| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-22 | |||
| 18:26:22 | mriedem | dansmith: you mean locked_by_owner yeah? | |
| 18:26:29 | dansmith | sure | |
| 18:26:33 | mriedem | where the logic would just be locked_by_owner = instance.locked_by == 'owner' | |
| 18:26:39 | dansmith | or locked_by=Owner|NotOwner | |
| 18:26:45 | dansmith | right | |
| 18:26:57 | dansmith | don't say admin, just say "not you" | |
| 18:27:03 | elbragstad | ++ | |
| 18:27:21 | mriedem | i'd be more ok with doing locked_by=owner|other | |
| 18:27:42 | mriedem | i'm trying to express this... | |
| 18:28:06 | dansmith | *simpsons | |
| 18:28:27 | mriedem | but doing a policy check when building a response based on policy for something that would be checked during a subsequent unlock request, seems very weird and capability-ish to me, which we don't do elsewhere. it's true we check policy when building a response to hide things for non-admins, like system details (hosts, fault traceback, etc), but those aren't capability things | |
| 18:28:30 | gmann | I am also ok for locked_by=owner|other if we cannot expose the UUID. | |
| 18:28:44 | mriedem | gmann: we don't store the uuid anywhere for who locked the instance | |
| 18:28:45 | elbragstad | if instances stored user information instead of project ids, we wouldn't need two lock properties, would we? | |
| 18:29:01 | gmann | mriedem: yeah, i mean if we do not want to change that as it will be complex | |
| 18:29:04 | dansmith | mriedem: so the reason is we don't do exactly this elsewhere and it "feels wrong" ? | |
| 18:29:22 | mriedem | correct | |
| 18:29:32 | dansmith | I mean, I understand that things feel wrong.. I don't understand why this one does, but that's okay | |
| 18:29:34 | dansmith | alright | |
| 18:29:36 | mriedem | i could argue it'd be nice to have a "can_be_resized" flag in the server response | |
| 18:29:41 | mriedem | that doesn't mean we're going to add that | |
| 18:29:55 | dansmith | people have asked for that :) | |
| 18:29:59 | mriedem | i'm sure they have | |
| 18:30:00 | dansmith | based on all the reasons you *can't* do that | |
| 18:30:11 | mriedem | and we've talked about that capabilities API for several years | |
| 18:30:18 | dansmith | how's that coming? :) | |
| 18:30:31 | mriedem | it's mostly rotted flesh in my storage room | |
| 18:30:32 | dansmith | because to me, | |
| 18:30:34 | mriedem | don't tell the cops please | |
| 18:31:00 | dansmith | the capaibiltiies api is the same thing, but all in one place, which IMHO lends further support to it being a good idea, but just not doable until we have enough other ones for critical mass :) | |
| 18:31:20 | dansmith | s/doable/palatable/ | |
| 18:31:24 | dansmith | and that's unfortunate | |
| 18:31:52 | dansmith | but anyway, exposing locked_by=owner|other seems like a reasonable compromise | |
| 18:32:05 | mriedem | i'd be fine with a GET /servers/{server_id}/capabilities API | |
| 18:32:14 | mriedem | and we start rolling shit into that like this | |
| 18:32:25 | dansmith | to be clear, | |
| 18:32:46 | dansmith | that means someone has to get the server, see that it's locked and a reason, then go fetch the capabilities of the server to see if they can unlock it | |
| 18:32:58 | dansmith | which is fine, but... | |
| 18:32:59 | mriedem | they could just try to unlock it | |
| 18:33:01 | mriedem | get the 403 | |
| 18:33:03 | mriedem | like always | |
| 18:33:12 | dansmith | sure | |
| 18:33:15 | mriedem | if locked_by=other | |
| 18:33:16 | elbragstad | ^ that's what people do now with other APIs (not necessarily in nova) | |
| 18:33:28 | mriedem | then there is a reasonable guess they might not be able to unlock it | |
| 18:33:32 | mriedem | which the reason should tell them why it's locked | |
| 18:33:35 | dansmith | I dunno, I'm just having trouble following all the logic spaghetti, but I don't really need to care | |
| 18:34:02 | elbragstad | does the reason include something like "this was locked by an admin, pay your bill pls" | |
| 18:34:11 | gmann | yeah lock_reason can tell, "this is locked by admin/system and ask them to unlock after xyz condition/time" | |
| 18:34:23 | gmann | elbragstad: yeah, i think it can be | |
| 18:34:32 | dansmith | elbragstad: I know they do, and I'm sure it doesn't set off any alarms to hit 403s, but it feels like trying sudo vi /etc/passwd on a system to see if you can, and if not, it's reported :) | |
| 18:35:29 | gmann | IMO with lock_reason info in response can explain the locked_by things. till now we did not have lock_reason so may be locked_by was important to know | |
| 18:35:38 | elbragstad | yeah - true... the same argument has been made to have a capabilities-like api in keystone, but we're obviously not there yet, either | |
| 18:35:49 | dansmith | lock_reason is for humans, locked_by is for computers, IMHO | |
| 18:35:54 | dansmith | so not really a substitute | |
| 18:36:20 | mriedem | i think i'm going to go lock myself in the garage with the car running | |
| 18:36:38 | dansmith | elbragstad: last time I went jiggling door handles in my neighborhood to see who has theirs locked, I was asked to not do that | |
| 18:36:48 | elbragstad | make sure you set locked_by=owner | |
| 18:36:56 | dansmith | mriedem: unfortunately if you have a catalytic converter that's going to take a really long time | |
| 18:37:16 | gmann | dansmith: ok so computer purpose still can be fulfilled by locked_by=other|owner. | |
| 18:37:22 | dansmith | gmann: yes | |
| 18:37:34 | elbragstad | dansmith yeah - not the best u-x, i agree | |
| 18:38:15 | gmann | i am thinking exposing exact UUID of locked person (admin etc) can be security leak ? | |
| 18:38:50 | gmann | so just say you can unlock it or not via locked_by=owner|other seems appropriate for me now | |
| 18:39:16 | gmann | for example exposing system admin id to end user etc | |
| 18:45:19 | openstackgerrit | melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749 | |
| 18:45:55 | melwitt | hm, wanted to get this other spec update done before moving specs but it's in merge conflict https://review.openstack.org/#/c/639033/1 | |
| 18:47:19 | mriedem | someone want to send this queens change through https://review.openstack.org/#/c/643219/ | |
| 18:47:23 | melwitt | er... I guess it's not. just somehow it makes the move specs patch in merge conflict | |
| 18:47:24 | mriedem | i'll start the release request | |
| 18:47:32 | mriedem | melwitt: i'll get that spec update | |
| 18:47:59 | melwitt | mriedem: thanks. I'll get the queens patch | |
| 19:01:14 | openstackgerrit | Merged openstack/nova-specs master: Update alloc-candidates-in-tree https://review.openstack.org/639033 | |
| 19:05:29 | openstackgerrit | Merged openstack/nova stable/pike: Fix bug case by none token context https://review.openstack.org/603044 | |
| 19:09:36 | temka | Uh, do we timestamp our logs in local time? | |
| 19:10:06 | fried_rice | dansmith: I thought to give more people time to look at it and comment. | |
| 19:11:02 | dansmith | fried_rice: roger | |
| 19:11:03 | fried_rice | dansmith: Also, the thing mriedem is working on with the multiattach capability has raised an interesting question that we're still working through. Namely, where do you put the required trait (which request group)? | |
| 19:11:21 | fried_rice | Not that that necessarily should hold up your spec, but it's giving me pause. | |
| 19:11:43 | mriedem | fried_rice: did you see my change on that patch? | |
| 19:11:46 | dansmith | yeah, would rather save that for implementation | |
| 19:11:47 | mriedem | i'm not using a request group anymore | |
| 19:12:00 | mriedem | i'm hitching a ride on the RequestSpec.flavor | |
| 19:12:20 | mriedem | cash, grass or ..., no traits ride for free | |
| 19:12:21 | fried_rice | mriedem: been afk for the past couple hours, if that's when you changed it. But eventually you're adding it to a request group, albeit maybe indirectly. | |
| 19:12:25 | dansmith | mriedem: I was trying to decide if I like or hate that | |
| 19:12:32 | mriedem | fried_rice: right indirectly | |
| 19:12:44 | openstackgerrit | melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749 | |
| 19:12:53 | dansmith | on the one hand it saves the "how do we request it" for the deeper layers, which might be good, | |
| 19:13:14 | fried_rice | on the other hand, it isn't part of the flavor | |
| 19:13:22 | dansmith | but also modifying the flavor, while it's the reason we added it and store it separately, strikes me as "this doesn't FEEL right" | |
| 19:13:37 | fried_rice | that's my gut too, but I haven't really thought about it | |
| 19:14:11 | mriedem | i did it for expediency b/c i couldn't figure out the semantics of the internal request group stuf | |
| 19:14:12 | mriedem | *stuff | |
| 19:14:34 | dansmith | yeah and the fact that you have to also worries me | |
| 19:14:41 | mriedem | it kept trying to add an empty numbered resources dict, and there are no resources in my request group | |
| 19:14:57 | mriedem | it's likely a bug in the resource request / group stuff internally | |
| 19:15:09 | fried_rice | where should it add it? | |
| 19:15:21 | mriedem | it was also just one of those, "oh hey i have like 2 hours left today to do something, how about i hack this together and see how it goes" thing | |
| 19:16:09 | dansmith | doesn't the base unnumbered group correspond to the whole request? | |