| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-05-02 | |||
| 16:38:00 | bauzas | my only concern is the traits inflation but that's a string | |
| 16:38:02 | sean-k-mooney | and report it statically if we are above that | |
| 16:38:09 | sean-k-mooney | i dont think we need a cofnig option | |
| 16:38:17 | bauzas | sean-k-mooney: that's why I was considering option 3 | |
| 16:38:35 | dansmith | sean-k-mooney: yeah, that's just a little annoying I think | |
| 16:38:39 | bauzas | which would be "meh dude, you don't have what I need, I'll just do what I can do" | |
| 16:38:45 | sean-k-mooney | bauzas: i dont think optional encypting is accpetable | |
| 16:38:47 | dansmith | because it becomes not as much a feature flag but a shadow version number | |
| 16:39:22 | bauzas | sean-k-mooney: true | |
| 16:39:28 | sean-k-mooney | if you asked for the storage to be enypeed we either need to do it or raise an error | |
| 16:40:05 | bauzas | sounds then reasonable to ERROR the instance | |
| 16:40:12 | enriquetaso | what a `new trait` involves? | |
| 16:40:18 | sean-k-mooney | dansmith: im not agaisn a min compute service version check in the api as well by the way | |
| 16:40:24 | bauzas | that's option 3 | |
| 16:40:25 | sean-k-mooney | i dont really think 2 is harsh | |
| 16:40:29 | bauzas | s/3/2 | |
| 16:40:46 | sean-k-mooney | we normally dont enabel feature untill the cloud is fully upgraded | |
| 16:40:54 | dansmith | I'm not saying that's how it needs to be, I'm just saying it feels like we're bordering on trait abuse here | |
| 16:40:56 | dansmith | so FWIW, | |
| 16:41:10 | sean-k-mooney | we have in the past done this on a per compute host bassis | |
| 16:41:15 | dansmith | we could also have a scheduler filter that requires a service version at or above a number | |
| 16:41:20 | sean-k-mooney | but in generall i think a min comptue version check is preferable | |
| 16:41:23 | dansmith | and we could add hints/advice to the scheduler for this sort of thing | |
| 16:41:31 | dansmith | which would be nice for this and other things I imagine | |
| 16:41:49 | bauzas | yeah this sounds quite a reasonable tradeoff | |
| 16:42:03 | sean-k-mooney | this being? | |
| 16:42:20 | bauzas | I'm just wondering whether we expose the service version on the scheduling side | |
| 16:42:31 | sean-k-mooney | i think you can already schedul based on the comptue service version | |
| 16:42:45 | sean-k-mooney | with either the json of comptue capablity filter | |
| 16:42:58 | sean-k-mooney | but in any case we want this to work without any configuration requried | |
| 16:43:20 | sean-k-mooney | bauzas: why not just do 2? | |
| 16:43:24 | gibi | I might miss something but if this feature nees compute code + libirt/qemu version then as simple compute version check is not enough | |
| 16:43:34 | dansmith | sean-k-mooney: Im saying make it integrated | |
| 16:43:44 | dansmith | sean-k-mooney: kinda like a prefilter | |
| 16:43:56 | bauzas | wait https://github.com/openstack/nova/blob/master/nova/scheduler/request_filter.py#L399 | |
| 16:44:17 | dansmith | gibi: yeah, you need service version and libvirt version and qemu version right? | |
| 16:44:20 | sean-k-mooney | so if feature x requries minv version y have a prefilter or simialr that will request a host with that min version | |
| 16:44:25 | bauzas | that's ephemeral encryption | |
| 16:44:50 | sean-k-mooney | ya thats seperate | |
| 16:44:56 | sean-k-mooney | we are talkign about encyption for nfs | |
| 16:45:09 | sean-k-mooney | for cinder volume backend that use nfs | |
| 16:45:10 | dansmith | gibi: that's kinda why I just think this is trait pollution because we end up with all these feature flags for any possible combination of several versions | |
| 16:45:24 | dansmith | and especially in three years, that trait is useless as everything exposes it all the time | |
| 16:45:55 | dansmith | so I'm not sure what the best plan is here, to be clear, I'm just saying none of these simple things feels right | |
| 16:46:07 | sean-k-mooney | dansmith: not really usign a trait we report an abstract capblity. but in general i would prefer both a trait and a compute version bump | |
| 16:46:43 | dansmith | that's the thing though: | |
| 16:47:02 | sean-k-mooney | enriquetaso: how is this feature requested on teh volume by the way? | |
| 16:47:08 | dansmith | just exposing a "can do nfs encryption" because all the versions are new enough just feels like we could have a thousand of those things | |
| 16:47:20 | enriquetaso | when attaching the volume sean-k-mooney | |
| 16:47:35 | sean-k-mooney | how exactly a atribute on the volume | |
| 16:47:44 | sean-k-mooney | enriquetaso: we have two distict but related problems | |
| 16:47:55 | sean-k-mooney | for attachemtn we know the host and have to check if the host suprot this | |
| 16:48:03 | sean-k-mooney | for new boots or move operattions | |
| 16:48:13 | sean-k-mooney | we need to find another host that also supports it | |
| 16:48:38 | bauzas | that's why I tend to lean on option 2 | |
| 16:48:40 | sean-k-mooney | and we need this to work without any operator configurign of aggreates ectra | |
| 16:48:46 | bauzas | it's an interim solution | |
| 16:48:54 | dansmith | bauzas: me too, but 2 is not enough right? | |
| 16:49:07 | sean-k-mooney | option 2 works if an only if our min libvirt/qemu version are new enough | |
| 16:49:10 | dansmith | because you could be running an older libvirt/qemu | |
| 16:49:11 | enriquetaso | mmh, the volume attr says it's encrypted and the volume image format is qcow2: https://review.opendev.org/c/openstack/nova/+/854030/6/nova/virt/libvirt/utils.py | |
| 16:49:18 | dansmith | and I suspect it also depends on brick versions? | |
| 16:49:24 | enriquetaso | not sure to understand the questions | |
| 16:49:42 | sean-k-mooney | enriquetaso: i was asking because if we are booting a new vm we would need to look that up | |
| 16:49:55 | sean-k-mooney | and then include it as an input to placemnt and the schduler in some way | |
| 16:50:00 | bauzas | dansmith: hah, true, I was considering an API check, but that would require the libvirt versions, so nevermind my foolness | |
| 16:50:24 | bauzas | yeah, so that'a tuple (libvirt, compute) for accepted versions | |
| 16:50:26 | sean-k-mooney | well it may or may not again it depend on if our min version is above or below the ersion that intoduced it | |
| 16:50:44 | bauzas | you need libvirt AND compute versions to be recent enough | |
| 16:50:44 | enriquetaso | oh, the volume doesnt have anything special, it just volume type=nfs and encrypted=true sean-k-mooney | |
| 16:51:22 | dansmith | sean-k-mooney: yeah, well, that's a good reason not to just add the compute+libvirt+qemu into a trait IMHO | |
| 16:51:35 | sean-k-mooney | dansmith: right which is not what i suggested | |
| 16:51:57 | dansmith | I know :) | |
| 16:51:57 | sean-k-mooney | i very explictly said have a triat for the capablity to supprot lux in qcow | |
| 16:52:06 | gibi | dansmith: on the trait pollution: either we encode a list of versions as a capability on the compute side, or we expose those versions to the scheduler / placement and code up a similar mapping of capability - versions in the sceduler side. We just move around similar logic | |
| 16:52:40 | dansmith | gibi: yeah, understand, it's just that traits are supposed to be timeless right? | |
| 16:52:47 | bauzas | we have 5 mins to find a solution or defer to a spec, honestly | |
| 16:53:00 | dansmith | I'm not saying I know which solution (combination) is best, I'm just saying nothing feels particularly natural to me | |
| 16:53:19 | sean-k-mooney | so i was suggestign have the driver check the requirements and report COMPUTE_LUKS_IN_QCOW if it supprots it | |
| 16:53:50 | sean-k-mooney | and have a prefilter requesst that if the voluem was nfs and qcow and encrypted=true | |
| 16:54:01 | gibi | dansmith: we will have a bunch of traits yes, but I don't see what problem that causes. We never remove min compute version checks from the code either | |
| 16:54:07 | sean-k-mooney | and addtional have a min compute service check for the rooling upgrade case | |
| 16:54:36 | sean-k-mooney | the other thing about traits is they are ment to be virt driver independent | |
| 16:55:08 | gibi | LUKS_IN_QCOW does not seem to be libvirt dependent | |
| 16:55:13 | sean-k-mooney | to dans timeless point i.e. if we add one it shoudl be resuable by other virt driver | |
| 16:55:19 | sean-k-mooney | ya i was just thinking that | |
| 16:55:25 | sean-k-mooney | its the qcow bit but | |
| 16:55:32 | sean-k-mooney | we have that alredy | |
| 16:56:17 | sean-k-mooney | https://github.com/openstack/os-traits/blob/master/os_traits/compute/ephemeral.py#L18 and https://github.com/openstack/os-traits/blob/master/os_traits/compute/image.py#L28 | |
| 16:56:23 | sean-k-mooney | almost would work togather | |
| 16:56:43 | sean-k-mooney | btu we cant assume that epmeral encryption supprot for lux means nfs also works | |
| 16:56:58 | enriquetaso | LUKS_IN_QCOW is already a config option on Nova gibi ? | |
| 16:57:13 | sean-k-mooney | enriquetaso: not really | |
| 16:57:23 | sean-k-mooney | or not that im aware of | |
| 16:57:35 | sean-k-mooney | in the context of cinder | |
| 16:57:39 | gibi | enriquetaso: we are discussing creating a new placement trait to represent if a compute supports luks in qcow | |
| 16:57:54 | enriquetaso | gibi++ thanks | |
| 16:58:19 | bauzas | I honestly think we need to settle the dust | |
| 16:58:49 | bauzas | given the very short time we have left I hereby propose enriquetaso to create a spec and describe the feature | |
| 16:58:58 | bauzas | we could then chime on the upgrade concerns | |