| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-25 | |||
| 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 | |
| 08:40:37 | bauzas | sean-k-mooney: can you then lookup another stable change ? | |
| 08:40:44 | bauzas | before asking for a stable release | |
| 08:40:55 | bauzas | sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/924514 | |
| 08:41:05 | bauzas | and the other backports | |
| 08:41:06 | sean-k-mooney | sure | |