Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-06-13
16:06:17 sdague honestly, I have wondered if we should stack our API changes a little differently so the functional is in one patch, samples tests in next patch, api-ref in 3rd? Then just -2 the bottom and do a flush
16:06:37 sdague because I do agree they get complicated to mentally context switch in the reviews
16:06:38 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Handle uuids in os-hypervisors API https://review.openstack.org/471899
16:06:39 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Use uuid for id in Services API https://review.openstack.org/464280
16:06:48 mriedem ^ is the latest before i have the api-ref working
16:06:54 mriedem per my os_api_ref problem this morning
16:07:12 mriedem my plan here was to get reviews on each separately, -2 on os-services until +2 on os-hypervisors, then squash them
16:07:37 mriedem there is going to be db api/object layer paging changes for the hypervisors one that will be separate
16:07:40 mriedem but things still get pretty big
16:07:46 cfriesen andreykurilin: you're my hero. :)
16:07:59 mriedem https://review.openstack.org/#/c/464280/11/nova/api/openstack/compute/services.py is the main functional piece in the os-services change, and isn't too bad
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 :)

Earlier   Later