| opendevreview | Merged openstack/nova stable/xena: Fix the wrong exception used to retry detach API calls https://review.opendev.org/c/openstack/nova/+/829049 | 00:08 |
|---|---|---|
| opendevreview | Ghanshyam proposed openstack/placement master: Policy defaults improvement spec https://review.opendev.org/c/openstack/placement/+/864385 | 04:05 |
| gmann | gibi: sean-k-mooney: ^^ added the BP link in the placement policy spec | 04:06 |
| gmann | bauzas: all done for placement project in LP, updated Driver and Maintainer to 'Nova Drivers' group https://launchpad.net/placement | 04:07 |
| opendevreview | Ghanshyam proposed openstack/placement master: Policy defaults improvement spec https://review.opendev.org/c/openstack/placement/+/864385 | 04:08 |
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: Fix how deprecated_reason of ovsdb_interface is logged https://review.opendev.org/c/openstack/os-vif/+/866102 | 05:13 |
| *** akekane is now known as abhishekk | 05:53 | |
| gibi | gmann: thanks, I'm +2 | 08:24 |
| bauzas | gmann: cool, thanks for the LP janitoring | 08:35 |
| opendevreview | Sahid Orentino Ferdjaoui proposed openstack/nova-specs master: fixing: allowing target state for evacuate https://review.opendev.org/c/openstack/nova-specs/+/866108 | 08:53 |
| *** labedz__ is now known as labedz | 09:12 | |
| opendevreview | Fabian Wiesel proposed openstack/nova master: Add more password generation options https://review.opendev.org/c/openstack/nova/+/865669 | 09:37 |
| opendevreview | Kirill proposed openstack/nova-specs master: new spec: support of vnc console for ironic https://review.opendev.org/c/openstack/nova-specs/+/863773 | 10:33 |
| opendevreview | Merged openstack/nova stable/victoria: Adds a repoducer for post live migration fail https://review.opendev.org/c/openstack/nova/+/863902 | 11:00 |
| sean-k-mooney | gmann: fyi im adding you to https://review.opendev.org/c/openstack/nova-specs/+/863773 to check my reasoning for why this would be ok to implemtne without an api microversion | 11:55 |
| opendevreview | Merged openstack/placement master: Policy defaults improvement spec https://review.opendev.org/c/openstack/placement/+/864385 | 12:18 |
| *** dasm|off is now known as dasm | 12:43 | |
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: Fix how deprecated_reason of ovsdb_interface is logged https://review.opendev.org/c/openstack/os-vif/+/866102 | 13:01 |
| opendevreview | Sylvain Bauza proposed openstack/nova stable/zed: Reproducer for bug 1951656 https://review.opendev.org/c/openstack/nova/+/866151 | 13:04 |
| opendevreview | Sylvain Bauza proposed openstack/nova stable/zed: Handle mdev devices in libvirt 7.7+ https://review.opendev.org/c/openstack/nova/+/866152 | 13:04 |
| opendevreview | Sylvain Bauza proposed openstack/nova stable/yoga: Reproducer for bug 1951656 https://review.opendev.org/c/openstack/nova/+/866153 | 13:06 |
| opendevreview | Sylvain Bauza proposed openstack/nova stable/yoga: Handle mdev devices in libvirt 7.7+ https://review.opendev.org/c/openstack/nova/+/866154 | 13:06 |
| opendevreview | Sylvain Bauza proposed openstack/nova stable/xena: Reproducer for bug 1951656 https://review.opendev.org/c/openstack/nova/+/866155 | 13:07 |
| opendevreview | Sylvain Bauza proposed openstack/nova stable/xena: Handle mdev devices in libvirt 7.7+ https://review.opendev.org/c/openstack/nova/+/866156 | 13:07 |
| opendevreview | Sylvain Bauza proposed openstack/nova stable/wallaby: Reproducer for bug 1951656 https://review.opendev.org/c/openstack/nova/+/866157 | 13:08 |
| opendevreview | Sylvain Bauza proposed openstack/nova stable/wallaby: Handle mdev devices in libvirt 7.7+ https://review.opendev.org/c/openstack/nova/+/866158 | 13:08 |
| *** akekane is now known as abhishekk | 13:48 | |
| gmann | sean-k-mooney: sure, I will check | 15:36 |
| *** labedz is now known as labedz_ | 16:49 | |
| gmann | dansmith: when you will start your leave? | 16:54 |
| dansmith | gmann: next weds is my first day gone | 16:54 |
| gmann | I wanted to discuss about service role in nova. we can discuss now if you are ok? | 16:54 |
| gmann | ok | 16:54 |
| dansmith | sure | 16:54 |
| gmann | dansmith: I was testing service role in tempest and it seems the APIs we are targeting for service role (swap volume, sever external) need to get the server and service role cannot do that, https://review.opendev.org/c/openstack/tempest/+/864595 | 16:56 |
| gmann | they need admin permission also to get the servers (as it is of other project) | 16:56 |
| dansmith | gmann: meaning you can't run the whole test with just the service role right? | 16:56 |
| gmann | dansmith: yes, it fail on 404 for server not found | 16:57 |
| gmann | either we need to allow get server to service user or keep these internal APIs as admin access | 16:57 |
| dansmith | right so you'll have to do everything *except* swap_volume with a user token, and only swap_volume with the service token right? | 16:57 |
| gmann | yeah | 16:58 |
| dansmith | isn't that what we expect cinder to do? *only* call swap_volume with the service user | 16:59 |
| gmann | but that need to be further tested if volume get in same situation as service role cannot get volume in cinder | 16:59 |
| dansmith | oh, you mean nova tries to get volume during swap_volume/ | 16:59 |
| gmann | not nova. may be server external event is good example | 16:59 |
| sean-k-mooney | gmann: for the tempest user could have both both admin and service | 17:00 |
| bauzas | dansmith: when you mean a "leave", you mean a PTO or something else ? | 17:00 |
| sean-k-mooney | and you coudl drop admin later | 17:00 |
| dansmith | sean-k-mooney: I think that's a bad idea | 17:00 |
| sean-k-mooney | dansmith: becasue we wont know which permission is working | 17:00 |
| dansmith | bauzas: PTO yeah, for the rest of the year, starting +1w from now | 17:00 |
| gmann | sean-k-mooney: we will not be able to drop it right? | 17:00 |
| dansmith | sean-k-mooney: and because we never want an operator to do that | 17:00 |
| bauzas | dansmith: ack, gtk | 17:00 |
| sean-k-mooney | gmann: we should be able to | 17:01 |
| gmann | service only role does not work so our main goal to make internal APIs for service only is not fulfil | 17:01 |
| dansmith | yeah, it's not the tempest user gmann is concerned about here right? | 17:01 |
| gmann | yes | 17:01 |
| dansmith | it's some composite operation where the service calling another ends up needing both? | 17:01 |
| dansmith | gmann: finish your example with external_event | 17:01 |
| sean-k-mooney | you said " they need admin permission also to get the servers " do you mean cinder | 17:02 |
| gmann | neutron call external event API with service user and then nova will return 404 | 17:02 |
| gmann | 404 for get server | 17:02 |
| dansmith | gmann: do you mean because nova internally does a get_server? | 17:02 |
| gmann | dansmith: sean-k-mooney either we need to access Db for server with hard coded admin inside the API | 17:03 |
| gmann | dansmith: yes, get server of the requested project if not admin | 17:03 |
| dansmith | are we still checking that at the db layer? | 17:03 |
| sean-k-mooney | dansmith: ya i think we are | 17:03 |
| gmann | I think it match the project_id from context unless it is admin | 17:03 |
| dansmith | okay, so external_event should get the server from the db with admin context, but do the usual policy check of "can you see this" on the result? | 17:04 |
| sean-k-mooney | the external events api si doign the server get | 17:04 |
| sean-k-mooney | to get the host | 17:04 |
| sean-k-mooney | so it know where to send the event | 17:04 |
| dansmith | well, also just to make sure it's for a legit server right? | 17:04 |
| gmann | dansmith: if we do with admin then no 'get server policy' come into pic | 17:04 |
| sean-k-mooney | yes also to ensure it exsits | 17:05 |
| sean-k-mooney | so we coudl make that db check supprot the service user too | 17:05 |
| gmann | if we are ok to use admin context inside those then it should work | 17:05 |
| dansmith | we need to make sure we don't leak the existence of servers via the external_event API because someone can call it and get a 403 vs 404, which I presume is why we look up the server with user creds now | 17:05 |
| sean-k-mooney | eventully we proably want to remvoe the db check but as a minimal interim step i think that would be ok | 17:05 |
| gmann | dansmith: yeah that is issue of 403 vs 404 then | 17:06 |
| gmann | and yes leak of server existence | 17:06 |
| dansmith | yeah, just need to be careful about that | 17:06 |
| sean-k-mooney | well the api is admin only now | 17:06 |
| gmann | now is ok, if we make it service only | 17:06 |
| dansmith | yeah, if they get stopped before the server get because they lack service role, then that's fine | 17:06 |
| gmann | I can pass server uuid to know if that exist or not | 17:06 |
| sean-k-mooney | it would only be an issue if you had the service role | 17:07 |
| sean-k-mooney | which no human shoudl ever have | 17:07 |
| dansmith | yeah | 17:07 |
| gmann | dansmith: yes, that policy check will be there for service role before they try getting sevrer | 17:07 |
| dansmith | for this case.. that might not be the case for all of ours, if we have any that are legit for humans and machines | 17:07 |
| dansmith | gmann: cool | 17:07 |
| gmann | sean-k-mooney: yes, only with service role | 17:07 |
| sean-k-mooney | dansmith: woudl you object ot adding the service role to the place where we check for admin in the db | 17:08 |
| dansmith | sean-k-mooney: I think that's a bad idea | 17:08 |
| sean-k-mooney | even if its just an interim step to reventually removing that in the db layer | 17:08 |
| sean-k-mooney | ok so you would prefer we internally escalate to admin context | 17:08 |
| dansmith | sean-k-mooney: not only because it would affect lots of other non-service role things, but also because it expands that check which we probably want to minimize | 17:08 |
| sean-k-mooney | ya that fiar | 17:09 |
| dansmith | I'd prefer we explicitly "elevate" to admin for service role things at the point of access | 17:09 |
| gmann | yeah | 17:09 |
| gmann | ok, let me go with approach 1. policy check for service role at start 2. fetch the things (server etc) with admin context | 17:09 |
| dansmith | ++ | 17:09 |
| sean-k-mooney | yep that works for me | 17:09 |
| sean-k-mooney | the same shoudl apply to swap volume | 17:10 |
| sean-k-mooney | its the same check that is failign right | 17:10 |
| gmann | ok, thanks dansmith sean-k-mooney | 17:10 |
| sean-k-mooney | the get_server | 17:10 |
| gmann | dansmith: sean-k-mooney btw if you have time, placement policy updates are ready to review too https://review.opendev.org/c/openstack/placement/+/865618 | 17:10 |
| gmann | gibi: ^^ | 17:10 |
| gmann | sean-k-mooney: yes | 17:10 |
| sean-k-mooney | gmann: realistically it will be next week before i have time to take a look but i set RP+1 and ill try to come back to itthen | 17:14 |
| gmann | sean-k-mooney: sure, thanks | 17:15 |
| opendevreview | melanie witt proposed openstack/nova stable/yoga: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866192 | 17:19 |
| opendevreview | melanie witt proposed openstack/nova stable/xena: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866193 | 17:20 |
| opendevreview | melanie witt proposed openstack/nova stable/wallaby: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866194 | 17:21 |
| opendevreview | melanie witt proposed openstack/nova stable/victoria: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866195 | 17:29 |
| opendevreview | melanie witt proposed openstack/nova stable/ussuri: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866196 | 17:31 |
| opendevreview | melanie witt proposed openstack/nova stable/train: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866201 | 18:05 |
| opendevreview | Merged openstack/nova-specs master: fixing: allowing target state for evacuate https://review.opendev.org/c/openstack/nova-specs/+/866108 | 18:07 |
| opendevreview | Merged openstack/nova stable/yoga: refactor: remove duplicated logic https://review.opendev.org/c/openstack/nova/+/855022 | 19:05 |
| *** Guest305 is now known as atmark | 21:07 | |
| opendevreview | Ghanshyam proposed openstack/nova master: Enable new defaults and scope checks by default https://review.opendev.org/c/openstack/nova/+/866218 | 21:07 |
| atmark | need help modifying https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4150-L4172 . I'd like to add to check VM property if it contains say a string 'NoRestart' in addition to ignored_states | 21:48 |
| opendevreview | Ghanshyam proposed openstack/nova master: Enable new defaults and scope checks by default https://review.opendev.org/c/openstack/nova/+/866218 | 21:54 |
| *** dasm is now known as dasm|off | 22:23 | |
| opendevreview | melanie witt proposed openstack/nova stable/train: Adapt websocketproxy tests for SimpleHTTPServer fix https://review.opendev.org/c/openstack/nova/+/866201 | 22:33 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!