| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-22 | |||
| 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 | |
| 17:26:43 | gibi | mriedem_afk: could you hit the bandwidth spec update when you are back https://review.openstack.org/#/c/644810/ ? | |
| 17:33:22 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova-specs master: Add "CPU selection with hypervisor consideration" spec https://review.openstack.org/645814 | |
| 17:34:44 | gmann | dansmith: mriedem_afk i think we should expose project_id in locked_by field which can be easily fetched for 'owner' case (based on DB enum value and context) but not sure how to fetch the admin project-id when server is locked by admin and GET by owner | |
| 17:35:01 | gmann | we need to change the locked_by DB from enum to UUID ? | |
| 17:35:01 | dansmith | gmann: yeah that's the obviously difficult one | |
| 17:35:46 | gmann | should we change DB schema also in this spec so that we always store project-id in locked_by while lock API | |
| 17:35:48 | dansmith | we could just start throwing a locked_by_uuid in sysmeta too, and maybe just add "id" to the enum, | |
| 17:36:07 | mnaser | kashyap: regarding your arm change to support 'virt' machine type, does that ship in as specific release of qemu? | |
| 17:36:08 | dansmith | but since it's an enum in the schema, it's not easy to just convert it to a freeform string | |
| 17:36:12 | elbragstad | mriedem_afk just catching up | |
| 17:36:22 | kashyap | mnaser: Hi | |
| 17:36:26 | mnaser | under centos 7 with qemu-kvm, i can't find a 'virt' machine type (this is the rhel shipped one) | |
| 17:36:37 | gmann | dansmith: ok. | |