From ed43be0a0f40c4b065472da27e6ef6e59a6789ca Mon Sep 17 00:00:00 2001 From: Rodrigo Duarte Sousa Date: Tue, 10 Jun 2014 10:22:51 -0300 Subject: [PATCH] Fixes bad performance when editing project members The bug is caused by consecutive client calls when fetching users' roles in a project. The fix was to use the /v3/role_assignments endpoint. Which retrieves all roles in a single client call (dashboards/admin/projects/workflows.py: UpdateProjectGroupsAction and UpdateProjectUsersAction). The compatibility to the previous keystoneclient version is maintained. In a scenario with 1000 users with a role in a project (using devstack), the time to render the Modify Users page dropped from 45s to 2.5s Change-Id: Ic72ebea0db226faf98c5e04d00d76dedd1fb01c1 Closes-bug: #1278920 --- openstack_dashboard/api/keystone.py | 34 +++++ .../dashboards/admin/projects/tests.py | 134 +++++++++++++----- .../dashboards/admin/projects/workflows.py | 25 ++-- .../test/test_data/keystone_data.py | 27 ++++ 4 files changed, 170 insertions(+), 50 deletions(-) diff --git a/openstack_dashboard/api/keystone.py b/openstack_dashboard/api/keystone.py index 880d49065c..ca74622a93 100644 --- a/openstack_dashboard/api/keystone.py +++ b/openstack_dashboard/api/keystone.py @@ -457,6 +457,16 @@ def remove_group_user(request, group_id, user_id): return manager.remove_from_group(group=group_id, user=user_id) +def role_assignments_list(request, project=None, user=None, role=None, + group=None, domain=None, effective=False): + if VERSIONS.active < 3: + raise exceptions.NotAvailable + + manager = keystoneclient(request, admin=True).role_assignments + return manager.list(project=project, user=user, role=role, group=group, + domain=domain, effective=effective) + + def role_create(request, name): manager = keystoneclient(request, admin=True).roles return manager.create(name) @@ -490,6 +500,30 @@ def roles_for_user(request, user, project): return manager.list(user=user, project=project) +def get_project_users_roles(request, project): + users_roles = {} + if VERSIONS.active < 3: + project_users = user_list(request, project=project) + + for user in project_users: + roles = roles_for_user(request, user.id, project) + roles_ids = [role.id for role in roles] + users_roles[user.id] = roles_ids + else: + project_role_assignments = role_assignments_list(request, + project=project) + for role_assignment in project_role_assignments: + if not hasattr(role_assignment, 'user'): + continue + user_id = role_assignment.user['id'] + role_id = role_assignment.role['id'] + if user_id in users_roles: + users_roles[user_id].append(role_id) + else: + users_roles[user_id] = [role_id] + return users_roles + + def add_tenant_user_role(request, project=None, user=None, role=None, group=None, domain=None): """Adds a role for a user on a tenant.""" diff --git a/openstack_dashboard/dashboards/admin/projects/tests.py b/openstack_dashboard/dashboards/admin/projects/tests.py index 468e4483ac..55085145dc 100644 --- a/openstack_dashboard/dashboards/admin/projects/tests.py +++ b/openstack_dashboard/dashboards/admin/projects/tests.py @@ -732,10 +732,13 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): 'user_list', 'roles_for_group', 'group_list', - 'role_list'), + 'role_list', + 'role_assignments_list'), quotas: ('get_tenant_quota_data', 'get_disabled_quotas')}) def test_update_project_get(self): + keystone_api_version = api.keystone.VERSIONS.active + project = self.tenants.first() quota = self.quotas.first() default_role = self.roles.first() @@ -744,6 +747,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): groups = self._get_all_groups(domain_id) roles = self.roles.list() proj_users = self._get_proj_users(project.id) + role_assignments = self.role_assignments.list() api.keystone.tenant_get(IsA(http.HttpRequest), self.tenant.id, admin=True) \ @@ -764,12 +768,20 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): .MultipleTimes().AndReturn(roles) api.keystone.group_list(IsA(http.HttpRequest), domain=domain_id) \ .AndReturn(groups) - api.keystone.user_list(IsA(http.HttpRequest), - project=self.tenant.id).AndReturn(proj_users) - for user in proj_users: - api.keystone.roles_for_user(IsA(http.HttpRequest), - user.id, - self.tenant.id).AndReturn(roles) + + if keystone_api_version >= 3: + api.keystone.role_assignments_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(role_assignments) + else: + api.keystone.user_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(proj_users) + + for user in proj_users: + api.keystone.roles_for_user(IsA(http.HttpRequest), + user.id, + self.tenant.id).AndReturn(roles) for group in groups: api.keystone.roles_for_group(IsA(http.HttpRequest), @@ -814,12 +826,16 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): 'remove_group_role', 'add_group_role', 'group_list', - 'role_list'), + 'role_list', + 'role_assignments_list'), api.nova: ('tenant_quota_update',), api.cinder: ('tenant_quota_update',), quotas: ('get_tenant_quota_data', 'get_disabled_quotas')}) + def test_update_project_save(self, neutron=False): + keystone_api_version = api.keystone.VERSIONS.active + project = self.tenants.first() quota = self.quotas.first() default_role = self.roles.first() @@ -829,6 +845,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): groups = self._get_all_groups(domain_id) proj_groups = self._get_proj_groups(project.id) roles = self.roles.list() + role_assignments = self.role_assignments.list() # get/init api.keystone.tenant_get(IsA(http.HttpRequest), @@ -853,13 +870,23 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): .MultipleTimes().AndReturn(roles) api.keystone.group_list(IsA(http.HttpRequest), domain=domain_id) \ .AndReturn(groups) - api.keystone.user_list(IsA(http.HttpRequest), - project=self.tenant.id).AndReturn(proj_users) + workflow_data = {} - for user in proj_users: - api.keystone.roles_for_user(IsA(http.HttpRequest), - user.id, - self.tenant.id).AndReturn(roles) + + if keystone_api_version >= 3: + api.keystone.role_assignments_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(role_assignments) + else: + api.keystone.user_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(proj_users) + + for user in proj_users: + api.keystone.roles_for_user(IsA(http.HttpRequest), + user.id, + self.tenant.id).AndReturn(roles) + for group in groups: api.keystone.roles_for_group(IsA(http.HttpRequest), group=group.id, @@ -1051,11 +1078,14 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): 'remove_group_role', 'add_group_role', 'group_list', - 'role_list'), + 'role_list', + 'role_assignments_list'), quotas: ('get_tenant_quota_data', 'get_disabled_quotas'), api.nova: ('tenant_quota_update',)}) def test_update_project_tenant_update_error(self): + keystone_api_version = api.keystone.VERSIONS.active + project = self.tenants.first() quota = self.quotas.first() default_role = self.roles.first() @@ -1064,6 +1094,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): groups = self._get_all_groups(domain_id) roles = self.roles.list() proj_users = self._get_proj_users(project.id) + role_assignments = self.role_assignments.list() # get/init api.keystone.tenant_get(IsA(http.HttpRequest), self.tenant.id, @@ -1085,15 +1116,24 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): .MultipleTimes().AndReturn(roles) api.keystone.group_list(IsA(http.HttpRequest), domain=domain_id) \ .AndReturn(groups) - api.keystone.user_list(IsA(http.HttpRequest), - project=self.tenant.id).AndReturn(proj_users) workflow_data = {} + + if keystone_api_version >= 3: + api.keystone.role_assignments_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(role_assignments) + else: + api.keystone.user_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(proj_users) + for user in proj_users: + api.keystone.roles_for_user(IsA(http.HttpRequest), + user.id, + self.tenant.id).AndReturn(roles) + + role_ids = [role.id for role in roles] for user in proj_users: - api.keystone.roles_for_user(IsA(http.HttpRequest), - user.id, - self.tenant.id).AndReturn(roles) - role_ids = [role.id for role in roles] if role_ids: workflow_data.setdefault(USER_ROLE_PREFIX + role_ids[0], []) \ .append(user.id) @@ -1155,11 +1195,14 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): 'remove_group_role', 'add_group_role', 'group_list', - 'role_list'), + 'role_list', + 'role_assignments_list'), quotas: ('get_tenant_quota_data', 'get_disabled_quotas'), api.nova: ('tenant_quota_update',)}) def test_update_project_quota_update_error(self): + keystone_api_version = api.keystone.VERSIONS.active + project = self.tenants.first() quota = self.quotas.first() default_role = self.roles.first() @@ -1169,6 +1212,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): groups = self._get_all_groups(domain_id) proj_groups = self._get_proj_groups(project.id) roles = self.roles.list() + role_assignments = self.role_assignments.list() # get/init api.keystone.tenant_get(IsA(http.HttpRequest), self.tenant.id, @@ -1190,15 +1234,22 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): .MultipleTimes().AndReturn(roles) api.keystone.group_list(IsA(http.HttpRequest), domain=domain_id) \ .AndReturn(groups) - api.keystone.user_list(IsA(http.HttpRequest), - project=self.tenant.id).AndReturn(proj_users) workflow_data = {} - for user in proj_users: - api.keystone.roles_for_user(IsA(http.HttpRequest), - user.id, - self.tenant.id).AndReturn(roles) + if keystone_api_version >= 3: + api.keystone.role_assignments_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(role_assignments) + else: + api.keystone.user_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(proj_users) + + for user in proj_users: + api.keystone.roles_for_user(IsA(http.HttpRequest), + user.id, + self.tenant.id).AndReturn(roles) for group in groups: api.keystone.roles_for_group(IsA(http.HttpRequest), @@ -1319,10 +1370,13 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): 'remove_group_role', 'add_group_role', 'group_list', - 'role_list'), + 'role_list', + 'role_assignments_list'), quotas: ('get_tenant_quota_data', 'get_disabled_quotas')}) def test_update_project_member_update_error(self): + keystone_api_version = api.keystone.VERSIONS.active + project = self.tenants.first() quota = self.quotas.first() default_role = self.roles.first() @@ -1331,6 +1385,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): proj_users = self._get_proj_users(project.id) groups = self._get_all_groups(domain_id) roles = self.roles.list() + role_assignments = self.role_assignments.list() # get/init api.keystone.tenant_get(IsA(http.HttpRequest), self.tenant.id, @@ -1352,14 +1407,23 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): .MultipleTimes().AndReturn(roles) api.keystone.group_list(IsA(http.HttpRequest), domain=domain_id) \ .AndReturn(groups) - api.keystone.user_list(IsA(http.HttpRequest), - project=self.tenant.id).AndReturn(proj_users) workflow_data = {} - for user in proj_users: - api.keystone.roles_for_user(IsA(http.HttpRequest), - user.id, - self.tenant.id).AndReturn(roles) + + if keystone_api_version >= 3: + api.keystone.role_assignments_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(role_assignments) + else: + api.keystone.user_list(IsA(http.HttpRequest), + project=self.tenant.id) \ + .AndReturn(proj_users) + + for user in proj_users: + api.keystone.roles_for_user(IsA(http.HttpRequest), + user.id, + self.tenant.id).AndReturn(roles) + for group in groups: api.keystone.roles_for_group(IsA(http.HttpRequest), group=group.id, diff --git a/openstack_dashboard/dashboards/admin/projects/workflows.py b/openstack_dashboard/dashboards/admin/projects/workflows.py index 82e6e61d16..0a6e3c6fd0 100644 --- a/openstack_dashboard/dashboards/admin/projects/workflows.py +++ b/openstack_dashboard/dashboards/admin/projects/workflows.py @@ -197,23 +197,18 @@ class UpdateProjectMembersAction(workflows.MembershipAction): # Figure out users & roles if project_id: try: - project_members = api.keystone.user_list(request, - project=project_id) + users_roles = api.keystone.get_project_users_roles(request, + project_id) except Exception: - exceptions.handle(request, err_msg) + exceptions.handle(request, + err_msg, + redirect=reverse(INDEX_URL)) - for user in project_members: - try: - roles = api.keystone.roles_for_user(self.request, - user.id, - project_id) - except Exception: - exceptions.handle(request, - err_msg, - redirect=reverse(INDEX_URL)) - for role in roles: - field_name = self.get_member_field_name(role.id) - self.fields[field_name].initial.append(user.id) + for user_id in users_roles: + roles_ids = users_roles[user_id] + for role_id in roles_ids: + field_name = self.get_member_field_name(role_id) + self.fields[field_name].initial.append(user_id) class Meta: name = _("Project Members") diff --git a/openstack_dashboard/test/test_data/keystone_data.py b/openstack_dashboard/test/test_data/keystone_data.py index 210454b4b2..7711215853 100644 --- a/openstack_dashboard/test/test_data/keystone_data.py +++ b/openstack_dashboard/test/test_data/keystone_data.py @@ -25,6 +25,7 @@ from keystoneclient.v2_0 import tenants from keystoneclient.v2_0 import users from keystoneclient.v3 import domains from keystoneclient.v3 import groups +from keystoneclient.v3 import role_assignments from openstack_auth import user as auth_user @@ -135,6 +136,7 @@ def data(TEST): TEST.users = utils.TestDataContainer() TEST.groups = utils.TestDataContainer() TEST.tenants = utils.TestDataContainer() + TEST.role_assignments = utils.TestDataContainer() TEST.roles = utils.TestDataContainer() TEST.ec2 = utils.TestDataContainer() @@ -236,6 +238,31 @@ def data(TEST): group4 = groups.Group(groups.GroupManager(None), group_dict) TEST.groups.add(group, group2, group3, group4) + role_assignments_dict = {'user': {'id': '1'}, + 'role': {'id': '1'}, + 'scope': {'project': {'id': '1'}}} + role_assignment1 = role_assignments.RoleAssignment( + role_assignments.RoleAssignmentManager, role_assignments_dict) + role_assignments_dict = {'user': {'id': '2'}, + 'role': {'id': '2'}, + 'scope': {'project': {'id': '1'}}} + role_assignment2 = role_assignments.RoleAssignment( + role_assignments.RoleAssignmentManager, role_assignments_dict) + role_assignments_dict = {'group': {'id': '1'}, + 'role': {'id': '2'}, + 'scope': {'project': {'id': '1'}}} + role_assignment3 = role_assignments.RoleAssignment( + role_assignments.RoleAssignmentManager, role_assignments_dict) + role_assignments_dict = {'user': {'id': '3'}, + 'role': {'id': '2'}, + 'scope': {'project': {'id': '1'}}} + role_assignment4 = role_assignments.RoleAssignment( + role_assignments.RoleAssignmentManager, role_assignments_dict) + TEST.role_assignments.add(role_assignment1, + role_assignment2, + role_assignment3, + role_assignment4) + tenant_dict = {'id': "1", 'name': 'test_tenant', 'description': "a test tenant.",