| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-13 | |||
| 16:08:18 | mriedem | i chose not to try and re-using the weird little private methods for the service action apis | |
| 16:08:22 | mriedem | like _enable and _disable | |
| 16:08:36 | mriedem | so it's a single update() method for the new microversion that handles all of the things in the request | |
| 16:12:08 | openstackgerrit | Sean Dague proposed openstack/python-novaclient master: Fix setting 'global_request_id' in SessionClient https://review.openstack.org/473224 | |
| 16:12:16 | sdague | sfinucan: it can actualy be a little smaller | |
| 16:13:30 | sfinucan | sdague: And even smaller? REQ_ID_HEADER looks unused | |
| 16:14:41 | sdague | sfinucan: oh, it might be now | |
| 16:14:49 | bauzas | mriedem: I did remember how cutting the relationship between services and hypervisors was a big thing, so I trust you when you say it's becoming a monster change | |
| 16:14:52 | sdague | I can swing around in a sec | |
| 16:15:08 | sfinucan | no rush | |
| 16:15:35 | bauzas | touching services and hypervisor seems like the most impactful things to do, because we have a shit ton of tests asserting things, plus a lot of logic around those | |
| 16:15:46 | openstackgerrit | Sean Dague proposed openstack/python-novaclient master: Fix setting 'global_request_id' in SessionClient https://review.openstack.org/473224 | |
| 16:15:50 | mriedem | it's definitely not fun | |
| 16:15:57 | bauzas | mriedem: any series I can start to review, so I could help ? | |
| 16:16:03 | mriedem | the os-services thing is pretty straight-forward though, after working on it for a couple of days | |
| 16:16:03 | bauzas | don't be afraid of WIPs | |
| 16:16:16 | mriedem | bauzas: https://review.openstack.org/#/c/464280/11 and below | |
| 16:16:27 | bauzas | mriedem: okay, I'll review those | |
| 16:16:32 | mriedem | bauzas: ^ is really basically done, except one todo in the code and i need to get the api-ref working | |
| 16:16:56 | mriedem | sdague: i wonder if my comments in the parameters.yaml is what's making os_api_ref puke? | |
| 16:17:10 | bauzas | mriedem: I can see the code | |
| 16:17:39 | mriedem | oh i guess not, there are already comments in there | |
| 16:18:50 | sdague | mriedem: you have the patch? | |
| 16:18:57 | mriedem | https://review.openstack.org/#/c/464280/ | |
| 16:21:25 | sdague | ok, let me poke | |
| 16:25:55 | sdague | mriedem: ok, it's a warning that's trying to trigger | |
| 16:26:31 | sdague | oh, i see what's going on, let me figure out the right fix | |
| 16:33:12 | sdague | oh, it looks like sphinx changed a call interface | |
| 16:34:05 | mriedem | ok but this would be the warning https://github.com/openstack/os-api-ref/blob/master/os_api_ref/__init__.py#L349 | |
| 16:34:15 | mriedem | i'm missing some path parameter | |
| 16:37:27 | openstackgerrit | Alex Szarka proposed openstack/nova master: Refactor create_delete_server_with_instance_update https://review.openstack.org/466296 | |
| 16:38:38 | openstackgerrit | Chris Friesen proposed openstack/python-novaclient master: 2.47: Show flavor info in server details https://review.openstack.org/435141 | |
| 16:38:38 | openstackgerrit | Chris Friesen proposed openstack/python-novaclient master: Clean up ShellTest unit tests https://review.openstack.org/473910 | |
| 16:38:55 | cfriesen | mriedem: andreykurilin: ^ | |
| 16:40:05 | mriedem | sdague: yeah if i restrict Sphinx<1.6.1 i get this | |
| 16:40:05 | mriedem | /home/user/git/nova/api-ref/source/api-ref/source/os-services.inc:250 .rst:: WARNING: No path parameter ``service_id`` found in rest_parameter stanza. | |
| 16:40:21 | sdague | yeh | |
| 16:40:28 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform missing delete notifications https://review.openstack.org/410297 | |
| 16:40:44 | sdague | so they seem to have changed the warn parameters on env | |
| 16:43:23 | mriedem | i'll open a bug quick to track it against os-api-ref | |
| 16:43:31 | cfriesen | mriedem: for https://review.openstack.org/#/c/465787/ we have some indication that it's not properly handling neutron floatingip-associate events | |
| 16:44:10 | openstackgerrit | Merged openstack/nova master: Fix the race condition with novnc https://review.openstack.org/470798 | |
| 16:53:40 | openstackgerrit | Merged openstack/nova master: Add missing query filter params for GET /os-services API https://review.openstack.org/473708 | |
| 16:55:51 | mriedem | dansmith: about that service not found thing last night, where it's not a compute service to do things like enable/disable, | |
| 16:55:59 | mriedem | we talked about returning a more useful error message in that case, | |
| 16:56:11 | mriedem | however, it's a chicken and egg problem, | |
| 16:57:04 | mriedem | well, maybe not - if it's not a compute service it won't have a host mapping and we'll get HostMappingNotFound, | |
| 16:57:18 | mriedem | if it is a compute service and really just doesn't exist, we'll get ServiceNotFound | |
| 16:59:12 | mriedem | anyway, now i'm having a hard time recalling what we talked about doing here | |
| 16:59:50 | dansmith | not 404ing if they try to disable a scheduler for example | |
| 17:00:13 | mriedem | oh right, so it is kind of a chicken and egg then | |
| 17:00:18 | mriedem | we don't know it's a scheduler or not | |
| 17:00:25 | mriedem | if there is no host mapping, that could be b/c it's not mapped yet | |
| 17:00:33 | mriedem | or because it's not a compute service | |
| 17:01:55 | mriedem | anyway, i think i'm going to write something up to the ML about this just to make people aware and see if anyone had any weird uses for disabling non-compute services, | |
| 17:02:01 | mriedem | to help answer what i should do in the new api | |
| 17:02:21 | dansmith | if we don't find a mapping, we can lookin cell0 for the service, and if we find, it we can assume it's not an unmapped compute, but a non-compute control service | |
| 17:02:37 | dansmith | there is no use for disabling a non-compute service, | |
| 17:02:44 | dansmith | all it does is set disabled=True, and that's only checked in the scheduler | |
| 17:02:53 | dansmith | I went through this with someone internal a few months ago | |
| 17:03:14 | mriedem | i know, but you never know what people are doing - which is why i just want to ask or let people be aware of this, | |
| 17:03:25 | mriedem | i think we still go forward with restricting updating to only compute services | |
| 17:03:57 | mriedem | i.e. i could totally see someone using service.update notifications to trigger some backend system to kill their nova-scheduler or something, so they can control things via the api | |
| 17:05:11 | dansmith | and your point is you just want to alert those people that we're killing that? .... RIGHT? | |
| 17:05:50 | mriedem | the point is i'm going to add a flurry of config options to call out to external hooks which change the behavior of this api | |
| 17:05:51 | mriedem | is that ok? | |
| 17:06:03 | dansmith | oh that? yeah, that's fine... reaaaal nice. | |
| 17:06:19 | mriedem | yes this is an alert | |
| 17:06:24 | mriedem | brown alert | |
| 17:06:47 | dansmith | heh | |
| 17:21:30 | mriedem | here she be http://lists.openstack.org/pipermail/openstack-operators/2017-June/013761.html | |
| 17:25:39 | edmondsw | dansmith mriedem sfinucan before I invest a bunch of time in UTs, what do y'all think of the course I'm on with https://review.openstack.org/#/c/472794/ ? | |
| 17:56:15 | dansmith | mriedem: the ironic driver is waiting for this to report some resources: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4627 | |
| 17:56:21 | dansmith | which will never happen | |
| 17:56:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Handle uuids in os-hypervisors API https://review.openstack.org/471899 | |
| 17:56:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use uuid for id in os-services API https://review.openstack.org/464280 | |
| 17:56:42 | dansmith | my discover change in that loop did find unmapped nodes though, so we definitely need that | |
| 17:56:43 | mriedem | dtp: ok os-services is done and ready for review ^ i'm going to -2 it until the os-hypervisors change is done | |
| 17:56:55 | dtp | ok | |
| 17:56:58 | mriedem | sdague: just squeeked in under 1000LOC :) | |
| 17:57:50 | mriedem | dansmith: hmm, how is it we never had to update GET /os-hypervisors/statistics for multi-cells? | |
| 17:57:54 | mriedem | no tempest test? | |
| 17:58:07 | dansmith | I think very few things hit that, | |
| 17:58:13 | dansmith | so yeah | |
| 17:58:54 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/hypervisors.py#L277 | |
| 17:59:02 | mriedem | yeah, so i guess we need to address that under your cells-aware bp | |
| 17:59:06 | dansmith | yeah | |
| 17:59:39 | mriedem | i worry a bit about what we haven't made cells aware due to lack of tempest api test coverage | |
| 17:59:55 | mriedem | but not so worried that i'm going to do a thorough audit right now :) | |
| 18:00:50 | dansmith | well, lack of test coverage is lack of test coverage | |
| 18:01:00 | dansmith | we're hitting one thing right here before real people do, so... | |
| 18:01:16 | mriedem | so kudos to us | |
| 18:01:46 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.rebuild-scheduled notification https://review.openstack.org/473929 | |
| 18:06:27 | mriedem | cfriesen: something still failing here https://review.openstack.org/#/c/435141/ | |
| 18:11:35 | dansmith | mriedem: there aren't even unit tests for that bit of HostAPI | |
| 18:12:11 | mriedem | nice | |
| 18:12:23 | mriedem | i've noticed we test the rest api but not the host api so well | |
| 18:15:58 | dansmith | I don't see rest tests for this either | |
| 18:24:04 | sdague | mriedem: oh, it's just a flat out broken sphinx thing | |
| 18:24:09 | sdague | gdi | |