diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index bfbdcbdecc..6109895ca4 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -355,10 +355,10 @@ #host = localhost # WARNING: This configuration option is part of the incomplete -# rolling upgrades work; changing this value has no effect. -# Used for rolling upgrades. Setting this option downgrades -# (or pins) the internal ironic RPC communication to the -# specified version to enable communication with older +# rolling upgrades work. Do not change this from the default +# value. Used for rolling upgrades. Setting this option +# downgrades (or pins) the internal ironic RPC communication +# to the specified version to enable communication with older # services. When doing a rolling upgrade from version X to # version Y, set (pin) this to X. To unpin, leave it unset. # Defaults to using the newest possible RPC behavior. (string diff --git a/ironic/conf/default.py b/ironic/conf/default.py index 1eaa8e49b3..b10454025c 100644 --- a/ironic/conf/default.py +++ b/ironic/conf/default.py @@ -266,8 +266,8 @@ service_opts = [ choices=versions.RELEASE_VERSIONS, # TODO(xek): mutable=True, help=_('WARNING: This configuration option is part of the ' - 'incomplete rolling upgrades work; changing this ' - 'value has no effect. ' + 'incomplete rolling upgrades work. Do not change this ' + 'from the default value. ' 'Used for rolling upgrades. Setting this option ' 'downgrades (or pins) the internal ironic RPC ' 'communication to the specified version to enable ' diff --git a/ironic/db/sqlalchemy/alembic/versions/868cb606a74a_add_version_field_in_base_class.py b/ironic/db/sqlalchemy/alembic/versions/868cb606a74a_add_version_field_in_base_class.py new file mode 100644 index 0000000000..d55f36c2a9 --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/868cb606a74a_add_version_field_in_base_class.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. + +"""add version field in base class + +Revision ID: 868cb606a74a +Revises: 3d86a077a3f2 +Create Date: 2016-12-15 12:31:31.629237 + +""" + +# revision identifiers, used by Alembic. +revision = '868cb606a74a' +down_revision = '3d86a077a3f2' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # NOTE(rloo): In db.sqlalchemy.models, we added the 'version' column + # to IronicBase class. All inherited classes/tables have + # this new column. + op.add_column('chassis', + sa.Column('version', sa.String(length=15), nullable=True)) + op.add_column('conductors', + sa.Column('version', sa.String(length=15), nullable=True)) + op.add_column('node_tags', + sa.Column('version', sa.String(length=15), nullable=True)) + op.add_column('nodes', + sa.Column('version', sa.String(length=15), nullable=True)) + op.add_column('portgroups', + sa.Column('version', sa.String(length=15), nullable=True)) + op.add_column('ports', + sa.Column('version', sa.String(length=15), nullable=True)) + op.add_column('volume_connectors', + sa.Column('version', sa.String(length=15), nullable=True)) + op.add_column('volume_targets', + sa.Column('version', sa.String(length=15), nullable=True)) + op.add_column('conductor_hardware_interfaces', + sa.Column('version', sa.String(length=15), nullable=True)) diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index f5f13ffe6c..622ab4ed9c 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -52,6 +52,8 @@ class IronicBase(models.TimestampMixin, metadata = None + version = Column(String(15), nullable=True) + def as_dict(self): d = {} for c in self.__table__.columns: diff --git a/ironic/objects/base.py b/ironic/objects/base.py index a8f2c5afac..0103197e63 100644 --- a/ironic/objects/base.py +++ b/ironic/objects/base.py @@ -17,7 +17,7 @@ from oslo_log import log from oslo_utils import versionutils from oslo_versionedobjects import base as object_base -import six +from oslo_versionedobjects import exception as ovo_exception from ironic.common import release_mappings as versions from ironic.conf import CONF @@ -80,20 +80,125 @@ class IronicObject(object_base.VersionedObject): self[field] != loaded_object[field]): self[field] = loaded_object[field] + def _convert_to_version(self, target_version): + """Convert to the target version. + + Subclasses should redefine this method, to do the conversion + of the object to the specified version. As a result of any + conversion, the object changes (self.obj_what_changed()) should + be retained. + + :param target_version: the desired version of the object + """ + pass + + def convert_to_version(self, target_version): + """Convert this object to the target version. + + _convert_to_version() does the actual work. + + :param target_version: the desired version of the object + """ + self._convert_to_version(target_version) + + # NOTE(rloo): self.__class__.VERSION is the latest version that + # is supported by this service. self.VERSION is the version of + # this object instance -- it may get set via e.g. the + # serialization or deserialization process, or here. + if (self.__class__.VERSION != target_version or + self.VERSION != self.__class__.VERSION): + self.VERSION = target_version + + def get_target_version(self): + """Returns the target version for this object. + + This is the version in which the object should be manipulated, e.g. + sent over the wire via RPC or saved in the DB. + + :returns: if pinned, returns the version of this object corresponding + to the pin. Otherwise, returns the version of the object. + :raises: ovo_exception.IncompatibleObjectVersion + """ + pin = CONF.pin_release_version + if pin: + version_manifest = versions.RELEASE_MAPPING[pin]['objects'] + pinned_version = version_manifest.get(self.obj_name()) + if pinned_version: + if not versionutils.is_compatible(pinned_version, + self.__class__.VERSION): + LOG.error( + 'For object "%(objname)s", the target version ' + '"%(target)s" is not compatible with its supported ' + 'version "%(support)s". The value ("%(pin)s") of the ' + '"pin_release_version" configuration option may be ' + 'incorrect.', + {'objname': self.obj_name(), 'target': pinned_version, + 'support': self.__class__.VERSION, 'pin': pin}) + raise ovo_exception.IncompatibleObjectVersion( + objname=self.obj_name(), objver=pinned_version, + supported=self.__class__.VERSION) + return pinned_version + + return self.__class__.VERSION + @staticmethod def _from_db_object(context, obj, db_object): """Converts a database entity to a formal object. + This always converts the database entity to the latest version + of the object. Note that the latest version is available at + object.__class__.VERSION. object.VERSION is the version of this + particular object instance; it is possible that it is not the latest + version. + :param context: security context :param obj: An object of the class. - :param db_object: A DB model of the object + :param db_object: A DB entity of the object :return: The object of the class with the database entity added + :raises: ovo_exception.IncompatibleObjectVersion """ + objname = obj.obj_name() + db_version = db_object['version'] + + if db_version is None: + # NOTE(rloo): This can only happen after we've updated the DB + # tables to include the 'version' column but haven't saved the + # object to the DB since the new column was added. This column is + # added in the Pike cycle, so if the version isn't set, use the + # version associated with the most recent release, i.e. '8.0'. + # The objects and RPC versions haven't changed between '8.0' and + # Ocata, which is why it is fine to use Ocata. + # Furthermore, if this is a new object that did not exist in the + # most recent release, we assume it is version 1.0. + # TODO(rloo): This entire if clause can be deleted in Queens + # since the dbsync online migration populates all the versions + # and it must be run to completion before upgrading to Queens. + db_version = versions.RELEASE_MAPPING['ocata']['objects'].get( + objname, '1.0') + + if not versionutils.is_compatible(db_version, obj.__class__.VERSION): + raise ovo_exception.IncompatibleObjectVersion( + objname=objname, objver=db_version, + supported=obj.__class__.VERSION) + for field in obj.fields: obj[field] = db_object[field] obj._context = context obj.obj_reset_changes() + + if db_version != obj.__class__.VERSION: + # convert to the latest version + obj.convert_to_version(obj.__class__.VERSION) + if obj.get_target_version() == db_version: + # pinned, so no need to keep these changes (we'll end up + # converting back to db_version if obj is saved) + obj.obj_reset_changes() + else: + # keep these changes around because they are needed + # when/if saving to the DB in the latest version + pass + return obj @classmethod @@ -111,36 +216,99 @@ class IronicObject(object_base.VersionedObject): return [cls._from_db_object(context, cls(), db_obj) for db_obj in db_objects] - def _get_target_version(self): - """Returns the target version for this object. + def do_version_changes_for_db(self): + """Do any changes to the object before saving it in the DB. - If pinned, returns the version of this object corresponding to - the pin. Otherwise, returns this (latest) version of the object. + This determines which version of the object should be saved to the + database, and if needed, updates the object (fields) to correspond to + the desired version. + + The version used to save the object is determined as follows: + + * If the object is pinned, we save the object in the pinned version. + Since it is pinned, we don't want to save in a newer version, in case + a rolling upgrade is happening and some services are still using the + older version of ironic, with no knowledge of this newer version. + * If the object isn't pinned, we save the object in the latest version. + + Because the object may be converted to a different object version, + this method should only be called just before saving the object to + the DB. + + :returns: a dictionary of changed fields and their new values + (could be an empty dictionary). """ - pinned_version = None - pin = CONF.pin_release_version - if pin: - version_manifest = versions.RELEASE_MAPPING[pin]['objects'] - pinned_version = version_manifest.get(self.obj_name()) - return pinned_version or self.__class__.VERSION + target_version = self.get_target_version() + + if (target_version != self.VERSION): + # Convert the object so we can save it in the target version. + self.convert_to_version(target_version) + db_version = target_version + else: + db_version = self.VERSION + + changes = self.obj_get_changes() + # NOTE(rloo): Since this object doesn't keep track of the version that + # is saved in the DB and we don't want to make a DB call + # just to find out, we always update 'version' in the DB. + changes['version'] = db_version + + return changes class IronicObjectSerializer(object_base.VersionedObjectSerializer): # Base class to use for object hydration OBJ_BASE_CLASS = IronicObject + def _process_object(self, context, objprim): + """Process the object. + + This is called from within deserialize_entity(). Deserialization + is done for any serialized entities from e.g. an RPC request; the + deserialization process converts them to Objects. + + This converts any IronicObjects to be in their latest versions, + so that the services (ironic-api and ironic-conductor) internally, + always deal objects in their latest versions. + + :param objprim: a serialized entity that represents an object + :returns: the deserialized Object + :raises ovo_exception.IncompatibleObjectVersion + """ + obj = super(IronicObjectSerializer, self)._process_object( + context, objprim) + if isinstance(obj, IronicObject): + if obj.VERSION != obj.__class__.VERSION: + obj.convert_to_version(obj.__class__.VERSION) + return obj + def serialize_entity(self, context, entity): - if isinstance(entity, (tuple, list, set, dict)): - entity = self._process_iterable(context, self.serialize_entity, - entity) - elif (hasattr(entity, 'obj_to_primitive') - and callable(entity.obj_to_primitive)): - target_version = entity._get_target_version() - # NOTE(xek): If the version is pinned, target_version is an older - # object version and entity's obj_make_compatible method is called - # to backport the object before serialization. - entity = entity.obj_to_primitive(target_version=target_version) - elif not isinstance(entity, (bool, float, type, six.integer_types, - six.string_types)) and entity: - LOG.warning("Entity %s was not serialized.", str(entity)) - return entity + """Serialize the entity. + + This serializes the entity so that it can be sent over e.g. RPC. + A serialized entity for an IronicObject is a dictionary with keys: + 'ironic_object.namespace', 'ironic_object.data', 'ironic_object.name', + 'ironic_object.version', and 'ironic_object.changes'. + + For IronicObjects, if the service (ironic-api or ironic-conductor) + is pinned, we want the object to be in the target/pinned version, + which is not necessarily the latest version of the object. + (Internally, the services deal with the latest versions of objects + so we know that these objects are always in the latest versions.) + + :param context: security context + :param entity: the entity to be serialized; may be an IronicObject + :returns: the serialized entity + :raises: ovo_exception.IncompatibleObjectVersion (via + .get_target_version()) + """ + if isinstance(entity, IronicObject): + target_version = entity.get_target_version() + if target_version != entity.VERSION: + # NOTE(xek): If the version is pinned, target_version is an + # older object version. We need to backport/convert to target + # version before serialization. + entity.convert_to_version(target_version) + + return super(IronicObjectSerializer, self).serialize_entity( + context, entity) diff --git a/ironic/objects/chassis.py b/ironic/objects/chassis.py index 1dc8624129..3279dceb28 100644 --- a/ironic/objects/chassis.py +++ b/ironic/objects/chassis.py @@ -138,7 +138,7 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): object, e.g.: Chassis(context) """ - values = self.obj_get_changes() + values = self.do_version_changes_for_db() db_chassis = self.dbapi.create_chassis(values) self._from_db_object(self._context, self, db_chassis) @@ -176,7 +176,7 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): A context should be set when instantiating the object, e.g.: Chassis(context) """ - updates = self.obj_get_changes() + updates = self.do_version_changes_for_db() updated_chassis = self.dbapi.update_chassis(self.uuid, updates) self._from_db_object(self._context, self, updated_chassis) diff --git a/ironic/objects/node.py b/ironic/objects/node.py index b5ad5c6a8f..08ee649834 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -323,7 +323,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): object, e.g.: Node(context) :raises: InvalidParameterValue if some property values are invalid. """ - values = self.obj_get_changes() + values = self.do_version_changes_for_db() self._validate_property_values(values.get('properties')) db_node = self.dbapi.create_node(values) self._from_db_object(self._context, self, db_node) @@ -365,12 +365,12 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): object, e.g.: Node(context) :raises: InvalidParameterValue if some property values are invalid. """ - updates = self.obj_get_changes() + updates = self.do_version_changes_for_db() self._validate_property_values(updates.get('properties')) if 'driver' in updates and 'driver_internal_info' not in updates: # Clean driver_internal_info when changes driver self.driver_internal_info = {} - updates = self.obj_get_changes() + updates = self.do_version_changes_for_db() db_node = self.dbapi.update_node(self.uuid, updates) self._from_db_object(self._context, self, db_node) diff --git a/ironic/objects/port.py b/ironic/objects/port.py index b1d0230731..6e13844150 100644 --- a/ironic/objects/port.py +++ b/ironic/objects/port.py @@ -227,7 +227,7 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): :raises: PortAlreadyExists if 'uuid' column is not unique """ - values = self.obj_get_changes() + values = self.do_version_changes_for_db() db_port = self.dbapi.create_port(values) self._from_db_object(self._context, self, db_port) @@ -270,7 +270,7 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): :raises: MACAlreadyExists if 'address' column is not unique """ - updates = self.obj_get_changes() + updates = self.do_version_changes_for_db() updated_port = self.dbapi.update_port(self.uuid, updates) self._from_db_object(self._context, self, updated_port) diff --git a/ironic/objects/portgroup.py b/ironic/objects/portgroup.py index 921bda5a23..a12542d7a7 100644 --- a/ironic/objects/portgroup.py +++ b/ironic/objects/portgroup.py @@ -218,7 +218,7 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): :raises: DuplicateName, MACAlreadyExists, PortgroupAlreadyExists """ - values = self.obj_get_changes() + values = self.do_version_changes_for_db() db_portgroup = self.dbapi.create_portgroup(values) self._from_db_object(self._context, self, db_portgroup) @@ -260,7 +260,7 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): :raises: PortgroupNotFound, DuplicateName, MACAlreadyExists """ - updates = self.obj_get_changes() + updates = self.do_version_changes_for_db() updated_portgroup = self.dbapi.update_portgroup(self.uuid, updates) self._from_db_object(self._context, self, updated_portgroup) diff --git a/ironic/objects/volume_connector.py b/ironic/objects/volume_connector.py index 2ea9b1684a..c66535f0bc 100644 --- a/ironic/objects/volume_connector.py +++ b/ironic/objects/volume_connector.py @@ -168,7 +168,7 @@ class VolumeConnector(base.IronicObject, :raises: VolumeConnectorAlreadyExists if a volume connector with the same UUID already exists """ - values = self.obj_get_changes() + values = self.do_version_changes_for_db() db_connector = self.dbapi.create_volume_connector(values) self._from_db_object(self._context, self, db_connector) @@ -199,7 +199,7 @@ class VolumeConnector(base.IronicObject, """Save updates to this VolumeConnector. Updates will be made column by column based on the result - of self.obj_get_changes(). + of self.do_version_changes_for_db(). :param context: security context. NOTE: This should only be used internally by the indirection_api. @@ -214,7 +214,7 @@ class VolumeConnector(base.IronicObject, fields :raises: InvalidParameterValue when the UUID is being changed """ - updates = self.obj_get_changes() + updates = self.do_version_changes_for_db() updated_connector = self.dbapi.update_volume_connector(self.uuid, updates) self._from_db_object(self._context, self, updated_connector) diff --git a/ironic/objects/volume_target.py b/ironic/objects/volume_target.py index 8fc555743d..20092c445a 100644 --- a/ironic/objects/volume_target.py +++ b/ironic/objects/volume_target.py @@ -194,7 +194,7 @@ class VolumeTarget(base.IronicObject, :raises: VolumeTargetAlreadyExists if a volume target with the same UUID exists """ - values = self.obj_get_changes() + values = self.do_version_changes_for_db() db_target = self.dbapi.create_volume_target(values) self._from_db_object(self._context, self, db_target) @@ -224,7 +224,7 @@ class VolumeTarget(base.IronicObject, """Save updates to this VolumeTarget. Updates will be made column by column based on the result - of self.obj_get_changes(). + of self.do_version_changes_for_db(). :param context: security context. NOTE: This should only be used internally by the indirection_api. @@ -237,7 +237,7 @@ class VolumeTarget(base.IronicObject, exists with the same node ID and boot index values :raises: VolumeTargetNotFound if the volume target cannot be found """ - updates = self.obj_get_changes() + updates = self.do_version_changes_for_db() updated_target = self.dbapi.update_volume_target(self.uuid, updates) self._from_db_object(self._context, self, updated_target) diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index 4f3934c4b5..3774c6d725 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -93,6 +93,7 @@ def remove_internal(values, internal): def node_post_data(**kw): node = utils.get_test_node(**kw) # These values are not part of the API object + node.pop('version') node.pop('conductor_affinity') node.pop('chassis_id') node.pop('tags') @@ -113,9 +114,9 @@ def node_post_data(**kw): def port_post_data(**kw): port = utils.get_test_port(**kw) - # node_id is not part of the API object + # These values are not part of the API object + port.pop('version') port.pop('node_id') - # portgroup_id is not part of the API object port.pop('portgroup_id') # NOTE(mgoddard): Physical network is not yet supported by the REST API. port.pop('physical_network') @@ -125,6 +126,8 @@ def port_post_data(**kw): def chassis_post_data(**kw): chassis = utils.get_test_chassis(**kw) + # version is not part of the API object + chassis.pop('version') internal = chassis_controller.ChassisPatchType.internal_attrs() return remove_internal(chassis, internal) @@ -142,7 +145,8 @@ def portgroup_post_data(**kw): """Return a Portgroup object without internal attributes.""" portgroup = utils.get_test_portgroup(**kw) - # node_id is not a part of the API object + # These values are not part of the API object + portgroup.pop('version') portgroup.pop('node_id') # NOTE(jroll): pop out fields that were introduced in later API versions, diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py index 812e79bf95..bc0e076365 100644 --- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/unit/db/sqlalchemy/test_migrations.py @@ -641,6 +641,16 @@ class MigrationCheckersMixin(object): self.assertIsInstance(ports.c.physical_network.type, sqlalchemy.types.String) + def _check_868cb606a74a(self, engine, data): + for table in ['chassis', 'conductors', 'node_tags', 'nodes', + 'portgroups', 'ports', 'volume_connectors', + 'volume_targets', 'conductor_hardware_interfaces']: + table = db_utils.get_table(engine, table) + col_names = [column.name for column in table.c] + self.assertIn('version', col_names) + self.assertIsInstance(table.c.version.type, + sqlalchemy.types.String) + def test_upgrade_and_version(self): with patch_with_engine(self.engine): self.migration_api.upgrade('head') diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 55239641cc..75f6469a11 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -20,6 +20,13 @@ from oslo_utils import timeutils from ironic.common import states from ironic.db import api as db_api from ironic.drivers import base as drivers_base +from ironic.objects import chassis +from ironic.objects import conductor +from ironic.objects import node +from ironic.objects import port +from ironic.objects import portgroup +from ironic.objects import volume_connector +from ironic.objects import volume_target def get_test_ipmi_info(): @@ -181,6 +188,7 @@ def get_test_node(**kw): "private_state": "secret value" } result = { + 'version': kw.get('version', node.Node.VERSION), 'id': kw.get('id', 123), 'name': kw.get('name', None), 'uuid': kw.get('uuid', '1be26c0b-03f2-4d2e-ae87-c02d7f33c123'), @@ -247,6 +255,7 @@ def create_test_node(**kw): def get_test_port(**kw): return { 'id': kw.get('id', 987), + 'version': kw.get('version', port.Port.VERSION), 'uuid': kw.get('uuid', '1be26c0b-03f2-4d2e-ae87-c02d7f33c781'), 'node_id': kw.get('node_id', 123), 'address': kw.get('address', '52:54:00:cf:2d:31'), @@ -284,6 +293,7 @@ def create_test_port(**kw): def get_test_volume_connector(**kw): return { 'id': kw.get('id', 789), + 'version': kw.get('version', volume_connector.VolumeConnector.VERSION), 'uuid': kw.get('uuid', '1be26c0b-03f2-4d2e-ae87-c02d7f33c781'), 'node_id': kw.get('node_id', 123), 'type': kw.get('type', 'iqn'), @@ -316,6 +326,7 @@ def get_test_volume_target(**kw): fake_properties = {"target_iqn": "iqn.foo"} return { 'id': kw.get('id', 789), + 'version': kw.get('version', volume_target.VolumeTarget.VERSION), 'uuid': kw.get('uuid', '1be26c0b-03f2-4d2e-ae87-c02d7f33c781'), 'node_id': kw.get('node_id', 123), 'volume_type': kw.get('volume_type', 'iscsi'), @@ -348,6 +359,7 @@ def create_test_volume_target(**kw): def get_test_chassis(**kw): return { 'id': kw.get('id', 42), + 'version': kw.get('version', chassis.Chassis.VERSION), 'uuid': kw.get('uuid', 'e74c40e0-d825-11e2-a28f-0800200c9a66'), 'extra': kw.get('extra', {}), 'description': kw.get('description', 'data-center-1-chassis'), @@ -376,6 +388,7 @@ def create_test_chassis(**kw): def get_test_conductor(**kw): return { 'id': kw.get('id', 6), + 'version': kw.get('version', conductor.Conductor.VERSION), 'hostname': kw.get('hostname', 'test-conductor-node'), 'drivers': kw.get('drivers', ['fake-driver', 'null-driver']), 'created_at': kw.get('created_at', timeutils.utcnow()), @@ -430,6 +443,7 @@ def get_test_redfish_info(): def get_test_portgroup(**kw): return { 'id': kw.get('id', 654), + 'version': kw.get('version', portgroup.Portgroup.VERSION), 'uuid': kw.get('uuid', '6eb02b44-18a3-4659-8c0b-8d2802581ae4'), 'name': kw.get('name', 'fooname'), 'node_id': kw.get('node_id', 123), @@ -464,6 +478,9 @@ def create_test_portgroup(**kw): def get_test_node_tag(**kw): return { + # TODO(rloo): Replace None below with the object NodeTag VERSION, + # after this lands: https://review.openstack.org/#/c/233357 + 'version': kw.get('version', None), "tag": kw.get("tag", "tag1"), "node_id": kw.get("node_id", "123"), 'created_at': kw.get('created_at'), diff --git a/ironic/tests/unit/objects/test_chassis.py b/ironic/tests/unit/objects/test_chassis.py index e046786f1a..c03c9e3b9a 100644 --- a/ironic/tests/unit/objects/test_chassis.py +++ b/ironic/tests/unit/objects/test_chassis.py @@ -58,6 +58,17 @@ class TestChassisObject(base.DbTestCase, obj_utils.SchemasTestMixIn): self.assertRaises(exception.InvalidIdentity, objects.Chassis.get, self.context, 'not-a-uuid') + def test_create(self): + chassis = objects.Chassis(self.context, **self.fake_chassis) + with mock.patch.object(self.dbapi, 'create_chassis', + autospec=True) as mock_create_chassis: + mock_create_chassis.return_value = utils.get_test_chassis() + + chassis.create() + + args, _kwargs = mock_create_chassis.call_args + self.assertEqual(objects.Chassis.VERSION, args[0]['version']) + def test_save(self): uuid = self.fake_chassis['uuid'] extra = {"test": 123} @@ -75,7 +86,8 @@ class TestChassisObject(base.DbTestCase, obj_utils.SchemasTestMixIn): mock_get_chassis.assert_called_once_with(uuid) mock_update_chassis.assert_called_once_with( - uuid, {'extra': {"test": 123}}) + uuid, {'version': objects.Chassis.VERSION, + 'extra': {"test": 123}}) self.assertEqual(self.context, c._context) res_updated_at = (c.updated_at).replace(tzinfo=None) self.assertEqual(test_time, res_updated_at) diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py index 2ec0510546..5fd7b07f18 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -93,7 +93,8 @@ class TestNodeObject(base.DbTestCase, obj_utils.SchemasTestMixIn): mock_update_node.assert_called_once_with( uuid, {'properties': {"fake": "property"}, 'driver': 'fake-driver', - 'driver_internal_info': {}}) + 'driver_internal_info': {}, + 'version': objects.Node.VERSION}) self.assertEqual(self.context, n._context) res_updated_at = (n.updated_at).replace(tzinfo=None) self.assertEqual(test_time, res_updated_at) @@ -124,7 +125,8 @@ class TestNodeObject(base.DbTestCase, obj_utils.SchemasTestMixIn): uuid, {'properties': {"fake": "property"}, 'driver': 'fake-driver', 'driver_internal_info': {}, - 'extra': {'test': 123}}) + 'extra': {'test': 123}, + 'version': objects.Node.VERSION}) self.assertEqual(self.context, n._context) res_updated_at = n.updated_at.replace(tzinfo=None) self.assertEqual(test_time, res_updated_at) @@ -214,7 +216,14 @@ class TestNodeObject(base.DbTestCase, obj_utils.SchemasTestMixIn): def test_create(self): node = objects.Node(self.context, **self.fake_node) - node.create() + with mock.patch.object(self.dbapi, 'create_node', + autospec=True) as mock_create_node: + mock_create_node.return_value = utils.get_test_node() + + node.create() + + args, _kwargs = mock_create_node.call_args + self.assertEqual(objects.Node.VERSION, args[0]['version']) def test_create_with_invalid_properties(self): node = objects.Node(self.context, **self.fake_node) diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 27f23f2462..fc1eb3529e 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -17,6 +17,7 @@ import datetime import iso8601 import mock +from oslo_utils import timeutils from oslo_versionedobjects import base as object_base from oslo_versionedobjects import exception as object_exception from oslo_versionedobjects import fixture as object_fixture @@ -39,13 +40,11 @@ class MyObj(base.IronicObject, object_base.VersionedObjectDictCompat): 'missing': fields.StringField(), } - def obj_make_compatible(self, primitive, target_version): - super(MyObj, self).obj_make_compatible(primitive, target_version) - if target_version == '1.4' and 'missing' in primitive: - del primitive['missing'] - def obj_load_attr(self, attrname): - setattr(self, attrname, 'loaded!') + if attrname == 'version': + setattr(self, attrname, None) + else: + setattr(self, attrname, 'loaded!') @object_base.remotable_classmethod def query(cls, context): @@ -68,6 +67,7 @@ class MyObj(base.IronicObject, object_base.VersionedObjectDictCompat): @object_base.remotable def save(self, context=None): + self.do_version_changes_for_db() self.obj_reset_changes() @object_base.remotable @@ -82,6 +82,12 @@ class MyObj(base.IronicObject, object_base.VersionedObjectDictCompat): self.save() self.foo = 42 + def _convert_to_version(self, target_version): + if target_version == '1.5': + self.missing = 'foo' + elif self.missing: + self.missing = '' + class MyObj2(object): @classmethod @@ -336,16 +342,227 @@ class _TestObject(object): self.assertEqual(set(myobj_fields) | set(myobj3_fields), set(TestSubclassedObject.fields.keys())) - def test_get_changes(self): + def _test_get_changes(self, target_version='1.5'): obj = MyObj(self.context) + self.assertEqual('1.5', obj.VERSION) + self.assertEqual(target_version, obj.get_target_version()) self.assertEqual({}, obj.obj_get_changes()) obj.foo = 123 self.assertEqual({'foo': 123}, obj.obj_get_changes()) obj.bar = 'test' - self.assertEqual({'foo': 123, 'bar': 'test'}, obj.obj_get_changes()) + obj.missing = 'test' # field which is missing in v1.4 + self.assertEqual({'foo': 123, 'bar': 'test', 'missing': 'test'}, + obj.obj_get_changes()) obj.obj_reset_changes() self.assertEqual({}, obj.obj_get_changes()) + def test_get_changes(self): + self._test_get_changes() + + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def test_get_changes_pinned(self, mock_release_mapping): + # obj_get_changes() is not affected by pinning + CONF.set_override('pin_release_version', + release_mappings.RELEASE_VERSIONS[-1]) + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'MyObj': '1.4', + } + } + self._test_get_changes(target_version='1.4') + + def test_convert_to_version_same(self): + # missing is added + obj = MyObj(self.context) + self.assertEqual('1.5', obj.VERSION) + obj.convert_to_version('1.5') + self.assertEqual('1.5', obj.VERSION) + self.assertEqual(obj.__class__.VERSION, obj.VERSION) + self.assertEqual({'missing': 'foo'}, obj.obj_get_changes()) + + def test_convert_to_version_new(self): + obj = MyObj(self.context) + obj.VERSION = '1.4' + obj.convert_to_version('1.5') + self.assertEqual('1.5', obj.VERSION) + self.assertEqual(obj.__class__.VERSION, obj.VERSION) + self.assertEqual({'missing': 'foo'}, obj.obj_get_changes()) + + def test_convert_to_version_old(self): + obj = MyObj(self.context) + obj.missing = 'something' + obj.obj_reset_changes() + obj.convert_to_version('1.4') + self.assertEqual('1.4', obj.VERSION) + self.assertEqual({'missing': ''}, obj.obj_get_changes()) + + @mock.patch.object(MyObj, 'convert_to_version', autospec=True) + def test_do_version_changes_for_db(self, mock_convert): + # no object conversion + obj = MyObj(self.context) + self.assertEqual('1.5', obj.VERSION) + self.assertEqual('1.5', obj.get_target_version()) + self.assertEqual({}, obj.obj_get_changes()) + obj.foo = 123 + obj.bar = 'test' + obj.missing = 'test' # field which is missing in v1.4 + self.assertEqual({'foo': 123, 'bar': 'test', 'missing': 'test'}, + obj.obj_get_changes()) + changes = obj.do_version_changes_for_db() + self.assertEqual({'foo': 123, 'bar': 'test', 'missing': 'test', + 'version': '1.5'}, changes) + self.assertEqual('1.5', obj.VERSION) + self.assertFalse(mock_convert.called) + + @mock.patch.object(MyObj, 'convert_to_version', autospec=True) + @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) + def test_do_version_changes_for_db_pinned(self, mock_target_version, + mock_convert): + # obj is same version as pinned, no conversion done + mock_target_version.return_value = '1.4' + + obj = MyObj(self.context) + obj.VERSION = '1.4' + self.assertEqual('1.4', obj.get_target_version()) + obj.foo = 123 + obj.bar = 'test' + self.assertEqual({'foo': 123, 'bar': 'test'}, obj.obj_get_changes()) + self.assertEqual('1.4', obj.VERSION) + changes = obj.do_version_changes_for_db() + self.assertEqual({'foo': 123, 'bar': 'test', 'version': '1.4'}, + changes) + self.assertEqual('1.4', obj.VERSION) + mock_target_version.assert_called_with(obj) + self.assertFalse(mock_convert.called) + + @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) + def test_do_version_changes_for_db_downgrade(self, mock_target_version): + # obj is 1.5; convert to 1.4 + mock_target_version.return_value = '1.4' + + obj = MyObj(self.context) + obj.foo = 123 + obj.bar = 'test' + obj.missing = 'something' + self.assertEqual({'foo': 123, 'bar': 'test', 'missing': 'something'}, + obj.obj_get_changes()) + self.assertEqual('1.5', obj.VERSION) + changes = obj.do_version_changes_for_db() + self.assertEqual({'foo': 123, 'bar': 'test', 'missing': '', + 'version': '1.4'}, changes) + self.assertEqual('1.4', obj.VERSION) + mock_target_version.assert_called_with(obj) + + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def _test__from_db_object(self, version, mock_release_mapping): + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'MyObj': '1.4', + } + } + missing = '' + if version == '1.5': + missing = 'foo' + obj = MyObj(self.context) + dbobj = {'created_at': timeutils.utcnow(), + 'updated_at': timeutils.utcnow(), + 'version': version, + 'foo': 123, 'bar': 'test', 'missing': missing} + MyObj._from_db_object(self.context, obj, dbobj) + self.assertEqual(obj.__class__.VERSION, obj.VERSION) + self.assertEqual(123, obj.foo) + self.assertEqual('test', obj.bar) + self.assertEqual('foo', obj.missing) + self.assertFalse(mock_release_mapping.called) + + def test__from_db_object(self): + self._test__from_db_object('1.5') + + def test__from_db_object_old(self): + self._test__from_db_object('1.4') + + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def test__from_db_object_no_version(self, mock_release_mapping): + # DB doesn't have version; get it from mapping + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'MyObj': '1.4', + } + } + obj = MyObj(self.context) + dbobj = {'created_at': timeutils.utcnow(), + 'updated_at': timeutils.utcnow(), + 'version': None, + 'foo': 123, 'bar': 'test', 'missing': ''} + MyObj._from_db_object(self.context, obj, dbobj) + self.assertEqual(obj.__class__.VERSION, obj.VERSION) + self.assertEqual(123, obj.foo) + self.assertEqual('test', obj.bar) + self.assertEqual('foo', obj.missing) + + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def test__from_db_object_map_version_bad(self, mock_release_mapping): + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'MyObj': '1.6', + } + } + obj = MyObj(self.context) + dbobj = {'created_at': timeutils.utcnow(), + 'updated_at': timeutils.utcnow(), + 'version': None, + 'foo': 123, 'bar': 'test', 'missing': ''} + self.assertRaises(object_exception.IncompatibleObjectVersion, + MyObj._from_db_object, self.context, obj, dbobj) + + def test_get_target_version_no_pin(self): + obj = MyObj(self.context) + self.assertEqual('1.5', obj.get_target_version()) + + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def test_get_target_version_pinned(self, mock_release_mapping): + CONF.set_override('pin_release_version', + release_mappings.RELEASE_VERSIONS[-1]) + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'MyObj': '1.4', + } + } + obj = MyObj(self.context) + self.assertEqual('1.4', obj.get_target_version()) + + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def test_get_target_version_pinned_no_myobj(self, mock_release_mapping): + CONF.set_override('pin_release_version', + release_mappings.RELEASE_VERSIONS[-1]) + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'NotMyObj': '1.4', + } + } + obj = MyObj(self.context) + self.assertEqual('1.5', obj.get_target_version()) + + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def test_get_target_version_pinned_bad(self, mock_release_mapping): + CONF.set_override('pin_release_version', + release_mappings.RELEASE_VERSIONS[-1]) + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'MyObj': '1.6', + } + } + obj = MyObj(self.context) + self.assertRaises(object_exception.IncompatibleObjectVersion, + obj.get_target_version) + def test_obj_fields(self): @base.IronicObjectRegistry.register_if(False) class TestObj(base.IronicObject, @@ -485,8 +702,9 @@ class TestObjectSerializer(test_base.TestCase): mock_indirection_api, my_version='1.6'): ser = base.IronicObjectSerializer() + backported_obj = MyObj() mock_indirection_api.object_backport_versions.return_value \ - = 'backported' + = backported_obj @base.IronicObjectRegistry.register class MyTestObj(MyObj): @@ -500,7 +718,7 @@ class TestObjectSerializer(test_base.TestCase): self.assertFalse( mock_indirection_api.object_backport_versions.called) else: - self.assertEqual('backported', result) + self.assertEqual(backported_obj, result) versions = object_base.obj_tree_get_versions('MyTestObj') mock_indirection_api.object_backport_versions.assert_called_with( self.context, primitive, versions) @@ -525,9 +743,35 @@ class TestObjectSerializer(test_base.TestCase): "Test object with unsupported (newer) version and revision" self._test_deserialize_entity_newer('1.7', '1.6.1', my_version='1.6.1') - @mock.patch.object(MyObj, 'obj_make_compatible') - def test_serialize_entity_no_backport(self, make_compatible_mock): - """Test single element serializer with no backport.""" + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def test_deserialize_entity_pin_ignored(self, mock_release_mapping): + # Deserializing doesn't look at pinning + CONF.set_override('pin_release_version', + release_mappings.RELEASE_VERSIONS[-1]) + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'MyTestObj': '1.0', + } + } + ser = base.IronicObjectSerializer() + + @base.IronicObjectRegistry.register + class MyTestObj(MyObj): + VERSION = '1.1' + + obj = MyTestObj(self.context) + primitive = obj.obj_to_primitive() + result = ser.deserialize_entity(self.context, primitive) + self.assertEqual('1.1', result.VERSION) + self.assertEqual('1.0', result.get_target_version()) + self.assertFalse(mock_release_mapping.called) + + @mock.patch.object(base.IronicObject, 'convert_to_version', autospec=True) + @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) + def test_serialize_entity_unpinned(self, mock_version, mock_convert): + """Test single element serializer with no backport, unpinned.""" + mock_version.return_value = MyObj.VERSION serializer = base.IronicObjectSerializer() obj = MyObj(self.context) obj.foo = 1 @@ -541,70 +785,69 @@ class TestObjectSerializer(test_base.TestCase): self.assertEqual('textt', data['missing']) changes = primitive['ironic_object.changes'] self.assertEqual(set(['foo', 'bar', 'missing']), set(changes)) - make_compatible_mock.assert_not_called() + mock_version.assert_called_once_with(mock.ANY) + self.assertFalse(mock_convert.called) + + @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) + def test_serialize_entity_pinned(self, mock_version): + """Test single element serializer with backport to pinned version.""" + mock_version.return_value = '1.4' - @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING') - def test_serialize_entity_backport(self, mock_release_mapping): - """Test single element serializer with backport.""" - CONF.set_override('pin_release_version', - release_mappings.RELEASE_VERSIONS[-1]) - mock_release_mapping.__getitem__.return_value = { - 'objects': { - 'MyObj': '1.4', - } - } serializer = base.IronicObjectSerializer() obj = MyObj(self.context) obj.foo = 1 obj.bar = 'text' - obj.missing = 'textt' + obj.missing = 'miss' + self.assertEqual('1.5', obj.VERSION) primitive = serializer.serialize_entity(self.context, obj) self.assertEqual('1.4', primitive['ironic_object.version']) data = primitive['ironic_object.data'] self.assertEqual(1, data['foo']) self.assertEqual('text', data['bar']) - self.assertNotIn('missing', data) + self.assertEqual('', data['missing']) changes = primitive['ironic_object.changes'] - self.assertEqual(set(['foo', 'bar']), set(changes)) + self.assertEqual(set(['foo', 'bar', 'missing']), set(changes)) + mock_version.assert_called_once_with(mock.ANY) + + @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) + def test_serialize_entity_invalid_pin(self, mock_version): + mock_version.side_effect = object_exception.InvalidTargetVersion( + version='1.6') - @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING') - def test_serialize_entity_invalid_pin(self, mock_release_mapping): - CONF.set_override('pin_release_version', - release_mappings.RELEASE_VERSIONS[-1]) - mock_release_mapping.__getitem__.return_value = { - 'objects': { - 'MyObj': '1.6', - } - } serializer = base.IronicObjectSerializer() obj = MyObj(self.context) self.assertRaises(object_exception.InvalidTargetVersion, serializer.serialize_entity, self.context, obj) + mock_version.assert_called_once_with(mock.ANY) - @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING') - def test_serialize_entity_no_pin(self, mock_release_mapping): - CONF.set_override('pin_release_version', - release_mappings.RELEASE_VERSIONS[-1]) - mock_release_mapping.__getitem__.return_value = { - 'objects': {} - } - serializer = base.IronicObjectSerializer() + @mock.patch.object(base.IronicObject, 'convert_to_version', autospec=True) + def test__process_object(self, mock_convert): obj = MyObj(self.context) - primitive = serializer.serialize_entity(self.context, obj) - self.assertEqual('1.5', primitive['ironic_object.version']) - - @mock.patch('ironic.objects.base.IronicObject._get_target_version') - @mock.patch('ironic.objects.base.LOG.warning') - def test_serialize_entity_unknown_entity(self, mock_warn, mock_version): - class Foo(object): - fields = {'foobar': fields.IntegerField()} + obj.foo = 1 + obj.bar = 'text' + obj.missing = 'miss' + primitive = obj.obj_to_primitive() serializer = base.IronicObjectSerializer() - obj = Foo() - primitive = serializer.serialize_entity(self.context, obj) - self.assertEqual(obj, primitive) - self.assertTrue(mock_warn.called) - mock_version.assert_not_called() + obj2 = serializer._process_object(self.context, primitive) + self.assertEqual(obj.foo, obj2.foo) + self.assertEqual(obj.bar, obj2.bar) + self.assertEqual(obj.missing, obj2.missing) + self.assertEqual(obj.VERSION, obj2.VERSION) + self.assertFalse(mock_convert.called) + + @mock.patch.object(base.IronicObject, 'convert_to_version', autospec=True) + def test__process_object_convert(self, mock_convert): + obj = MyObj(self.context) + obj.foo = 1 + obj.bar = 'text' + obj.missing = '' + obj.VERSION = '1.4' + primitive = obj.obj_to_primitive() + + serializer = base.IronicObjectSerializer() + serializer._process_object(self.context, primitive) + mock_convert.assert_called_once_with(mock.ANY, '1.5') class TestRegistry(test_base.TestCase): diff --git a/ironic/tests/unit/objects/test_port.py b/ironic/tests/unit/objects/test_port.py index a4aaf21359..375efe78d0 100644 --- a/ironic/tests/unit/objects/test_port.py +++ b/ironic/tests/unit/objects/test_port.py @@ -68,6 +68,17 @@ class TestPortObject(base.DbTestCase, obj_utils.SchemasTestMixIn): self.assertRaises(exception.InvalidIdentity, objects.Port.get, self.context, 'not-a-uuid') + def test_create(self): + port = objects.Port(self.context, **self.fake_port) + with mock.patch.object(self.dbapi, 'create_port', + autospec=True) as mock_create_port: + mock_create_port.return_value = utils.get_test_port() + + port.create() + + args, _kwargs = mock_create_port.call_args + self.assertEqual(objects.Port.VERSION, args[0]['version']) + def test_save(self): uuid = self.fake_port['uuid'] address = "b2:54:00:cf:2d:40" @@ -85,7 +96,8 @@ class TestPortObject(base.DbTestCase, obj_utils.SchemasTestMixIn): mock_get_port.assert_called_once_with(uuid) mock_update_port.assert_called_once_with( - uuid, {'address': "b2:54:00:cf:2d:40"}) + uuid, {'version': objects.Port.VERSION, + 'address': "b2:54:00:cf:2d:40"}) self.assertEqual(self.context, p._context) res_updated_at = (p.updated_at).replace(tzinfo=None) self.assertEqual(test_time, res_updated_at) diff --git a/ironic/tests/unit/objects/test_portgroup.py b/ironic/tests/unit/objects/test_portgroup.py index dee46c24bf..05a1d2108d 100644 --- a/ironic/tests/unit/objects/test_portgroup.py +++ b/ironic/tests/unit/objects/test_portgroup.py @@ -77,6 +77,17 @@ class TestPortgroupObject(base.DbTestCase, obj_utils.SchemasTestMixIn): self.context, 'not:a_name_or_uuid') + def test_create(self): + portgroup = objects.Portgroup(self.context, **self.fake_portgroup) + with mock.patch.object(self.dbapi, 'create_portgroup', + autospec=True) as mock_create_portgroup: + mock_create_portgroup.return_value = utils.get_test_portgroup() + + portgroup.create() + + args, _kwargs = mock_create_portgroup.call_args + self.assertEqual(objects.Portgroup.VERSION, args[0]['version']) + def test_save(self): uuid = self.fake_portgroup['uuid'] address = "b2:54:00:cf:2d:40" @@ -95,7 +106,8 @@ class TestPortgroupObject(base.DbTestCase, obj_utils.SchemasTestMixIn): mock_get_portgroup.assert_called_once_with(uuid) mock_update_portgroup.assert_called_once_with( - uuid, {'address': "b2:54:00:cf:2d:40"}) + uuid, {'version': objects.Portgroup.VERSION, + 'address': "b2:54:00:cf:2d:40"}) self.assertEqual(self.context, p._context) res_updated_at = (p.updated_at).replace(tzinfo=None) self.assertEqual(test_time, res_updated_at) diff --git a/ironic/tests/unit/objects/test_volume_connector.py b/ironic/tests/unit/objects/test_volume_connector.py index 44eaca01f0..3e9a461659 100644 --- a/ironic/tests/unit/objects/test_volume_connector.py +++ b/ironic/tests/unit/objects/test_volume_connector.py @@ -154,7 +154,8 @@ class TestVolumeConnectorObject(base.DbTestCase): mock_get_volume_connector.assert_called_once_with(uuid) mock_update_connector.assert_called_once_with( uuid, - {'connector_id': connector_id}) + {'version': objects.VolumeConnector.VERSION, + 'connector_id': connector_id}) self.assertEqual(self.context, c._context) res_updated_at = (c.updated_at).replace(tzinfo=None) self.assertEqual(test_time, res_updated_at) diff --git a/ironic/tests/unit/objects/test_volume_target.py b/ironic/tests/unit/objects/test_volume_target.py index bf53e81c81..267ecf1028 100644 --- a/ironic/tests/unit/objects/test_volume_target.py +++ b/ironic/tests/unit/objects/test_volume_target.py @@ -164,9 +164,10 @@ class TestVolumeTargetObject(base.DbTestCase): target.save() mock_get_volume_target.assert_called_once_with(uuid) - mock_update_target.assert_called_once_with(uuid, - {'boot_index': - boot_index}) + mock_update_target.assert_called_once_with( + uuid, + {'version': objects.VolumeTarget.VERSION, + 'boot_index': boot_index}) self.assertEqual(self.context, target._context) res_updated_at = (target.updated_at).replace(tzinfo=None) self.assertEqual(test_time, res_updated_at)