| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-01-27 | |||
| 17:00:11 | gmann | or i can do on top of that to see how it look like | |
| 17:00:20 | gmann | ok | |
| 17:27:27 | openstackgerrit | Merged openstack/nova stable/victoria: Add regression test for bug #1899649 https://review.opendev.org/c/openstack/nova/+/764611 | |
| 17:27:30 | openstack | bug 1899649 in OpenStack Compute (nova) "Volume marked as available after a failure to build" [Undecided,In progress] https://launchpad.net/bugs/1899649 - Assigned to Lee Yarwood (lyarwood) | |
| 17:33:17 | kashyap | gibi: stephenfin: When you get a minute, can you please have a gander at this (but it is from chengsheng, a new contributor, fixing a good problem): https://review.opendev.org/c/openstack/nova/+/762330/ | |
| 17:33:45 | gibi | kashyap: ack, added to my queue | |
| 17:33:46 | kashyap | It is related using the correct CPU APIs | |
| 17:34:05 | kashyap | And he's addressed some feedback of mine in the past; and it also checks off one of the items in this old spec: | |
| 17:34:26 | kashyap | https://opendev.org/openstack/nova-specs/commit/70811da221035044e27 (Add "CPU selection with hypervisor consideration" spec) | |
| 17:35:39 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Pass context objects to oslo.policy directly https://review.opendev.org/c/openstack/placement/+/766785 | |
| 17:35:40 | openstackgerrit | Lance Bragstad proposed openstack/placement master: policy: Suppress policy deprecation warnings https://review.opendev.org/c/openstack/placement/+/771852 | |
| 17:35:41 | openstackgerrit | Lance Bragstad proposed openstack/placement master: policy: Don't persist default rule changes in tests https://review.opendev.org/c/openstack/placement/+/772508 | |
| 17:35:46 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for resource providers https://review.opendev.org/c/openstack/placement/+/760240 | |
| 17:35:47 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for aggregates https://review.opendev.org/c/openstack/placement/+/760235 | |
| 17:35:48 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for allocations https://review.opendev.org/c/openstack/placement/+/760236 | |
| 17:35:52 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for allocation candidates https://review.opendev.org/c/openstack/placement/+/760237 | |
| 17:35:58 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for inventories https://review.opendev.org/c/openstack/placement/+/760238 | |
| 17:36:02 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for resource classes https://review.opendev.org/c/openstack/placement/+/760239 | |
| 17:36:08 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for traits https://review.opendev.org/c/openstack/placement/+/760241 | |
| 17:36:13 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for usage https://review.opendev.org/c/openstack/placement/+/760242 | |
| 17:36:16 | openstackgerrit | Lance Bragstad proposed openstack/placement master: policy: Remove the deprecated 'placement' rule https://review.opendev.org/c/openstack/placement/+/772333 | |
| 17:36:19 | openstackgerrit | Lance Bragstad proposed openstack/placement master: policy: Deprecate 'admin_api' rule https://review.opendev.org/c/openstack/placement/+/772334 | |
| 17:36:23 | lbragstad | stephenfin gmann sean-k-mooney since i needed to go through and fix things early in the series, i cleaned up a bunch of the one-off patches later in the series | |
| 17:36:39 | lbragstad | i abandoned the ones i had the ability to abandon | |
| 17:36:46 | sean-k-mooney | lbragstad: yep just saw them come in | |
| 17:37:00 | gmann | lbragstad: thanks, | |
| 17:37:00 | lbragstad | and i left comments on others that can be abandoned because i rolled them into subsequent patches | |
| 17:37:27 | sean-k-mooney | i need to look at how the roles compose and this works again because it sound like its working muc differently then i tought | |
| 17:37:43 | sean-k-mooney | to the point where im questinging the feature | |
| 17:38:00 | sean-k-mooney | i hope i am just missundering how the tests are written | |
| 17:39:54 | sean-k-mooney | basically the point im trying to figure out is as a SYSTEM_ADMIN do i only have the admin role or do i also have the member and reader role | |
| 17:40:00 | sean-k-mooney | im hoping its the former | |
| 17:40:10 | sean-k-mooney | and that we have ineritence between the roles | |
| 17:40:45 | stephenfin | ack | |
| 17:41:30 | sean-k-mooney | if we dont have inheritance betwen the roles then i dont think it make sense to have personas and reference them in the policy rules | |
| 17:44:44 | sean-k-mooney | if im reading https://specs.openstack.org/openstack/keystone-specs/specs/keystone/queens/system-scope.html#system-roles-implied-roles-inherited-roles correctly | |
| 17:45:20 | sean-k-mooney | if i have role:admin i should have role:member and role:reader trasitivly as they are implied roles | |
| 17:45:31 | sean-k-mooney | but i shoudl not have to add them to my user explictly | |
| 17:46:44 | sean-k-mooney | so im very confused by https://review.opendev.org/c/openstack/placement/+/760240/21/placement/tests/functional/gabbits/resource-provider-legacy-rbac.yaml#9 | |
| 17:47:09 | stephenfin | sean-k-mooney: I'm pretty sure it is implied, and because it's implied both the main role and the implied roles are included in the HTTP_X_ROLES header in CSV form | |
| 17:47:57 | stephenfin | so the implied link is encoded in keystone, and oslo.context and oslo.policy don't need to know about it. They can simply check for the role they care about | |
| 17:48:05 | sean-k-mooney | right i think including them in the x roles header explictly is odd | |
| 17:48:21 | lbragstad | if you have the admin role on a project and you look at the token response, you'll see keystone expands those out for you | |
| 17:48:42 | stephenfin | Yeah, that seems perfectly sensible to me | |
| 17:48:51 | lbragstad | the reason why we set them in the headers explicitly is so you don't have a dependency on keystone via keystonemiddleware | |
| 17:48:58 | sean-k-mooney | lbragstad: ok so the gabit test data is not the input date for the user to create | |
| 17:49:07 | sean-k-mooney | its a mix of both the input and the expect output | |
| 17:49:36 | lbragstad | the header input should represent what keystonemiddleware would translate | |
| 17:49:43 | lbragstad | since we're cutting that out of these tests | |
| 17:49:55 | sean-k-mooney | ah ok | |
| 17:49:58 | sean-k-mooney | can we add that as a note | |
| 17:50:01 | stephenfin | sean-k-mooney: You're simulating the request. In a real-world deployment keystonemiddleware would add those headers for you | |
| 17:50:04 | sean-k-mooney | i was not expect use to cut that out | |
| 17:50:05 | stephenfin | yeah | |
| 17:50:09 | sean-k-mooney | since these are functional tests | |
| 17:50:11 | sean-k-mooney | not unit tests | |
| 17:50:18 | sean-k-mooney | so we normally would not mock that | |
| 17:50:37 | stephenfin | *nova functional tests | |
| 17:50:40 | kashyap | gibi: Also, just to note, chengsheng, the reporter has done real live migration tests w/ the patch. I've discussed with them in December | |
| 17:50:48 | lbragstad | if placement wants to add functional test with keystone - then i'd say just use tempest? | |
| 17:50:57 | lbragstad | functional tests* | |
| 17:50:58 | stephenfin | sean-k-mooney: https://review.opendev.org/c/openstack/placement/+/760240/22/placement/auth.py | |
| 17:51:08 | sean-k-mooney | well im not suggesting keystone i was expecting a test fixture | |
| 17:51:21 | sean-k-mooney | that would do the translation instead of use doing it by hand | |
| 17:51:40 | sean-k-mooney | but if you add a note to the test saying that is what happeing then im ok with it | |
| 17:52:07 | sean-k-mooney | these vars are bing used as the request headers in the test | |
| 17:52:24 | gmann | in nova also we prepared the context like that by adding admin, member, reader in admin or so | |
| 17:52:32 | sean-k-mooney | so i was expecting them to be differnet | |
| 17:52:47 | stephenfin | we could rework that test fixture to associate known usernames with specific roles and set the headers that way, but what lbragstad has done is far more obvious IMO | |
| 17:52:57 | gmann | https://github.com/openstack/nova/blob/master/nova/tests/unit/policies/base.py#L57 | |
| 17:53:23 | sean-k-mooney | stephenfin: the bit i was missing was keystone auto expanding all the roles you have and flatening them | |
| 17:53:45 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP zuul: Increase SWIFT_LOOPBACK_DISK_SIZE within nova-lvm job https://review.opendev.org/c/openstack/nova/+/772702 | |
| 17:53:47 | sean-k-mooney | i was expecting that to be a list of your direct roles only | |
| 17:54:33 | sean-k-mooney | lbragstad: if i add a role i dont have to that maully keystone midelware will validate that on the project api side right | |
| 17:54:51 | sean-k-mooney | lbragstad: e.g. it will hit keystone to valideate it | |
| 17:55:01 | sean-k-mooney | as part of normal token validation | |
| 17:55:41 | lbragstad | sean-k-mooney ksm cleans out those headers and repopulates them based on the token validation response | |
| 17:55:42 | sean-k-mooney | assuming yes then i think i understand how the tests are workign now | |
| 17:56:22 | sean-k-mooney | lbragstad: ok so the imporant thin is in the project we can trust the roles in the context because ksm has checked for us | |
| 17:56:31 | lbragstad | https://opendev.org/openstack/keystonemiddleware/src/branch/master/keystonemiddleware/auth_token/_request.py#L75 | |
| 17:56:38 | sean-k-mooney | thats what i was expecting but just wanted to confirm | |
| 17:56:49 | lbragstad | https://opendev.org/openstack/keystonemiddleware/src/branch/master/keystonemiddleware/auth_token/_request.py#L224-L227 | |
| 17:57:29 | openstackgerrit | Stephen Finucane proposed openstack/placement master: policy: Add note about keystone's expansion of roles https://review.opendev.org/c/openstack/placement/+/772752 | |
| 17:57:29 | sean-k-mooney | lbragstad: thanks perfect | |
| 17:57:37 | lbragstad | yep | |
| 17:57:40 | stephenfin | sean-k-mooney: lemme know if that's useful ^ | |
| 17:58:04 | stephenfin | We can refine wording if so | |
| 17:58:20 | sean-k-mooney | stephenfin: yep no that make sense to me | |
| 17:58:41 | sean-k-mooney | im not sure why gerrit thinks you change the last 3 lins | |
| 17:59:04 | sean-k-mooney | i should have a , after yep | |
| 17:59:30 | stephenfin | Oh no, I really did add spacing :) | |
| 17:59:37 | stephenfin | a'ight, and with that | |
| 17:59:56 | stephenfin | kashyap: I'll take a look at that tomorrow | |
| 18:00:13 | kashyap | stephenfin: Thank you; it is fixing some real gnarly problem | |
| 18:35:43 | openstackgerrit | Merged openstack/nova stable/victoria: compute: Don't detach volumes when RescheduledException raised without retry https://review.opendev.org/c/openstack/nova/+/764612 | |
| 19:15:01 | gmann | stephenfin: lbragstad replied on this. as per my finding/run oslo.policy does not change rule's self.check_str even it only change rule's self.check which is internal attribute for oslo policy to play with - https://review.opendev.org/c/openstack/placement/+/772508/1//COMMIT_MSG#13 | |
| 19:15:53 | gmann | I ran few scenario and did not find oslo policy changing defined rule check_str. | |
| 19:18:10 | openstackgerrit | Merged openstack/nova stable/victoria: Use subqueryload() instead of joinedload() for (system_)metadata https://review.opendev.org/c/openstack/nova/+/761809 | |
| 19:37:40 | openstackgerrit | melanie witt proposed openstack/nova stable/ussuri: Use subqueryload() instead of joinedload() for (system_)metadata https://review.opendev.org/c/openstack/nova/+/761810 | |
| 20:24:28 | openstackgerrit | Vlad Gusev proposed openstack/nova stable/train: Use subqueryload() instead of joinedload() for (system_)metadata https://review.opendev.org/c/openstack/nova/+/761811 | |