diff --git a/nova/db/sqlalchemy/api_migrations/migrate_repo/versions/051_nested_resource_providers.py b/nova/db/sqlalchemy/api_migrations/migrate_repo/versions/051_nested_resource_providers.py new file mode 100644 index 0000000000..4745a1341d --- /dev/null +++ b/nova/db/sqlalchemy/api_migrations/migrate_repo/versions/051_nested_resource_providers.py @@ -0,0 +1,50 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import Column +from sqlalchemy import ForeignKey +from sqlalchemy import Index +from sqlalchemy import Integer +from sqlalchemy import MetaData +from sqlalchemy import Table + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + resource_providers = Table('resource_providers', meta, autoload=True) + columns_to_add = [ + ('root_provider_id', + Column('root_provider_id', Integer, + ForeignKey('resource_providers.id'))), + ('parent_provider_id', + Column('parent_provider_id', Integer, + ForeignKey('resource_providers.id'))), + ] + for col_name, column in columns_to_add: + if not hasattr(resource_providers.c, col_name): + resource_providers.create_column(column) + + indexed_columns = set() + for idx in resource_providers.indexes: + for c in idx.columns: + indexed_columns.add(c.name) + + if 'root_provider_id' not in indexed_columns: + index = Index('resource_providers_root_provider_id_idx', + resource_providers.c.root_provider_id) + index.create() + if 'parent_provider_id' not in indexed_columns: + index = Index('resource_providers_parent_provider_id_idx', + resource_providers.c.parent_provider_id) + index.create() diff --git a/nova/db/sqlalchemy/api_models.py b/nova/db/sqlalchemy/api_models.py index 3982606996..7e5e7b19f9 100644 --- a/nova/db/sqlalchemy/api_models.py +++ b/nova/db/sqlalchemy/api_models.py @@ -293,6 +293,10 @@ class ResourceProvider(API_BASE): schema.UniqueConstraint('uuid', name='uniq_resource_providers0uuid'), Index('resource_providers_name_idx', 'name'), + Index('resource_providers_root_provider_id_idx', + 'root_provider_id'), + Index('resource_providers_parent_provider_id_idx', + 'parent_provider_id'), schema.UniqueConstraint('name', name='uniq_resource_providers0name') ) @@ -301,6 +305,13 @@ class ResourceProvider(API_BASE): uuid = Column(String(36), nullable=False) name = Column(Unicode(200), nullable=True) generation = Column(Integer, default=0) + # Represents the root of the "tree" that the provider belongs to + root_provider_id = Column(Integer, ForeignKey('resource_providers.id'), + nullable=True) + # The immediate parent provider of this provider, or NULL if there is no + # parent. If parent_provider_id == NULL then root_provider_id == id + parent_provider_id = Column(Integer, ForeignKey('resource_providers.id'), + nullable=True) class Inventory(API_BASE): diff --git a/nova/exception.py b/nova/exception.py index 52f56aeb76..13794226dd 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -2065,6 +2065,11 @@ class ResourceClassNotFound(NotFound): msg_fmt = _("No such resource class %(resource_class)s.") +class CannotDeleteParentResourceProvider(NovaException): + msg_fmt = _("Cannot delete resource provider that is a parent of " + "another. Delete child providers first.") + + class ResourceProviderInUse(NovaException): msg_fmt = _("Resource provider has allocations.") diff --git a/nova/objects/resource_provider.py b/nova/objects/resource_provider.py index 1987aa9b29..77f02862ec 100644 --- a/nova/objects/resource_provider.py +++ b/nova/objects/resource_provider.py @@ -27,6 +27,7 @@ from oslo_db import exception as db_exc from oslo_log import log as logging import six import sqlalchemy as sa +from sqlalchemy import exc as sqla_exc from sqlalchemy import func from sqlalchemy import sql from sqlalchemy.sql import null @@ -399,13 +400,22 @@ def _get_provider_by_uuid(context, uuid): """ conn = conn = context.session.connection() rpt = sa.alias(_RP_TBL, name="rp") + parent = sa.alias(_RP_TBL, name="parent") + root = sa.alias(_RP_TBL, name="root") + # TODO(jaypipes): Change this to an inner join when we are sure all + # root_provider_id values are NOT NULL + rp_to_root = sa.outerjoin(rpt, root, rpt.c.root_provider_id == root.c.id) + rp_to_parent = sa.outerjoin(rp_to_root, parent, + rpt.c.parent_provider_id == parent.c.id) cols = [ rpt.c.id, rpt.c.uuid, rpt.c.name, rpt.c.generation, + root.c.uuid.label("root_provider_uuid"), + parent.c.uuid.label("parent_provider_uuid"), ] - sel = sa.select(cols).where(rpt.c.uuid == uuid) + sel = sa.select(cols).select_from(rp_to_parent).where(rpt.c.uuid == uuid) res = conn.execute(sel).fetchone() if not res: raise exception.NotFound( @@ -568,14 +578,100 @@ def _set_traits(context, rp, traits): rp.generation = _increment_provider_generation(conn, rp) +@db_api.api_context_manager.reader +def _has_child_providers(context, rp_id): + """Returns True if the supplied resource provider has any child providers, + False otherwise + """ + child_sel = sa.select([_RP_TBL.c.id]) + child_sel = child_sel.where(_RP_TBL.c.parent_provider_id == rp_id) + child_res = context.session.execute(child_sel.limit(1)).fetchone() + if child_res: + return True + return False + + +@db_api.api_context_manager.writer +def _set_root_provider_id(context, rp_id, root_id): + """Simply sets the root_provider_id value for a provider identified by + rp_id. Used in online data migration. + + :param rp_id: Internal ID of the provider to update + :param root_id: Value to set root provider to + """ + upd = _RP_TBL.update().where(_RP_TBL.c.id == rp_id) + upd = upd.values(root_provider_id=root_id) + context.session.execute(upd) + + +ProviderIds = collections.namedtuple( + 'ProviderIds', 'id uuid parent_id parent_uuid root_id root_uuid') + + +def _provider_ids_from_uuid(context, uuid): + """Given the UUID of a resource provider, returns a namedtuple + (ProviderIds) with the internal ID, the UUID, the parent provider's + internal ID, parent provider's UUID, the root provider's internal ID and + the root provider UUID. + + :returns: ProviderIds object containing the internal IDs and UUIDs of the + provider identified by the supplied UUID + :param uuid: The UUID of the provider to look up + """ + # SELECT + # rp.id, rp.uuid, + # parent.id AS parent_id, parent.uuid AS parent_uuid, + # root.id AS root_id, root.uuid AS root_uuid + # FROM resource_providers AS rp + # LEFT JOIN resource_providers AS parent + # ON rp.parent_provider_id = parent.id + # LEFT JOIN resource_providers AS root + # ON rp.root_provider_id = root.id + me = sa.alias(_RP_TBL, name="me") + parent = sa.alias(_RP_TBL, name="parent") + root = sa.alias(_RP_TBL, name="root") + cols = [ + me.c.id, + me.c.uuid, + parent.c.id.label('parent_id'), + parent.c.uuid.label('parent_uuid'), + root.c.id.label('root_id'), + root.c.uuid.label('root_uuid'), + ] + # TODO(jaypipes): Change this to an inner join when we are sure all + # root_provider_id values are NOT NULL + me_to_root = sa.outerjoin(me, root, me.c.root_provider_id == root.c.id) + me_to_parent = sa.outerjoin(me_to_root, parent, + me.c.parent_provider_id == parent.c.id) + sel = sa.select(cols).select_from(me_to_parent) + sel = sel.where(me.c.uuid == uuid) + res = context.session.execute(sel).fetchone() + if not res: + return None + return ProviderIds(**dict(res)) + + @base.NovaObjectRegistry.register_if(False) class ResourceProvider(base.NovaObject): + SETTABLE_FIELDS = ('name', 'parent_provider_uuid') fields = { 'id': fields.IntegerField(read_only=True), 'uuid': fields.UUIDField(nullable=False), 'name': fields.StringField(nullable=False), 'generation': fields.IntegerField(nullable=False), + # UUID of the root provider in a hierarchy of providers. Will be equal + # to the uuid field if this provider is the root provider of a + # hierarchy. This field is never manually set by the user. Instead, it + # is automatically set to either the root provider UUID of the parent + # or the UUID of the provider itself if there is no parent. This field + # is an optimization field that allows us to very quickly query for all + # providers within a particular tree without doing any recursive + # querying. + 'root_provider_uuid': fields.UUIDField(nullable=False), + # UUID of the direct parent provider, or None if this provider is a + # "root" provider. + 'parent_provider_uuid': fields.UUIDField(nullable=True, default=None), } def create(self): @@ -588,20 +684,28 @@ class ResourceProvider(base.NovaObject): if 'name' not in self: raise exception.ObjectActionError(action='create', reason='name is required') + if 'root_provider_uuid' in self: + raise exception.ObjectActionError( + action='create', + reason=_('root provider UUID cannot be manually set.')) + + self.obj_set_defaults() updates = self.obj_get_changes() - db_rp = self._create_in_db(self._context, updates) - self._from_db_object(self._context, self, db_rp) + self._create_in_db(self._context, updates) + self.obj_reset_changes() def destroy(self): self._delete(self._context, self.id) def save(self): updates = self.obj_get_changes() - if updates and list(updates.keys()) != ['name']: + if updates and any(k not in self.SETTABLE_FIELDS + for k in updates.keys()): raise exception.ObjectActionError( action='save', reason='Immutable fields changed') self._update_in_db(self._context, self.id, updates) + self.obj_reset_changes() @classmethod def get_by_uuid(cls, context, uuid): @@ -670,46 +774,151 @@ class ResourceProvider(base.NovaObject): _set_traits(self._context, self, traits) self.obj_reset_changes() - @staticmethod @db_api.api_context_manager.writer - def _create_in_db(context, updates): + def _create_in_db(self, context, updates): + parent_id = None + root_id = None + # User supplied a parent, let's make sure it exists + parent_uuid = updates.pop('parent_provider_uuid') + if parent_uuid is not None: + # Setting parent to ourselves doesn't make any sense + if parent_uuid == self.uuid: + raise exception.ObjectActionError( + action='create', + reason=_('parent provider UUID cannot be same as ' + 'UUID. Please set parent provider UUID to ' + 'None if there is no parent.')) + + parent_ids = _provider_ids_from_uuid(context, parent_uuid) + if parent_ids is None: + raise exception.ObjectActionError( + action='create', + reason=_('parent provider UUID does not exist.')) + + parent_id = parent_ids.id + root_id = parent_ids.root_id + updates['root_provider_id'] = root_id + updates['parent_provider_id'] = parent_id + self.root_provider_uuid = parent_ids.root_uuid + db_rp = models.ResourceProvider() db_rp.update(updates) context.session.add(db_rp) - return db_rp + context.session.flush() + + self.id = db_rp.id + self.generation = db_rp.generation + + if root_id is None: + # User did not specify a parent when creating this provider, so the + # root_provider_id needs to be set to this provider's newly-created + # internal ID + db_rp.root_provider_id = db_rp.id + context.session.add(db_rp) + context.session.flush() + self.root_provider_uuid = self.uuid @staticmethod @db_api.api_context_manager.writer def _delete(context, _id): + # Do a quick check to see if the provider is a parent. If it is, don't + # allow deleting the provider. Note that the foreign key constraint on + # resource_providers.parent_provider_id will prevent deletion of the + # parent within the transaction below. This is just a quick + # short-circuit outside of the transaction boundary. + if _has_child_providers(context, _id): + raise exception.CannotDeleteParentResourceProvider() + # Don't delete the resource provider if it has allocations. - rp_allocations = context.session.query(models.Allocation).filter( - models.Allocation.resource_provider_id == _id).count() + rp_allocations = context.session.query(models.Allocation).\ + filter(models.Allocation.resource_provider_id == _id).\ + count() if rp_allocations: raise exception.ResourceProviderInUse() # Delete any inventory associated with the resource provider context.session.query(models.Inventory).\ - filter(models.Inventory.resource_provider_id == _id).delete() + filter(models.Inventory.resource_provider_id == _id).\ + delete(synchronize_session=False) # Delete any aggregate associations for the resource provider # The name substitution on the next line is needed to satisfy pep8 RPA_model = models.ResourceProviderAggregate context.session.query(RPA_model).\ filter(RPA_model.resource_provider_id == _id).delete() - # Now delete the RP records - result = context.session.query(models.ResourceProvider).\ - filter(models.ResourceProvider.id == _id).delete() + # Now delete the RP record + try: + result = context.session.query(models.ResourceProvider).\ + filter(models.ResourceProvider.id == _id).\ + delete(synchronize_session=False) + except sqla_exc.IntegrityError: + # NOTE(jaypipes): Another thread snuck in and parented this + # resource provider in between the above check for + # _has_child_providers() and our attempt to delete the record + raise exception.CannotDeleteParentResourceProvider() if not result: raise exception.NotFound() - @staticmethod @db_api.api_context_manager.writer - def _update_in_db(context, id, updates): + def _update_in_db(self, context, id, updates): + if 'parent_provider_uuid' in updates: + # TODO(jaypipes): For now, "re-parenting" and "un-parenting" are + # not possible. If the provider already had a parent, we don't + # allow changing that parent due to various issues, including: + # + # * if the new parent is a descendant of this resource provider, we + # introduce the possibility of a loop in the graph, which would + # be very bad + # * potentially orphaning heretofore-descendants + # + # So, for now, let's just prevent re-parenting... + my_ids = _provider_ids_from_uuid(context, self.uuid) + parent_uuid = updates.pop('parent_provider_uuid') + if parent_uuid is not None: + parent_ids = _provider_ids_from_uuid(context, parent_uuid) + # User supplied a parent, let's make sure it exists + if parent_ids is None: + raise exception.ObjectActionError( + action='create', + reason=_('parent provider UUID does not exist.')) + if (my_ids.parent_id is not None and + my_ids.parent_id != parent_ids.id): + raise exception.ObjectActionError( + action='update', + reason=_('re-parenting a provider is not ' + 'currently allowed.')) + + updates['root_provider_id'] = parent_ids.root_id + updates['parent_provider_id'] = parent_ids.id + self.root_provider_uuid = parent_ids.root_uuid + else: + if my_ids.parent_id is not None: + raise exception.ObjectActionError( + action='update', + reason=_('un-parenting a provider is not ' + 'currently allowed.')) + db_rp = context.session.query(models.ResourceProvider).filter_by( id=id).first() db_rp.update(updates) - db_rp.save(context.session) + try: + db_rp.save(context.session) + except sqla_exc.IntegrityError: + # NOTE(jaypipes): Another thread snuck in and deleted the parent + # for this resource provider in between the above check for a valid + # parent provider and here... + raise exception.ObjectActionError( + action='update', + reason=_('parent provider UUID does not exist.')) @staticmethod + @db_api.api_context_manager.writer # Needed for online data migration def _from_db_object(context, resource_provider, db_resource_provider): + # Online data migration to populate root_provider_id + # TODO(jaypipes): Remove when all root_provider_id values are NOT NULL + if db_resource_provider['root_provider_uuid'] is None: + rp_id = db_resource_provider['id'] + uuid = db_resource_provider['uuid'] + db_resource_provider['root_provider_uuid'] = uuid + _set_root_provider_id(context, rp_id, rp_id) for field in resource_provider.fields: setattr(resource_provider, field, db_resource_provider[field]) resource_provider._context = context @@ -1230,11 +1439,32 @@ class ResourceProviderList(base.ObjectListBase, base.NovaObject): # and we want to make sure those class names aren't incorrect. resources = {_RC_CACHE.id_from_string(r_name): amount for r_name, amount in resources.items()} - query = context.session.query(models.ResourceProvider) + rp = sa.alias(_RP_TBL, name="rp") + root_rp = sa.alias(_RP_TBL, name="root_rp") + parent_rp = sa.alias(_RP_TBL, name="parent_rp") + + cols = [ + rp.c.id, + rp.c.uuid, + rp.c.name, + rp.c.generation, + root_rp.c.uuid.label("root_provider_uuid"), + parent_rp.c.uuid.label("parent_provider_uuid"), + ] + + # TODO(jaypipes): Convert this to an inner join once all + # root_provider_id values are NOT NULL + rp_to_root = sa.outerjoin(rp, root_rp, + rp.c.root_provider_id == root_rp.c.id) + rp_to_parent = sa.outerjoin(rp_to_root, parent_rp, + rp.c.parent_provider_id == parent_rp.c.id) + + query = sa.select(cols).select_from(rp_to_parent) + if name: - query = query.filter(models.ResourceProvider.name == name) + query = query.where(rp.c.name == name) if uuid: - query = query.filter(models.ResourceProvider.uuid == uuid) + query = query.where(rp.c.uuid == uuid) # If 'member_of' has values join with the PlacementAggregates to # get those resource providers that are associated with any of the @@ -1246,13 +1476,13 @@ class ResourceProviderList(base.ObjectListBase, base.NovaObject): resource_provider_id = _RP_AGG_TBL.c.resource_provider_id rps_in_aggregates = sa.select( [resource_provider_id]).select_from(join_statement) - query = query.filter(models.ResourceProvider.id.in_( - rps_in_aggregates)) + query = query.where(rp.c.id.in_(rps_in_aggregates)) if not resources: # Returns quickly the list in case we don't need to check the # resource usage - return query.all() + res = context.session.execute(query).fetchall() + return [dict(r) for r in res] # NOTE(sbauza): In case we want to look at the resource criteria, then # the SQL generated from this case looks something like: @@ -1289,8 +1519,8 @@ class ResourceProviderList(base.ObjectListBase, base.NovaObject): # comes from the above filters # First JOIN between inventories and RPs is here - join_clause = _RP_TBL.c.id == _INV_TBL.c.resource_provider_id - query = query.join(_INV_TBL, join_clause) + inv_join = sa.join(rp_to_parent, _INV_TBL, + rp.c.id == _INV_TBL.c.resource_provider_id) # Now, below is the LEFT JOIN for getting the allocations usage usage = sa.select([_ALLOC_TBL.c.resource_provider_id, @@ -1301,8 +1531,7 @@ class ResourceProviderList(base.ObjectListBase, base.NovaObject): usage = usage.group_by(_ALLOC_TBL.c.resource_provider_id, _ALLOC_TBL.c.resource_class_id) usage = sa.alias(usage, name='usage') - query = query.outerjoin( - usage, + usage_join = sa.outerjoin(inv_join, usage, sa.and_( usage.c.resource_provider_id == ( _INV_TBL.c.resource_provider_id), @@ -1322,13 +1551,15 @@ class ResourceProviderList(base.ObjectListBase, base.NovaObject): amount % _INV_TBL.c.step_size == 0 ) for (r_idx, amount) in resources.items()] - query = query.filter(sa.or_(*where_clauses)) - query = query.group_by(_RP_TBL.c.id) + query = query.select_from(usage_join) + query = query.where(sa.or_(*where_clauses)) + query = query.group_by(rp.c.id) # NOTE(sbauza): Only RPs having all the asked resources can be provided query = query.having(sql.func.count( sa.distinct(_INV_TBL.c.resource_class_id)) == len(resources)) - return query.all() + res = context.session.execute(query).fetchall() + return [dict(r) for r in res] @classmethod def get_all_by_filters(cls, context, filters=None): diff --git a/nova/tests/functional/db/api/test_migrations.py b/nova/tests/functional/db/api/test_migrations.py index 4bb1f52b23..26e5f115c5 100644 --- a/nova/tests/functional/db/api/test_migrations.py +++ b/nova/tests/functional/db/api/test_migrations.py @@ -646,6 +646,14 @@ class NovaAPIMigrationsWalk(test_migrations.WalkVersionsMixin): def _check_050(self, engine, data): self.assertColumnExists(engine, 'flavors', 'description') + def _check_051(self, engine, data): + for column in ['root_provider_id', 'parent_provider_id']: + self.assertColumnExists(engine, 'resource_providers', column) + self.assertIndexExists(engine, 'resource_providers', + 'resource_providers_root_provider_id_idx') + self.assertIndexExists(engine, 'resource_providers', + 'resource_providers_parent_provider_id_idx') + class TestNovaAPIMigrationsWalkSQLite(NovaAPIMigrationsWalk, test_base.DbTestCase, diff --git a/nova/tests/functional/db/test_resource_provider.py b/nova/tests/functional/db/test_resource_provider.py index cef156aa21..865093924d 100644 --- a/nova/tests/functional/db/test_resource_provider.py +++ b/nova/tests/functional/db/test_resource_provider.py @@ -88,6 +88,30 @@ class ResourceProviderTestCase(ResourceProviderBaseCase): self.assertRaises(exception.ObjectActionError, resource_provider.create) + def test_create_unknown_parent_provider(self): + """Test that if we provide a parent_provider_uuid value that points to + a resource provider that doesn't exist, that we get an + ObjectActionError. + """ + rp = rp_obj.ResourceProvider( + context=self.ctx, + name='rp1', + uuid=uuidsentinel.rp1, + parent_provider_uuid=uuidsentinel.noexists) + exc = self.assertRaises(exception.ObjectActionError, rp.create) + self.assertIn('parent provider UUID does not exist', str(exc)) + + def test_create_with_parent_provider_uuid_same_as_uuid_fail(self): + """Setting a parent provider UUID to one's own UUID makes no sense, so + check we don't support it. + """ + cn1 = rp_obj.ResourceProvider( + context=self.ctx, uuid=uuidsentinel.cn1, name='cn1', + parent_provider_uuid=uuidsentinel.cn1) + + exc = self.assertRaises(exception.ObjectActionError, cn1.create) + self.assertIn('parent provider UUID cannot be same as UUID', str(exc)) + def test_create_resource_provider(self): created_resource_provider = rp_obj.ResourceProvider( context=self.ctx, @@ -110,6 +134,97 @@ class ResourceProviderTestCase(ResourceProviderBaseCase): self.assertEqual(0, created_resource_provider.generation) self.assertEqual(0, retrieved_resource_provider.generation) + def test_root_provider_population(self): + """Simulate an old resource provider record in the database that has no + root_provider_uuid set and ensure that when grabbing the resource + provider object, the root_provider_uuid field in the table is set to + the provider's UUID. + """ + rp_tbl = rp_obj._RP_TBL + conn = self.api_db.get_engine().connect() + + # First, set up a record for an "old-style" resource provider with no + # root provider UUID. + ins_stmt = rp_tbl.insert().values( + id=1, + uuid=uuidsentinel.rp1, + name='rp-1', + root_provider_id=None, + parent_provider_id=None, + generation=42, + ) + conn.execute(ins_stmt) + + rp = rp_obj.ResourceProvider.get_by_uuid(self.ctx, uuidsentinel.rp1) + + # The ResourceProvider._from_db_object() method should have performed + # an online data migration, populating the root_provider_id field + # with the value of the id field. Let's check it happened. + sel_stmt = sa.select([rp_tbl.c.root_provider_id]).where( + rp_tbl.c.id == 1) + res = conn.execute(sel_stmt).fetchall() + self.assertEqual(1, res[0][0]) + # Make sure the object root_provider_uuid is set on load + self.assertEqual(rp.root_provider_uuid, uuidsentinel.rp1) + + def test_inherit_root_from_parent(self): + """Tests that if we update an existing provider's parent provider UUID, + that the root provider UUID of the updated provider is automatically + set to the parent provider's root provider UUID. + """ + rp1 = rp_obj.ResourceProvider( + context=self.ctx, + name='rp1', + uuid=uuidsentinel.rp1, + ) + rp1.create() + + # Test the root was auto-set to the create provider's UUID + self.assertEqual(uuidsentinel.rp1, rp1.root_provider_uuid) + + # Create a new provider that we will make the parent of rp1 + parent_rp = rp_obj.ResourceProvider( + context=self.ctx, + name='parent', + uuid=uuidsentinel.parent, + ) + parent_rp.create() + self.assertEqual(uuidsentinel.parent, parent_rp.root_provider_uuid) + + # Now change rp1 to be a child of parent and check rp1's root is + # changed to that of the parent. + rp1.parent_provider_uuid = parent_rp.uuid + rp1.save() + + self.assertEqual(uuidsentinel.parent, rp1.root_provider_uuid) + + def test_save_root_provider_failed(self): + """Test that if we provide a root_provider_uuid value we get an + ObjectActionError if we save the object. + """ + rp = rp_obj.ResourceProvider( + context=self.ctx, + name='rp1', + uuid=uuidsentinel.rp1, + ) + rp.create() + rp.root_provider_uuid = uuidsentinel.noexists + self.assertRaises(exception.ObjectActionError, rp.save) + + def test_save_unknown_parent_provider(self): + """Test that if we provide a parent_provider_uuid value that points to + a resource provider that doesn't exist, that we get an + ObjectActionError if we save the object. + """ + rp = rp_obj.ResourceProvider( + context=self.ctx, + name='rp1', + uuid=uuidsentinel.rp1, + ) + rp.create() + rp.parent_provider_uuid = uuidsentinel.noexists + self.assertRaises(exception.ObjectActionError, rp.save) + def test_save_resource_provider(self): created_resource_provider = rp_obj.ResourceProvider( context=self.ctx, @@ -125,6 +240,118 @@ class ResourceProviderTestCase(ResourceProviderBaseCase): ) self.assertEqual('new-name', retrieved_resource_provider.name) + def test_save_reparenting_fail(self): + """Tests that we prevent a resource provider's parent provider UUID + from being changed from a non-NULL value to another non-NULL value. + """ + cn1 = rp_obj.ResourceProvider( + context=self.ctx, uuid=uuidsentinel.cn1, name='cn1') + cn1.create() + + cn2 = rp_obj.ResourceProvider( + context=self.ctx, uuid=uuidsentinel.cn2, name='cn2') + cn2.create() + + cn3 = rp_obj.ResourceProvider( + context=self.ctx, uuid=uuidsentinel.cn3, name='cn3') + cn3.create() + + # First, make sure we can set the parent for a provider that does not + # have a parent currently + cn1.parent_provider_uuid = uuidsentinel.cn2 + cn1.save() + + # Now make sure we can't change the parent provider + cn1.parent_provider_uuid = uuidsentinel.cn3 + exc = self.assertRaises(exception.ObjectActionError, cn1.save) + self.assertIn('re-parenting a provider is not currently', str(exc)) + + # Also ensure that we can't "un-parent" a provider + cn1.parent_provider_uuid = None + exc = self.assertRaises(exception.ObjectActionError, cn1.save) + self.assertIn('un-parenting a provider is not currently', str(exc)) + + def test_nested_providers(self): + """Create a hierarchy of resource providers and run through a series of + tests that ensure one cannot delete a resource provider that has no + direct allocations but its child providers do have allocations. + """ + root_rp = rp_obj.ResourceProvider( + context=self.ctx, + uuid=uuidsentinel.root_rp, + name='root-rp', + ) + root_rp.create() + + child_rp = rp_obj.ResourceProvider( + context=self.ctx, + uuid=uuidsentinel.child_rp, + name='child-rp', + parent_provider_uuid=uuidsentinel.root_rp, + ) + child_rp.create() + + grandchild_rp = rp_obj.ResourceProvider( + context=self.ctx, + uuid=uuidsentinel.grandchild_rp, + name='grandchild-rp', + parent_provider_uuid=uuidsentinel.child_rp, + ) + grandchild_rp.create() + + # Verify that the root_provider_uuid of both the child and the + # grandchild is the UUID of the grandparent + self.assertEqual(root_rp.uuid, child_rp.root_provider_uuid) + self.assertEqual(root_rp.uuid, grandchild_rp.root_provider_uuid) + + # Create some inventory in the grandchild, allocate some consumers to + # the grandchild and then attempt to delete the root provider and child + # provider, both of which should fail. + invs = [ + rp_obj.Inventory( + resource_provider=grandchild_rp, + resource_class=fields.ResourceClass.VCPU, + total=1, + reserved=0, + allocation_ratio=1.0, + min_unit=1, + max_unit=1, + step_size=1, + ), + ] + inv_list = rp_obj.InventoryList( + resource_provider=grandchild_rp, + objects=invs + ) + grandchild_rp.set_inventory(inv_list) + + allocs = [ + rp_obj.Allocation( + resource_provider=grandchild_rp, + resource_class=fields.ResourceClass.VCPU, + consumer_id=uuidsentinel.consumer, + used=1, + ), + ] + alloc_list = rp_obj.AllocationList(self.ctx, objects=allocs) + alloc_list.create_all() + + self.assertRaises(exception.CannotDeleteParentResourceProvider, + root_rp.destroy) + self.assertRaises(exception.CannotDeleteParentResourceProvider, + child_rp.destroy) + + # Cannot delete provider if it has allocations + self.assertRaises(exception.ResourceProviderInUse, + grandchild_rp.destroy) + + # Now remove the allocations against the child and check that we can + # now delete the child provider + alloc_list.delete_all() + grandchild_rp.destroy() + child_rp.destroy() + root_rp.destroy() + def test_destroy_resource_provider(self): created_resource_provider = rp_obj.ResourceProvider( context=self.ctx, diff --git a/nova/tests/unit/objects/test_resource_provider.py b/nova/tests/unit/objects/test_resource_provider.py index 95160ddf90..5968d2c3d6 100644 --- a/nova/tests/unit/objects/test_resource_provider.py +++ b/nova/tests/unit/objects/test_resource_provider.py @@ -16,7 +16,6 @@ import six import nova from nova import context from nova import exception -from nova import objects from nova.objects import fields from nova.objects import resource_provider from nova import test @@ -26,9 +25,9 @@ from nova.tests import uuidsentinel as uuids _RESOURCE_CLASS_NAME = 'DISK_GB' _RESOURCE_CLASS_ID = 2 -IPV4_ADDRESS_ID = objects.fields.ResourceClass.STANDARD.index( +IPV4_ADDRESS_ID = fields.ResourceClass.STANDARD.index( fields.ResourceClass.IPV4_ADDRESS) -VCPU_ID = objects.fields.ResourceClass.STANDARD.index( +VCPU_ID = fields.ResourceClass.STANDARD.index( fields.ResourceClass.VCPU) _RESOURCE_PROVIDER_ID = 1 @@ -39,7 +38,23 @@ _RESOURCE_PROVIDER_DB = { 'uuid': _RESOURCE_PROVIDER_UUID, 'name': _RESOURCE_PROVIDER_NAME, 'generation': 0, + 'root_provider_uuid': _RESOURCE_PROVIDER_UUID, + 'parent_provider_uuid': None, } + +_RESOURCE_PROVIDER_ID2 = 2 +_RESOURCE_PROVIDER_UUID2 = uuids.resource_provider2 +_RESOURCE_PROVIDER_NAME2 = uuids.resource_name2 +_RESOURCE_PROVIDER_DB2 = { + 'id': _RESOURCE_PROVIDER_ID2, + 'uuid': _RESOURCE_PROVIDER_UUID2, + 'name': _RESOURCE_PROVIDER_NAME2, + 'generation': 0, + 'root_provider_uuid': _RESOURCE_PROVIDER_UUID, + 'parent_provider_uuid': _RESOURCE_PROVIDER_UUID, +} + + _INVENTORY_ID = 2 _INVENTORY_DB = { 'id': _INVENTORY_ID, @@ -73,28 +88,6 @@ def _fake_ensure_cache(ctxt): class TestResourceProviderNoDB(test_objects._LocalTest): USES_DB = False - @mock.patch('nova.objects.resource_provider._get_provider_by_uuid', - return_value=_RESOURCE_PROVIDER_DB) - def test_object_get_by_uuid(self, mock_db_get): - resource_provider_object = resource_provider.ResourceProvider.\ - get_by_uuid(mock.sentinel.ctx, _RESOURCE_PROVIDER_UUID) - self.assertEqual(_RESOURCE_PROVIDER_ID, resource_provider_object.id) - self.assertEqual(_RESOURCE_PROVIDER_UUID, - resource_provider_object.uuid) - - @mock.patch('nova.objects.resource_provider.ResourceProvider.' - '_create_in_db', return_value=_RESOURCE_PROVIDER_DB) - def test_create(self, mock_db_create): - obj = resource_provider.ResourceProvider(context=self.context, - uuid=_RESOURCE_PROVIDER_UUID, - name=_RESOURCE_PROVIDER_NAME) - obj.create() - self.assertEqual(_RESOURCE_PROVIDER_UUID, obj.uuid) - self.assertIsInstance(obj.id, int) - mock_db_create.assert_called_once_with( - self.context, {'uuid': _RESOURCE_PROVIDER_UUID, - 'name': _RESOURCE_PROVIDER_NAME}) - def test_create_id_fail(self): obj = resource_provider.ResourceProvider(context=self.context, uuid=_RESOURCE_PROVIDER_UUID, @@ -107,6 +100,17 @@ class TestResourceProviderNoDB(test_objects._LocalTest): self.assertRaises(exception.ObjectActionError, obj.create) + def test_create_with_root_provider_uuid_fail(self): + obj = resource_provider.ResourceProvider( + context=self.context, + uuid=_RESOURCE_PROVIDER_UUID, + name=_RESOURCE_PROVIDER_NAME, + root_provider_uuid=_RESOURCE_PROVIDER_UUID, + ) + + exc = self.assertRaises(exception.ObjectActionError, obj.create) + self.assertIn('root provider UUID cannot be manually set', str(exc)) + class TestProviderSummaryNoDB(test_objects._LocalTest): USES_DB = False @@ -126,18 +130,36 @@ class TestProviderSummaryNoDB(test_objects._LocalTest): class TestResourceProvider(test_objects._LocalTest): - def test_create_in_db(self): - updates = {'uuid': _RESOURCE_PROVIDER_UUID, - 'name': _RESOURCE_PROVIDER_NAME} - db_rp = resource_provider.ResourceProvider._create_in_db( - self.context, updates) - self.assertIsInstance(db_rp.id, int) - self.assertEqual(_RESOURCE_PROVIDER_UUID, db_rp.uuid) - self.assertEqual(_RESOURCE_PROVIDER_NAME, db_rp.name) + def test_create_no_parent(self): + rp = resource_provider.ResourceProvider( + self.context, uuid=_RESOURCE_PROVIDER_UUID, + name=_RESOURCE_PROVIDER_NAME) + rp.create() + self.assertIsInstance(rp.id, int) + self.assertEqual(_RESOURCE_PROVIDER_UUID, rp.uuid) + self.assertEqual(_RESOURCE_PROVIDER_NAME, rp.name) + self.assertEqual(_RESOURCE_PROVIDER_UUID, rp.root_provider_uuid) + self.assertIsNone(rp.parent_provider_uuid) + + def test_create_in_db_with_parent_provider_uuid(self): + parent = resource_provider.ResourceProvider( + self.context, uuid=uuids.parent, name="parent") + parent.create() + child = resource_provider.ResourceProvider( + self.context, uuid=uuids.child, name="child", + parent_provider_uuid=uuids.parent) + child.create() + self.assertEqual(uuids.child, child.uuid) + self.assertEqual(uuids.parent, child.parent_provider_uuid) + self.assertEqual(uuids.parent, child.root_provider_uuid) def test_save_immutable(self): - fields = {'id': 1, 'uuid': _RESOURCE_PROVIDER_UUID, - 'generation': 1} + fields = { + 'id': 1, + 'uuid': _RESOURCE_PROVIDER_UUID, + 'generation': 1, + 'root_provider_uuid': _RESOURCE_PROVIDER_UUID, + } for field in fields: rp = resource_provider.ResourceProvider(context=self.context) setattr(rp, field, fields[field]) @@ -152,6 +174,8 @@ class TestResourceProvider(test_objects._LocalTest): self.context, _RESOURCE_PROVIDER_UUID) self.assertEqual(rp.uuid, retrieved_rp.uuid) self.assertEqual(rp.name, retrieved_rp.name) + self.assertEqual(rp.root_provider_uuid, + retrieved_rp.root_provider_uuid) def test_get_by_uuid_from_db_missing(self): self.assertRaises(exception.NotFound,