Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-22
15:35:45 mriedem so hold up
15:36:06 tssurya mriedem: okay
15:44:40 mriedem tssurya: done
15:44:46 tssurya thanks
15:45:02 mriedem elbragstad: so we've got this silly locked_by field on the instance object in our db which is an enum of 'admin' or 'owner',
15:45:24 mriedem https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4016
15:46:05 mriedem i guess in this case admin is a non-project admin, so like a system scope admin
15:46:20 mriedem anyway, there is a proposal to expose that out of the rest api https://review.openstack.org/#/c/638629/3/specs/train/approved/add-locked-reason.rst@117
15:46:37 mriedem do you see any issues with future policy scope creep weirdness with that?
15:46:48 dansmith sure seems like that should be a userid and not an enum
15:47:14 dansmith or
15:47:14 dansmith (before we expose it I mean)
15:47:18 mriedem it's hella old
15:47:22 dansmith I know
15:47:45 gmann mriedem: dansmith as in future it can be locked by system.admin and project.admin as well we should make it user-id.
15:47:46 dansmith so when we expose it, maybe we should make it possible for it to be owner|admin|other, so that later if we store a user-id, we can say "other" there
15:48:06 gmann otherwise we will not know it is locked by project admin or system admin
15:48:19 mriedem gmann: today if you lock as project admin it's 'owner'
15:48:32 mriedem it's only 'admin' if the request comes from an admin in a different project
15:49:55 dansmith tssurya: just added a comment about that
15:50:04 gmann mriedem: no, it is 'admin' if project admin lock any non-admin user server of same project
15:50:07 tssurya dansmith: checking
15:50:18 mriedem looks like it was added in https://review.openstack.org/#/c/38196/
15:50:37 dansmith god the world was so much simpler in 2013
15:50:43 mriedem gmann: ? is_owner = instance.project_id == context.project_id
15:50:53 mriedem as long as the project id matches, it's owner
15:50:56 mriedem despite the role
15:51:01 gmann ohh yeah we only match project id. soory
15:51:03 gmann sorry
15:51:48 dansmith "soory" correct canadian for "sorry"
15:52:01 gmann :)
15:52:01 mriedem he's fitting right in
15:52:21 gmann so we assume 'admin' is system-admin which again might be confusing
15:52:36 mriedem hmm, i wonder if locked_by is still used on unlock https://review.openstack.org/#/c/38196/13/nova/compute/api.py@2490
15:52:51 dansmith it's what gates whether or not you can unlock right?
15:52:54 gmann I am adding lock as [system, project] scope_type in my PoC of scope_type - https://review.openstack.org/#/c/645452/2/nova/policies/lock_server.py
15:53:06 mriedem ah it is
15:53:49 gmann but we have overridden unlock also i think that is for admin to unlock owner server
15:53:52 gmann owner locked server
15:54:11 mriedem os_compute_api:os-lock-server:unlock:unlock_override
15:54:12 mriedem yuck
15:54:23 mriedem baking policy into the db
15:54:44 gmann yeah, we should remove that also. not sure any reason to keep that
15:54:44 dansmith ready for friday to be over already?
15:56:29 gmann are we going to expose 'locked_by' also ? not checked the spec yet
15:57:27 tssurya dansmith: umm you want to add "other" where ? as in locked_by enum schema needs to change ? or just in the response api terminology for future needs ?
15:57:42 dansmith tssurya: just the api schema for the future
15:57:54 tssurya okay
15:58:00 dansmith we should get some agreement on that (and the actual enum value) first though
15:58:06 tssurya yea
15:58:07 mriedem dansmith: i replied to that comment,
15:58:11 dansmith and mriedem is too busy barfing
15:58:13 mriedem Instance.locked_by is just a string
15:58:21 mriedem which will get spewed back out of the api response
15:58:36 mriedem so if we allow storing a user_id in there in the future, it's a db schema change but shouldn't really mean any change to the instance object or api
15:58:41 dansmith oh sorry, I assumed it was an enum in the json schema too
15:58:52 mriedem it might be in tempest...checking
15:59:06 mriedem oh nvm, we don't expose it today
15:59:08 gmann we only expose 'lock' as string in API response
15:59:09 dansmith it says admin or owner
15:59:12 mriedem right
15:59:16 gmann we do not expose 'locked_by'
15:59:22 mriedem tempest is the only thing that would have a response schema check
15:59:25 dansmith mriedem: right the thing she's adding says "admin or owner"
15:59:39 mriedem i know, but that's just the possible values in the db schema today
15:59:42 dansmith so I assumed that meant a schema enum
15:59:44 mriedem tempest could just allow 'string'
15:59:56 mriedem nova doesn't have any response schema
16:00:01 dansmith well let's be clear about it then because it looks like those are the only two values
16:00:05 dansmith I mean in the spec
16:00:06 mriedem sure
16:01:27 mriedem 11am and i forgot what i was actually going to try and get done today...
16:04:48 mriedem need a stable core to hit this https://review.openstack.org/#/c/633493/
16:05:01 mriedem https://review.openstack.org/#/c/640116/ is also trivial on queens
16:05:05 mriedem for fast approval
16:05:23 mriedem and https://review.openstack.org/#/c/637391/
16:05:38 mriedem once we get those we could probably release queens
16:07:48 mriedem thanks dansmith
16:08:39 dansmith uar
16:24:37 mdbooth https://bugs.launchpad.net/nova/+bug/1821373
16:24:38 openstack Launchpad bug 1821373 in OpenStack Compute (nova) "Most instance actions can be called concurrently" [Undecided,New]
16:25:33 melwitt dansmith: want to hit these release bot changes? https://review.openstack.org/645461 and https://review.openstack.org/645463
16:25:45 sean-k-mooney mdbooth: i assume you are going to remove the logic that shortcircuts if the value is set the same value and have it hit the db
16:26:14 mdbooth sean-k-mooney: Probably, but I'll have to meditate carefully on the safety of whatever.
16:26:29 mdbooth sean-k-mooney: And right now I'm a bit hungry and a bit checked out for the weekend :)
16:26:59 sean-k-mooney mdbooth: fixing db curuptions is definetly not a firday evning activity
16:27:11 mdbooth Don't try to fix db races on an empty stomach :)
16:27:47 dansmith melwitt: I think you're safe to fast-approve things like that :)
16:27:57 melwitt can't be too careful
16:28:30 melwitt (joke)
16:31:06 dansmith mdbooth: hmm, check_instance_state has a task state to try to prevent some of what you describe, not sure why that's not being used (although it won't close it entirely)
16:31:59 dansmith but I agree, I think either doing a dedicated task_state check in the db if we're going to punt due to no-updates will retain as much of the optimization as possible and close that window
16:32:13 dansmith letting it all go through even if no updates is an option, but it's heavy
16:32:21 dansmith (i.e. what we were doing before)
16:40:43 mdbooth dansmith: Yeah. I started thinking about whether we can safely bypass the trip to the db, but that's not a Friday afternoon thought-train.
16:41:13 dansmith I don't think we can entirely based on your reasoning, but we can make it efficient
16:41:22 dansmith but yes, a tuesday conversation
16:44:03 dansmith fried_rolls: are you not approving this because of your comment or for sequencing reasons? https://review.openstack.org/#/c/644625/5
16:44:20 dansmith fried_rolls: if the former, I'll just add it to unblock progress, but it seems entirely duplicative to me
16:52:16 openstackgerrit melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749

Earlier   Later