| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-02-19 | |||
| 18:15:01 | sean-k-mooney | you were asking about aviablity zone checks | |
| 18:15:13 | sshmlbrg | but it doesn't work | |
| 18:15:31 | sean-k-mooney | so by default htere are no aviablity zone checks for nova and cinder voluems. | |
| 18:15:46 | sean-k-mooney | we have a config option to opt into the non default beahvior of preventign cross az attach | |
| 18:16:18 | sean-k-mooney | its possible this only works when cross az atach is allow or when an instance is pinned to an az | |
| 18:19:53 | sean-k-mooney | ill see if i can quickly test it | |
| 18:20:27 | sshmlbrg | But this function lacks a cross-AZ check, and we proceed to compare the availability zone of the volume and the instance. https://github.com/openstack/nova/blob/master/nova/compute/api.py#L5143 | |
| 18:22:07 | sean-k-mooney | so cross_az attach is a comput enode option | |
| 18:22:19 | sean-k-mooney | a shevle offloaded instance does not have a compute service | |
| 18:22:41 | sean-k-mooney | so you cant actuly reason about if this shoudl or shoudl not work | |
| 18:23:50 | sean-k-mooney | the az check i enabeld by this check https://github.com/openstack/nova/blob/master/nova/compute/api.py#L5065 | |
| 18:23:56 | sean-k-mooney | *this default | |
| 18:24:02 | sean-k-mooney | because we are not passsing that variable | |
| 18:24:35 | opendevreview | Sylvain Bauza proposed openstack/nova master: Add a new ImagePropertiesWeigher https://review.opendev.org/c/openstack/nova/+/940642 | |
| 18:24:36 | opendevreview | Sylvain Bauza proposed openstack/nova master: per-property ImageMetaPropsWeigher https://review.opendev.org/c/openstack/nova/+/941601 | |
| 18:24:47 | bauzas | dansmith: sean-k-mooney: rebased as asked | |
| 18:25:27 | sshmlbrg | Then, should we disable this option so that attach works for shelved_offloaded VMs? | |
| 18:25:51 | dansmith | bauzas: okay did you want me to make the mods to use the new method? | |
| 18:26:03 | sean-k-mooney | there is more context here | |
| 18:26:24 | sean-k-mooney | sshmlbrg: https://github.com/openstack/nova/commit/07a24dcef7ce6767b4b5bab0c8d3166cbe5b39c0 | |
| 18:27:20 | sshmlbrg | i need a few minutes to get it :) | |
| 18:28:39 | sean-k-mooney | sshmlbrg: it looks like the curent check was added to fix https://bugs.launchpad.net/nova/+bug/1694844 | |
| 18:28:57 | sean-k-mooney | but it may have broken seveled atatch in the process | |
| 18:29:59 | sean-k-mooney | skimming the patch it maybe looks like the intent was to fall back to the default schduler zone | |
| 18:30:31 | sean-k-mooney | i think in this sepcific case it woudl be valide to pass false | |
| 18:31:00 | sean-k-mooney | if an donly if instnace.az was None in either the request spec or the instance table in teh cell db | |
| 18:31:35 | sean-k-mooney | basically if the isntance is shelve_offloaded. and it not pinend to any az | |
| 18:32:04 | sean-k-mooney | then we can possibly disable the afinity check | |
| 18:32:33 | sean-k-mooney | but that may lead to issue unshevling | |
| 18:32:46 | sean-k-mooney | unless we pinned the shleved instance to the relevent az | |
| 18:33:10 | sean-k-mooney | that is only vailid to do however if cros_az_attach=false | |
| 18:34:17 | sean-k-mooney | since we can now unshlve to an az | |
| 18:34:27 | sshmlbrg | Yes, and also, we can unshelve the VM to another AZ, and I’m not sure how the volumes will behave in that case. | |
| 18:34:45 | sean-k-mooney | the workaround for tha twould be to return an error and make the user explictly request the az they want to unshvle to and that woudl have to match the volume az | |
| 18:35:07 | sean-k-mooney | sshmlbrg: its blocked if you use the non default beahvior | |
| 18:35:30 | sshmlbrg | ok | |
| 18:35:34 | sean-k-mooney | sshmlbrg: teh reals fix is to delete the ablity for nova to prevent cross_az entirly | |
| 18:35:44 | sean-k-mooney | that never shoudl have been a per compute config option | |
| 18:36:02 | sean-k-mooney | it should be a attibute on the cinder volume | |
| 18:36:25 | sean-k-mooney | i.e. when attaching a volume you shoudl be able to choose if cross az storage is allowed or not | |
| 18:36:35 | sean-k-mooney | the reason it snot that way, beyond legacy | |
| 18:36:55 | sean-k-mooney | is as a end user you dont knwo if the strage backend is accable from the current host | |
| 18:37:41 | sean-k-mooney | its tricky | |
| 18:38:35 | sshmlbrg | ok, i hope i got it, need to summarize it. Is it bug or ...? | |
| 18:49:35 | sean-k-mooney | its proably a but im just not sure how to fix it | |
| 18:50:06 | sean-k-mooney | im inclided to say pass false and then requrie pepoel to unshlve to the same az fi thats how the clodu is confiugred | |
| 18:51:08 | sean-k-mooney | we shoudl have a better solution to this, that the problem with config driven api bevhior and why we dont add it anymore if we can avoid it at all costs | |
| 18:53:30 | sshmlbrg | I was thinking about storing the old AZ, writing the AZ to it when shelving, and releasing it when unshelving. Then, when attaching the volume, we could check that specific availability zone. | |
| 18:54:04 | sean-k-mooney | so that not correct | |
| 18:54:25 | sean-k-mooney | instnace that dont request a az shoudl not be pinned to one by default | |
| 18:54:32 | sshmlbrg | understand why | |
| 18:55:20 | sean-k-mooney | if they have a cidner volume and land on a host with cross_az_attach=false and it does not violate the requirement nova will pin it | |
| 18:56:34 | sean-k-mooney | sshmlbrg: did the instance have a voluem before it was shelved? | |
| 18:56:50 | sshmlbrg | yes, volume-backed instance | |
| 18:57:03 | sean-k-mooney | ok then it should be pinned already | |
| 18:57:21 | sean-k-mooney | so the bug is that its proably not usign the az form the request spec if its not set in the host | |
| 18:57:28 | sean-k-mooney | * in the instance table | |
| 18:58:07 | sshmlbrg | yes | |
| 18:58:37 | sean-k-mooney | ok so tha that specific edge case is fixable | |
| 18:59:22 | sean-k-mooney | we proably cant fix the case where it did not have a volume before it was shelved eaisly | |
| 18:59:32 | sean-k-mooney | but we coudl fix this case. | |
| 18:59:46 | sean-k-mooney | can you write that up as a bug | |
| 18:59:58 | sean-k-mooney | sshmlbrg: im not sure if its somethign you feel comfortable trying to fix | |
| 19:00:20 | sshmlbrg | get availability_zone from RequestSpec for instance? | |
| 19:01:27 | sean-k-mooney | if its shelve offloaded | |
| 19:01:47 | sshmlbrg | I would like to fix it under your guidance :) I will create a bug tomorrow. | |
| 19:01:51 | sean-k-mooney | and if its not pinend then just proceed as if it passed i think | |
| 19:03:26 | sshmlbrg | sean-k-mooney: Thanks a lot for your attention. Stay in touch | |
| 19:04:34 | sean-k-mooney | sshmlbrg: no worreis, https://github.com/openstack/nova/blob/88a36a5a0099aa52f9ea93905c7c56911b2bf0b3/nova/objects/request_spec.py#L781-L791 is your starting point | |
| 19:05:01 | sean-k-mooney | you can use Requestspec.get_by_instance_uuid() to get the request spec object if its shelve offloaded | |
| 19:05:32 | sean-k-mooney | and then grab the az https://github.com/openstack/nova/blob/88a36a5a0099aa52f9ea93905c7c56911b2bf0b3/nova/objects/request_spec.py#L76 | |
| 19:05:56 | sean-k-mooney | if that is not none then you need to compare to the volume | |
| 19:06:09 | sean-k-mooney | if it is then i think returnign form the function might be ok | |
| 19:06:16 | sean-k-mooney | we will need to see what others think | |
| 19:11:46 | mikal | Morning | |
| 19:12:34 | sean-k-mooney | mikal: o/ | |
| 19:30:03 | opendevreview | Mengyang Zhang proposed openstack/nova-specs master: Add Burst Length Support to Cinder QoS https://review.opendev.org/c/openstack/nova-specs/+/932653 | |
| 20:58:10 | opendevreview | sean mooney proposed openstack/nova master: Deprecate linux bridge support https://review.opendev.org/c/openstack/nova/+/942291 | |
| 21:00:07 | sean-k-mooney | bauzas: ^ thats the nova side fo the deprecation | |
| #openstack-nova - 2025-02-20 | |||
| 02:47:14 | opendevreview | Merged openstack/os-vif master: Drop kuryr-kubernetes-tempest https://review.opendev.org/c/openstack/os-vif/+/941568 | |
| 02:51:43 | opendevreview | sean mooney proposed openstack/nova master: Add a new ImagePropertiesWeigher https://review.opendev.org/c/openstack/nova/+/940642 | |
| 02:51:44 | opendevreview | sean mooney proposed openstack/nova master: per-property ImageMetaPropsWeigher https://review.opendev.org/c/openstack/nova/+/941601 | |
| 03:00:17 | opendevreview | sean mooney proposed openstack/nova master: Per-Property ImageMetaPropsWeigher https://review.opendev.org/c/openstack/nova/+/941601 | |
| 04:27:10 | opendevreview | melanie witt proposed openstack/nova master: libvirt: Fix regression of listDevices() return type https://review.opendev.org/c/openstack/nova/+/942334 | |
| 04:28:55 | melwitt | sean-k-mooney, dansmith: partial revert is proposed ^. I wasn't sure if we should have a release note as I think none of the branches have been released yet with the regression? | |
| 04:30:57 | opendevreview | melanie witt proposed openstack/nova master: libvirt: Fix regression of listDevices() return type https://review.opendev.org/c/openstack/nova/+/942334 | |
| 04:35:18 | opendevreview | melanie witt proposed openstack/nova master: libvirt: Fix regression of listDevices() return type https://review.opendev.org/c/openstack/nova/+/942334 | |
| 11:21:20 | sean-k-mooney | melwitt: i think your right that we have not had a release with it. its possibel someone picked it up from master or stabel in a downstream or directly but its unlikely. | |
| 14:54:03 | dansmith | gibi: did you want to re-review the imageprops weigher or should I +W? | |
| 15:11:56 | opendevreview | Johannes Kulik proposed openstack/nova master: libvirt: fix maxphysaddr passthrough dom parsing https://review.opendev.org/c/openstack/nova/+/942371 | |
| 15:26:37 | gibi | dansmith: go ahead I will have no time today to get back to it. I did not review the original impl either just discussed the high level design. | |
| 15:26:46 | dansmith | okay | |
| 15:27:07 | gibi | dansmith: I think we are in agreement of the design, and I trust your review about the impl | |
| 15:27:14 | dansmith | gibi: ack | |
| 15:27:36 | dansmith | bauzas: if you want to +W the patch below the first two can land, waiting on a recheck of the third | |
| 15:27:48 | bauzas | dansmith: sure | |
| 15:45:53 | sean-k-mooney | while we are talking about merging things can someone +2w this https://review.opendev.org/c/openstack/os-vif/+/941585 so i can propose a os-vif release | |
| 15:46:57 | sean-k-mooney | we need to do that today so we can bump it in uc before the non clinet lib freeze | |
| 15:47:42 | sean-k-mooney | also once https://review.opendev.org/c/openstack/nova/+/940873 is merged we can merge the spice direct series | |
| 15:50:50 | sean-k-mooney | well almost we also need to merge https://review.opendev.org/c/openstack/tempest/+/942023 but thats approved | |