Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-01-29
11:00:12 sean-k-mooney frickler: my real concer is that the sdk or some other code has made the same assumation
11:04:13 sean-k-mooney quick glance says no, they are constucitng fake ides usign uuid4 but prepeending user-id https://github.com/openstack/openstacksdk/blob/c6459ae0b0983530d8db75178bbfe213c8022396/openstack/tests/unit/identity/v3/test_proxy.py#L38
11:11:57 opendevreview John Garbutt proposed openstack/nova master: ironic: Fix ConflictException when deleting server https://review.opendev.org/c/openstack/nova/+/883411
11:17:30 johnthetubaguy sean-k-mooney: I have reworded the commit message on this old patch, because it is now really urgent. We have a big regression on the Ironic driver after merging those convert to SDK patches, basically we lost the ability to retry on HTTP Conflict for the silly unplug_vif code. This old patch looks to fix the problem. It would be ace if you have chance to review this one please (you added some comments saying you wanted to look at
11:17:30 johnthetubaguy this in the past): https://review.opendev.org/c/openstack/nova/+/883411
11:29:23 opendevreview Merged openstack/nova master: Add ServersViewBuilderTestV296 unit test class https://review.opendev.org/c/openstack/nova/+/939673
11:32:51 sean-k-mooney johnthetubaguy: so retrying on http conflict is not really a thing we shoudl do in general
11:33:38 sean-k-mooney a 409 means the request is invaild for the current state and we shoudl not expect resending it to work
11:34:54 sean-k-mooney johnthetubaguy: with that said i dont crrently have the context of this patch loaded so i will take a look
11:39:01 sean-k-mooney johnthetubaguy: i had previoulys agreed to allow fixign the current sate even thought he ironci workfow violates our driver api
11:39:30 johnthetubaguy [@_oftc_sean-k-mooney:matrix.org](https://matrix.to/#/@_oftc_sean-k-mooney:matrix.org)I agree with you, we should add more retry on conflict back in. But in this specific case, after we retry for around 15-30 mins, what we get is Bad request, and only then we consider the operation complete. Basically when unprovision has succeded, all the other work has already been done for us.
11:39:34 sean-k-mooney so johnthetubaguy with that in mind i assume you have tested this?
11:40:56 sean-k-mooney we do have a greeen ironic-tempest-ipa-wholedisk-bios-agent_ipmitool-tinyipa but that boot and deelete like 1 instance
11:41:09 johnthetubaguy [@_oftc_sean-k-mooney:matrix.org](https://matrix.to/#/@_oftc_sean-k-mooney:matrix.org)I haven't tested this on master, but it has been running in some old clouds for a while. I haven't heard back if this works on Caracal yet either I am afraid. Just had this reported to me yesterday evening internally.
11:41:22 sean-k-mooney so this obviouly does nto entrily break everythign but it does not validate that the node can be used again
11:41:49 sean-k-mooney johnthetubaguy: ack
11:42:09 johnthetubaguy Sure, but we call unprovison, and that goes into either error or available, let me find that graph a sec in the ironic docs to check.
11:42:43 sean-k-mooney johnthetubaguy: that not really the issue
11:43:02 johnthetubaguy OK, what bit are you worried about? I could easily be missing something?
11:43:58 sean-k-mooney so there are two conferns one is practically does this work which is what you were answering. and i trust you when you say that unprovision will do all the actions that cleanup_deploy does
11:44:57 sean-k-mooney the issue with that is ironic is not allow to modify neturon ports or cinder volume attachments assocated with a nova instnace. so it doing the cleanup as part of unprovision by makign api calls ironic si not allowed to do
11:45:25 sean-k-mooney so long term that is a probelem that shoudl be fixed
11:46:41 johnthetubaguy Ah, I was missing your concern before. Now I see what you mean. It is a “different dance” with Ironic, for sure. And we do hit issues because of the oddities going on there, e.g.: https://bugs.launchpad.net/nova/+bug/2085432
11:46:46 sean-k-mooney i agreed that we can move forward with the current state. that does not mean the current state is correct. i condier this to just be a workaround
11:48:45 sean-k-mooney johnthetubaguy: well its not jsut that its also this https://security.openstack.org/ossa/OSSA-2023-003.html
11:49:11 johnthetubaguy I mean, this is Nova better respecting Ironic’s API semantics, in my book, but I don’t think we have connects on what we want it to look like. I know Ironic are keen to re-work the networking stack for their standalone cases, and making NGS suck much less would be ace… I should take a look at that soon ish.
11:50:01 johnthetubaguy Ah, so, volumes and Ironic, I mostly hard ignore myself. So that probably is an aspect we need someone to test better. Or at least get some lovely Ironic people to take a look at our reasoning here.
11:50:31 sean-k-mooney form my perspecitve ironci is not allwoed ot make any calls to neutorn or cinder for ports or volumes port are assocated with a nove instance
11:51:27 sean-k-mooney johnthetubaguy: nova does not allow you to add/remove voluvmes or port to/form an instnace via the cidner/neutrion api inluciding createing/modifying volume attachments or port bindings
11:51:36 sean-k-mooney johnthetubaguy: ironic is doign both
11:51:45 johnthetubaguy Yeah, true.
11:52:53 sean-k-mooney so unprovision in ironic should not be cleanign up the neutorn prots ro removing the volume attahcments
11:53:12 sean-k-mooney that shoudl be done by our generic code in the compute manager as part of the instance delete flow
11:53:51 sean-k-mooney johnthetubaguy: anyway what importnt right now is this is impacting actual users
11:54:05 sean-k-mooney so we need to do the best we can in the current state
11:54:37 sean-k-mooney but if we have simialr CVEs in the future ironic integation may break
11:58:36 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for hosts APIs https://review.opendev.org/c/openstack/nova/+/937047
11:58:37 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for instance actions https://review.opendev.org/c/openstack/nova/+/937048
11:58:37 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for hypervisors APIs (1/3) https://review.opendev.org/c/openstack/nova/+/937245
11:58:38 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for hypervisors APIs (2/3) https://review.opendev.org/c/openstack/nova/+/937246
11:58:38 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for hypervisors APIs (3/3) https://review.opendev.org/c/openstack/nova/+/937247
11:58:39 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server IPs APIs https://review.opendev.org/c/openstack/nova/+/937608
11:58:39 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for remote consoles https://review.opendev.org/c/openstack/nova/+/940114
11:58:40 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for keypairs APIs https://review.opendev.org/c/openstack/nova/+/940115
11:58:40 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for image metadata APIs https://review.opendev.org/c/openstack/nova/+/940299
11:58:42 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server topology API https://review.opendev.org/c/openstack/nova/+/940300
11:58:42 opendevreview Stephen Finucane proposed openstack/nova master: api: project/tenant and user IDs are not UUIDs https://review.opendev.org/c/openstack/nova/+/940369
11:59:54 sean-k-mooney johnthetubaguy: anyway +2
12:00:48 sean-k-mooney johnthetubaguy: there is another ironci change that you might want to take a look at if you feel like doing a review
12:01:16 sean-k-mooney it fixes a regression added in dalmation so you may not have hit it yet
12:03:29 sean-k-mooney https://bugs.launchpad.net/nova/+bug/2092570
12:03:42 sean-k-mooney johnthetubaguy: https://review.opendev.org/c/openstack/nova/+/939411
12:40:42 johnthetubaguy I will take a look now.
15:09:00 johnthetubaguy [@_oftc_sean-k-mooney:matrix.org](https://matrix.to/#/@_oftc_sean-k-mooney:matrix.org)got distracted by chasing outage fun, but I am +2 that patch. I don't feel in-sync enough to +W right now, but that sure looks broken. I guess we don't test rebuild in that ironic CI you mentioned?
15:09:32 johnthetubaguy (as in we look broken without that patch)
15:09:48 sean-k-mooney we have exactly 1 job and it boot 1 vms and deletes it
15:09:53 sean-k-mooney so ya no rebuild testing
15:09:58 sean-k-mooney we really shoudl fix that...
15:10:30 sean-k-mooney this is a simple case where mypy and type checking would have saved use. we just mixed up the paramater order
15:10:57 sean-k-mooney johnthetubaguy: if no one else has ocmmend ill loop back and +w next week
15:11:33 sean-k-mooney mikal: looking at https://review.opendev.org/c/openstack/nova/+/924844 melwitt comment form pathset 18 have not been adressed
15:12:28 kpdev hi all, need review on https://review.opendev.org/c/openstack/nova/+/927474
15:12:44 kpdev I had also mentioned at https://etherpad.opendev.org/p/nova-bug-triage-20240904
15:13:09 sean-k-mooney mikal: sorry https://review.opendev.org/c/openstack/nova/+/924844/24
15:14:01 sean-k-mooney mikal: i shoudl slow down https://review.opendev.org/c/openstack/nova/+/924844/24/nova/api/openstack/compute/remote_consoles.py the api change relate to the get_spice_console api that was remvoed in 2.6
15:15:22 sean-k-mooney kpdev: that change is not valid
15:17:07 kpdev @sean-k-mooney: what is missing ? Can you please reply on pull request ?
15:17:23 sean-k-mooney the bug report is invliad
15:17:45 sean-k-mooney if we were to implemnt what you propsoe it would be a feature with a non tivial upgrade impact
15:19:54 sean-k-mooney kpdev: nova assumes that all cpus can be use by default, if that is not the case you are requried to use the [compute]cpu_shared_set and [compute]cpu_dedicated_set or the legacy [default]vcpu_pin_set to specify which cpu can be used
15:20:48 sean-k-mooney if your host cgroups are configure in such a way as they cannot be assigned to the vm becuase of the cpu_exclisve bit or otherwise and you do not use the provided config option that is not a nova bug
15:21:55 kpdev so you mean, using provided config option there is no need of this PR
15:22:42 sean-k-mooney correct i dont think this change is required if you correctly confire nova via the cpu_share_set
15:23:29 sean-k-mooney nova should not be parssing the cgoup configuration to determin which cpu can be used
15:23:42 sean-k-mooney the the installer shoudl be configuring that via the aviabel options
15:25:27 kpdev ok
15:26:48 kpdev I have added this on bug and update after checking with engineer
15:32:02 sean-k-mooney i have tiraged the bug as opipion, if you stongly feel that the ux should be imporved i think we would need to have a disgin dicussion on how before proceedign with this as a new feature
15:32:11 sean-k-mooney i dont think this is in the scope of nova to adress however
15:37:12 kpdev let me check internally as you are saying its not then in scope of nova to address.
15:43:07 opendevreview Rajesh Tailor proposed openstack/nova master: Add support for showing image properties in server show response https://review.opendev.org/c/openstack/nova/+/939649
16:26:06 melwitt sean-k-mooney: hey I just read your comment on the power management patch. are you saying that you think we should not be offlining CPU for STOPPED instances?
17:35:36 opendevreview Masahito Muroi proposed openstack/nova master: Use defaultdict object for request_specs_dict in the _list_view https://review.opendev.org/c/openstack/nova/+/939658
17:43:47 opendevreview Masahito Muroi proposed openstack/nova master: Use defaultdict object for request_specs_dict in the _list_view https://review.opendev.org/c/openstack/nova/+/939658
17:45:42 masahito gibi sean-k-mooney: thank you for the review for the patch. I did the further investigation for the current response value pattern. Please check the result and document update, too. https://review.opendev.org/c/openstack/nova/+/939658
18:47:00 mikal sean-k-mooney: I think you're saying you just want the changes to that file that you commented on removed? Is that right?
18:47:30 sean-k-mooney so im unsure
18:47:56 sean-k-mooney mikal: melwitt made a comment that that api was remvoed in 2.6
18:48:01 mikal sean-k-mooney: IIRC Mel was confused by an old removed API and in the end I didn't need to do anything. I am trying to reload state on that file at the moment.
18:48:03 sean-k-mooney so we shoudl not need to update it
18:48:37 sean-k-mooney and as a result we likely dont need that addtional validation schema
18:48:51 mikal sean-k-mooney: yeah, Mel wanted a test for the HTTP GET on that API, which was removed.
18:49:06 sean-k-mooney because if you were to use the 2.5 microversion it shoudl not return the new spice console anyway
18:49:30 sean-k-mooney so it was removed in a micorversion but we did not remove the functionaltiy
18:49:51 sean-k-mooney if we did the funciton woudl be returnign a 410
18:50:06 melwitt this sounds familiar and maybe I forgot to reply (and don't remember at the moment)
18:50:30 sean-k-mooney melwitt: context is i started reviewing mikal spice console serise again yesterday
18:50:39 sean-k-mooney and noticed your comment did not seam to be adressed
18:51:18 mikal So I can revert the changes to that file. Honestly I'm not across the validation stuff much, but if you think that's correct I believe you.
18:51:25 melwitt ok, I'll take a look

Earlier   Later