Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-07-25
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
14:49:08 TheJulia yup
14:53:52 dansmith you know what we should do
14:54:10 dansmith we should add a disk_format of 'gpt' to glance,
14:54:19 dansmith and start rejecting raw in nova over time
14:54:51 dansmith then we can assert that a gpt image actually has a gpt header and we could stop with this "anything that doesn't match another format must be raw" silliness,
14:55:01 dansmith which would really help a lot I think
14:55:21 dansmith then raw can be more like what TheJulia says above (which is correct, just with caveats today)
14:57:48 TheJulia yeah
14:58:09 TheJulia gpt would be nice actually, since there are distinctions but really it should get treated exactly like raw
14:58:16 JayF a partition image, so to speak, would always have to be raw, yeah?
14:58:19 TheJulia the VM firmware... otoh
14:58:31 TheJulia JayF: it could be qcow2 as a container
14:58:38 TheJulia and when unpacked it is just bytes
14:59:14 dansmith TheJulia: right it would be treated like raw inside of nova and for qemu, but we could assert that it's not just random bytes but at least looks like a gpt on the surface,
14:59:34 dansmith refuse to boot something that is claimed as raw in glance (which could be a kernel or ramdisk or text file)
14:59:53 dansmith and not have the "else: must be raw" behavior everywhere in nova, which is the source of lots of problems
15:00:05 dansmith because to qemu, an iso is raw because it doesn't support it
15:00:36 TheJulia yeah
15:00:39 TheJulia makes sense actually
15:00:41 dansmith and ami/ari/aki images also have to be treated as raw because qemu-img doesn't support them when we're comparing notes
15:01:36 TheJulia yeah
15:01:48 dansmith basically this sort of hack: https://github.com/openstack/nova/blob/master/nova/virt/images.py#L186
15:04:43 andreykurilin sean-k-mooney: er… I cannot find the actual nova’s code that does it, but 403 is automatically propagated from neutron’s call. So my simplified case was totally irrelevant. Sorry for misleading you. It is only about 400 as a part of removeSecurityGroup now.
15:38:25 dansmith ralonsoh: does your comment on the ami snapshot patch mean it solves the problem for your neutron stuff?
15:39:46 dansmith ah looks like it from the neutron DNM patch, cool
15:39:48 dansmith sean-k-mooney: ^
17:07:15 priteau Looks like https://review.opendev.org/c/openstack/nova/+/924734 is ready to merge
17:23:02 frickler dansmith: ^^ do you want to have another look?
17:23:02 gmann sean-k-mooney: andreykurilin: on SG error. I agree to handle both 403 as well 400 in both Add/Remove SG cases which is always better than 500. NOTE: neither 400 or 403 need microversion bump as those are always existing error code from all the APIs so we can just fix this with bug fix with no microversion bump
17:23:36 dansmith frickler: +Wd
17:24:27 gmann sean-k-mooney: andreykurilin: 500 -> '400, 403, 404 or 415' is all ok. https://docs.openstack.org/nova/latest/contributor/microversions.html#id3 "The exception to not needing a microversion when returning a previously unspecified error code is the 400, 403, 404 and 415 cases. "
17:29:31 opendevreview Merged openstack/nova master: Remove AMI snapshot format special case https://review.opendev.org/c/openstack/nova/+/924866
17:29:41 gmann sean-k-mooney: andreykurilin but for 403 we should define new exception which can clearly have msg about neutron access error otherwise user can confuse on nova access issue. Something inherited from PolicyNotAuthorized or Forbidden but a different error msg - https://github.com/openstack/nova/blob/master/nova/exception.py#L191
17:31:22 gmann andreykurilin: just read your last msg about 403 is already propagated. can you check what error msg it say?
17:49:09 gmann andreykurilin: bcz I am not sure where we handle the neutron 403, but it is propagated in both cases addSecurityGroup and removeSecurityGroup ? https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/security_groups.py#L421-L430 https://github.com/openstack/nova/blob/eb5e3374bcb5de3b5986b66438a59c73be7fb596/nova/api/openstack/wsgi.py#L667
18:15:54 andreykurilin gmann: `RESP BODY: {"forbidden": {"code": 403, "message": "rule:update_port is disallowed by policy\nNeutron server returns request_ids: ['req-97c54f6e-6876-4cf4-9657-b8b3be322f57’]”}}`. vs `RESP BODY: {"forbidden": {"code": 403, "message": "Policy doesn't allow os_compute_api:os-security-groups:add to be performed.”}}` in case of Nova’s native policy error.
18:16:26 andreykurilin Despite the fact neutron errors have an extra 'suffix' line with request-id, mo, adding some prefix to error's from neutron should give more visibility of "guilty" side.
18:22:32 andreykurilin gmann: https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L213-L214 neutronclient’s 403 is handled on ‘globally’

Earlier   Later