| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-01-29 | |||
| 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 | |
| 18:52:22 | mikal | sean-k-mooney: I'm just removing the validation for the HTTP GET right, not the POST? | |
| 18:52:24 | sean-k-mooney | mikal: i think you can revert the changes to that file and also remove the other changes melwitt mention in ps 18 | |
| 18:52:47 | sean-k-mooney | i would also need ot load context | |
| 18:52:56 | sean-k-mooney | im looking for the patch now | |
| 18:53:21 | sean-k-mooney | ok so https://review.opendev.org/c/openstack/nova/+/924844/24/nova/api/openstack/compute/remote_consoles.py | |
| 18:53:51 | sean-k-mooney | so the changes to create are correct | |
| 18:54:22 | sean-k-mooney | schema.get_spice_console_v298 is not needed and the changes to get_spice_console can be reverted | |
| 18:54:25 | mikal | Ok, doing that now. | |
| 18:55:11 | sean-k-mooney | mel mention you can remvoe the scheme here https://review.opendev.org/c/openstack/nova/+/924844/comment/bcfe7166_8d9bb51c/ | |
| 18:56:16 | sean-k-mooney | mikal: i think all the other comments were adress looking over them quickly | |
| 18:56:36 | mikal | Yeah honestly I thought I'd addressed them all but I definitely missed that one. | |
| 18:57:01 | sean-k-mooney | beyond that i dont see anything quickly that i would -1 for | |
| 18:57:22 | sean-k-mooney | i need to finsih reading the release note but i suspect we could proceed ith it as is | |
| 18:57:32 | sean-k-mooney | or adress other issue in a followup | |
| 18:58:07 | mikal | As an aside, I learned the other day that libvirt's equivalent to oVirt VDSM hooks _does_ _not_ allow modifying the domain XML or qemu command line on instance start. There are some times you can change the XML, mostly around live migration, but not for a domain create. | |
| 18:58:14 | sean-k-mooney | oh ya one trivial issue in the release note | |
| 18:58:24 | sean-k-mooney | "spice-direct" shoudl be ``spice-direct`` | |
| 18:58:35 | sean-k-mooney | double backtics to make it bold | |
| 18:58:41 | sean-k-mooney | btu honstly it fine with quotes | |
| 18:58:41 | mikal | I cna fix that now too. | |
| 18:59:07 | sean-k-mooney | same for spice_direct_proxy_base_url | |
| 18:59:08 | mikal | Do you want it bold in the API reelase notes too? | |
| 18:59:34 | sean-k-mooney | you mena the api ref | |