| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-10 | |||
| 14:31:54 | sean-k-mooney | intell moved it to the cpu 12 years ago with nehelem | |
| 14:32:05 | gibi | OK, then I'm fine with the socket policy series as is | |
| 14:32:07 | bauzas | that reminds me, any barebone server you could recommend me for running an openstack env ? | |
| 14:32:20 | artom | gibi, cool, thanks! | |
| 14:32:23 | gibi | let's merge it and do whathever refactor you agree on as a followup | |
| 14:32:59 | sean-k-mooney | bauzas: i got one from ebay for 1600 or so but i allo have older ones for about 500 | |
| 14:33:01 | gibi | it also means that Sean's port policy patch should go on top | |
| 14:33:17 | sean-k-mooney | bauzas: thre are lots you can get second hand depending on your budget | |
| 14:33:20 | bauzas | sean-k-mooney: woah that's expensive | |
| 14:33:28 | sean-k-mooney | bauzas: if you dont have an atic get a tower server | |
| 14:33:30 | gmann | stephenfin: on shelve block: by this you mean you will return this in new code based on config right ? "the current block we have uses https://github.com/openstack/nova/blob/ab07507e5cfce6232fef373d07ff92ea704541da/nova/exception.py#L158-L159" | |
| 14:33:34 | gmann | sean-k-mooney: ^^ | |
| 14:33:38 | gmann | not stephenfin | |
| 14:33:54 | bauzas | sean-k-mooney: I have a cave, but it's already fully packed so a barebone would get my preference | |
| 14:34:10 | gmann | stephenfin: ack, sorry i missed that part of filtering. I will check that | |
| 14:34:24 | bauzas | a tower is nice but too huge for the place I'd put it in there | |
| 14:34:41 | sean-k-mooney | bauzas: i have to run but we can talk about it later if you want you can get ceaper system if you hapy to go older in 200-500 price point | |
| 14:34:57 | bauzas | sean-k-mooney: I'd appreciate it | |
| 14:35:06 | bauzas | I honestly don't need a lot of power | |
| 14:35:13 | bauzas | I just want rapid disks and RAM | |
| 14:35:20 | bauzas | for spinning VMs | |
| 14:35:39 | sean-k-mooney | gmann am it would return that for vms with cyborg prots and a similer new excpetion for vms with sriov port | |
| 14:35:45 | bauzas | I was both frustrated and ashamed by gibi testing my own series :p | |
| 14:35:50 | sean-k-mooney | gmann: the new one woudl also just inherit form forbidden | |
| 14:35:58 | stephenfin | gmann: Ack. Like I said, I don't really like it myself. Relying on this obscure aggregate metadata property that was previously only used for a specific filter and optional pre-filter feels wrong | |
| 14:35:59 | sean-k-mooney | gmann: so no change in error code | |
| 14:36:22 | gibi | bauzas: no worries. I'm lucky as I have access to a lab with decent hardware (even with SRIOV capable NICs) | |
| 14:36:39 | bauzas | gibi: that's what I honestly'd like to have | |
| 14:36:44 | stephenfin | gmann: If it were me, I'd probably just drop the idea and remove that idea from the spec but perhaps there are people who really want this? :-D | |
| 14:36:57 | sean-k-mooney | but ya basically return that for shelve based vnic_type in (direct,direct-pyshical ...) all the sriov or cyborg vnic types | |
| 14:37:01 | gmann | sean-k-mooney: +1, as long as it inherit from forbidden error, wsgi layer will handle it as 403 otherwise you need to handle it explicitly in API controller. | |
| 14:37:28 | sean-k-mooney | ok ill add you to the patch to review if that is ok when i get time to write it | |
| 14:37:43 | gmann | sean-k-mooney: and yes 403 return for error cases does not need microversion as it is already generic return code as you mentioned. | |
| 14:37:52 | yonglihe | gibi： hope you have some bandwidth . I can fix comments at you guys night. wish me lucky. -:) | |
| 14:38:11 | gibi | yonglihe: I plan to get to the smartnic series before I leave for today | |
| 14:38:15 | gibi | yonglihe: have a nice night | |
| 14:38:22 | gmann | stephenfin: but if we drop we again go back to the problem of using PROJECT_AMDIN for POST /server API on specific host | |
| 14:38:58 | stephenfin | gmann: That's a fair point | |
| 14:39:00 | yonglihe | gibi: thanks have a good day. | |
| 14:39:14 | stephenfin | gmann: In that case, we can persist with it. I wasn't sure of the use case but that sounds like a good one. | |
| 14:49:00 | gmann | stephenfin: 1 comment, for case where there is no 'filter_tenant_id' in host's aggregate then that host we can return for project admin | |
| 14:49:48 | stephenfin | oh, that makes things more useful | |
| 14:50:06 | stephenfin | isn't that a lot of information to expose though? | |
| 14:51:11 | stephenfin | Maybe not. They'll just see the summary view so ID, hostname, state, and status | |
| 14:51:14 | gibi | that would basically mean that all project admin in a non tenant separated cloud see every compute | |
| 14:51:38 | gmann | yeah | |
| 14:51:43 | gibi | do we allow project admins to boot on all the computes in that case directly? | |
| 14:52:30 | gibi | I mean what exactly this means? | |
| 14:52:30 | gibi | 15:38 < gmann> stephenfin: but if we drop we again go back to the problem of using | |
| 14:52:30 | gmann | all you mean all or only host's computes with no 'filter_tenant_id' | |
| 14:52:33 | gibi | PROJECT_AMDIN for POST /server API on specific host | |
| 14:53:10 | gibi | does the above mean that we want to allow project admins to boot on specific compute host? | |
| 14:53:22 | stephenfin | I'm looking and we already do | |
| 14:53:24 | gmann | if it pass the filter_tenant_id filter (host match with filter_tenant_id or there is host with no filter_tenant_id) then that compute canbe allowed and rest 403 | |
| 14:53:51 | gibi | hm, that feels powerful | |
| 14:54:00 | stephenfin | but gmann's comment there says we should change that to system admin | |
| 14:54:03 | gibi | especially in a non tenant separated cloud | |
| 14:54:48 | gmann | stephenfin: no, i mean system reader get everything, and project admin only go with those filter_tenant_id filters | |
| 14:55:21 | stephenfin | isn't that what I've done? | |
| 14:55:22 | gmann | may be we have to check if context is scoped to system or project | |
| 14:55:50 | gmann | this one? https://review.opendev.org/c/openstack/nova/+/765798/5..6/nova/api/openstack/compute/hypervisors.py#197 | |
| 14:56:01 | gmann | and you will keep default policy as SYSTEM_READER? | |
| 14:57:02 | stephenfin | oh, are you talking about the context I use in my call to 'objects.Aggregate.get_by_metadata'? | |
| 14:58:02 | stephenfin | I'm changing the 'os_compute_api:os-hypervisors:list' policy to project admin or system reader | |
| 14:58:32 | stephenfin | but you'll note that check doesn't pass a context argument | |
| 14:58:45 | stephenfin | so it will never pass for a project admin, right? | |
| 14:59:03 | stephenfin | while it will pass for a system admin or system reader | |
| 14:59:08 | stephenfin | since they don't need context | |
| 14:59:28 | stephenfin | it's a bit of a hack, I'll admit, but it seemed logical to me | |
| 14:59:50 | gmann | stephenfin: it will pass with project admin too right with ' 'os_compute_api:os-hypervisors:list' policy to project admin or system reader' | |
| 15:00:02 | gmann | it will fail for project member/reader | |
| 15:00:51 | stephenfin | the check will fail (non-fatally) for project admins, which means we'll enter that block | |
| 15:01:29 | gmann | but with this it would fail right?> -https://review.opendev.org/c/openstack/nova/+/765798/6/nova/policies/hypervisors.py#37 | |
| 15:01:33 | gmann | *would not | |
| 15:02:21 | stephenfin | I'm confused. Let's start from the top | |
| 15:02:44 | stephenfin | The policy has been changed from "system reader" to "system reader or project admin" | |
| 15:02:49 | stephenfin | We check that here https://review.opendev.org/c/openstack/nova/+/765798/5..6/nova/api/openstack/compute/hypervisors.py#277 | |
| 15:03:17 | stephenfin | If e.g. a project reader tries to access this API, they'll get a HTTP 403 | |
| 15:03:47 | stephenfin | we then check the *same policy* again here https://review.opendev.org/c/openstack/nova/+/765798/5..6/nova/api/openstack/compute/hypervisors.py#197 | |
| 15:04:13 | stephenfin | only this time we do not provide "target={'project_id': context.project_id}" | |
| 15:04:15 | gmann | yeah 277 protect from project member/reader | |
| 15:04:33 | stephenfin | you must provide that to pass any project checks | |
| 15:04:40 | gmann | ahh, i did not notice the target | |
| 15:04:54 | gmann | got it, little confusing to read | |
| 15:04:54 | stephenfin | so that check will return False for project members but true for system admin/member/reader | |
| 15:05:02 | stephenfin | *project admins | |
| 15:05:08 | gmann | yeah | |
| 15:05:13 | stephenfin | yeah, like I said, it's a bit of hack | |
| 15:05:28 | stephenfin | that's what I was trying to call out with this comment | |
| 15:05:30 | stephenfin | # don't pass project information, forcing the check to be a system | |
| 15:05:32 | stephenfin | # admin check | |
| 15:05:48 | gmann | or simple we can check context.system_scope == 'all' or whatever exact value | |
| 15:05:57 | stephenfin | I should have said "don't provide project information as part of the target" | |
| 15:06:05 | stephenfin | that would be simpler :) | |
| 15:06:28 | stephenfin | what happens if we don't have scoping enabled though? | |
| 15:06:47 | stephenfin | would that result in admins not being able to list all hypervisors? | |
| 15:07:35 | gmann | https://github.com/openstack/oslo.context/blob/0d02866365bc8b779aef9ebd0b79a52c96ae40e5/oslo_context/context.py#L251 | |
| 15:08:23 | gmann | humm that is tricky, in old way we allowed to list all hypervisors | |
| 15:09:36 | gmann | so as long as we support deprecated policy it will pass the 277 and @197 if you can check "if CONF.oslo_policy.enforce_scope and context.system_scope == 'all'" then we can support old way too | |
| 15:09:57 | gmann | i mean only enable that check for CONF.oslo_policy.enforce_scope=true | |