Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-11-30
19:15:37 dansmith gmann: well, it just assumes member if not admin, which all the current users of that fixture are going to expect.. I'm saying it needs to be extended to allow you to tell it what roles you want
19:15:43 gmann and later we can add user_id as 'member', 'reader' etc and add role accodringl;y
19:15:48 dansmith gmann: otherwise non-admin stuff will all fail too because roles=[] can't do aything
19:16:02 gmann yeah that is true
19:16:36 dansmith if we fix the deprecated rule bit, then this doesn't matter just yet, but before we can test with deprecated rules disabled, we'll have to fix this I think
19:17:51 gmann dansmith: right for No legacy tests we need that. and we can add we can add user_id == 'member' and add only member role and else part as add reader
19:18:05 dansmith sure, or just pass roles= to the fixture
19:18:27 dansmith right now, the user that is used is something other than "member" so we probably need a bunch of changes for that first
19:18:57 gmann +1, much better
19:30:41 opendevreview Dan Smith proposed openstack/nova master: Make API fixture pass roles https://review.opendev.org/c/openstack/nova/+/819907
19:30:47 dansmith gmann: like this-ish ^
19:33:45 gmann dansmith: +1 yeah, and you want default to reader only (lower level of access) ? here https://review.opendev.org/c/openstack/nova/+/819907/2/nova/tests/functional/api/client.py#140
19:34:30 dansmith gmann: that defaults to member, not reader.. since that's what everything else would expect currently, it seemed like that would be the best default
19:34:54 dansmith looks like maybe it's only used in one other place though,
19:35:04 dansmith so I guess I could default it even lower and fix that one too
19:35:32 dansmith default should be roles=[] right? since that's what you get if you're not explicitly granted reader/member/admin on a project
19:35:40 gmann dansmith: yeah. but if it need mroe test fix then we can leave as of now and later we can do while moving func tests also to reader/mem,ber
19:35:43 dansmith oh nm, it's just used here
19:37:41 opendevreview Dan Smith proposed openstack/nova master: Make API fixture pass roles https://review.opendev.org/c/openstack/nova/+/819907
19:37:51 dansmith I thought client was used a few other places directly, but it's not, so we should be good to default to roles=[] ^
19:39:04 gmann +1. yeah its self.api and self.admin_api
19:39:06 gmann as of now
19:39:19 dansmith yup
19:39:27 sean-k-mooney well default to roles=None
19:39:39 sean-k-mooney rather then []
19:39:40 opendevreview Ghanshyam proposed openstack/nova master: Introduce 'admin' policy base rule https://review.opendev.org/c/openstack/nova/+/819389
19:39:49 sean-k-mooney sicne you should not use mutable defaults but same effect
19:39:56 gmann dansmith: ^^ this is for 'admin' basically renaming 'context_is_admin' to 'admin'
19:40:02 dansmith sean-k-mooney: did you look at the patch? :)
19:40:11 dansmith gmann: cool thanks
19:40:24 sean-k-mooney nope just saw the converstaion scroll by
19:40:43 dansmith sean-k-mooney: trying not to take offense that you think I don't know not to use mutable defaults :)
19:41:50 sean-k-mooney :)
19:42:58 sean-k-mooney i still see it in code often enough but ya you already did it the right way https://review.opendev.org/c/openstack/nova/+/819907/3/nova/tests/functional/api/client.py#140
19:49:52 gmann I think we have hacking rule for that.
20:13:37 dansmith gmann: I don't think your admin patch works for me by itself,
20:13:44 dansmith since it's still requiring role:admin, which isn't what we have today
20:13:56 dansmith it needs to be is_admin in order to work as-is right?
20:24:52 gmann dansmith: context set is_admin based on this rule itself https://github.com/openstack/nova/blob/d630615a02469442fb50ed4aa7e092206a28166a/nova/context.py#L138
20:24:58 gmann https://review.opendev.org/c/openstack/nova/+/819389/4/nova/policy.py
20:29:16 gmann but as it is two level deprecated rule combined I hope each one is logical ORed by oslo policy. but is it failing on your patch? if so then we can avoid to rename it for now. and add DEPRECATED_ADMIN_POLICY in is_context_admin rule only
20:39:49 gmann dansmith: yeah, i ran your patch with my change and it does not work. oslo policy add only one level of deprecated rule in logical OR.
20:40:56 gmann dansmith: I will abandon my patch and let's rename CONTEXT_ADMIN to ADMIN in your patch but keeping rule:is_context_admin. commented in https://review.opendev.org/c/openstack/nova/+/816206/comment/543216d1_15fecfb1/
21:43:47 dansmith gmann: okay, but that's not enough, it needs to be "role:admin or is_admin:True"
21:44:21 dansmith so do you want me to do that on context_is_admin with a deprecated_rule= or add it to admin_api?
21:44:51 gmann dansmith: yeah that will add ORed both
21:45:59 gmann dansmith: yeah deprecated_rule=DEPRECATED_ADMIN_POLICY .
21:46:13 dansmith okay that's not enough either, I need this: https://termbin.com/bnqh
21:46:15 dansmith you okay with that?
21:46:27 dansmith oh wait
21:46:37 dansmith it's enough to use it as-is for today, you're right
21:46:45 dansmith I'm thinking this needs to work for both old and new, but not as of this patch
21:46:58 dansmith so yeah, I will just add the deprecated_admin_policy there
21:47:12 gmann if you do this then it should ADMIN = 'rule:context_is_admin'
21:47:58 dansmith yep, hadn't saved that before paste :)
21:48:31 dansmith running all the tests now, but expect this to work now
21:48:40 gmann dansmith: so with that you do not need this right? 'check_str='is_admin:True or role:admin','
21:48:52 dansmith correct, for just this patch, I don't
21:48:59 gmann yeah.
21:50:40 dansmith there are just too many balls in the air with this stuff for my tiny brain :D
23:06:26 artom dansmith, for the record, I'm painfully aware of the door that your last line opened, but am steadfastly refusing to step through it
23:06:54 dansmith artom: lol, I *literally* almost pinged you, asking if you wanted to do the mriedem thing in his stead :D
23:07:48 artom I'm all grown up now
23:08:33 dansmith riiight
23:09:09 artom No really, I'm organizing expensive calls and everything :P
23:37:08 opendevreview Dan Smith proposed openstack/nova master: Revert project-specific APIs for servers https://review.opendev.org/c/openstack/nova/+/816206
23:37:08 opendevreview Dan Smith proposed openstack/nova master: Allow per-context rule in error messages https://review.opendev.org/c/openstack/nova/+/816865
23:42:42 opendevreview Ghanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope https://review.opendev.org/c/openstack/nova/+/819390
23:44:28 opendevreview Ghanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope https://review.opendev.org/c/openstack/nova/+/819390
23:44:33 opendevreview Ghanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope https://review.opendev.org/c/openstack/nova/+/819390
#openstack-nova - 2021-12-01
02:29:47 opendevreview Ghanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope https://review.opendev.org/c/openstack/nova/+/819390
08:42:49 luk4s Good morning all
08:44:07 luk4s We enabled tenant isolation with placement https://docs.openstack.org/nova/wallaby/admin/aggregates.html#tenant-isolation-with-placement and have two aggregates: one for private tenants and one for generic use.
08:45:55 luk4s For private tenants we want the allocations to happened first on the private aggregate and when that is full we want to move on to the generic one once the private one is full.
08:46:14 luk4s Is this possible?
08:48:23 luk4s I have added the relevant project id to both aggregates, but the allocation is a little bit random. Can priority/weight be assigned to prioritise one over other, i.e. always prioritise the private over generic one.
09:46:19 bauzas luk4s: hola
09:46:37 bauzas luk4s: so you want to stack first for one aggregate and then another one ?
09:49:34 luk4s bauzas, yes.
09:51:11 luk4s i.e. always fill up private aggregate first before moving on to the generic one.
09:51:13 bauzas for this tenant ?
09:51:37 bauzas if so, you need a weigher
09:52:32 luk4s I have already looked at that but it looks like existing weighters may not do what I want - I presume I would have to write a new one?
09:53:53 luk4s would like to avoid writing one if possible :)
09:57:32 bauzas luk4s: AFAIK, there are no ways to tell Nova to do this for the moment
09:58:50 luk4s bauzas, cheers and thank you for the information
09:59:10 luk4s I mean thank you for the information :)
11:14:28 bauzas cores, a documentation change simple https://review.opendev.org/c/openstack/nova/+/814561
12:59:57 stephenfin gibi: care to send https://review.opendev.org/c/openstack/nova/+/814561 on its way
12:59:58 stephenfin ?
13:07:39 sean-k-mooney stephenfin: has you mock to unittest.mock patch merged yet
13:07:53 brinzhang_ gmann: hi, would you like to review the series of remove tenant_id patches https://review.opendev.org/q/topic:%22bp%252Fremove-tenant-id%22+(status:open%20OR%20status:merged)
13:08:33 sean-k-mooney hum no https://review.opendev.org/c/openstack/nova/+/714676 oh takashi thing its causing nova.tests.unit.api.openstack.test_wsgi_app.WSGIAppTest.test_init_application_called_twice to fail
13:36:26 gibi bauzas, stephenfin: on itr
13:38:55 gibi done
13:49:55 bauzas gibi: t
13:49:58 bauzas ta even
14:16:03 artom Am I the only one for whom `git review -d` appears to timeout?
14:16:20 sean-k-mooney which review
14:16:22 sean-k-mooney ill try it

Earlier   Later