| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-22 | |||
| 17:42:49 | gmann | dansmith: in current spec- https://review.openstack.org/#/c/638629/3/specs/train/approved/add-locked-reason.rst@117 | |
| 17:42:50 | kashyap | mnaser: Yep, that is a good idea; and x86 will "soon" get it. There have been some mind-showering sessions on QEMU usptream list | |
| 17:42:53 | dansmith | gmann: if you're really concerned about that and are willing to do the work underneath to make it not have to, then okay | |
| 17:42:54 | gmann | yeah | |
| 17:43:31 | kashyap | mnaser: So expect to see a "virt" board for x86 (that is expressely designed for guests, including of course Nova instances). | |
| 17:43:44 | mnaser | kashyap: cool, that would be super neat | |
| 17:44:24 | gmann | because this is the only API we say 'admin' or 'owner' in response. and 'admin' will be extended in scope_type system-admin or project-admin | |
| 17:44:58 | dansmith | gmann: ack, well, raise that on the spec I guess | |
| 17:47:26 | mnaser | is "urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host='198.72.124.41', port=443): Read timed out. (read timeout=60)" under tempest.scenario.test_shelve_instance.TestShelveInstance a common failure? | |
| 17:47:34 | elbragstad | i think i stumbled across an article like this a while ago when we were trying to add an attribute to an enum http://komlenic.com/244/8-reasons-why-mysqls-enum-data-type-is-evil/ | |
| 17:56:27 | elbragstad | mriedem i thinking keeping the `admin` or `owner` details out of the database would be a good think moving forward | |
| 17:57:05 | elbragstad | a lot of the work we're doing with scope types introduces different types of "admins", so we can't necessarily assume "admin" is always the cloud administrator/operator | |
| 18:01:40 | dansmith | elbragstad: right, that's why we're talking about this: | |
| 18:01:48 | dansmith | it's already in the db, but not exposed to the user | |
| 18:02:00 | dansmith | the spec is proposing exposing that admin|user notion to the api user | |
| 18:02:28 | dansmith | so we either need to make the api sufficiently generic or change a bunch of stuff before we can do the other part, which is locked reason | |
| 18:02:50 | elbragstad | so a user would be able to see who locked an instance? | |
| 18:02:52 | dansmith | gmann_afk: elbragstad mriedem: What if instead of exposing the person that locked it for now, we just expose a boolean which is "can I unlock it?" | |
| 18:02:56 | dansmith | elbragstad: right | |
| 18:03:11 | dansmith | but we could paper over a lot of details by just saying "you can unlock this" or "you can not unlock this: | |
| 18:03:15 | elbragstad | dansmith the "can I unlock it" bit seems like a good middle ground? | |
| 18:03:22 | dansmith | right | |
| 18:03:35 | mriedem | dansmith: that depends on policy though, which could change between the time they have that response and when they try to unlock | |
| 18:03:38 | dansmith | at least lets us push the refactoring of the data store away from the current spec | |
| 18:03:38 | elbragstad | as a user - i probably care less about who locked my stuff and i just want to know if i can lock it | |
| 18:03:40 | mriedem | granted, i wouldn't expect policy to change often | |
| 18:04:01 | mriedem | the 'can i unlock' is a policy check | |
| 18:04:01 | dansmith | mriedem: sure, but same for exposing admin|user .. you have to now what the policy is to know whether it matters | |
| 18:04:31 | elbragstad | well - to calculate that boolean you'd just need to run the context through that policy check - then populate that accordingly in the data model (and response?) | |
| 18:04:43 | dansmith | right | |
| 18:04:54 | mriedem | not the data model, but the response yes | |
| 18:04:55 | dansmith | but if you don't use the boolean, and expose "locked_by: admin" | |
| 18:05:07 | dansmith | the user has to know whether policy considers them an admin or not right? | |
| 18:05:16 | dansmith | whereas "no you can't unlock this" is pretty clear | |
| 18:05:20 | openstackgerrit | Merged openstack/nova stable/queens: libvirt: Add workaround to cleanup instance dir when using rbd https://review.openstack.org/628726 | |
| 18:05:27 | elbragstad | ++ | |
| 18:05:29 | openstackgerrit | Merged openstack/nova stable/queens: Avoid BadRequest error log on volume attachment https://review.openstack.org/640116 | |
| 18:05:55 | dansmith | the fact that it could change between check and attempt is pretty much the case for everything :) | |
| 18:05:56 | elbragstad | "admin" is kinda confusing, what is locked by a project admin, a domain admin or a system admin? | |
| 18:06:11 | elbragstad | s/what/was/ | |
| 18:06:17 | mriedem | in this case project admin == 'owner' | |
| 18:06:24 | mriedem | same as if the owner of the instance locked it themselves | |
| 18:06:28 | mriedem | it's the system admin case that is wonky | |
| 18:07:00 | elbragstad | i'm missing the part why the system-admin case is wonky | |
| 18:07:26 | mriedem | i mean the case where the locked_by is 'admin' today is if someone outside the project that owns the instance locks the instance | |
| 18:07:38 | mriedem | project admin locking the instance treats locked_by as 'owner' | |
| 18:08:18 | elbragstad | ... interesting | |
| 18:08:26 | mriedem | idk how i feel about returning an at-the-time-of-request policy check boolean saying if you can or cannot unlock it with a subsequent unlock call, i don't think we have anything else like that in the api | |
| 18:08:27 | elbragstad | someone outside the project? | |
| 18:08:53 | dansmith | I totally don't understand the concern over the policy thing | |
| 18:09:01 | dansmith | we return data (or not) based on policy | |
| 18:09:32 | mriedem | for faults and event traceback yeah | |
| 18:09:33 | mriedem | true | |
| 18:09:47 | dansmith | and we used to for things like flavor extra specs, iirc | |
| 18:09:47 | mriedem | and the numa blob that is being proposed | |
| 18:09:58 | dansmith | and hostnames if you're admin | |
| 18:10:12 | dansmith | or fake host ids if you're not | |
| 18:10:16 | mriedem | it's slightly different though, those are just exposing things or not | |
| 18:10:45 | dansmith | if it really matters, then unlockable:true or omit it entirely, but I don't think that's better :) | |
| 18:10:47 | mriedem | i'd be much more comfortable with this locked_reason spec if we didn't have to deal with gd locked_by in the first place | |
| 18:11:58 | mriedem | locked_by is just internal meta and i'm not sure why we need to expose it, and it's only related to a policy check on unlock | |
| 18:12:12 | mriedem | if you can't unlock you find out with a 403 as normal | |
| 18:12:13 | dansmith | well, we can do that too | |
| 18:12:19 | dansmith | but I don't understand the concern | |
| 18:12:30 | mriedem | i wouldn't block on it | |
| 18:12:59 | mriedem | just seems like we're maybe trying to solve a problem we're making for ourselves | |
| 18:13:04 | mriedem | which we maybe don't need to care about | |
| 18:13:20 | dansmith | I tell you who's making problems.. mriedem :) | |
| 18:13:33 | mriedem | i've said a million times that guy is an insufferable asshole | |
| 18:13:41 | dansmith | heh | |
| 18:14:30 | elbragstad | dumb question: does exposing the attribute actually have an impact on someone being able to unlock or lock an instance? | |
| 18:14:55 | mriedem | it's an indication if they can unlock it | |
| 18:15:10 | gmann | i agree that 'can i unlock' is policy control things. exposing locked_by I consider the benefit of as user i know my instances is locked by someone else like admin (system-admin or project-admin) and i can ask them to unlock | |
| 18:15:21 | dansmith | I imagine the impetus for this whole thing is, someone tries to do something, can't because locked, so they don't understand why and have to call support to figure out why.. if there was a "locked by an admin, reason: because you didn't pay" then .. they know | |
| 18:15:23 | elbragstad | but if that attribute wasn't exposed to them, could they still build the same request and send it to nova and have it behave the same way? | |
| 18:15:35 | dansmith | elbragstad: yes | |
| 18:15:49 | dansmith | elbragstad: but they may not understand why it fails if they don't see it was locked by someone else | |
| 18:15:50 | mriedem | right i see the value in the reason field for sure | |
| 18:16:04 | gmann | then we can leave locked_by to expose | |
| 18:16:23 | openstackgerrit | Merged openstack/nova stable/queens: Allow utime call to fail on qcow2 image base file https://review.openstack.org/633493 | |
| 18:16:39 | openstackgerrit | Merged openstack/nova master: Update master for stable/stein https://review.openstack.org/645463 | |
| 18:16:41 | mriedem | elbragstad: this is the check code on unlock https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/lock_server.py#L47 | |
| 18:16:47 | openstackgerrit | Merged openstack/nova stable/stein: Update .gitreview for stable/stein https://review.openstack.org/645461 | |
| 18:17:01 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4025 | |
| 18:17:07 | dansmith | gmann: you're saying you think exposing "can I unlock" is bad right? | |
| 18:17:28 | dansmith | what if we just expose locked_by_me:True|False or locked_by=user|other | |
| 18:17:29 | gmann | yeah. that is all control by policy | |
| 18:17:40 | dansmith | okay I don't understand that at all, but whatever :) | |
| 18:17:44 | gmann | if locked by other then policy should say 403 | |
| 18:18:14 | dansmith | gmann: right, but you're going to make them try to determine that instead of just doing the check ourselves? we can ask policy about it, just like we do for the conditional exposure of fault | |
| 18:18:25 | dansmith | try the put I mean | |
| 18:18:51 | elbragstad | if the policy is written like "(role:admin and system_scope:all) or rule:owner" and locked_by_me is True|False, is locked_by useful? | |
| 18:18:58 | mriedem | if i'm following this unlock logic, if an admin in my project locks my server, i can unlock it. but if an admin outside my project (global god like admin) locks my server, then i can only unlock it if i pass the override policy check right? | |
| 18:19:14 | elbragstad | oh - maybe i was missing ^ that bit | |
| 18:19:37 | gmann | mriedem: right | |
| 18:19:44 | mriedem | and the override policy defaults to rule:is_admin i.e. system level god admin | |
| 18:19:47 | mriedem | in legacy terms | |
| 18:20:57 | gmann | elbragstad: we compare only project_id (not user id) for deciding it is owner or not. | |
| 18:21:00 | mriedem | so with dan's proposal, on response, we checking if you pass is_expected_locked_by and if not, do you pass the override policy, exactly like the unlock code, | |
| 18:21:16 | mriedem | are we going to want to perform that policy check when listing servers with details? ^ for each server? | |