]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Update quotas to handle domain acting as project
authorRyan McNair <rdmcnair@us.ibm.com>
Tue, 1 Mar 2016 18:54:37 +0000 (18:54 +0000)
committerRyan McNair <rdmcnair@us.ibm.com>
Mon, 7 Mar 2016 17:06:28 +0000 (17:06 +0000)
The Keystone change Ib22a0f3007cb7ef6b4df6f48da5f4d018e905f55 sets
the domain_id as the top-level parent project. However, since a
domain is not a project and therefore has no effect on quota nesting,
the domain "parent" should not be considered in the nested quota code.
This patch updates the quota code to ignore the domain_id if it's
present in the parent tree.

Change-Id: I221cfc14b886ea9819e065d4c79e8c4bfc94df45
Closes-Bug: #1551911

cinder/quota_utils.py
cinder/tests/unit/test_quota.py
cinder/tests/unit/test_quota_utils.py

index d7cd82a4754c4b224d3a1cbc16a9bc5c4fb8cd71..3ab0579947e009bd374d032d71e84146283b8bea 100644 (file)
@@ -103,6 +103,19 @@ def get_volume_type_reservation(ctxt, volume, type_id,
     return reservations
 
 
+def _filter_domain_id_from_parents(domain_id, tree):
+    """Removes the domain_id from the tree if present"""
+    new_tree = None
+    if tree:
+        parent, children = next(iter(tree.items()))
+        # Don't add the domain id to the parents hierarchy
+        if parent != domain_id:
+            new_tree = {parent: _filter_domain_id_from_parents(domain_id,
+                                                               children)}
+
+    return new_tree
+
+
 def get_project_hierarchy(context, project_id, subtree_as_ids=False,
                           parents_as_ids=False):
     """A Helper method to get the project hierarchy.
@@ -110,6 +123,8 @@ def get_project_hierarchy(context, project_id, subtree_as_ids=False,
     Along with hierarchical multitenancy in keystone API v3, projects can be
     hierarchically organized. Therefore, we need to know the project
     hierarchy, if any, in order to do nested quota operations properly.
+    If the domain is being used as the top most parent, it is filtered out from
+    the parent tree and parent_id.
     """
     try:
         keystone = _keystone_client(context)
@@ -118,11 +133,18 @@ def get_project_hierarchy(context, project_id, subtree_as_ids=False,
             project = keystone.projects.get(project_id,
                                             subtree_as_ids=subtree_as_ids,
                                             parents_as_ids=parents_as_ids)
-            generic_project.parent_id = project.parent_id
+
+            generic_project.parent_id = None
+            if project.parent_id != project.domain_id:
+                generic_project.parent_id = project.parent_id
+
             generic_project.subtree = (
                 project.subtree if subtree_as_ids else None)
-            generic_project.parents = (
-                project.parents if parents_as_ids else None)
+
+            generic_project.parents = None
+            if parents_as_ids:
+                generic_project.parents = _filter_domain_id_from_parents(
+                    project.domain_id, project.parents)
     except exceptions.NotFound:
         msg = (_("Tenant ID: %s does not exist.") % project_id)
         raise webob.exc.HTTPNotFound(explanation=msg)
index 1b3e6e256e2360e4a049ee1e811d70f167d57fd3..f9084f0fbbf9e5579c0cfe2d041bcc29ff7afb36 100644 (file)
@@ -1387,6 +1387,7 @@ class NestedDbQuotaDriverBaseTestCase(DbQuotaDriverBaseTestCase):
             def __init__(self, parent_id):
                 self.parent_id = parent_id
                 self.parents = {parent_id: None}
+                self.domain_id = 'default'
 
         def fake_get_project(project_id, subtree_as_ids=False,
                              parents_as_ids=False):
index 598c546696b92dbd727bc6aa0d38dba38da27534..dc45e041d07699107f702e2bbdba5df4e7a332de 100644 (file)
@@ -35,6 +35,8 @@ class QuotaUtilsTest(test.TestCase):
             self.id = id
             self.parent_id = parent_id
             self.subtree = None
+            self.parents = None
+            self.domain_id = 'default'
 
     def setUp(self):
         super(QuotaUtilsTest, self).setUp()
@@ -92,6 +94,61 @@ class QuotaUtilsTest(test.TestCase):
             self.context.project_id, parents_as_ids=False, subtree_as_ids=True)
         self.assertEqual(expected_project.__dict__, project.__dict__)
 
+    def _setup_mock_ksclient(self, mock_client, version='v3',
+                             subtree=None, parents=None):
+        keystoneclient = mock_client.return_value
+        keystoneclient.version = version
+        proj = self.FakeProject(self.context.project_id)
+        proj.subtree = subtree
+        if parents:
+            proj.parents = parents
+            proj.parent_id = next(iter(parents.keys()))
+        keystoneclient.projects.get.return_value = proj
+
+    @mock.patch('keystoneclient.client.Client')
+    def test__filter_domain_id_from_parents_domain_as_parent(
+            self, mock_client):
+        # Test with a top level project (domain is direct parent)
+        self._setup_mock_ksclient(mock_client, parents={'default': None})
+        project = quota_utils.get_project_hierarchy(
+            self.context, self.context.project_id, parents_as_ids=True)
+        self.assertIsNone(project.parent_id)
+        self.assertIsNone(project.parents)
+
+    @mock.patch('keystoneclient.client.Client')
+    def test__filter_domain_id_from_parents_domain_as_grandparent(
+            self, mock_client):
+        # Test with a child project (domain is more than a parent)
+        self._setup_mock_ksclient(mock_client,
+                                  parents={'bar': {'default': None}})
+        project = quota_utils.get_project_hierarchy(
+            self.context, self.context.project_id, parents_as_ids=True)
+        self.assertEqual('bar', project.parent_id)
+        self.assertEqual({'bar': None}, project.parents)
+
+    @mock.patch('keystoneclient.client.Client')
+    def test__filter_domain_id_from_parents_no_domain_in_parents(
+            self, mock_client):
+        # Test that if top most parent is not a domain (to simulate an older
+        # keystone version) nothing gets removed from the tree
+        parents = {'bar': {'foo': None}}
+        self._setup_mock_ksclient(mock_client, parents=parents)
+        project = quota_utils.get_project_hierarchy(
+            self.context, self.context.project_id, parents_as_ids=True)
+        self.assertEqual('bar', project.parent_id)
+        self.assertEqual(parents, project.parents)
+
+    @mock.patch('keystoneclient.client.Client')
+    def test__filter_domain_id_from_parents_no_parents(
+            self, mock_client):
+        # Test that if top no parents are present (to simulate an older
+        # keystone version) things don't blow up
+        self._setup_mock_ksclient(mock_client)
+        project = quota_utils.get_project_hierarchy(
+            self.context, self.context.project_id, parents_as_ids=True)
+        self.assertIsNone(project.parent_id)
+        self.assertIsNone(project.parents)
+
     @mock.patch('cinder.quota_utils._keystone_client')
     def test_validate_nested_projects_with_keystone_v2(self, _keystone_client):
         _keystone_client.side_effect = exceptions.VersionNotAvailable