Earlier  
Posted Nick Remark
#openstack-nova - 2024-05-20
19:22:09 sean-k-mooney or decypt the image
19:22:15 melwitt the main thing is I think we want to avoid an unpleasant surprise on that. similar to the encrypted rescue image, not to just decrypt it
19:22:47 melwitt I think the desire based on current code review comments is that it would decrypt in that case
19:22:51 sean-k-mooney personally im feeling like hw:ephemeral_encryption=false and os_glance_encrypt_format=luks woudl be a 409 conflict at least for now
19:23:09 melwitt yeah, I think it would be fair to start with that
19:23:50 sean-k-mooney the reason i hesitate there is it would allow anyoen in the same project (realy that has access to the image secret) to trivially/acidentaly decypt the image
19:24:14 melwitt and tack on a separate patch that enables the ability to request decryption
19:24:18 melwitt because it's like a user experience thing. not really "required" but nice to have to give users that ability
19:24:23 sean-k-mooney its not uncommon for peopel to query flavors by size and select the first one that they find
19:24:30 melwitt yeah. that would be my only concern with it as well
19:25:03 sean-k-mooney to be fair if thte image had os_glance_encrypt_format=luks hw_ephemeral_encryption=ture
19:25:27 sean-k-mooney i think we all agree it should be a 409 conflict if the flavor has hw:ephemeral_encryption=false
19:26:00 sean-k-mooney but since the arguement is being made that os_glance_encrypt_format=luks effectivly implies hw_ephemeral_encryption=ture
19:26:33 sean-k-mooney im wonderin if we shoudl also imply it when comparing ot a flavor with hw:ephemeral_encryption=false
19:26:35 melwitt I expect we all agree that's an ok first step. we've been trying to keep patches smaller in this thing as well so I have been splitting smaller things like that into their own patch
19:26:49 sean-k-mooney ok
19:27:28 sean-k-mooney so before this converstaion i was tending to say "ok ill approve the spec and we can take it to the implemeation" no i feel like we were not on the same page but have got much closer
19:27:45 sean-k-mooney so my stomach say i shoudl have breakfast/dinner
19:27:55 sean-k-mooney but i also dont really want to leave this open
19:28:04 sean-k-mooney so melwitt how do you want to proceed
19:28:18 melwitt yeah so I was thinking if os_glance_encrypt_format=luks (and really mainly just the presence of os_glance_encrypt_key_id) then if hw:ephemeral_encryption=false is in direct conflict and would act the same as the image conflict. until we offer decryption later if we do that
19:29:29 sean-k-mooney ack i think that is logicaly consitent
19:30:03 sean-k-mooney i feel like os_glance_encrypt_key_id is only valid if all the other os_glance_encrypt_* are dfeind
19:30:15 melwitt sean-k-mooney: I think it would probably be best to update the spec now. either way I will update it -- if it gets approved as-is I will still have an amendment proposed shortly. to not forget all this stuff
19:30:17 melwitt so either way is fine with me
19:30:26 melwitt I agree. I think that is the intent in the standardized properties anyway
19:30:29 sean-k-mooney so its not clear to me if we shoudl be checking any spefici one of those keys or if any of them exist
19:32:16 melwitt yeah.. I'll probably add validation for that just to make a better experience. I'm not sure if they are all required to be set from a glance perspective. like, will glance reject upload of an image that only sets one or a subset of them? or maybe if it does allow it, it will fill it defaults itself
19:32:29 melwitt so either way on the nova side we would expect/require them all to be set if one is set
19:32:38 sean-k-mooney so the glance spec used ot have a seperate nova spec https://review.opendev.org/c/openstack/nova-specs/+/608696/11/specs/train/approved/image-encryption.rst
19:32:43 sean-k-mooney for how we would use those
19:33:27 melwitt from my perspective at a minimum if os_glance_encrypt_key_id is set, then os_glance_encrypt_format is required
19:33:31 sean-k-mooney in that they explictly talk about adding supprot for decypting the encyped image when booting the instnace
19:33:59 sean-k-mooney melwitt: ya i think those two are requried and likely the type as well i.e. symetic
19:34:15 melwitt ah ok
19:34:17 sean-k-mooney having said that i think we are only going to support symmetric encption
19:34:26 melwitt ah right
19:34:30 sean-k-mooney so i guess if its single valued
19:34:34 sean-k-mooney it does not matter for now
19:36:01 sean-k-mooney melwitt: you know what would help
19:36:14 melwitt yeah, we can adapt that as we go along. if we think more than just those two should be required to be present to be valid
19:36:18 melwitt because the other use case is an individual user uploading an encrypted image
19:36:26 sean-k-mooney in your spec can you define the only values of the os_glance_encrypt* properties that will actully work for now
19:36:28 melwitt so not sure if they will be required to provide all image properties
19:37:53 sean-k-mooney like will you be using os_glance_decrypt_size?
19:38:11 sean-k-mooney i dont think you are your using virt_size + 1G in the check right
19:38:15 melwitt sure I can add specifics about which ones we are requiring
19:38:35 melwitt there is a check for that somewhere actually
19:38:39 sean-k-mooney well it might hjsut be good to say hay nova is not using os_glance_decrypt_size
19:39:12 melwitt today we validate image size in some way and I had to add the "plus overhead" to the calc if encrypted for it to pass
19:39:19 sean-k-mooney right
19:39:41 sean-k-mooney but if i rememebr reviewing that code we do not use os_glance_decrypt_size
19:40:22 melwitt yeah I guess really i could use os_glance_decrypt_size instead for the validation if the image is encrypted. since that's what we actually care about, is if the decrypted size will fit in the chosen flavor
19:40:46 melwitt no we didn't have os_glance* properties proposed at the time
19:40:49 sean-k-mooney os_glance_decrypt_size and os_glance_decrypt_container_format i think are for the usecase of booting form an encypted iamge and decypting it on the host
19:40:56 melwitt well, sorry
19:41:03 sean-k-mooney which would imply they expect the virtual_size to include the overhead
19:41:06 melwitt they were proposed already but we were not planning to use them initially
19:41:09 sean-k-mooney when its encypted
19:41:24 melwitt but at the ptg we agreed to use them so I will be changing where needed to use those instead of what I had put
19:41:52 sean-k-mooney sure but os_glance_decrypt_size is defied as "size after payload decryption"
19:42:07 sean-k-mooney which would be the size of the data insid ethe luks encyption
19:42:15 sean-k-mooney not the size on disk fo the encypted images
19:42:45 sean-k-mooney so i dont think it actully helps us
19:42:52 melwitt right.. maybe I'm getting confused
19:43:11 sean-k-mooney i think this is explcitly for the usecase of booting non encypted instance form an encypted image
19:43:26 melwitt the point I was meaning really is I'll use them when applicable
19:43:26 sean-k-mooney so that we woudl know how big it is after we strip the encyption from it
19:43:34 sean-k-mooney ack
19:44:04 sean-k-mooney ok ill chat ot you tomorrow o/
19:44:14 melwitt ok I see what you're saying
19:44:18 melwitt ok, seeya tomorrow o/
21:41:25 opendevreview Merged openstack/nova master: Enable virtio-scsi in nova-next https://review.opendev.org/c/openstack/nova/+/918458
#openstack-nova - 2024-05-21
04:36:36 opendevreview melanie witt proposed openstack/nova-specs master: Re-propose specs for ephemeral encryption https://review.opendev.org/c/openstack/nova-specs/+/907654
06:05:22 opendevreview xielijie proposed openstack/nova master: Fix Timestamp Inconsistency During VM Creation https://review.opendev.org/c/openstack/nova/+/920074
07:50:16 opendevreview Andrew Bonney proposed openstack/nova master: Set port dns_name when unshelving instances https://review.opendev.org/c/openstack/nova/+/919961
07:52:34 opendevreview Andrew Bonney proposed openstack/nova master: Set port dns_name when unshelving instances https://review.opendev.org/c/openstack/nova/+/919961
08:59:22 opendevreview Merged openstack/nova-specs master: Add openapi spec https://review.opendev.org/c/openstack/nova-specs/+/909448
09:18:25 opendevreview Andrew Bonney proposed openstack/nova master: Don't reset port dns_name when shelving instances https://review.opendev.org/c/openstack/nova/+/919961
10:59:43 opendevreview sean mooney proposed openstack/nova master: ensure correct cleanup of multi-attach volumes https://review.opendev.org/c/openstack/nova/+/916322
10:59:43 opendevreview sean mooney proposed openstack/nova master: add functional repoducer for bug 2048837 https://review.opendev.org/c/openstack/nova/+/919979
11:11:17 opendevreview sean mooney proposed openstack/nova master: fix py312 tox definitions https://review.opendev.org/c/openstack/nova/+/920090
11:11:18 opendevreview sean mooney proposed openstack/nova master: tox: Drop envdir https://review.opendev.org/c/openstack/nova/+/920091
11:12:57 sean-k-mooney dansmith: can you look at https://review.opendev.org/c/openstack/nova/+/907591 and https://review.opendev.org/c/openstack/nova/+/920090/1 to fix our tox usage when your about
13:30:23 opendevreview sean mooney proposed openstack/nova master: ensure correct cleanup of multi-attach volumes https://review.opendev.org/c/openstack/nova/+/916322
13:30:23 opendevreview sean mooney proposed openstack/nova master: add functional repoducer for bug 2048837 https://review.opendev.org/c/openstack/nova/+/919979
13:38:57 dansmith sean-k-mooney: ah sorry I had made the tox change locally and I guess I didn't get it into my commit, sorry
13:39:25 sean-k-mooney no worries i just noticed it was running the wrong thing when i saw a ci run fail
13:41:27 sean-k-mooney on the plus side the job is still passing when its runing to correct content so looks like we have not regressed since we merged it
15:17:46 opendevreview Merged openstack/nova master: tox: Drop envdir https://review.opendev.org/c/openstack/nova/+/907591
16:09:38 dansmith just did my fifth recheck trying to get the de-UEC patch merged.. none of the failures were kernel crashes, but dang.. seems bad the last couple days
16:19:40 sean-k-mooney is that includign the one i did this morning
16:19:52 sean-k-mooney but ya im not sure what has changed
16:57:38 opendevreview sean mooney proposed openstack/nova master: add functional repoducer for bug 2065927 https://review.opendev.org/c/openstack/nova/+/920119
17:13:59 opendevreview Merged openstack/osc-placement master: tox: Drop envdir https://review.opendev.org/c/openstack/osc-placement/+/907596
17:43:29 opendevreview Merged openstack/nova master: fix py312 tox definitions https://review.opendev.org/c/openstack/nova/+/920090
18:19:33 opendevreview Alexey Stupnikov proposed openstack/nova master: [doc] Improve description for nova-manage db purge https://review.opendev.org/c/openstack/nova/+/919746
18:19:48 opendevreview Merged openstack/placement master: tox: Drop envdir https://review.opendev.org/c/openstack/placement/+/907590
#openstack-nova - 2024-05-22
02:44:52 opendevreview Merged openstack/nova master: [doc] Improve description for nova-manage db purge https://review.opendev.org/c/openstack/nova/+/919746
07:06:23 opendevreview Rajesh Tailor proposed openstack/nova master: Handle neutron-client conflict https://review.opendev.org/c/openstack/nova/+/918048

Earlier   Later