Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-01-27
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 lbragstad and i left comments on others that can be abandoned because i rolled them into subsequent patches
17:37:00 gmann lbragstad: thanks,
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 sean-k-mooney lbragstad: thanks perfect
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: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
20:31:13 openstackgerrit Vlad Gusev proposed openstack/nova stable/train: Use subqueryload() instead of joinedload() for (system_)metadata https://review.opendev.org/c/openstack/nova/+/761811
20:34:14 openstackgerrit Vlad Gusev proposed openstack/nova stable/stein: Use subqueryload() instead of joinedload() for (system_)metadata https://review.opendev.org/c/openstack/nova/+/761812
20:35:35 openstackgerrit Vlad Gusev proposed openstack/nova stable/rocky: Use subqueryload() instead of joinedload() for (system_)metadata https://review.opendev.org/c/openstack/nova/+/761813
20:37:04 openstackgerrit Vlad Gusev proposed openstack/nova stable/queens: Use subqueryload() instead of joinedload() for (system_)metadata https://review.opendev.org/c/openstack/nova/+/761814
21:08:17 openstackgerrit Merged openstack/nova stable/stein: Update pci stat pools based on PCI device changes https://review.opendev.org/c/openstack/nova/+/761727

Earlier   Later