| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-24 | |||
| 22:30:15 | sean-k-mooney | the one for when your deleting things | |
| 22:30:26 | sean-k-mooney | ya i always liked that | |
| 22:30:28 | melwitt | yeah. I love that | |
| 22:31:03 | sean-k-mooney | i feell like if we added that to reset-state i would feel alttile better about that existing | |
| 22:31:54 | melwitt | same | |
| 22:31:59 | sean-k-mooney | ok im geting tired so im goign to go get some rest and ill take a look at dansmith's patch again in the morning | |
| 22:32:30 | melwitt | ok, seeya o/ | |
| 22:59:08 | opendevreview | melanie witt proposed openstack/nova master: docs: Correct unified limits CLI commands https://review.opendev.org/c/openstack/nova/+/924888 | |
| 23:38:50 | opendevreview | Merged openstack/nova master: [CI] Replace deprecated regex https://review.opendev.org/c/openstack/nova/+/922212 | |
| #openstack-nova - 2024-07-25 | |||
| 07:34:12 | opendevreview | Alexander Hild proposed openstack/nova master: feat: "vm uuid" in baseBoard asset tag https://review.opendev.org/c/openstack/nova/+/924907 | |
| 10:30:46 | opendevreview | Alexander Hild proposed openstack/nova master: feat: Include "vm uuid" in baseBoard asset tag in libvirt domain xml https://review.opendev.org/c/openstack/nova/+/924916 | |
| 11:35:43 | sean-k-mooney | gibi: bauzas: can one of ye hit https://review.opendev.org/c/openstack/nova/+/924733 | |
| 11:36:27 | gibi | looking | |
| 11:38:47 | gibi | +A | |
| 11:39:49 | sean-k-mooney | gibi: thanks | |
| 11:40:12 | sean-k-mooney | gibi: i left some other comments on https://review.opendev.org/c/openstack/nova/+/924866 by the way in case you want to weigh in there | |
| 11:40:41 | sean-k-mooney | im +2 on it but want to get dans opion before proceeding | |
| 11:41:21 | andreykurilin | If not, can this case be considered as a bug? And the last question - if it is a bug, does it require a microversion bump or not, as the 'removeSecurityGroup' API already defines a 400 error code? | |
| 11:41:21 | andreykurilin | Hi folks! I have a custom Neutron plugin that restricts adding&removing specific security groups from ports. It works excellently with Nova's `addSecurityGroup` server action, i.e., Nova propagates an original error from Neutron to the end-user. But 'removeSecurityGroup' behaves differently - it raises 500 if something goes wrong on the Neutron side. Is the behavior difference between similar server actions intentional or not? | |
| 11:44:08 | gibi | sean-k-mooney: ack. My limited understanding of the related code tells me that our defaults probably OK and if we start reconfiure things we will hit edge cases. So lets wait for Dan but I think we can land this as is and chalk up one more thing to care about when we rewrite the image code in nova. | |
| 11:44:53 | sean-k-mooney | yep thats where im at too | |
| 11:46:11 | sean-k-mooney | im going for an "eyes fully open appoch" just to be transparent and say hay this edge case exist but it existed before so not a regression | |
| 11:47:07 | sean-k-mooney | it does double down on the fact we need better tempest coverage of the formats we intend to supprot in the nova ci however | |
| 11:47:14 | sean-k-mooney | even if those are just in a perodic job | |
| 11:47:37 | sean-k-mooney | or one that only runs on cahgnes to the image code | |
| 11:55:59 | gibi | I agree. We only caught our regression becuase we were lucky to have a job with ami images | |
| 12:02:55 | sean-k-mooney | andreykurilin: we do not provide any supprot in nova for third party enturon exteions | |
| 12:03:58 | sean-k-mooney | andreykurilin: so unless this is in tree in neutron its by defientioan not a nova bug | |
| 12:04:43 | sean-k-mooney | if there is an aooved neutron api exteion for this beaviro then nova can be enhanced to supprot that when its enabeld | |
| 12:04:49 | opendevreview | Balazs Gibizer proposed openstack/nova master: DNM: test downstream cross repo testing https://review.opendev.org/c/openstack/nova/+/924823 | |
| 12:06:21 | sean-k-mooney | the behaivor being differnet is likely not intentional but im not sure if we shoudl fix it | |
| 12:07:01 | sean-k-mooney | if we were to fix this a micorversoin would not be approiate | |
| 12:07:57 | sean-k-mooney | andreykurilin: your custom neutron plugin is effectivly impelmeting "custom api policy" internaly which we also do not supprot | |
| 12:12:07 | opendevreview | Stephen Finucane proposed openstack/osc-placement master: Remove use of distutils https://review.opendev.org/c/openstack/osc-placement/+/924926 | |
| 12:12:54 | stephenfin | sean-k-mooney: gibi: Can you folks review that when you get a chance? osc-placement is currently broken on Python 3.12, which is the default Python on Fedora 40 (and has been out since October 2023) | |
| 12:18:41 | sean-k-mooney | stephenfin: 3.12 is alos only optional in this release and that was a hard battel to get to even that point | |
| 12:18:54 | sean-k-mooney | but im very much in favor of fixing any 3.12 issues | |
| 12:19:03 | sean-k-mooney | so yes ill review it now | |
| 12:19:07 | sean-k-mooney | https://github.com/openstack/governance/blob/master/reference/runtimes/2024.2.rst | |
| 12:19:10 | sean-k-mooney | for refernce | |
| 12:19:37 | sean-k-mooney | ah its this | |
| 12:19:56 | sean-k-mooney | so the other option is to use microversion_parse form oslo? | |
| 12:20:02 | gibi | stephenfin: Is it intetional that we ignore the patch version from semver? | |
| 12:20:09 | sean-k-mooney | but im ok with your in tree version too | |
| 12:20:40 | sean-k-mooney | gibi: depends on the schem | |
| 12:21:21 | sean-k-mooney | patch only exsits in the 4 . version i.e. major.minor.bugfix.patch | |
| 12:21:24 | stephenfin | gibi: It's not semver, it's a microversion. to the best of my knowledge, microversions never have a patch version | |
| 12:21:33 | sean-k-mooney | and ya microvesion dont use it | |
| 12:21:41 | stephenfin | and won't ever use it | |
| 12:21:50 | sean-k-mooney | https://pypi.org/project/microversion-parse/ | |
| 12:22:06 | sean-k-mooney | that is the shared dep we maintain fo doitng htis by the way | |
| 12:22:29 | stephenfin | yeah, see my commit message. This is small enough that I don't see the point in dragging in a new (and borderline unmaintained library) | |
| 12:22:53 | sean-k-mooney | right but in genreally i want use to not cargo cult this to every project | |
| 12:23:08 | sean-k-mooney | so im fine with this for now but either we shoudl pull that in or add this to oslo.utils | |
| 12:23:22 | stephenfin | If it belongs anywhere, it's probably osc-lib | |
| 12:23:35 | sean-k-mooney | well we need it for the sdk and in opentack tooo | |
| 12:23:50 | sean-k-mooney | by openstack i mean nova et al | |
| 12:23:57 | stephenfin | but this is going away once we switch osc-placement to sdk | |
| 12:24:24 | sean-k-mooney | oh i just mean i have seen this exact issue in pbr and other projects | |
| 12:24:26 | stephenfin | microversion-parse handles things like parsing headers. I think using it in nova makes sense | |
| 12:24:47 | gibi | ahh OK I did not realized that we are parsing microversion | |
| 12:25:14 | sean-k-mooney | so i think we are both ok with the osc_placement patch for now? | |
| 12:25:15 | gibi | and I cannot blame it to lack of coffee as I just had one | |
| 12:25:22 | gibi | I put my +2 ther | |
| 12:25:23 | gibi | e | |
| 12:25:29 | sean-k-mooney | ok ill +w it so | |
| 12:25:34 | gibi | cool | |
| 12:25:38 | gibi | thanks stephenfin for fixing it | |
| 12:26:58 | stephenfin | np | |
| 12:50:21 | opendevreview | Merged openstack/osc-placement master: Remove use of distutils https://review.opendev.org/c/openstack/osc-placement/+/924926 | |
| 13:01:57 | andreykurilin | sean-k-mooney: thanks for reply. “custom api policy” - yes and no. I can simplify the case - restricting to manipulate security groups for ‘less-privileged’ roles. It does not require a custom policy and reuses everything ‘builtin’. addSecurityGroup action reraises neutron’s policy error which gives end-user enough information what is going on, versus 500 by removeSecurityGroup | |
| 13:10:41 | andreykurilin | Neutron plugin I mentioned previously is entirely on neutron’s project side. It is not nova specific, i.e., it does not modify/extend nova-api. I just tried to give more context about what I’m trying to achieve. it looks like I gave more misleading info rather than useful one :) | |
| 13:12:03 | sean-k-mooney | andreykurilin: right btu nova does not supprot neutron srbac funcitonaltiy or any non default policy/roles | |
| 13:13:12 | sean-k-mooney | andreykurilin: so https://docs.openstack.org/neutron/latest/admin/config-rbac.html is not supproted by nova because the netruon folks never created a spec or worked with use to add support | |
| 13:13:35 | sean-k-mooney | we ocationaly get one of request ot enable subset of it | |
| 13:14:01 | sean-k-mooney | but there has never been an effort to actully supprot it or update it when neturon adds new fucntionaltiy | |
| 13:14:54 | sean-k-mooney | andreykurilin: if you want to use any fucntionaly restricte by that then you cannot use nova proxy apis anymore | |
| 13:15:17 | sean-k-mooney | you have to do the secrity group config via neutron api only | |
| 13:16:09 | andreykurilin | sean-k-mooney: yes, I understand that nova should not care about neutron rbac. The only problem for me here is that the end user who sees a 500 internal error (with fill out a bug report message) is less happy than someone who sees a propagated error from neutron. As I mentioned, one method already does this. | |
| 13:16:49 | sean-k-mooney | what error code does neutron return in this case | |
| 13:16:52 | andreykurilin | 400 | |
| 13:17:47 | sean-k-mooney | 400 bad request is technially not correct for a permission denial | |
| 13:17:56 | sean-k-mooney | do you have an example of the neutorn error respocne you can post | |
| 13:18:18 | andreykurilin | I could be incorrect, let me recheck quickly | |
| 13:19:09 | andreykurilin | ps: I was referencing https://github.com/openstack/nova/blob/master/nova/network/security_group_api.py#L635-L638 versus https://github.com/openstack/nova/blob/master/nova/network/security_group_api.py#L695-L699 | |
| 13:19:55 | sean-k-mooney | so its partly because removing a secrity group is never ment to be somethign that can fail | |
| 13:21:20 | sean-k-mooney | we may be able to handel the 400 case there safely | |
| 13:21:35 | sean-k-mooney | we woudl need to knwo if there is any other case that neutron returns a 400 | |
| 13:21:50 | sean-k-mooney | but we are only updatign the securit_group part of the port in that requst | |
| 13:21:54 | sean-k-mooney | so its proably ok | |
| 13:23:01 | sean-k-mooney | i dont like that we are returing a 400 form nova in this case by the way | |
| 13:24:03 | sean-k-mooney | it really should be a 403 | |
| 13:24:47 | andreykurilin | sean-k-mooney: rechecked, I was incorrect, sorry. Neutron actually raises 403. I thought that my simplified case for custom neutron logic is similar to policy errors | |
| 13:25:25 | sean-k-mooney | ok so a 403 is what i would expect becuase this is being deigned based on the permission of the roles the request was made with | |
| 13:26:51 | sean-k-mooney | so nova should be handleling that gracefuly and returning a 403 as well so i think we can catch that and provide a better error message | |
| 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 | |