| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-01-28 | |||
| 16:59:26 | bauzas | #endmeeting | |
| 16:59:46 | fwiesel | Thanks, and sorry for the noise. | |
| 16:59:56 | masahito | thank you | |
| 17:02:31 | gibi | thanks | |
| 17:12:47 | gibi | bauzas, dansmith, melwitt, sean-k-mooney: I just discovered that we missed the rebuild API action response when we considered the image properties in server details https://review.opendev.org/c/openstack/nova/+/939649/4#message-7d57a602cdd025a52c15c2470091baa6f0ee0340 | |
| 17:12:58 | gibi | would you be OK to amend the spec with this impact? | |
| 17:16:18 | gibi | I need to drop soon but I will read back | |
| 17:21:09 | priteau | bauzas and sean-k-mooney, could you please review this backport? Same as yesterday but for Bobcat: https://review.opendev.org/c/openstack/nova/+/940272 | |
| 17:21:59 | dviroel | gibi: i hit the same issue in https://review.opendev.org/c/openstack/nova/+/938604 - so the same decision should also impact the scheduler_hints spec... | |
| 17:25:53 | bauzas | gibi: I need to reload the context but if you ask for updating a spec post-approval-freeze, that's absolutely fine to me | |
| 17:29:45 | sean-k-mooney | priteau: +2 | |
| 17:53:46 | dansmith | gibi: yep, commented | |
| 18:05:02 | opendevreview | Stephen Finucane proposed openstack/nova master: api: Only check minimum API version https://review.opendev.org/c/openstack/nova/+/940325 | |
| 18:05:11 | stephenfin | sean-k-mooney: took a different approach ^ | |
| 18:06:06 | sean-k-mooney | ya that could work as well. although if your only checkign the min version why is it not positional | |
| 18:06:45 | stephenfin | It effectively is. Do you mean using the forward slash operator? | |
| 18:06:47 | sean-k-mooney | this https://review.opendev.org/c/openstack/nova/+/940325/1/nova/api/openstack/api_version_request.py#300 does not need to be a kw arg | |
| 18:07:17 | stephenfin | I don't understand the question? Are you thinking Python here or Go? :D | |
| 18:07:41 | sean-k-mooney | in python | |
| 18:07:48 | sean-k-mooney | if we always expect to pass version | |
| 18:07:52 | sean-k-mooney | instead of is_supported(req, version=_MIN_API_VERSION): | |
| 18:07:56 | stephenfin | Oh | |
| 18:08:01 | sean-k-mooney | it should be is_supported(req, version): | |
| 18:08:20 | stephenfin | Okay, you were asking why does it take a default | |
| 18:08:21 | sean-k-mooney | you just dont call this if your not enforcing a min right? | |
| 18:08:31 | sean-k-mooney | yes | |
| 18:08:32 | stephenfin | yup, 100%. Good call | |
| 18:08:44 | opendevreview | Stephen Finucane proposed openstack/nova master: api: Only check minimum API version https://review.opendev.org/c/openstack/nova/+/940325 | |
| 18:08:57 | stephenfin | Done. Thanks | |
| 18:09:41 | sean-k-mooney | ok so for make you just using not min | |
| 18:09:47 | sean-k-mooney | ya that works | |
| 18:10:03 | stephenfin | Like I said, if you really want a max check just negate the call | |
| 18:10:25 | sean-k-mooney | well i was just reviewing https://review.opendev.org/c/openstack/nova/+/940325/2/nova/api/openstack/compute/create_backup.py#68 | |
| 18:10:28 | sean-k-mooney | where your doing that | |
| 18:10:52 | stephenfin | I'm nothing if not consistent | |
| 18:11:38 | sean-k-mooney | +1 for now i looked at the first couple of files and ill loop back to it once i have reveiewd the previosu patches | |
| 18:14:49 | sean-k-mooney | sicne your here why did you add a bunch of commans an reflow things in https://review.opendev.org/c/openstack/nova/+/936369/7/nova/api/validation/parameter_types.py | |
| 18:16:39 | stephenfin | sean-k-mooney: literally leaving a comment on that now. I ran ruff by mistake on the file. I thought I had undone most of it manually in a respin but apparently not (or I've undone the undo) | |
| 18:17:20 | stephenfin | If it's impossible to review, I can respin. If not, this code is *very* rarely touched in backports and therefore shouldn't cause issues down the line. I didn't do it in any other change that I'm aware of | |
| 18:17:21 | sean-k-mooney | ack, can ruff be configure to not add the trailing commas im pretty sure black can | |
| 18:17:39 | stephenfin | It can now. It was a preview feature until recently | |
| 18:17:49 | sean-k-mooney | its not imposibel to revieww it just dislike the stypel which i normally dont -1 for | |
| 18:18:13 | stephenfin | https://docs.astral.sh/ruff/settings/#format_skip-magic-trailing-comma | |
| 18:18:38 | sean-k-mooney | oh i dont mind the backport impact its trivial, its just annoying to read when it expand the enum in particalar | |
| 18:19:14 | sean-k-mooney | we are not actully using ruff, at least right now but it might be nice ot add that to our pyproject.toml anyway | |
| 19:19:03 | sean-k-mooney | stephenfin: i belive we have a bug in the openapi schma serise. you are asusmign that project id and user id have to be uuids but as afar as i cna tell looking at keystone and our own code they are not required to eb uuids | |
| 19:22:05 | opendevreview | Michael Still proposed openstack/nova master: libvirt: direct SPICE console database changes https://review.opendev.org/c/openstack/nova/+/926877 | |
| 19:22:05 | opendevreview | Michael Still proposed openstack/nova master: libvirt: direct SPICE console object changes https://review.opendev.org/c/openstack/nova/+/926876 | |
| 19:22:06 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add extra spec for sound device. https://review.opendev.org/c/openstack/nova/+/926126 | |
| 19:22:06 | opendevreview | Michael Still proposed openstack/nova master: libvirt: allow direct SPICE connections to qemu https://review.opendev.org/c/openstack/nova/+/924844 | |
| 19:22:07 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add extra specs for USB redirection. https://review.opendev.org/c/openstack/nova/+/927354 | |
| 19:22:22 | mikal | sean-k-mooney: ^--- is a rebase and the tweaked USB controller patch. | |
| 20:11:40 | opendevreview | Merged openstack/nova master: Fix typo in release note https://review.opendev.org/c/openstack/nova/+/935681 | |
| 20:11:50 | opendevreview | Merged openstack/nova stable/2023.2: libvirt: Wrap un-proxied listDevices() and listAllDevices() https://review.opendev.org/c/openstack/nova/+/940272 | |
| 22:32:51 | opendevreview | Merged openstack/nova master: zuul: Add missing context comments for nova-next https://review.opendev.org/c/openstack/nova/+/935682 | |
| #openstack-nova - 2025-01-29 | |||
| 02:14:07 | opendevreview | melanie witt proposed openstack/nova master: pwr mgmt: power down free PCPUS when updating compute node https://review.opendev.org/c/openstack/nova/+/932926 | |
| 08:09:37 | opendevreview | Balazs Gibizer proposed openstack/nova-specs master: Extend image-properties-in-server-show spec with rebuild https://review.opendev.org/c/openstack/nova-specs/+/940360 | |
| 08:12:11 | gibi | dansmith, melwitt, sean-k-mooney: thanks for the quick response. Here are the spec update ^^ | |
| 08:17:30 | masahito | gibi: hi, could you review the small bug fixes for the v2.96 or later API bug if you have time? It has been +2ed already so it needs one more W+1. https://review.opendev.org/c/openstack/nova/+/939673 and https://review.opendev.org/c/openstack/nova/+/939658 | |
| 08:21:46 | gibi | masahito: looking... | |
| 08:48:42 | gibi | masahito: I've approved the reproducer and left some API concerns in the fix. We probably need other cores to express opinion about the API impact. | |
| 08:49:03 | gibi | sean-k-mooney: ^^ | |
| 09:33:22 | masahito | gibi: thanks for the quick review :) | |
| 09:34:00 | mikal | sean-k-mooney: that SPICE patch series has now been updated and passes CI. | |
| 09:57:37 | opendevreview | Mark Goddard proposed openstack/nova master: ironic: Let Ironic handle deployment cleanup actions during destroy https://review.opendev.org/c/openstack/nova/+/883411 | |
| 10:02:55 | stephenfin | sean-k-mooney: feck :( | |
| 10:04:12 | stephenfin | sean-k-mooney: Yeah, that checks out. Let me think on it. If you're doing further reviews, maybe just ignore/call out the places I've made those assumption and move on. I'll rejig | |
| 10:05:17 | sean-k-mooney[m] | sure we have merged one already the flavor acess schema | |
| 10:05:23 | sean-k-mooney[m] | we should fix that | |
| 10:05:44 | sean-k-mooney[m] | but there are very few apis that take either a project id or user id | |
| 10:15:10 | stephenfin | sean-k-mooney[m]: Potential proposal. I'm going to drop https://review.opendev.org/c/openstack/nova/+/937013 from the stack and see how clean the rebase is | |
| 10:15:40 | stephenfin | If it's horrific, I might keep it and rejig the commit message to indicate that no, these things aren't UUIDs but they often look like UUIDs so this is a little more realistic?? | |
| 10:16:17 | stephenfin | Hopefully not an issue but I'd _love_ to avoid a multi-hour rebase if possible 😇 | |
| 10:16:17 | sean-k-mooney[m] | sure, i would jsut assert that its a stiring between 1 and 64 printable characters | |
| 10:16:43 | stephenfin | printable or ascii? | |
| 10:16:51 | stephenfin | *alphanumeric | |
| 10:17:08 | stephenfin | (if you know off the top of your head) | |
| 10:17:25 | sean-k-mooney[m] | you can look at the keystone def its effectivly base64 | |
| 10:17:31 | sean-k-mooney[m] | i think | |
| 10:17:55 | sean-k-mooney[m] | alphanumeric and _ perhaps - | |
| 10:18:14 | sean-k-mooney[m] | i dont recall exactly but a subset of ascii | |
| 10:19:39 | sean-k-mooney[m] | stephenfin: https://github.com/openstack/keystone/blob/master/keystone/api/validation/parameter_types.py#L67-L73 | |
| 10:21:40 | stephenfin | ideal, copy-paste job so | |
| 10:39:39 | opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server topology API https://review.opendev.org/c/openstack/nova/+/940300 | |
| 10:53:57 | frickler | well this is more explicit, so likely for domains the full answer is "UUID4 without dashes or 'default'"? https://opendev.org/openstack/keystone/src/branch/master/keystone/resource/schema.py#L249 | |
| 10:54:46 | sean-k-mooney | frickler: domain are not used in any nova api | |
| 10:54:55 | sean-k-mooney | project and user ids are and have different rules | |
| 10:55:43 | frickler | ah, ok | |
| 10:56:00 | sean-k-mooney | frickler: user ids for federated keystone are the first 64 bytes of the sha256 of the user objects fields by default btu thte hash is both plugable and configurable | |
| 10:56:50 | sean-k-mooney | frickler: this basically all stems out of the fact that when using ldap/active directory they dont produce uuids for the identifer | |
| 10:57:07 | sean-k-mooney | so orginally keystone pass that through directly | |
| 10:57:31 | sean-k-mooney | the sql backend that most use does produced the hex dump of a uuid4 | |
| 10:59:50 | sean-k-mooney | its one of those things where it will almost alwasy be a uuid in practice but in reality we cant depend on that | |
| 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 | this in the past): 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: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 | |