Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-08-02
10:23:58 sean-k-mooney it would still be save not remote_save()
10:24:34 gibi ahh
10:24:53 gibi why we are too clever with these namings
10:24:54 gibi ?!
10:25:08 gibi but you are right
10:25:22 gibi the incoming save might fail
10:25:24 gibi hm
10:25:38 sean-k-mooney you can add a try_save()
10:25:39 gibi actually the incoming save RPC will call Instance.save that will do the checks again
10:25:43 sean-k-mooney function that delegate to save
10:25:56 sean-k-mooney try_save can check if it should save and then call save
10:26:17 gibi try_save is bad as I don't want to replace all the instance.save calls with instance.try_save
10:26:36 gibi but I need the check at every instance.save call
10:26:37 sean-k-mooney ya that was what i was about to say you would have to do that
10:26:59 sean-k-mooney so the only ohte way to do this if you dont use a decorator
10:27:22 sean-k-mooney is to have the context manage replace instance.save within it
10:27:25 sean-k-mooney which is kind of messy
10:28:33 gibi as the decorator is applied both on the local and on the remote side with the decorator we also call the extra check twice. I think the effect is the same either if I use the decorator or a save -> remote_save indirection
10:28:38 gibi but I will try it out after lunch
10:28:41 gibi thanks again
10:29:11 sean-k-mooney cool let me knwo how it goes, enjoy lunch
10:29:17 gibi thanks
11:07:08 opendevreview Elod Illes proposed openstack/nova stable/rocky: Move 'check-cherry-picks' test to gate, n-v check https://review.opendev.org/c/openstack/nova/+/804654
11:22:00 opendevreview sean mooney proposed openstack/nova master: enable blocked VDPA move operations https://review.opendev.org/c/openstack/nova/+/832330
11:36:53 opendevreview Elod Illes proposed openstack/nova stable/wallaby: Gracefull recovery when attaching volume fails https://review.opendev.org/c/openstack/nova/+/829434
11:54:20 opendevreview Balazs Gibizer proposed openstack/nova master: Avoid saving instance under mutated migration context https://review.opendev.org/c/openstack/nova/+/851832
11:54:20 opendevreview Balazs Gibizer proposed openstack/nova master: Prevent instance.save() under mutated migration context https://review.opendev.org/c/openstack/nova/+/850746
11:54:53 gibi sean-k-mooney[m]: locally this works ^^ lets see if it passes tempest and grenade
11:55:05 gibi dansmith_: meanwhile with sean-k-mooney[m] we figured a solution for my question ^^
12:07:07 artom gibi, I have a concern on https://review.opendev.org/c/openstack/nova/+/851832 - does it sound legit to you? I'm not too sure myself...
12:55:01 opendevreview Merged openstack/nova master: [docs] Fix mention of custom scheduling after Wallaby https://review.opendev.org/c/openstack/nova/+/851807
12:55:26 opendevreview Guillaume Espanel proposed openstack/nova master: Skip useless qemu-img convert when snapshotting https://review.opendev.org/c/openstack/nova/+/851854
13:00:10 gibi artom: you and sean-k-mooney[m] both raised this, I'm waiting for grenade to prove that it is a real problem or not. If it is then the only way to keep this work is to decorate save() to modify it so the name is kept.
13:00:41 gibi I'm not sure how the names of the RPC is mapped
13:01:03 gibi in you example you need an old conductor
13:01:17 gibi but I think we say, upgrade your control service first
13:01:28 sean-k-mooney gibi: dansmith_ might be able to help you figure that out but i can point to where that is done of the top of my head
13:01:31 gibi so in theory the conductor will have both save and _remote_save
13:01:47 gibi during a rolling upgrade
13:01:56 gibi old computes will call save over RPC and that exists
13:02:08 gibi new computes probably will call _remote_save and that also exists in a new conductor
13:02:24 gibi sean-k-mooney: ack
13:23:25 artom gibi, for a major version upgrade, yeah, control tier first, so it's not a problem
13:23:31 artom It's only a problem if we backport
13:23:43 sean-k-mooney[m] my laptop over heated again
13:24:21 sean-k-mooney[m] ill be upstream only for a bit while it cools down and i get the vpn set up elsewhere
13:25:03 artom Pour mineral oil in your bathtub and dunk your laptop in there?
13:28:48 gibi artom: hm, if we backport then both the old and the new side will have both save and _remote_save so I don't see the problem about the backport either
13:29:26 opendevreview Radosław Piliszek proposed openstack/nova stable/yoga: [docs] Fix mention of custom scheduling after Wallaby https://review.opendev.org/c/openstack/nova/+/851870
13:32:11 opendevreview Balazs Gibizer proposed openstack/nova master: Remove double mocking https://review.opendev.org/c/openstack/nova/+/851445
13:32:11 opendevreview Balazs Gibizer proposed openstack/nova master: hacking: force explicit import of python's mock https://review.opendev.org/c/openstack/nova/+/708768
13:32:12 opendevreview Balazs Gibizer proposed openstack/nova master: Remove the PowerVM driver https://review.opendev.org/c/openstack/nova/+/850346
13:33:52 kashyap gibi: Thx for cleaning up the PowerVM!
13:34:00 gibi that wasn't me
13:34:03 gibi I just rebased
13:34:09 kashyap Err, stephenfin++ :)
13:34:23 kashyap What a diffstat: +10 -9350
13:45:39 dansmith_ artom: gibi sean-k-mooney[m]: I'm really -2 on that approach in general, but -1 for politeness
13:46:17 opendevreview Elod Illes proposed openstack/nova stable/train: DNM: test preinstall of python3-yaml https://review.opendev.org/c/openstack/nova/+/851861
13:46:32 dansmith_ gibi: to answer your question, the methods are mapped automatically, which means it's a problem for minor and major upgrades as artom noted, and why you can't find the method mapping
13:46:48 gibi dansmith: thanks for the feedback
13:48:08 gibi dansmith: so during minor upgrade we allow new compute code to run before the conductor is upgraded?
13:48:27 dansmith yes, any service in any order
13:49:03 gibi that is news for me
13:49:08 gibi anyhowe
13:49:18 dansmith but as I said on the review, this is really an impedance mismatch between compute and manager, it sounds like, and some refactoring there needs to happen instead of doubling down on the original thing, IMHO
13:49:30 dansmith gibi: okay, that's why artom specifically called out minor updates
13:49:58 dansmith basically, we expect anyone should be able to "yum upgrade" on any node at any time
13:50:11 gibi ack, I learned something new today
13:50:16 gibi going back to the original problem
13:51:00 gibi I don't know what will break if we remove the mutated migration context from the rollback_live_migration_at_destination call
13:51:26 gibi It was added there on purpose as far as I see
13:51:51 artom Yeah, it was so that we roll back the stuff claimed on the destination
13:51:54 dansmith it's something in driver.destroy() that looks to see if there's a migration context, and does more/less stuff during destroy as a result right?
13:52:19 artom Actually, maybe not, ignore me until I look at the code again
13:52:53 dansmith as noted, I think that was probably the first bad move
13:53:06 dansmith the "temporarily mutate instance so I don't have to change code elsewhere"
13:53:23 dansmith ended up having a side-effect that we didn't expect, so more flags to prevent side effects is just compounding the problem :)
13:55:00 gibi dansmith: I agree with general idea not to add more flags. So lets see if we can figure out how to untangle what we have
13:55:07 dansmith ++
13:58:12 gibi dansmith: if we are already at the problem the follow up patch https://review.opendev.org/c/openstack/nova/+/850746/3 show another occurence of the save under mutated migration context codepath.
13:58:21 gibi we call driver.rebuild https://review.opendev.org/c/openstack/nova/+/850746/3 under a mutated context
13:58:24 kashyap gibi: Hey, is there some "openstack server show" command to see the machine type configured on the compute node? Or 'grep'ing the nova.conf / `sudo virsh dumpxml $instance` the only way?
13:58:59 dansmith gibi: I have to jump on a call, might have a few minutes after before the next one at the top of the next hour
13:59:01 gibi and ironic virt driver saves the instance https://review.opendev.org/c/openstack/nova/+/850746/3
13:59:01 bauzas kashyap: you would need to be an admin at least if so
13:59:09 gibi dansmith: ack
13:59:15 gibi dansmith: no worries
13:59:19 kashyap bauzas: Right, let's say admin. Is there an admin Nova command?
13:59:31 bauzas this is a conf opt
13:59:43 bauzas so this shouldn't be an API extension
14:00:21 bauzas in general, we don't want to provide an API for knowing about some config option
14:00:56 kashyap bauzas: Right; fair enough
14:01:49 kashyap bauzas: Note, this is also a metadata and extra_spec property as well
14:01:57 kashyap "this" == guest machine type
14:03:47 kashyap bauzas: So if a user configures a Glance image with that image meta property, they should be able to see
14:05:38 kashyap bauzas: So, `openstack image show` should show it if a user sets it
14:06:33 gibi dansmith: so if you have a minute at some point I summarized my second question here https://review.opendev.org/c/openstack/nova/+/850746/3/nova/compute/manager.py#3797
14:06:52 opendevreview Elod Illes proposed openstack/nova stable/train: DNM: test preinstall of python3-yaml https://review.opendev.org/c/openstack/nova/+/851861
14:08:53 gibi kashyap: while I understand the need to see what machine type an instance uses, I don't think we have a single place we persist it for the insance. I guess we rely on the fact that we can always regenerate the machine type from the flavor + image + compute config.

Earlier   Later