| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-08-25 | |||
| 14:26:42 | gmann | that is how other policy changes we do | |
| 14:27:56 | gmann | old microversion in this case i mean >2.3 for GET and >2.75 for PUT/Rebuild as this attribute was added for them. | |
| 14:40:44 | gibi | gmann: does this mean we are not implementing the POST /servers change to allow passing hostname in Xena? If we do implement that then delaying the policy change (at least of the new microversion) with deprecation does not make sense from usability perspective | |
| 14:41:15 | gibi | I want see what I passed in | |
| 14:43:58 | gibi | so what if in Xena we deprecate the policy BUT also in xena in the microversion that allows passing hostname in server create we introduce the hostname in the responses. Then in Yoga we remove the policy from the hostname attribute and that results that hostname will be shown in every microversion | |
| 14:44:41 | gibi | this allows that the current POST change be usable in Xena and also follows deprecation policy | |
| 14:52:44 | gmann | gibi: in that case we have to mention that policy is controlled with microversion. for >=2.90 you can see this attribute even previously you have restricted it for non-admin and even non-admin does not pass hostname in POST request. | |
| 14:55:28 | gibi | gmann: purely from policy perspective yes, this would be a microversion that temporarily changes the policy. After Yoga where the policy is removed this microversion controlls policy situation would be resolved | |
| 14:55:59 | gibi | honestly I don't want to delay the possibility to set hostnames | |
| 14:56:18 | gibi | but only allow to set them in 2.90 but not allow to see what you set feels bad | |
| 15:02:36 | gmann | gibi: they can see with policy change, but yes with default policy they would not be able to see | |
| 15:02:44 | gmann | *policy override | |
| 15:03:01 | gibi | gmann: is there a way to only override the policy for the hostname attribute only? | |
| 15:06:09 | gmann | gibi: no, it is with other server extended attributes | |
| 15:06:38 | gibi | but we don't want to show all the exteneded attribute as there are sensitive informations there like the hypervisor_hostname | |
| 15:06:51 | gmann | humm | |
| 15:06:52 | gibi | but the hostname of the VM is harmless | |
| 15:07:02 | gibi | that does not reveal any infra informatiuon | |
| 15:07:11 | gmann | yeah | |
| 15:09:55 | gmann | gibi: I think in this exceptional case when there is related API change, I am getting your point. | |
| 15:10:18 | gmann | let me rethink if any other way it can create inconsistency other seems ok to me | |
| 15:10:23 | gmann | otherwise | |
| 15:11:07 | gibi | gmann: for me it is the case when our strict rules (microversion and policy deprecation) get in our way to provide a meaningful and (for me) safe change for the end user | |
| 15:12:05 | gmann | yeah. | |
| 15:13:09 | gibi | and I support our strict rules in general as it is a safety net to avoid breaking users, but in this specific case I think we can allow an exception for the rules | |
| 15:14:12 | gmann | i agree. especially with the point of using 2.90 changes completely (request and see the hostname by end users) | |
| 15:15:22 | gmann | I think I am convinced now :). thanks | |
| 15:17:15 | opendevreview | Merged openstack/placement master: Bump os-traits to latest 2.6.0 https://review.opendev.org/c/openstack/placement/+/805830 | |
| 15:24:27 | gibi | gmann: thank you for the discussion | |
| 15:29:33 | melwitt | lyarwood: thanks for noting that I need to rebase the consumer types set, going to do that now and will need +W reapplied after | |
| 15:29:59 | lyarwood | melwitt: I think we can get away without a rebase now | |
| 15:30:09 | lyarwood | melwitt: zuul should do it for us once it's rechecked | |
| 15:30:33 | melwitt | o rly | |
| 15:30:35 | melwitt | ok | |
| 15:30:39 | melwitt | I'm glad I said something. thanks I can do that | |
| 15:30:47 | stephenfin | gmann: gibi: Sorry, I had meetings. It _sounds_ like you've come to a conclusion and are ultimately happy with what I've done? | |
| 15:31:22 | gmann | stephenfin: yeah, it looks good to me, reviewing that patch... | |
| 15:33:14 | gibi | ohh consumer_types are going in \o/ | |
| 15:36:30 | melwitt | yes \o/ | |
| 15:36:54 | lyarwood | assuming it's rebased by zuul ;) | |
| 15:39:26 | lyarwood | cool looks like it did | |
| 15:39:58 | lyarwood | brb | |
| 16:07:09 | gmann | stephenfin: left few comments on test and documentation | |
| 16:36:31 | artom | def _do_old_style_instance_list_for_poor_cellsv1_users() | |
| 16:36:39 | artom | Right, who's the joker responsible for that? | |
| 16:37:25 | artom | Mr Smith. | |
| 16:37:29 | sean-k-mooney | someone who took pitty on the cell v1 users | |
| 16:37:39 | sean-k-mooney | then need all the care we can give them | |
| 16:37:41 | artom | (Yes, this is very old code - looking at a Queens bug report) | |
| 16:39:33 | sean-k-mooney | ********* | |
| 16:39:58 | sean-k-mooney | they are mising url quoting | |
| 16:40:00 | sean-k-mooney | https://github.com/sqlalchemy/sqlalchemy/blob/master/lib/sqlalchemy/engine/url.py#L542 | |
| 16:43:23 | artom | ? | |
| 16:44:27 | sean-k-mooney | the query args secotion fo the url is enccoded using quote_plus | |
| 16:44:40 | sean-k-mooney | where spaces are replaced with + | |
| 16:44:58 | sean-k-mooney | but the protocol seciont of the url has a + in it that is not a spec | |
| 16:45:02 | sean-k-mooney | *space | |
| 16:45:27 | sean-k-mooney | so if i decode it with unqoute technically the queary args wont be decoded properly | |
| 16:45:51 | artom | Ohhh | |
| 16:46:33 | sean-k-mooney | which means if i want to properly decode it i need to hande eveything beofre the ? one way and everything after the other way | |
| 16:46:47 | sean-k-mooney | i htink im just going to skip that and replcae % with %% | |
| 16:47:06 | sean-k-mooney | and hope that config parser is ok with the url encoded value | |
| 16:47:35 | artom | Yeah | |
| 16:47:51 | artom | We'll get the tripleo job to run with the fix as a dependency | |
| 16:48:06 | sean-k-mooney | ya i can add a depends on | |
| 16:48:09 | sean-k-mooney | that works with ooo right | |
| 16:48:18 | sean-k-mooney | via a dnm patch | |
| 16:50:46 | artom | It should | |
| 16:51:23 | artom | I was thinking the other way around, have an ooo DNM patch depending on the nova one, but whatever works | |
| 16:53:11 | sean-k-mooney | artom: that is what im going to do | |
| 17:01:33 | lbragstad | gmann so - i'm working through a test where a system-admin creates a server in a project (specifically trying to find what needs to change in KSA to make that possible) | |
| 17:02:21 | lbragstad | and it looks like the first failure is https://github.com/openstack/neutron/blob/master/neutron/notifiers/nova.py#L265 because neutron is using a novaclient with a project-scoped token (instead of a system-scoped one) | |
| 17:03:35 | sean-k-mooney | that nova client should be using the token set in the neutron.conf | |
| 17:03:53 | sean-k-mooney | which woudl normally be the nova user or a service user | |
| 17:03:59 | lbragstad | yeah - exaclty | |
| 17:04:00 | lbragstad | and it does | |
| 17:04:02 | lbragstad | so - that's good | |
| 17:04:28 | sean-k-mooney | so the nova user would have to be a project member or if it was a system_memeber we need a new parmater to specifcy a project id i ugess | |
| 17:04:31 | sean-k-mooney | *guess | |
| 17:04:55 | sean-k-mooney | the api its calling however is admin only and not really proejct specfic | |
| 17:05:15 | sean-k-mooney | im surpised that is failing | |
| 17:05:25 | lbragstad | in https://github.com/openstack/neutron/blob/master/neutron/notifiers/nova.py#L265 specifically - the nova user would need to be a system-admin https://github.com/openstack/nova/blob/master/nova/policies/server_external_events.py#L27 | |
| 17:05:40 | lbragstad | iiuc | |
| 17:05:41 | sean-k-mooney | yes | |
| 17:06:48 | sean-k-mooney | so the nova user woul dhave to be a system_admin | |
| 17:06:52 | lbragstad | right | |
| 17:07:10 | lbragstad | so that neutron can use that user with a client to create external events in nova | |
| 17:07:38 | sean-k-mooney | yes | |
| 17:07:46 | sean-k-mooney | its calling this endpoint https://docs.openstack.org/api-ref/compute/?expanded=run-events-detail#run-events | |
| 17:08:06 | lbragstad | in addition to that - we'd need a way to make sure neutron is using the right scope when building the auth request | |
| 17:08:24 | lbragstad | https://github.com/openstack/neutron/blob/master/neutron/notifiers/nova.py#L61 | |
| 17:09:35 | sean-k-mooney | yep we woudl need to add a new config parmater ofor the scop to use | |
| 17:09:41 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/policies/server_external_events.py#L24-L35 | |
| 17:09:54 | sean-k-mooney | in this case system scope | |
| 17:10:05 | lbragstad | and i imagine that's going to be a pattern we need else where? | |
| 17:10:43 | lbragstad | i'm expecting this is just the first time i'm hitting this issue, but there are other layers to the onion | |
| 17:11:00 | sean-k-mooney | well its going to happen but worse for volume resize | |
| 17:11:17 | sean-k-mooney | cinder currently has bug where it calls that endpoing with the users token | |
| 17:11:17 | lbragstad | sure - that makes sense | |
| 17:11:24 | sean-k-mooney | instead of actully using an admin one | |