| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-11-30 | |||
| 16:32:41 | bauzas | #agreed https://blueprints.launchpad.net/nova/+spec/policy-defaults-refresh-2 accepted as a specless BP as the direction is already explained in https://review.opendev.org/c/openstack/governance/+/815158/ | |
| 16:32:50 | bauzas | moving on, last topic | |
| 16:33:12 | bauzas | (ganso) Raising awareness of vif_multiqueue_enabled in flavor work that is ready to be reviewed/merged | |
| 16:33:15 | bauzas | ganso: around ? | |
| 16:33:21 | ganso | o/ | |
| 16:33:34 | bauzas | #link https://blueprints.launchpad.net/nova/+spec/multiqueue-flavor-extra-spec | |
| 16:33:49 | ganso | so as the topic titles says: https://review.opendev.org/q/topic:%22bp%252Fmultiqueue-flavor-extra-spec%22+(status:open%20OR%20status:merged) | |
| 16:34:14 | ganso | we've discussed 2-3 weeks ago about this and that it could/may be specless, but it was approved to be specless ~6 months ago | |
| 16:34:20 | bauzas | ganso: nothing changed during the implementation phase requiring further discussion ? | |
| 16:34:45 | ganso | bauzas: as far as I know, nothing changed and the code is complete | |
| 16:35:05 | bauzas | the BP was previously approved as specless so I don't see problems approving it again providing there were no changes in design | |
| 16:35:08 | ganso | I rebased it and it is passing CI | |
| 16:35:13 | bauzas | (requiring further discussions) | |
| 16:36:11 | ganso | I'm pretty much shepherding this set of changes now, but the work was done by stephenfin | |
| 16:36:12 | bauzas | ganso: I guess you're taking over stephenfin's work ? | |
| 16:36:22 | ganso | yes | |
| 16:36:25 | bauzas | OK, that's crystal clear then | |
| 16:36:39 | bauzas | I don't have any problems reapproving it | |
| 16:36:49 | ganso | great =) | |
| 16:36:49 | bauzas | anyone else disagreeing ? | |
| 16:37:28 | bauzas | #agreed https://blueprints.launchpad.net/nova/+spec/multiqueue-flavor-extra-spec to approve it again as a specless BP for the yoga release cycle | |
| 16:37:50 | bauzas | we're at the end of the agenda, anything else to mention ? | |
| 16:38:15 | bauzas | I'm happy to say we were quick this time :) | |
| 16:38:24 | gibi | \o/ | |
| 16:38:30 | bauzas | if not, | |
| 16:38:34 | opendevmeet | Log: https://meetings.opendev.org/meetings/nova/2021/nova.2021-11-30-16.00.log.html | |
| 16:38:34 | opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/nova/2021/nova.2021-11-30-16.00.txt | |
| 16:38:34 | opendevmeet | Minutes: https://meetings.opendev.org/meetings/nova/2021/nova.2021-11-30-16.00.html | |
| 16:38:34 | opendevmeet | Meeting ended Tue Nov 30 16:38:34 2021 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | |
| 16:38:34 | bauzas | #endmeeting* | |
| 16:38:49 | bauzas | man, I'm fed up with my typing issues | |
| 16:40:03 | bauzas | thanks all | |
| 16:40:22 | elodilles | thanks bauzas o/ | |
| 16:40:57 | bauzas | I'm a gross person, I haven't officially thanked you all at the end of the meeting | |
| 16:42:09 | elodilles | :D | |
| 17:07:13 | bauzas | gmann: can I put you assignee on https://blueprints.launchpad.net/nova/+spec/policy-defaults-refresh-2 ? | |
| 17:14:48 | opendevreview | Nicolas Parquet proposed openstack/nova master: Add v2.91 microversion, allowing @ and dot (.) characters in keypair name https://review.opendev.org/c/openstack/nova/+/781076 | |
| 18:05:52 | gmann | bauzas: sure, thanks | |
| 18:07:59 | opendevreview | Dan Smith proposed openstack/nova master: Revert project-specific APIs for servers https://review.opendev.org/c/openstack/nova/+/816206 | |
| 18:08:00 | opendevreview | Dan Smith proposed openstack/nova master: Make API fixture pass roles https://review.opendev.org/c/openstack/nova/+/819907 | |
| 18:08:26 | dansmith | gmann: this has the fixture change on top ^ so you can see it not working without it, and then working when we change that | |
| 18:08:42 | dansmith | I haven't done more digging on why this is required, but hoping it's something you can spot easily | |
| 18:08:53 | gmann | dansmith: ok, checking | |
| 18:08:54 | dansmith | I wonder if we're not really testing fake policy like we think in functional? | |
| 18:09:47 | gmann | dansmith: which is good :). I wanted to remove those fake policy from unit as well as from functional tests completely but that might need more tests modification but something we should do. | |
| 18:10:29 | dansmith | don't disagree that it would be better, I just don't know why this is required right now, | |
| 18:10:40 | dansmith | since I would expect us to at most be testing with old defaults OR'd in | |
| 18:11:13 | gmann | sure, I will check it where we are missing things. | |
| 18:11:46 | gmann | dansmith: did you see my comment https://review.opendev.org/c/openstack/nova/+/816206/9/nova/policies/base.py | |
| 18:12:12 | gmann | dansmith: if CONTEXT_ADMIN if more readable then it is fine otherwise we can ad ADMIN ? | |
| 18:12:27 | dansmith | gmann: oh yeah I did, I just got distracted by the functional failures | |
| 18:12:59 | gmann | I started converting SYSTEM_READER to SYSTEM_ADMIN on top of your patch so doing it in base patch will avoid rebase or so | |
| 18:13:00 | gmann | sure | |
| 18:57:52 | gmann | dansmith: this is reason for functional test failure https://review.opendev.org/c/openstack/nova/+/816206/comment/510a59e0_5ffbc2f5/ | |
| 18:58:12 | gmann | dansmith: functional test using the real policy helped us to capture it. | |
| 19:00:04 | dansmith | gmann: ahh, I was probably conflating that rule with the admin_or_owner below it when thinking that we'd still have the old default | |
| 19:01:40 | gmann | dansmith: yeah, and those role hierarchy fix in 819907 made test passing because of 'admin' being used as user-id for functional test https://review.opendev.org/c/openstack/nova/+/819907/comment/eba56840_d585da0a/ | |
| 19:02:17 | dansmith | gmann: you mean that's why they worked before I switched the rule... | |
| 19:03:14 | dansmith | gmann: should I use admin_or_owner for those flavor-extra-spec rules, or add the DEPRECATED_ADMIN_OR_OWNER to context_admin/ | |
| 19:05:22 | gmann | dansmith: I think DEPRECATED_ADMIN_POLICY as CONTEXT_ADMIN is going to replace SYSTEM_ADMIN only . I was trying to do it this way https://review.opendev.org/c/openstack/nova/+/819389 | |
| 19:06:07 | gmann | and PROJECT_ADMIN going to be with DEPRECATED_ADMIN_OR_OWNER which has project_id in that | |
| 19:08:22 | gmann | dansmith: let me update my patch and then you can use the new ADMIN rule for place of role:admin | |
| 19:08:56 | dansmith | gmann: ack | |
| 19:09:06 | dansmith | gmann: we still want my patch to pass proper roles from the fixture though right? | |
| 19:09:32 | dansmith | presumably we need to also let you get a fixture with no (or foo) roles for testing that member is enforced | |
| 19:14:38 | gmann | dansmith: for is_admin L1092 yeah it is ok but else part make reader also give member authority https://review.opendev.org/c/openstack/nova/+/819907/1/nova/tests/fixtures/nova.py#1092 | |
| 19:15:22 | gmann | if we remove the else part and let real role being tested what test use then it should be ok | |
| 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 | :) | |