| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-11-09 | |||
| 18:30:40 | mriedem | so, i see in placement handler code where it handles the "allocations are being removed on PUT" case, and it ensures a consumer exists, but then i don't see where that consumer is deleted | |
| 18:30:46 | mriedem | like the note in the compute code | |
| 18:32:41 | mriedem | https://github.com/openstack/nova/blob/e27905f482ba26d2bbf3ae5d948dee37523042d5/nova/api/openstack/placement/handlers/allocation.py#L404 | |
| 18:34:53 | mriedem | oh i guess it should happen here https://github.com/openstack/nova/blob/e27905f482ba26d2bbf3ae5d948dee37523042d5/nova/api/openstack/placement/objects/resource_provider.py#L2099 | |
| 18:36:28 | mriedem | https://github.com/openstack/nova/blob/e27905f482ba26d2bbf3ae5d948dee37523042d5/nova/api/openstack/placement/objects/consumer.py#L70 might be broken | |
| 18:36:38 | mriedem | in the same way that ensure was broken https://github.com/openstack/nova/commit/730936e535e67127c76d4f27649a16d8cf05efc9#diff-fcca11e34c1b5fce52a4ddbc418aa2d5 | |
| 18:37:26 | openstackgerrit | Merged openstack/nova stable/queens: conductor: Recreate volume attachments during a reschedule https://review.openstack.org/612496 | |
| 18:37:32 | openstackgerrit | Merged openstack/nova master: Update the description to make it more accuracy https://review.openstack.org/615362 | |
| 18:39:58 | mriedem | i can't really tell where delete_consumers_if_no_allocations is tested though... | |
| 18:40:04 | mriedem | some gabbit i'm sure | |
| 18:41:23 | dansmith | seems easily unit testable, | |
| 18:41:31 | dansmith | and I definitely can't look at that and tell that it works | |
| 18:41:59 | dansmith | since it's joining on consume id and asserting that it's none in one case | |
| 18:42:15 | mriedem | yeah i can't do anything with the sql w/o testing it | |
| 18:42:43 | dansmith | time to pack up, back later | |
| 18:42:58 | mriedem | oh i guess DeleteConsumerIfNoAllocsTestCase | |
| 18:43:04 | mriedem | i'll tweak that after lunch | |
| 18:53:28 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add debug logs when doubling-up allocations during scheduling https://review.openstack.org/617016 | |
| 18:53:29 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Log consumers_to_check when calling delete_consumers_if_no_allocations https://review.openstack.org/617017 | |
| 18:53:30 | mriedem | jaypipes: debug logging needed for this gate bug ^ | |
| 18:54:42 | jaypipes | still reading back, sorry | |
| 19:04:33 | jaypipes | dansmith, mriedem_hangry: there is a check at the end of the server-side of PUT /allocations that will auto-delete the consumer record if there are no allocations still referring to it. | |
| 19:07:44 | jaypipes | dansmith: and yes, I've said for a long time that we should have a GET /consumers endpoint. There are placement devs that vehemently disagreed with that. | |
| 19:27:08 | sean-k-mooney | mriedem_hangry: would you have any objection to backporting https://review.openstack.org/#/c/591607/9 to newton? im pretty sure we have a customer that is hitting this as they reported instance restarting after a host reboot missing interfaces that show up in nova interface-list | |
| 19:34:29 | sean-k-mooney | mriedem_hangry: actullly i just realised newton is way older then i remembered and is eol | |
| 19:34:49 | mriedem | jaypipes: yup found that, and the related functional test | |
| 19:35:19 | mriedem | sean-k-mooney: not to mention that isn't even approved on master | |
| 19:35:54 | sean-k-mooney | mriedem: yes :) im aware. i was more asking do you think this is something that can be backported in general upstream | |
| 19:36:02 | sean-k-mooney | once it lands in master | |
| 19:36:52 | mriedem | idk | |
| 19:37:09 | mriedem | it seems to be pretty controversial | |
| 19:37:22 | mriedem | like my david bowie costume on halloween | |
| 19:38:15 | sean-k-mooney | im waiting on more logs for the downstream bug to confirm this is actully the issue | |
| 19:40:18 | sean-k-mooney | mriedem: ill try to review and digest these chagnes more on monday | |
| 19:41:00 | sean-k-mooney | mriedem: are you flying out to berlin today/tomorow? | |
| 19:41:04 | mriedem | the problem the huawei ops team ran into was policy changed on the neutron side which started returning an empty list of ports, which was then saved into the info cache in the nova db, | |
| 19:41:18 | mriedem | and the heal periodic relies on the info cache rather than the source of truth to fix the cache | |
| 19:41:22 | mriedem | tonight | |
| 19:41:59 | mriedem | there are other ways to simply rebuild the cache if that's what is needed, e.g. https://docs.openstack.org/python-novaclient/latest/cli/nova.html#nova-refresh-network | |
| 19:42:06 | sean-k-mooney | right. i think we discussed this in the past at some point too it feels familar but i have not reviewd this before | |
| 19:42:18 | mriedem | it came up at the ptg i think | |
| 19:42:39 | mriedem | and the public cloud sig has brought it up before (OVH obviously) | |
| 19:42:51 | sean-k-mooney | mriedem: perhaps. oh so we can force a rebuild via the nova client today? | |
| 19:43:01 | mriedem | it doesn't rebuild from neutron | |
| 19:43:14 | mriedem | i don't think anyway | |
| 19:43:21 | mriedem | it just sends a network-changed event to the compute | |
| 19:43:41 | sean-k-mooney | oh it rebuilds form the vif table in the nova db? | |
| 19:43:53 | mriedem | yes | |
| 19:43:57 | mriedem | well, | |
| 19:43:59 | mriedem | from the info cache | |
| 19:44:18 | mriedem | iow, | |
| 19:44:26 | sean-k-mooney | ok but if the info cache got currpted when the host rebooted your still stuck | |
| 19:44:32 | mriedem | the network-changed event and _heal_instance_info_cache periodic do the same thing today | |
| 19:44:36 | mriedem | correct | |
| 19:44:48 | mriedem | hence the reason for making the periodic actually "heal" from the source of truth, which is neutron | |
| 19:44:53 | mriedem | and not our potentially corrupted cache | |
| 19:45:13 | sean-k-mooney | so i suggested they detach the missing interfaces and reattach them as a workaround for now to try and force nova and neutron to resync | |
| 19:45:30 | sean-k-mooney | i think that would work in this case but its not ideal | |
| 19:47:04 | sean-k-mooney | mriedem: anyway thanks. i was sraching my head for the last day or so trying to parse what was going on from incomplete logs but im 98% sure this is it. | |
| 19:47:49 | sean-k-mooney | mriedem: have a safe trip. | |
| 20:48:18 | mriedem | thanks | |
| 20:58:12 | dansmith | mriedem: so you found a test that confirmed the behavior of that thing? | |
| 20:58:19 | dansmith | mriedem: that deletes the consumer? | |
| 21:02:45 | mriedem | DeleteConsumerIfNoAllocsTestCase is the functional test that covers that case, | |
| 21:02:50 | mriedem | and it looks like a correct test to me, | |
| 21:03:02 | mriedem | creates 2 consumers each with 2 allocations on different resource classes, | |
| 21:03:09 | mriedem | clears the allocations for one of them and asserts the consumer is gone | |
| 21:03:29 | mriedem | i think we're just hitting a race with the shelve offloaded status change before we cleanup the allocations | |
| 21:03:40 | mriedem | but i've posted a couple of patches to add debug logs to help determine if that's the case | |
| 21:03:51 | mriedem | https://review.openstack.org/617016 | |
| 21:04:55 | dansmith | okay I'm not sure how we could race and see no allocations but a consumer and get that generation conflict | |
| 21:05:13 | dansmith | it'd be one thing if we thought the consumer was there and then disappeared out from under us | |
| 21:17:11 | mriedem | during unshelve the scheduler does see allocations | |
| 21:17:31 | mriedem | and it thinks we're doing a move | |
| 21:17:56 | dansmith | okay I thought you pasted a line showing that there was only one allocation going back to placement | |
| 21:18:07 | mriedem | there are 3 PUTs for allocations | |
| 21:18:11 | mriedem | 1. create the server - initial | |
| 21:18:23 | mriedem | 2. shelve offload - wipe the allocations to {} - which should delete the consumer | |
| 21:18:33 | mriedem | 3. unshelve - scheduler claims resources with the wrong consumer generation | |
| 21:18:45 | mriedem | and when 3 happens, the scheduler gets allocations for hte consumer and they are there, | |
| 21:18:47 | dansmith | ...right | |
| 21:18:55 | mriedem | so it uses the consumer generation (1) from those allocations | |
| 21:19:03 | mriedem | then i think what happens is, | |
| 21:19:05 | dansmith | oh, so it passes generation=1 instead of generation=0, meaning new consumer? | |
| 21:19:12 | mriedem | placement recreates the consumer which will have generation null | |
| 21:19:15 | mriedem | yes | |
| 21:19:22 | dansmith | okay I see | |
| 21:19:46 | dansmith | I thought you were seeing consumer generation was null or zero or whatever in the third put, but still getting a conflict | |
| 21:19:49 | dansmith | but that makes sense now | |
| 21:20:03 | mriedem | ment.concurrent_update", "detail": "There was a conflict when trying to complete your request.\n\n consumer generation conflict - expected null but got 1 ", "title": "Conflict"}]} | |
| 21:20:03 | mriedem | Nov 06 19:48:37.013780 ubuntu-xenial-inap-mtl01-0000379614 nova-scheduler[12154]: WARNING nova.scheduler.client.report [None req-f266a0ff-2840-413d-9877-4500e61512f5 tempest-ServersNegativeTestJSON-477704048 tempest-ServersNegativeTestJSON-477704048] Failed to save allocation for 6665f00a-dcf1-4286-b075-d7dcd7c37487. Got HTTP 409: {"errors": [{"status": 409, "request_id": "req-c9ba6cbd-3b6e-4e5d-b550-9588be8a49d2", "code": "p | |
| 21:20:12 | mriedem | consumer generation conflict - expected null but got 1 | |
| 21:20:20 | mriedem | yup - so new consumer but we're passing a generation of 1 | |
| 21:20:25 | mriedem | from the old, now deleted consumer | |
| 21:21:07 | dansmith | cool | |
| 21:21:26 | mriedem | so, | |
| 21:21:42 | dansmith | I wish there was something better to communicate that, but any time we get "expected null" in that case, we should be able to re-try but as a non-move sort of thing | |
| 21:21:43 | mriedem | we can paper over this by deleting the allocations before marking the instance as shelved offloaded, but that's whack-a-moley | |
| 21:21:49 | dansmith | yeah | |
| 21:22:00 | mriedem | right we need to retry from claim_resources but i'm not sure what's the best way to do that | |