Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-06-13
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 bauzas don't be afraid of WIPs
16:16:03 mriedem the os-services thing is pretty straight-forward though, after working on it for a couple of days
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: Clean up ShellTest unit tests https://review.openstack.org/473910
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:55 cfriesen mriedem: andreykurilin: ^
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:05 mriedem sdague: yeah if i restrict Sphinx<1.6.1 i get this
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: Use uuid for id in os-services API https://review.openstack.org/464280
17:56:31 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Handle uuids in os-hypervisors API https://review.openstack.org/471899
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
18:24:41 cfriesen mriedem: we found the bug in https://review.openstack.org/#/c/465787 and I updated the review
18:26:56 cfriesen mriedem: I think that's new after rebasing...I only reran the test_shell unit tests after changing stuff
18:28:48 openstackgerrit Dan Smith proposed openstack/nova master: DNM: Canary https://review.openstack.org/473565
18:28:49 openstackgerrit Dan Smith proposed openstack/nova master: Make compute_node_statistics() work across cells https://review.openstack.org/473931
18:29:28 cfriesen mriedem: this is weird...novaclient.tests.unit.test_client.SessionClientTest.test_global_id passes locally
18:30:41 cfriesen mriedem: and I didn't change anything related to that code, that I can see at least
18:37:21 cfriesen mriedem: locally it looks fine. what's with the key.lower() in the traceback of the failing test at http://logs.openstack.org/41/435141/13/check/gate-python-novaclient-python27-ubuntu-xenial/df4a44b/testr_results.html.gz ?
18:42:07 cfriesen andreykurilin: can you see any reason why https://review.openstack.org/#/c/473910 would cause the failure in test_global_id at http://logs.openstack.org/10/473910/1/check/gate-python-novaclient-python27-ubuntu-xenial/76e7e46/testr_results.html.gz ?
18:44:18 mriedem cfriesen: the global id thing is being fixed
18:44:23 mriedem sdague has a patch for it
18:45:17 mriedem cfriesen: good catch on https://review.openstack.org/#/c/465787/3/nova/network/neutronv2/api.py@2322
18:56:21 cfriesen mriedem: can't take credit personally...we ran into it during testing and one of the guys tracked it down

Earlier   Later