Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-07-25
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’
18:23:09 gmann andreykurilin: right. did not check that. ++
18:23:20 andreykurilin Yeah, It took me a while to find it
18:24:05 gmann andreykurilin: then you can just add 400 case in Remove SG case
18:24:18 gmann all these handling are so deep in code and hard to find :)
18:26:20 andreykurilin gmann: thank you! I’ll update my patch yesterday with a proper release note.
18:26:35 gmann andreykurilin: ++ thanks
18:34:25 andreykurilin *tomorrow (need to relax )
19:18:10 opendevreview Merged openstack/nova stable/2023.1: Change force_format strategy to catch mismatches https://review.opendev.org/c/openstack/nova/+/924734
20:08:21 opendevreview Jay Faulkner proposed openstack/nova master: [ironic] Factor out metadata and send to ironic https://review.opendev.org/c/openstack/nova/+/923910
21:13:32 JayF sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/923910 + https://review.opendev.org/c/openstack/ironic/+/924887 are tested working in devstack. Still have to write a small migration script and some tempest tests, but wanted to get general agreement on the shape of the change before going further. If you could have a look I'd appreciate it. (/me goes and asks the same of Ironic cores)
21:46:21 opendevreview sean mooney proposed openstack/nova stable/2024.1: Remove AMI snapshot format special case https://review.opendev.org/c/openstack/nova/+/924982
#openstack-nova - 2024-07-26
01:33:15 opendevreview Wenping Song proposed openstack/nova master: remove exception.RescheduledException check as _validate_instance_group_policy doesnot rasie this exception https://review.opendev.org/c/openstack/nova/+/924988
05:29:54 ralonsoh dansmith, yes, thanks. Sorry for not replying yesterday, it was public holiday in some regions of Spain
05:30:20 ralonsoh I think we are good now in Neutron
08:30:39 opendevreview Pavlo Shchelokovskyy proposed openstack/nova master: Fix image conversion check in finish_migration https://review.opendev.org/c/openstack/nova/+/897842
08:36:17 opendevreview sean mooney proposed openstack/nova stable/2023.2: Remove AMI snapshot format special case https://review.opendev.org/c/openstack/nova/+/924999
08:37:48 opendevreview sean mooney proposed openstack/nova stable/2023.1: Remove AMI snapshot format special case https://review.opendev.org/c/openstack/nova/+/925000
08:38:40 sean-k-mooney bauzas: gmann: if ye are around im backporting the fix for the ami snapshot regression
08:39:01 sean-k-mooney bauzas: gmann https://review.opendev.org/c/openstack/nova/+/924982
08:39:03 bauzas cve sage, season 2, episode 203
08:39:09 bauzas saga*
08:39:34 sean-k-mooney once those merged im going to create the release patches
08:39:51 sean-k-mooney and ill add ye to review them

Earlier   Later