| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-23 | |||
| 15:43:23 | gmann | dansmith: at least now it will protect s to dis-allow system admin | |
| 15:43:23 | bauzas | will remove my -1 | |
| 15:43:28 | dansmith | which is also why I think we need to be working towards a future where operators aren't expected to be able to tweak it like they are today | |
| 15:44:44 | gmann | having project_id in check_str is good way to differentiate from system user. once we enable scope as hardcoded then we can remove all these noop checks with context.project_id | |
| 15:46:11 | bauzas | gmann: explain me why we need to touch https://review.opendev.org/c/openstack/nova/+/828670/4/nova/policies/attach_interfaces.py | |
| 15:46:27 | bauzas | and why the role no longer needs admins | |
| 15:46:53 | bauzas | oh, because we want to remove those wrong system-scoped roles | |
| 15:47:16 | gmann | bauzas: yeah, and GET project reader can do and create/delete interface project member | |
| 15:47:45 | bauzas | gmann: but then, if I'm admin of project 10, I can't longer attach interfaces for servers owned by project 2 ? | |
| 15:48:19 | bauzas | or even list them ? | |
| 15:48:45 | bauzas | sorry if those sound silly questions, but I better need to ramp up on the policies things in order to be able to merge this stuff before tomorrow | |
| 15:48:55 | gmann | bauzas: with new policy only yes, that is isolation we are doing. keep project admin restrictive to their project operatrions (except list all project resources if we have in that API) | |
| 15:49:42 | gmann | projectA admin will be restrictive to do projectA things not projectB things. (with all project resource list case) | |
| 15:49:51 | bauzas | gmann: is that a breaking change if we merge this change by now ? | |
| 15:50:08 | bauzas | or can admins use the legacy policies for a while ? | |
| 15:50:25 | gmann | bauzas: we do support legacy policy as deprecated so until we remove them it will keep woprking with old token | |
| 15:50:48 | gmann | bauzas: yes, by default new policy are disabled. scope check as well as new default | |
| 15:51:04 | bauzas | ok | |
| 15:51:11 | bauzas | I better understand :) | |
| 15:51:40 | bauzas | gmann: just tbc, until https://review.opendev.org/c/openstack/nova/+/828670/4/nova/policies/attach_interfaces.py#30 is removed or default changes to new policies, nothing changes ? | |
| 15:51:46 | gmann | bauzas: this is good way to know what all allowed as default with these policy change https://review.opendev.org/c/openstack/nova/+/828670/4/nova/tests/unit/policies/test_attach_interfaces.py#29 | |
| 15:52:07 | opendevreview | Jonathan Race proposed openstack/nova master: driver/secheduler/docs for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 15:52:08 | opendevreview | Jonathan Race proposed openstack/nova master: zuul-job for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/828372 | |
| 15:52:08 | bauzas | gmann: heh, the patch is hairy, I'm not yet there | |
| 15:52:27 | bauzas | gosh, we're rushing out of time | |
| 15:52:29 | gmann | bauzas: true, or operator enable it with config option enforce_scope=True, enforce_new_defaults=True | |
| 15:52:51 | bauzas | gmann: ok, perfect and I guess this is correctly documented so I'm not afraid | |
| 15:53:03 | gmann | both config are false (disabled) by default in yoga | |
| 15:53:06 | bauzas | gmann: thanks gmann, continuing to review | |
| 15:53:25 | chateaulav | gibi: added final changes in this last patchset to the driver patch, and the zuul ci should be good this time around (fingers crossed) | |
| 15:53:57 | gibi | chateaulav: ack, I will check it before I leave today | |
| 15:55:00 | chateaulav | ccol | |
| 15:55:28 | bauzas | gibi: fwiw, I'm on the policies series that you +1d, I'm leaving a ton of comments to help you better understandingh | |
| 15:57:33 | gibi | bauzas: thanks that will help if you want to convince me to upgrade my vote | |
| 15:58:43 | gmann | bauzas: gibi thanks for reviews. I need to go away for 20 min (taking breakfast) and then will return if any query while review. | |
| 16:17:37 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Ignore LibvirtConfigObject kwargs https://review.opendev.org/c/openstack/nova/+/830644 | |
| 16:17:37 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Remove unnecessary TODO https://review.opendev.org/c/openstack/nova/+/830645 | |
| 16:17:38 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Add vIOMMU device to guest https://review.opendev.org/c/openstack/nova/+/830646 | |
| 16:17:38 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Rename some config objects https://review.opendev.org/c/openstack/nova/+/830647 | |
| 16:17:39 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Shuffle methods around https://review.opendev.org/c/openstack/nova/+/830648 | |
| 16:17:39 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Remove handling for older libvirt versions https://review.opendev.org/c/openstack/nova/+/830649 | |
| 16:17:40 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Return objects from Guest.get_interfaces https://review.opendev.org/c/openstack/nova/+/830650 | |
| 16:17:40 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Don't fetch guest architecture repeatedly https://review.opendev.org/c/openstack/nova/+/830651 | |
| 16:17:41 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Prepare for manual PCIe address management https://review.opendev.org/c/openstack/nova/+/830652 | |
| 16:17:42 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Start managing PCIe address allocation https://review.opendev.org/c/openstack/nova/+/830653 | |
| 16:18:01 | stephenfin | Ignore those. They're nowhere near done. Just pushing so I remember to finish them at some point | |
| 16:19:03 | sean-k-mooney | stephenfin: am... i tought we were not gong to tdo that | |
| 16:19:06 | sean-k-mooney | but ok | |
| 16:19:51 | sean-k-mooney | i mean its been in our downstream backlog for ever | |
| 16:21:57 | opendevreview | Pedro Monteiro Azevedo de Moura Almeida proposed openstack/nova master: Update live_migration_downtime definition https://review.opendev.org/c/openstack/nova/+/828387 | |
| 16:26:31 | opendevreview | Elod Illes proposed openstack/nova stable/wallaby: skip test_tagged_attachment in nova-next https://review.opendev.org/c/openstack/nova/+/830656 | |
| 16:27:37 | bauzas | gmann: once you're back, question in https://review.opendev.org/c/openstack/nova/+/829626/9/nova/api/openstack/compute/views/servers.py#443 | |
| 16:28:17 | gmann | checking | |
| 16:31:49 | bauzas | gibi: other cores : I'm +2 on the whole new-policy-rules series except one patch, reviews welcome | |
| 16:33:33 | bauzas | gmann: can you then please answer my question ? | |
| 16:33:57 | bauzas | I have a hardstop in 30 mins and I want to briefly look at the last unified-limits patch before I go | |
| 16:34:43 | gmann | ok | |
| 16:39:00 | gmann | bauzas: replied, by default if target are not passed in policy it take context'sproject_id itself | |
| 16:39:44 | gmann | and DB query are based on requester project_id so it gets only requesting project instances until all-tenant is requested | |
| 16:40:36 | bauzas | gmann: ok, then I'm confused, why did we need to add the target be explicitely project_id ? | |
| 16:41:55 | gmann | bauzas: same as discussed before. PROJECT_ADMIN|MEMBER|READER has project_id i check_str of rule so we need to pass project_id in target which can pass. it is done to separate out the system user if scope are disabled. otherwise system reader can also pass policy. | |
| 16:42:18 | stephenfin | sean-k-mooney: We're still not necessarily going to do it, but it's come up so often that it seems something we should _eventually_ do | |
| 16:42:20 | gmann | bauzas: once we remove the enforce_scope as configurable (enable it hardcoded) then we can remove those and cleanup | |
| 16:42:29 | stephenfin | Now just to find the time to actually do it, heh | |
| 16:42:55 | gmann | bauzas: passing requester project_id and checking that against requester itself is ally bogus thing we have to do for now. | |
| 16:43:20 | bauzas | ok, it's a bit of a spaghetti code to me but I trust you | |
| 16:43:40 | bauzas | either way, +2d the whole series | |
| 16:43:57 | bauzas | you need to find someone next to jab it | |
| 16:43:57 | gmann | bauzas: this is TODO to cleanup once scope are enabled https://github.com/openstack/nova/blob/134c4791ff8233264b7862db692e058f60ae2f80/nova/policy.py#L180 | |
| 16:44:10 | gmann | bauzas: thanks a lot. | |
| 16:44:21 | bauzas | gmann: ok, I see | |
| 16:44:28 | bauzas | thanks for the explanations | |
| 16:44:53 | gmann | bauzas: and other key bit we need to improve after secure rbac is to remove the admin checks from DB which will make it more cleaner | |
| 16:45:03 | gmann | but those are things to do after we ship secure rbac as default | |
| 17:01:40 | sean-k-mooney | stephenfin: ack | |
| 17:03:41 | bauzas | also +2d the whole unified-limits series, melwitt | |
| 17:04:21 | bauzas | don't be afraid folks, if you get +2/+W before Friday, you're all good | |
| 17:08:02 | melwitt | thank you bauzas! | |
| 17:32:23 | gibi | bauzas: ack, I will look first thing in the morning tomorrow | |
| 18:07:04 | whoami-rajat | dansmith, bauzas hey, just to clear my understanding regarding your comments on change https://review.opendev.org/c/openstack/nova/+/820368 | |
| 18:07:39 | whoami-rajat | the ask is to divide the patch into two parts, one adding new event (requires MV bump) and other adding rebuild support (requires yet another MV bump) | |
| 18:07:52 | whoami-rajat | so the two patches will be 2.91 and 2.92 | |
| 18:07:57 | dansmith | no, | |
| 18:08:05 | dansmith | the event doesn't need a version bump, just your api change | |
| 18:08:12 | dansmith | event first, then api change | |
| 18:08:34 | dansmith | I would put the cinder volume change and test in a separate early patch too and we can sink that quickly, IMHO | |
| 18:10:22 | whoami-rajat | hmm, I'm still slightly confused, the addition of a new event has required an MV bump, so you're saying don't do the bump at all or use 2.91 for it as well ? https://review.opendev.org/c/openstack/nova/+/820368/8/nova/api/openstack/compute/schemas/server_external_events.py | |
| 18:11:43 | dansmith | uh | |
| 18:12:18 | dansmith | okay I didn't realize we started adding microversions for new event names | |
| 18:13:03 | dansmith | pretty sure that didn't happen in the earlier days (was never my intent) but clearly has been happening since v51 or so | |
| 18:13:28 | dansmith | so yeah maybe not worth splitting in that case, I guess | |
| 18:13:40 | dansmith | I'd update with the other comments and then we can see what bauzas thinks | |
| 18:13:54 | dansmith | it's just such a large cut of things all over the API | |
| 18:15:36 | whoami-rajat | I can understand it's a burden to review it but having no prior experience working in nova, I also don't have much ideas to do it in a better way | |
| 18:15:53 | whoami-rajat | I will update with the given tests (and continue work on the tempest test) until then | |
| 18:16:00 | whoami-rajat | s/tests/comments | |
| 18:16:05 | dansmith | yep, sounds good | |
| 18:16:19 | whoami-rajat | cool, thanks! | |
| 18:17:06 | whoami-rajat | dansmith, i think we also have novaclient release this week right? | |
| 18:17:48 | dansmith | I dunno, but seems likely | |