| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-13 | |||
| 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 | |
| 18:57:43 | cfriesen | mriedem: you okay with https://review.openstack.org/#/c/435141/ otherwise, or did you still have issues with the json blob output? | |
| 18:58:00 | mriedem | haven't looked at it yet | |
| 18:58:06 | mriedem | getting https://review.openstack.org/#/c/473224/ sorted out | |
| 18:58:09 | cfriesen | kay, no worries | |
| 18:59:33 | mriedem | cfriesen: i definitely don't like dumping a json blob in a table cell, | |
| 18:59:39 | mriedem | but as you replied, we do it elsewhere | |
| 19:00:45 | mriedem | the alternative is we dump the flavor name | |
| 19:00:51 | mriedem | with the server details | |
| 19:01:00 | mriedem | and then if you want details on the flavor, you do nova flavor-show <flavor name> | |
| 19:03:57 | bauzas | mriedem: dansmith: just a note that I'll be on PTO tomorrow | |
| 19:04:16 | mriedem | ok | |
| 19:04:59 | bauzas | that, next week and the other week | |
| 19:05:39 | cfriesen | mriedem: that doesn't work if the flavor is deleted or modified though | |
| 19:05:52 | mriedem | cfriesen: true | |
| 19:06:07 | mriedem | cfriesen: would be nice at least to dump key-value pairs in a table cell rather than a json blob | |
| 19:06:13 | mriedem | but i don't know how easy that is | |
| 19:06:19 | mriedem | with prettytable | |
| 19:06:42 | mriedem | we might have done something like that with nova-status, checking | |