From b10f11d7e8e1afb7a12a470f92c42bf3c23eca95 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Mon, 26 Sep 2016 20:52:49 -0400 Subject: [PATCH] placement: add nested resource providers Adds initial support for storing the relationship between parent and child resource providers. Nested resource providers are essential for expressing certain types of resources -- in particular SR-IOV physical functions and certain SR-IOV fully-programmable gate arrays. The resources that these providers expose are of resource class SRIOV_NET_VF and we will need a way of indicating that the physical function providing these virtual function resources is tagged with certain traits (representing vendor_id, product_id or the physical network the PF is attached to). The compute host is a resource provider which has an SR-IOV-enabled physical function (NIC) as a child resource provider. The physical function has an inventory containing some total amount of SRIOV_NET_VF resources. These SRIOV_NET_VF resources are allocated to zero or more consumers (instances) on the compute host. compute host (parent resource provider) | | SR-IOV PF (child resource provider) : / \ / \ VF1 VF2 (inventory of child provider) The resource provider model gets two new fields: - root_provider_uuid: The "top" or "root" of the tree of nested providers - parent_provider_uuid: The immediate parent of the provider, or None if the provider is a root provider. The database schema adds two new columns to the resource_providers table that contain the internal integer IDs that correspond to the user-facing UUID values: - root_provider_id - parent_provider_id The root_provider_uuid field is non-nullable in the ResourceProvider object definition, and this code includes an online data migration to automatically populate the root_provider_id field with the value of the resource_providers.id field for any resource providers already in the DB. The root_provider_id field value is populated automatically when a provider is created. If the parent provider UUID is set, then the root_provider_id is set to the root_provider_id value of the parent. If parent is unset, root_provider_id is set to the value of the id attribute of the provider being created. The corresponding UUID values for root and parent provider are fetched in the queries that retrieves resource provider data using two self-referential joins. The root_provider_id column allows us to do extremely quick lookups of an entire tree of providers without needing to perform any recursive database queries. Logic in this patch ensures that no resource provider can be deleted if any of its children have any allocations active on them. We also check to ensure that when created or updated, a resource provider's parent provider UUID actually points to an existing provider. It's important to point out that qualitative trait information is only associated with a resource provider entity, not the resources that resource provider has in its inventory. This is the reason why nested resource providers are necessary. In the case of things like NUMA nodes or SRIOV physical functions, if a compute host had multiple SRIOV physical functions, each associated with a different network trait, there would be no way to differentiate between the SRIOV_NET_VF resources that those multiple SRIOV physical functions provided if the containing compute host had a single inventory item containing the total number of VFs exposed by both PFs. Change-Id: I2d8df57f77a03cde898d9ec792c5d59b75f61204 blueprint: nested-resource-providers Co-Authored-By: Moshe Levi --- .../versions/051_nested_resource_providers.py | 50 +++ nova/db/sqlalchemy/api_models.py | 11 + nova/exception.py | 5 + nova/objects/resource_provider.py | 289 ++++++++++++++++-- .../functional/db/api/test_migrations.py | 8 + .../functional/db/test_resource_provider.py | 227 ++++++++++++++ .../unit/objects/test_resource_provider.py | 94 +++--- 7 files changed, 620 insertions(+), 64 deletions(-) create mode 100644 nova/db/sqlalchemy/api_migrations/migrate_repo/versions/051_nested_resource_providers.py 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 daa47d5dc2..6632284ac1 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,