Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-01-28
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
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?

Earlier   Later