Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-09-30
16:57:20 r-taketn Thank you
16:57:28 elodilles thanks o/
16:58:10 gibi o/
16:58:42 sean-k-mooney Uggla: im triaging https://bugs.launchpad.net/nova/+bug/2125738 as low but its valid just an fyi
16:58:42 elodilles Uggla: fyi, the nova ptg etherpad link is not updated on the ptg page: https://ptg.opendev.org/etherpads.html
16:59:05 opendevreview Lajos Katona proposed openstack/nova master: Use SDK for Neutron floating IPs https://review.opendev.org/c/openstack/nova/+/962604
16:59:17 elodilles Uggla: i think it can be updated via the openinfraptg bot in #openinfra-events channel
16:59:41 sean-k-mooney its a fairly simpel fix in out bindep.txt to account for the package change
17:00:45 lajoskatona Uggla: Hi, just a quick note that I slowly progress with the migration to SDK from neutronclient hell: https://review.opendev.org/q/topic:%22sdk_for_neutron%22 , after we are over the release week we can discuss how should it be consumed the easiest way :-)
17:00:58 sean-k-mooney its not actully a bug requriing nova code change really beyond bindeps.txt to just document the relevent dep
17:01:45 sean-k-mooney lajoskatona: ya you have broken it up that helps
17:02:18 sean-k-mooney i likmed the orginal patch and saw you were updating it but i have not looked at it in any detial
17:02:34 sean-k-mooney s/likmed/skimed/
17:04:43 sean-k-mooney lajoskatona: we have a related topic for the wtacher ptg about how/wen to start the convertion tothe sdk there. there is a depency on the neutron client in watcher too https://github.com/openstack/watcher/blob/master/requirements.txt#L38
17:05:28 sean-k-mooney but really not much of one today so i was sugesting we start with movaing the novaclinet and perhaps other over to the sdk next cycle
17:06:09 Uggla sean-k-mooney, elodilles I will look at above later.
17:09:11 lajoskatona sean-k-mooney: The thing that I run into and have to think about is that in n-client there are plenty of exotic exceptions like overquota and similar, but SDK has only basic exceptions like conflict
17:10:09 lajoskatona at the moment I am not sure if using the simple conflict, notfound etc is enough or there is a need for cover the neutronclient exotic exceptions in SDK for example
17:11:57 sean-k-mooney lajoskatona: ah so we may need to add some more detailed excption n nova for those diffent usecases to make the rest of the logic continue to work the same
17:12:31 sean-k-mooney and your tryign to understand if that is the correct approch or if the nova code should be adapted to the geneirc expctions?
17:12:46 sean-k-mooney it that the crux of the disucssion you want to have
17:13:24 sean-k-mooney i.e. adtapt the generic excption to specific ones locally at the callsite or not?
17:16:20 sean-k-mooney lajoskatona: for the most part as you have seen here https://review.opendev.org/c/openstack/nova/+/928022/12/nova/network/neutron.py#2933 we have tired to convert the neutron client excption into nova specific ones rather then propagate them outside ot the nova.network.neutron module
17:16:56 sean-k-mooney so ideally code outside of nova/network/neutron.py shoudl not need to deal with sdk or neutron client excptions
17:17:08 sean-k-mooney lajoskatona: alhtough i dont know if that is actully what you are asking
17:19:11 lajoskatona sean-k-mooney: yes that is my problem now, I think the fresh patch for FIPs has some of these cases
17:19:14 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/962604/1/nova/network/neutron.py#b2949
17:19:20 sean-k-mooney this is perhaps a good example
17:19:56 sean-k-mooney before if it was not a client excption we loged the ecetpion and reaised
17:20:37 lajoskatona or some real exotic ones: https://review.opendev.org/c/openstack/nova/+/962604/1/nova/network/neutron.py#3100 :-)
17:20:42 sean-k-mooney we still proably shoudl actch other clent excptions and log them and then raise them again
17:21:06 sean-k-mooney ah
17:21:17 sean-k-mooney except (neutron_client_exc.IpAddressGenerationFailureClient, neutron_client_exc.ExternalIpAddressExhaustedClient
17:21:30 lajoskatona SDK has general httpException, that can be used for such
17:22:35 opendevreview Takashi Kajinami proposed openstack/nova master: AMD SEV: omit iommu='on' for virtio devices https://review.opendev.org/c/openstack/nova/+/909635
17:22:38 sean-k-mooney so neutron_client_exc.OverQuotaClient apars ot be the oen that is missing
17:22:55 sean-k-mooney right we are nologner rasign FloatingIpLimitExceeded
17:23:35 sean-k-mooney so for partity we would need to disambiuage the sdk_exc.ConflictException
17:23:57 sean-k-mooney and look at the message to know if its a quota issue or if we are out of ips
17:25:26 sean-k-mooney i think in both case we would expect the allocation to fail and we would want to return a 409
17:25:47 sean-k-mooney so merging those together while it makes debuging hard does not seam to alter the api behavior
17:26:48 sean-k-mooney lajoskatona: are you wonderign if nova shoudl do that disambiguation ro if it shoudl be in the sdk
17:27:08 sean-k-mooney or are you wonderign if its required to treat them differently vs merging them into a singel expction
17:27:14 lajoskatona sean-k-mooney: +1, I check it tomorrow to look inside the message
17:27:57 lajoskatona sean-k-mooney: My first idea was to merge them
17:28:19 sean-k-mooney i think as long as the end result at the api/externally does nto change i.e. 409
17:28:28 sean-k-mooney adn we have somethign reasonable in the debug logs
17:28:36 lajoskatona sean-k-mooney: for tomorrow I will have more test results from zuul, so I will see if this aproach can work :-)
17:28:37 sean-k-mooney so we can tell which it actully was its proably oke to merge them
17:28:46 sean-k-mooney ack
17:29:22 sean-k-mooney so for th ecase like this you could consider doing the merge in a seperate refactor first
17:29:54 sean-k-mooney then the adopt sdk patch is just doing that
17:30:17 sean-k-mooney i have not reviewd this in detail but in most cases i tlooks like you have found 1:1 mappings
17:30:37 sean-k-mooney but sure let me know what you find after zuul reports
17:31:17 sean-k-mooney lajoskatona: as an aside i think https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.py#L729-L744 is the only useage of neutronclient in watcher
17:31:38 sean-k-mooney and i dont think we actully need that info really any more
17:31:47 sean-k-mooney its part of the create_instance helper https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.py#L641
17:31:56 sean-k-mooney and that on my todolist to see fi we can entrily delete
17:32:40 sean-k-mooney lajoskatona: so if nothing else im goign to see if we can remove the neutronclient dep entirly one way or another in 2026.1
17:34:04 sean-k-mooney watcher used ot have its onw impleation of evacuate/coldmigrate where it woudl snapshot a vm, delete it and create a new one
17:34:33 sean-k-mooney i belive this is mostly dead code now
17:41:15 lajoskatona sean-k-mooney: thanks I add watcher to my todolist, I have anyway some pending patches waiting for the release to not bother people (like in manila-ui)
18:29:07 melwitt dansmith, bauzas: just some fyi, I have been realizing some issues while working on tempest tests for this series. I have added a few comments about it on the spec re-proposal here https://review.opendev.org/c/openstack/nova-specs/+/961564/1/specs/2026.1/approved/vtpm-live-migration.rst
18:30:15 melwitt (about [libvirt]supported_tpm_secret_security valid values and overloading of the image_hw_tpm_secret_security instance sysmeta key)
18:30:22 dansmith melwitt: I mostly gave up reviewing the spec re-proposal because it isn't easy to see the changes. So I've just been using the commit message to remind myself of the changes while looking at the code
18:30:26 dansmith I figure it's better to spend my time on the code anyway
18:31:56 melwitt dansmith: that's a reasonable approach, that was my thinking in enumerating the changes in the commit message. just saying I have noticed some things while doing tempest stuff and thought that might be the best place to note my findings
18:32:12 dansmith ack
18:33:56 melwitt it's all about the "opt in via resize" part of it. I have been writing tempest tests that do resize-confirm, resize-revert, and now I'm doing a rebuild-with-image-property one to see what happens
18:34:24 melwitt I am unsure how comprehensive we will want to be with the ability for a user to change their TPM secret security policy
18:48:24 melwitt dansmith: do you think that we should support a user _changing_ their secret security policy via a resize? or do you think the resize should be limited to legacy instances that don't have a policy set? because the former becomes a problem with us overloading the sysmeta key for it
18:49:30 melwitt *the image property sysmeta key for it
18:56:13 sean-k-mooney melwitt: interesting i had assumed the later that you would be able change via resize
18:57:46 melwitt sean-k-mooney: you mean the former I think :) but yes I also went in with that assumption but I want to pause and make sure that's what we want, bc if we want that, I think we need to not use "image_hw_tpm_secret_security" as the key to stash in the instance sysmeta. bc it makes flavor/resize conflict errors
18:58:37 melwitt (I have been finding things while adding more func tests and tempest tests)
18:59:04 melwitt *flavor/image conflict errors
18:59:22 sean-k-mooney well the flavor image conflcit check shoudl be checkign the glance image keys
18:59:27 sean-k-mooney are we using the stashed copy
18:59:28 melwitt , when the user may not have ever used an image prop
18:59:44 sean-k-mooney i guess we might be to ensure that out of band glance changes dont creap in
19:00:05 sean-k-mooney so we do not need ot use the same key
19:00:12 melwitt we are yeah. for the check virt.hardware.get_tpm_secret_security_constraint() for example
19:00:23 sean-k-mooney that was suggested just to minimise the number of places we have to check
19:01:06 melwitt yeah. using the same key becomes a problem if the user never used an image with that key in it and we have set it internally for other reasons
19:01:12 sean-k-mooney are you thinking we woudl add a new instance system metadata key ?
19:01:31 sean-k-mooney or were you leanign to only allowing resize if its not a confilct
19:01:51 sean-k-mooney meaning we shoudl not set it on update but treat it not being set as the old user policy
19:01:56 melwitt if we want to be able to convert via resize, yes. like convert from host => deployment, etc. unless someone has a different idea
19:01:58 sean-k-mooney i.e. defaulting in code instead of in the db
19:02:45 sean-k-mooney if we dont use a new key then the simplest thing to do is treat key not in data as the USER policy
19:02:50 melwitt because if we set image_hw_tpm_secret_security NOT from an image, it will get locked into the instance as if it had the image property, and it will reject any flavor hw:tpm_secret_security that does not agree with it
19:03:25 sean-k-mooney then the key will only be set if it actully comes form the image
19:03:32 melwitt everything currently works fine for legacy => host and legacy => deployment
19:03:51 melwitt the problem comes when you have user => host, user => deployment, host => deployment, etc. new instances basically
19:04:07 sean-k-mooney ya
19:04:26 melwitt but yeah I see what you are saying
19:04:45 melwitt that would work also
19:05:10 sean-k-mooney basicly image_metadata.get("image_hw_tpm_secret_security", "user") in virt.hardware.get_tpm_secret_security_constraint()
19:05:48 sean-k-mooney well
19:05:54 sean-k-mooney hum

Earlier   Later