Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-11-07
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
16:37:52 sean-k-mooney dansmith: oh i disagree i alwasy conisderd a comptue version bump more expensive or at most the same
16:38:17 dansmith sean-k-mooney: well, we disagree then.. we bump service versions all the time for stuff like this, where there isn't even a rpc bump to correlate
16:38:37 dansmith and it is a single integer in one tree versus a new enum in traits, a package release, a dep update, and then a nova patch
16:38:39 bauzas tbh, I'd rather prefer having the prefilter asking 'get me a libvirt compute' rather than 'get me a compute that supports foo' since this feature is very QEMU-centric
16:39:05 bauzas sean-k-mooney: the fact is, this service version check can drop next cycle
16:39:05 sean-k-mooney bauzas: that would be a misuse of traits
16:39:26 bauzas sean-k-mooney: starting with D, all computes will support a libvirt recent enough
16:39:29 dansmith sean-k-mooney: how is that a misuse of traits?
16:39:32 sean-k-mooney if we want to also have a servic verion bump and an additon check in the api prior to the call to the schduler we can
16:39:52 sean-k-mooney dansmith: we previosly said we didnt want to have a trait for which virt driver is in use
16:39:58 bauzas if two virt drivers were proposing the same feature, then yah a trait sounds good to me
16:40:07 sean-k-mooney we can revert that if we want but i know there was pushback to that in the past
16:40:30 bauzas but here that capability is purely qemu-based
16:40:37 sean-k-mooney bauzas: so is the only reason your takign this stance because we did the min libvirt bump last cycle
16:40:43 dansmith sean-k-mooney: we have traits for which type of hardware is on the host, this seems similar, but we can also filter on hypervisor type from host state anyway right? so we can do it without a trait
16:40:50 bauzas say some other feature does the same, we gonna add another trait and another prefilter
16:41:19 sean-k-mooney bauzas: yes we should anytime thre is a capablity that is not supproted by all supproted drivers
16:41:25 bauzas and boom, traits explosion, plus the fact we yet again push hypervisor features upfront
16:41:44 sean-k-mooney bauzas: traits are cheap and placment was built to deal with many of them
16:41:59 dansmith honestly, this particular feature is probably not critical, right? meaning:
16:42:13 bauzas service version checks that can get rid next cycle are cheaper IMHO and,
16:42:21 bauzas no traits is cheaper than a single one :)=

Earlier   Later