Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-11-07
16:19:46 dansmith bauzas: second review I mean?
16:19:48 bauzas https://review.opendev.org/q/(project:openstack/nova-specs)+status:open+NOT+owner:self+NOT+label:Workflow%253C%253D-1+label:Verified%253E%253D1%252Czuul+NOT+reviewedby:self+is:mergeable+NOT+label:Code-Review%253C%253D-1%252Cnova-core+label:Code-Review%253E%253D2
16:19:59 bauzas not sure the link will render correctly
16:20:11 bauzas but tl;dr: dansmith you have the sole spec that needs a second +2 :)
16:20:37 bauzas and yeah, there are reproposals on the way
16:20:44 bauzas btw. I wonder why they don't show up
16:21:09 dansmith bauzas: ah bummer.. maybe gibi can circle back on that .. that's the only one I can't finish :)
16:21:14 bauzas https://review.opendev.org/q/project:openstack/nova-specs+status:open+label:Code-Review%253E%253D2
16:21:31 dansmith but yeah I'll look at the re-proposals
16:21:38 bauzas sorry, my dash link seems to be wrong
16:21:59 bauzas fwiw, had no time yet to correctly write a mdev live-migration spec
16:22:01 auniyal6 on CI bugs, nova-emulation job is failing too (I wanted to check if its failing for more patches, but builds page is not opening for me right now )
16:22:04 bauzas I'll bug folks directly :)
16:22:04 sean-k-mooney i can also try and loop back
16:22:09 sean-k-mooney ill be doing more review later today
16:22:21 sean-k-mooney dansmith: on your spec
16:22:41 johnthetubaguy (I keep trying to do the ironic re-proposal, but I keep getting distracted, sorry)
16:22:43 opendevreview Dmitriy Rabotyagov proposed openstack/nova stable/2023.1: Fix rebuild compute RPC API exception for rolling-upgrades https://review.opendev.org/c/openstack/nova/+/900336
16:22:44 opendevreview Dmitriy Rabotyagov proposed openstack/nova stable/2023.1: Adding server actions tests to grenade-multinode https://review.opendev.org/c/openstack/nova/+/900337
16:22:58 bauzas gibi: you left a +1 because of Uggla_'s concern but AFAICR, it was resolved
16:23:11 bauzas so I just went +2
16:23:18 gibi bauzas: OK, thanks
16:23:31 opendevreview Dmitriy Rabotyagov proposed openstack/nova stable/2023.2: add a regression test for all compute RPCAPI 6.x pinnings for rebuild https://review.opendev.org/c/openstack/nova/+/900309
16:23:40 bauzas okay, moving on
16:24:21 bauzas dvo-plv wanted us to do a group discussion on https://review.opendev.org/c/openstack/nova-specs/+/895924/ since I asked to *not* use a trait but let's not discuss this now
16:24:32 opendevreview Dmitriy Rabotyagov proposed openstack/nova stable/2023.2: Fix rebuild compute RPC API exception for rolling-upgrades https://review.opendev.org/c/openstack/nova/+/900338
16:24:33 opendevreview Dmitriy Rabotyagov proposed openstack/nova stable/2023.2: Adding server actions tests to grenade-multinode https://review.opendev.org/c/openstack/nova/+/900339
16:24:41 bauzas if people want, we could discuss this during open discussion
16:25:17 bauzas (or just read my gerrit comments, you'll get my point, which is please avoid adding traits for just a libvirt version check that's already supported for all computes except Antelope and olders)
16:25:53 bauzas anyway, moving on
16:26:16 dansmith yeah
16:26:28 dansmith needs to be capability-based, IMHO
16:26:38 dansmith "supports X" not "is version X"
16:27:09 sean-k-mooney yep
16:27:17 sean-k-mooney so supprot virtio-packed format
16:27:20 sean-k-mooney is a capablity
16:27:22 bauzas I said we could resolve that with a service version check
16:27:22 sean-k-mooney not a version check
16:27:27 sean-k-mooney so it shoudl be a trait
16:27:35 sean-k-mooney and we shoudl not need a comptue service check
16:27:43 bauzas sean-k-mooney: that's not exactly what's written in both code and spec
16:27:48 sean-k-mooney as this is not a feature that is impemnted at the comptue mager level
16:27:56 bauzas the requirement is "is libvirt >6.7"
16:28:12 sean-k-mooney right that is the requireemtn for the feature too work
16:28:33 bauzas since bobcat supports 7.0, we're good
16:28:35 sean-k-mooney but it is modeled by reportign a triat for the host where the capablitys is supproted
16:28:38 sean-k-mooney yep
16:28:51 sean-k-mooney but we now need to supprot n-2 and diffent oss
16:29:00 sean-k-mooney *operating systems
16:29:02 bauzas so it only leaves the rolling upgrade case with a caracal env + an antelope node
16:29:21 bauzas which will no longer be a problem with D
16:29:29 sean-k-mooney well this featur like many other can be docusmeed as only supproted after a full upgrade is complete
16:29:49 bauzas so again, I'm very against adding a trait for modeling a libvirt version that's gonna be minimum for all our computes next cycle anyway
16:29:59 sean-k-mooney bauzas: that not what its modeling
16:30:27 sean-k-mooney and its required in my view for as long as we have any virt driver (other then ironic and libvirt) that supprot vms
16:30:36 bauzas https://review.opendev.org/c/openstack/nova/+/876075/25/nova/virt/libvirt/driver.py
16:30:57 bauzas that exactly models the libvirt version
16:31:14 dansmith not really
16:31:16 sean-k-mooney its the only way to detect it because libvirt does not have a way to detect it without a version check
16:31:19 sean-k-mooney but
16:31:26 sean-k-mooney since we now have the min versio nrequiremnet
16:31:31 dansmith it's exposing whether or not the feature is supported, but the way it does that is by the libvirt version internally
16:31:40 sean-k-mooney this would be a static trait that is exposed by computes using the libvirt driver
16:31:42 dansmith that's different than exposing a trait of the actual version
16:31:52 sean-k-mooney dansmith: exactly
16:31:54 bauzas sean-k-mooney: then I'd suggest a prefilter that would only ensure that we land on a libvirt hostr
16:32:08 dansmith the prefilter can be the trait though
16:32:15 sean-k-mooney bauzas: i belive the prefilter was aldreay in the spec
16:32:34 sean-k-mooney bauzas: its somethign i have defintly dicussed for this feature previously
16:32:37 bauzas yeah, but again, I don't like us adding yet another trait for this
16:32:56 sean-k-mooney bauzas: i belive this is exactly what traits shoudl be used for
16:32:58 dansmith but this is what we do right? all the other traits are just like this I think
16:33:08 sean-k-mooney dansmith: more or less
16:33:17 bauzas then, let's just provide this trait without a conditional
16:33:28 sean-k-mooney bauzas: yes i also said that in my comments
16:33:40 sean-k-mooney and i said this last cycle too when we origianlly proposed it
16:33:41 bauzas if it's really about directing to libvirt nodes
16:33:41 dansmith we can do that if we hard-require the min version
16:34:20 sean-k-mooney bauzas: basically the context yoru missing is we had prviously agree that if we enforced the min verison we would update the sepc depending on which feature landed first
16:34:27 sean-k-mooney kasyaps min version bump
16:34:29 sean-k-mooney or this feature
16:34:48 sean-k-mooney in the repopoal it should be updated to drop the check because the min verion bump already happend in bobcat
16:35:05 dansmith so to be clear, we're already hard-requiring the min version needed for this, and thus the trait is just for the upgrade case where we might have old computes?
16:35:11 opendevreview Dmitriy Rabotyagov proposed openstack/nova stable/zed: Fix rebuild compute RPC API exception for rolling-upgrades https://review.opendev.org/c/openstack/nova/+/900341
16:35:12 opendevreview Dmitriy Rabotyagov proposed openstack/nova stable/zed: Adding server actions tests to grenade-multinode https://review.opendev.org/c/openstack/nova/+/900342
16:35:21 bauzas what *you* missed is that I figured this out before the meeting (that spec is older than our min bump) but I missed the fact we want to direct to libvirt-only nodes, hence my mistake
16:35:28 sean-k-mooney dansmith: upgrade case or where your mixing virt drivers
16:35:41 sean-k-mooney dansmith: but use oru current min is 7.0.0 i belvie and the feature is in 6.x
16:35:48 dansmith sean-k-mooney: okay libvirt and ironic being the only possibilities there :)
16:35:50 bauzas dansmith: the former (upgrade case) was my concern
16:35:58 bauzas sean-k-mooney: it requires 6.7
16:36:26 dansmith so I mean, I would probably lean towards just a service version check to not let this work until everything is upgraded, but the multi-virt driver thing is a fair point
16:36:27 sean-k-mooney bauzas: yep i said 6.x because i know we met the min requirement with our min supported version
16:36:31 bauzas I don't want us to buy a new trait for something that's only because we support N-2 this cycle
16:36:40 dansmith and even though we're dropping basically all the others, we could have a new one in the future without this support, so...
16:36:57 sean-k-mooney dansmith: well we dont need a new compute service verion for this feature in general
16:37:24 sean-k-mooney altough i guess we coudl do oen for the new prefilter
16:37:27 bauzas dansmith: so you're on the same page than me, a service version check for upgrades is enough, but if we really want to avoid other hypervisors we could need a trait
16:37:28 dansmith sean-k-mooney: we don't need it, but we could use it (cheaper than a trait) for the usual purpose of not exposing features until everything is upgraded
16:37:51 dansmith bauzas: no, I said I lean that way, but I'm also fine with a trait because of the virt driver possibility

Earlier   Later