Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-07-25
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
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

Earlier   Later