| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-07-27 | |||
| 13:19:53 | opendevreview | Sylvain Bauza proposed openstack/nova master: api: Drop generating a keypair and add special chars to naming https://review.opendev.org/c/openstack/nova/+/849133 | |
| 13:20:14 | bauzas | rebased my keypair api microversion due to unshelve_to_host merge ^ | |
| 13:20:24 | bauzas | people can review it | |
| 13:27:56 | opendevreview | Kashyap Chamarthy proposed openstack/nova stable/victoria: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851223 | |
| 13:31:38 | gibi | bauzas: +2, do you have an python binding patch and or osc patch with the client side change? | |
| 13:31:45 | bauzas | not yet | |
| 13:31:53 | opendevreview | Kashyap Chamarthy proposed openstack/nova stable/victoria: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851223 | |
| 13:32:04 | gibi | Uggla: similar question to you too, do you have a client patch for the unshelve to host change? | |
| 13:32:08 | gibi | bauzas: ack | |
| 13:32:31 | sean-k-mooney | technically we dont need thos to merge the nova change but ya woudl be nice to see them sooner rather then later | |
| 13:32:38 | Uggla | gibi, yes give you the ids in one sec | |
| 13:32:39 | gibi | in the past we said we would at least like to see the client patches proposed (not merged) before we approve a microversion | |
| 13:32:45 | gibi | Uggla: awesome | |
| 13:32:49 | sean-k-mooney | ill be dropping soon but ill see if i have time to review before i do | |
| 13:33:32 | opendevreview | Kashyap Chamarthy proposed openstack/nova stable/ussuri: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851224 | |
| 13:34:22 | Uggla | gibi: https://review.opendev.org/c/openstack/python-novaclient/+/831651 and https://review.opendev.org/c/openstack/python-openstackclient/+/831902 | |
| 13:34:33 | gibi | Uggla: thanks | |
| 13:35:03 | Uggla | gibi, there is also a related tempest test: 841088: Tempest test for checking unshelve to host | https://review.opendev.org/c/openstack/tempest/+/841088 | |
| 13:35:16 | gibi | ack | |
| 13:38:26 | opendevreview | Kashyap Chamarthy proposed openstack/nova stable/train: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851226 | |
| 13:40:21 | kashyap | bauzas: gibi: If you have the stomach for it today still (other backports): https://review.opendev.org/q/Iec387dcbc49ddb91ebf5cfd188224eaf6021c0e1 | |
| 13:40:41 | bauzas | gibi: kashyap: was working on the novaclient change | |
| 13:40:49 | kashyap | No worries | |
| 13:46:31 | opendevreview | Merged openstack/nova master: Add a workaround to skip hypervisor version check on LM https://review.opendev.org/c/openstack/nova/+/851034 | |
| 14:04:35 | gibi | Uggla: I left feedback in the python-novaclient patch and +1d the osc patch. nice work! | |
| 14:05:14 | Uggla | gibi, ok I'll have a look ASAP. | |
| 14:12:57 | opendevreview | Sylvain Bauza proposed openstack/python-novaclient master: Add support for 2.92 : keypair import mandatory https://review.opendev.org/c/openstack/python-novaclient/+/851231 | |
| 14:13:02 | bauzas | gibi: ^ | |
| 14:13:09 | gibi | bauzas: thanks | |
| 14:13:13 | bauzas | now working on the OSC patch | |
| 14:14:14 | gibi | bauzas: you probably want to put your novaclient patch top of Uggla's | |
| 14:14:33 | bauzas | gibi: there is a depends-on my api patch | |
| 14:14:39 | bauzas | gibi: but I can rebase it | |
| 14:15:00 | gibi | yeah, keep the depends on, but rebase your client patch on Uggla's client patch | |
| 14:15:08 | gibi | otherwise you will be in merge conflict | |
| 14:16:39 | bauzas | gibi: cool, yeah I know about the merge conflict but the fix will be simple | |
| 14:16:51 | gibi | yepp | |
| 14:16:59 | gibi | othervise your client patch looks good to me | |
| 14:20:00 | opendevreview | Sylvain Bauza proposed openstack/python-novaclient master: Add support for 2.92 : keypair import mandatory https://review.opendev.org/c/openstack/python-novaclient/+/851231 | |
| 14:20:19 | bauzas | gibi: done | |
| 14:20:55 | gibi | bauzas: thanks, reapplied my +@ | |
| 14:20:56 | gibi | +2 | |
| 14:21:11 | bauzas | thanks | |
| 14:21:44 | gibi | your home has hair? ;) | |
| 14:45:49 | bauzas | gibi: sorry, haircut *at* home :p | |
| 14:46:25 | bauzas | my wife and I have a hairdresser visiting uds | |
| 14:46:30 | gibi | ohh, nice | |
| 14:46:39 | gibi | my wife tend to cut mine | |
| 14:46:39 | bauzas | cheaper and better | |
| 14:46:55 | gibi | that is the cheapest :) | |
| 14:47:28 | bauzas | yeah but if I use a hair mower, then my hair will have issues :) | |
| 14:48:19 | bauzas | I prefer to have a pro using scissors | |
| 15:48:14 | artom | gibi, so rollback_live_migration_at_destination() is the cast that contains the mutated migrated context, right? | |
| 15:48:26 | artom | So what if we just change it to a call instead of cast to make it blocking? | |
| 15:48:41 | artom | That way, we're sure that by the time we drop the claim, we're out of the mutated context? | |
| 15:49:16 | gibi | that is a dirty thing that will mean we will have a short time window where the db contains the wrong info, then then the second call will return the db content to the correct state | |
| 15:49:23 | gibi | so we will have a smaller race window | |
| 15:49:26 | gibi | but we still have the race | |
| 15:50:02 | artom | You're assuming that mutating the context saves to the DB? | |
| 15:50:05 | artom | I don't think that's the case | |
| 15:50:16 | gibi | we mutate the context and then save the instance today | |
| 15:50:25 | gibi | just the mutate is not an issue | |
| 15:50:57 | gibi | the instance.save happens in _cleanup in the driver | |
| 15:51:06 | gibi | and that is run under the mutated context | |
| 15:52:55 | gibi | links to the code are here https://bugs.launchpad.net/nova/+bug/1982497/comments/2 | |
| 15:53:08 | artom | gibi, oh, right | |
| 15:53:32 | artom | We mutate the context in the compute manager, and in that context we eventually call down to instance.save() in the libvirt driver | |
| 15:53:34 | gibi | hence my second patch that forbids instance.save under the mutated context to avoid this in the fututre | |
| 15:53:41 | gibi | artom: yes | |
| 15:54:00 | artom | Wait, so why did you jump through the sync loops in your reproducer test? | |
| 15:54:05 | artom | The Condition(), for example? | |
| 15:54:56 | gibi | I needed a special serialization order to make sure that we avoid the case when we save the wrong state, then right after we save the good state. | |
| 15:55:25 | artom | Ah | |
| 15:55:53 | gibi | _rollback_live_migration makes the DB invalid then drop_move_claim_at_destination fixes it. | |
| 15:56:31 | gibi | so in that order it is just a small race window (same as if we would make _rollback_live_migration a call instead of a cast) | |
| 15:57:24 | gibi | interestingly in a real environment _rollback_live_migration tend to take longer and finishing _after_ drop_move_claim_at_destination probably due to _rollback_live_migration doing disk IO that can be slow | |
| 15:58:34 | artom | I mean we could just remove the instance.save() from the cleanrup()... | |
| 15:58:41 | artom | And count on the compute manager doing it later... | |
| 15:58:43 | artom | Sounds fragile | |
| 15:59:03 | gibi | artom: yepp that is one way, we can do that and then figure out the fallout | |
| 16:00:04 | gibi | it seems we have that instance.save as delete_instance_files() can fail and we want to retry | |
| 16:00:11 | gibi | later in a periodic if that fails | |
| 16:02:06 | gibi | unfortunately _cleanup not just set instance.cleaned but also init instance.system_metadata['clean_attempts'] that is used in _run_pending_deletes | |
| 16:02:56 | gibi | and honestly not 100% sure if that is the only instance update we do during the full destroy codepath | |
| 16:03:34 | gibi | I think that is the only instance.save but maybe other pieces of code also changes the instance that we would loose if we remove the instance.save | |
| 16:08:07 | melwitt | gmann: fyi the tempest tests for the volume extend coverage have merged, so I think the nova patch to start running the tests is ready for review https://review.opendev.org/c/openstack/nova/+/843700 | |
| 16:08:29 | gmann | melwitt: checking | |
| 16:08:37 | melwitt | thanks! | |
| 16:57:42 | sean-k-mooney | im going to go nap soon i think but i might review the keypair change before or after | |
| 16:57:53 | sean-k-mooney | is that the next microverion in the queue | |
| 17:14:30 | gibi | t is | |
| 17:14:31 | gibi | it is | |
| 17:14:49 | bauzas | I'm just about to upload the OSC patch in 5 mins | |
| 17:14:57 | bauzas | after creating the relnote | |
| 17:15:10 | bauzas | so we'll have all the meat for 2.92 except the tempest tests | |
| 17:15:27 | sean-k-mooney[m] | ok cool | |
| 17:21:57 | bauzas | WTF | |
| 17:22:20 | bauzas | [sbauza@sbauza python-openstackclient]$ git review -s | |
| 17:22:20 | bauzas | Problems encountered installing commit-msg hook | |
| 17:22:20 | bauzas | The following command failed with exit code 255 | |