| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-25 | |||
| 13:27:30 | andreykurilin | But what about 400 ? :) Are you still ok with handling it the same way as in addSecurityGroup ? | |
| 13:27:31 | sean-k-mooney | but i would also consider this to be skirting what is and is not in scope to fix in nova | |
| 13:28:10 | sean-k-mooney | andreykurilin: i would prefer that to also be a 403 but that woudl require a microverison update | |
| 13:28:16 | sean-k-mooney | and im not sure we want that | |
| 13:29:14 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/exception.py#L1054 | |
| 13:29:31 | sean-k-mooney | that unfortunetly inhrits form invlaide and default to a 400 as a result | |
| 13:30:04 | sean-k-mooney | that was a bug form when we first added the proxy api https://github.com/openstack/nova/commit/c3ed3dfcf99e9162616960bad5c7b7a36d3b2453 | |
| 13:30:28 | sean-k-mooney | we have got better at using sematicly correct http respoce codes since then | |
| 13:30:50 | andreykurilin | Yes, it is 400. It is set separately here https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/security_groups.py#L430 | |
| 13:32:07 | sean-k-mooney | so if we want to avoid a micor version bump we might be forced to use 400 https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/security_groups.py#L432 | |
| 13:32:26 | sean-k-mooney | although i belive we are alwasy allowed to return a 401 and 403 | |
| 13:32:34 | sean-k-mooney | btu we shoudl disucss that with gmann ^ | |
| 13:32:46 | sean-k-mooney | andreykurilin: if you want to create a patch for this do | |
| 13:33:11 | sean-k-mooney | we can discuss the exacat details of the respoce code and message content as part of the review | |
| 13:34:14 | andreykurilin | sean-k-mooney: great, thanks! I already have a patch for handling 400 as a part of removeSecurityGroup action https://review.opendev.org/c/openstack/nova/+/924399 . I’ll add 403 handling there as well | |
| 13:35:14 | sean-k-mooney | can you actully file a bug for this too | |
| 13:35:29 | sean-k-mooney | and add a release note please. | |
| 13:35:39 | sean-k-mooney | we would ideally have both if we are going to backport this | |
| 13:35:39 | andreykurilin | sure | |
| 14:09:36 | dansmith | JayF: TheJulia: Someone told me that ironic is using glance's ARI format in some way, presumably to store your ramdisk image, is that right? | |
| 14:11:24 | opendevreview | Merged openstack/nova stable/2023.2: Change force_format strategy to catch mismatches https://review.opendev.org/c/openstack/nova/+/924733 | |
| 14:15:56 | TheJulia | dansmith: yeah, aki and ari for the raw image artifacts set by an admin to facilitate ops, they never ever get handed off to a conversion operation | |
| 14:16:39 | dansmith | TheJulia: okay, I feel like that's the wrong image type to be using.. what gets stored in the aki? | |
| 14:17:02 | sean-k-mooney | those are not required as far as i am aware by the way | |
| 14:17:04 | dansmith | oh, the kernel for a pxeboot operation? | |
| 14:17:14 | TheJulia | dansmith: yes | |
| 14:17:27 | sean-k-mooney | what i ment by that is using ari and aki | |
| 14:17:38 | sean-k-mooney | to store the ramdisk and kernel images | |
| 14:17:42 | dansmith | the reason is that AKI, ARI are not just kernel and ramdisk, they are *amazon* kernel and ramdisk, which at least when it was defined meant "a paravirt kernel" and "a ramdisk with paravirt drivers in it" | |
| 14:17:58 | dansmith | that said, TheJulia, those things never flow through nova at all, is that right? | |
| 14:18:04 | sean-k-mooney | i dont think there is any ckecking that the ramdisk image and kernel iamge uuid are in those formats | |
| 14:18:12 | TheJulia | dansmith: well, it is possible to do, given how they can be constructed | |
| 14:18:15 | dansmith | asking because I want to (thermo) nuke AMI,ARI,AKI from nova | |
| 14:18:16 | TheJulia | dansmith: never flow through nova | |
| 14:18:28 | dansmith | okay | |
| 14:19:08 | frickler | dansmith: sean-k-mooney: I lost track a bit, are there fixes already that should make https://review.opendev.org/c/openstack/nova/+/924734 pass or does that still need further investigation? or just enough rechecks? ;) | |
| 14:19:33 | sean-k-mooney | dansmith: TheJulia nova support suing sepreate ram/kernel disk indepenent of the format of the root disk as far as im aware | |
| 14:19:44 | sean-k-mooney | so if you took an ami and uploaed all 3 parts as raw | |
| 14:19:50 | sean-k-mooney | it would work | |
| 14:20:10 | TheJulia | sean-k-mooney: yeah, it *should* and I would expect it to actually | |
| 14:20:10 | sean-k-mooney | as would uploading a qcow as qcow and refeince seperate kerenl/ram disk | |
| 14:20:18 | TheJulia | I think this goes back to age old convention | |
| 14:20:19 | dansmith | sean-k-mooney: yeah, I'm sure we're not doing any specific checking on the aki,ari bits | |
| 14:20:30 | TheJulia | set like 10 years ago because people didn't think of raw | |
| 14:20:39 | sean-k-mooney | to me the only purposue today is lableing them as not bootable | |
| 14:20:44 | TheJulia | perfect | |
| 14:20:45 | sean-k-mooney | for filtering the images | |
| 14:21:05 | dansmith | TheJulia: no, raw has been around forever, it's because the disk image of an ARI,AKI booted thing is not a disk | |
| 14:21:25 | TheJulia | dansmith: indeed and correct | |
| 14:21:47 | TheJulia | ari/aki has also been in the docs for ages for some human delination purpose I never understood | |
| 14:21:57 | sean-k-mooney | frickler: checkign | |
| 14:22:03 | TheJulia | other than it is an explicit kernel and explicit ramdisk | |
| 14:22:19 | dansmith | TheJulia: it's not for humans specifically, it's for amazon compatibility which has long since gone away in direct form | |
| 14:22:31 | sean-k-mooney | frickler: i think there is a requirement repo change to bump packaging to the 2023.2 version to fix the setuptools issues | |
| 14:22:49 | TheJulia | dansmith: wasn't explicitly aware of that, but likely humans just didn't know to update ironic's docs. | |
| 14:23:03 | dansmith | TheJulia: virtual machines booted this way don't use a bootloader on the disk and are sort of warm booted from a pre-crafted memory state | |
| 14:23:21 | frickler | sean-k-mooney: ah, that was merged two days ago, so I'll try to recheck then https://review.opendev.org/c/openstack/requirements/+/924764 | |
| 14:23:34 | sean-k-mooney | yep that the patch i was looking for | |
| 14:23:36 | sean-k-mooney | perfect | |
| 14:23:36 | dansmith | TheJulia: right, I think the human problem is that people saw "ramdisk" and "kernel" and thought "oh these are what I should use" without considering what the "amazon-" prefix meant in context | |
| 14:24:00 | dansmith | TheJulia: anyway, the thing I wanted to confirm is that if we were to drop all that special behavior from nova it's not going to impact ironic and it sounds like that's true | |
| 14:24:16 | TheJulia | dansmith: yeah, its just a doc issue really | |
| 14:24:27 | TheJulia | dansmith: correct | |
| 14:24:51 | dansmith | TheJulia: you mean a doc issue because you could easily store all those in 'raw' images and it would all work fine right? | |
| 14:26:13 | TheJulia | yeah | |
| 14:26:16 | dansmith | ack | |
| 14:26:56 | TheJulia | at least *should* afaik | |
| 14:27:30 | TheJulia | I would expect it to in other words | |
| 14:28:25 | sean-k-mooney | unless ye have also specical cased it for some archane reason im not sure why it woudl not | |
| 14:28:48 | sean-k-mooney | the format you mark somethign in in glance really does not change the content of the data | |
| 14:29:08 | dansmith | right, glance is going to start using inspector to enforce the content of the stream at some point, | |
| 14:29:29 | dansmith | and if we were to get particular about it, we'd need to decide what we're going to accept for those formats | |
| 14:32:41 | TheJulia | Can the format be changed post upload? | |
| 14:32:59 | dansmith | no | |
| 14:34:11 | TheJulia | oh joy | |
| 14:34:12 | TheJulia | okay | |
| 14:34:37 | TheJulia | folks would just have to explicitly correct any incorrect usage | |
| 14:34:54 | sean-k-mooney | TheJulia: glance does not allow the format to be updated after the fact | |
| 14:34:57 | dansmith | I'm not saying you need to do that, to be clear, | |
| 14:35:07 | dansmith | I'm saying it's wrong, historically wrong, and I want to remove that specialized support from nova | |
| 14:35:11 | sean-k-mooney | TheJulia: so yes but by uploading and using new iamges with the corect format | |
| 14:35:13 | TheJulia | yeah, I grok that | |
| 14:35:22 | dansmith | but you guys can continue being wrong, I just don't want to break ironic if doing so in nova would be a problem | |
| 14:35:23 | TheJulia | sean-k-mooney: yeah, figured that | |
| 14:35:39 | TheJulia | dansmith: no worries, we should fix it anyway just to remove confusion | |
| 14:35:52 | JayF | We have plenty of other ways to keep being wrong instead ;) | |
| 14:36:09 | dansmith | JayF: I was being overly dramatic in my use of wrong of course :) | |
| 14:36:50 | JayF | oh, I took it as jovial :D | |
| 14:38:11 | dansmith | okay good :) | |
| 14:42:54 | TheJulia | JayF: looks like our devstack plugin uses the wrong convention too, but that also should be an easy/quick fix as far as I can see | |
| 14:45:07 | JayF | yeah we've been telling people aki/ari since forever | |
| 14:45:30 | JayF | I'm probably at least somewhat responsible for that, at least as the last person left from the team that made the agent lol | |
| 14:46:05 | TheJulia | JayF: https://bugs.launchpad.net/ironic/+bug/2074090 | |
| 14:46:18 | dansmith | glance probably needs an image format type of "non-bootable-artifact" or something like that | |
| 14:47:05 | dansmith | raw is technically okay for those things, but people think raw means "a MBR/GPT disk that I should be able to boot in nova" too | |
| 14:47:32 | dansmith | perhaps raw but setting hw_arch to be "linux/x86" or something to make it clear it's not a machine-bootable thing or something like that | |
| 14:48:03 | TheJulia | Simple is likely best here, since raw is just bytes, not a disk image under a certian format | |
| 14:48:17 | JayF | Yeah, I think that's likely why we'd have gone with the aki/ari back in the day, like you said above, pattern matching on "kernel" and "ramdisk" and there's no other choice that fits | |
| 14:48:33 | JayF | it's almost like we're trying to combine "purpose" and "image format" in the same field, and it's resisting for good reason :D | |
| 14:49:00 | dansmith | yeah, and the fact that all three are also options for container_format makes it even less sensical | |