From 9bc06783ecaff5eef58535904abc12ea15982329 Mon Sep 17 00:00:00 2001 From: Grzegorz Grasza Date: Tue, 6 Dec 2016 13:40:45 +0100 Subject: [PATCH] Add RPC and object version pinning To support rolling upgrades, capping of RPC communication and Ironic objects is required. Old RPC services and objects may still be running while an upgrade is in progress. This makes sure that these old services are called and all objects are used in a supported RPC and objects version. This patch adds the configuration option "pin_release_version". Setting this option caps (downgrades) the internal RPC communication to the specified version to enable communication with older services. When doing a rolling upgrade from version X to Y, set this to X. It defaults to using the newest (current) possible RPC behavior and object versions. Change-Id: Ie2342d4051f85392a8b10d39ebffc287da57bf2b Partial-Bug: #1526283 Co-Authored-By: Szymon Borkowski Co-Authored-By: Ruby Loo --- doc/source/dev/releasing.rst | 26 +++++ etc/ironic/ironic.conf.sample | 9 ++ ironic/common/release_mappings.py | 86 +++++++++++++++++ ironic/conductor/rpcapi.py | 7 +- ironic/conf/default.py | 10 ++ ironic/objects/base.py | 33 +++++++ .../unit/common/test_release_mappings.py | 95 +++++++++++++++++++ ironic/tests/unit/conductor/test_rpcapi.py | 17 ++++ ironic/tests/unit/objects/test_objects.py | 91 ++++++++++++++++++ 9 files changed, 372 insertions(+), 2 deletions(-) create mode 100644 ironic/common/release_mappings.py create mode 100644 ironic/tests/unit/common/test_release_mappings.py diff --git a/doc/source/dev/releasing.rst b/doc/source/dev/releasing.rst index 54c14654a..71ce39ade 100644 --- a/doc/source/dev/releasing.rst +++ b/doc/source/dev/releasing.rst @@ -47,11 +47,24 @@ Things to do before releasing TEMPEST_BAREMETAL_MAX_MICROVERSION in devstack/lib/ironic to make sure that unsupported API tempest tests are skipped on stable branches. +* To support rolling upgrades, add this new release version (and release name + if it is a named release) into ironic/common/release_mappings.py: + + * in RELEASE_MAPPING, make a copy of the 'master' entry, and rename the first + 'master' entry to the new semver release version. + * If this is a named release, add a RELEASE_MAPPING entry for the named + release. Its value should be the same as that of the latest semver one + (that you just added above). + * Regenerate the sample config file, so that the choices for the + ``[DEFAULT]/pin_release_version`` configuration are accurate. + .. _`standards`: http://docs.openstack.org/developer/ironic/dev/faq.html#know-if-a-release-note-is-needed-for-my-change Things to do after releasing ============================ +When a release is done that results in a stable branch +------------------------------------------------------ When a release is done that results in a stable branch for the project, the release automation will push a number of changes that need to be approved. @@ -82,8 +95,21 @@ Additionally, changes need to be made on master to: and `pbr documentation `_ for details. +For all releases +---------------- For all releases, whether or not it results in a stable branch: * update the specs repo to mark any specs completed in the release as implemented. + * remove any -2s on patches that were blocked until after the release. + + * to support rolling upgrades, make these changes in + ironic/common/release_mappings.py: + + * if the release was a named release, delete any entries from + RELEASE_MAPPING associated with the oldest named release. Since we + support upgrades between adjacent named releases, the master branch will + only support upgrades from the most recent named release to master. + * regenerate the sample config file, so that the choices for the + ``[DEFAULT]/pin_release_version`` configuration are accurate. diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 001ad5e4d..f71df190a 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -354,6 +354,15 @@ # value) #host = localhost +# Used for rolling upgrades. Setting this option downgrades +# 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 +# this to X. Defaults to using the newest possible RPC +# behavior. (string value) +# Allowed values: ocata, 7.0 +#pin_release_version = + # Path to the rootwrap configuration file to use for running # commands as root. (string value) #rootwrap_config = /etc/ironic/rootwrap.conf diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py new file mode 100644 index 000000000..063e14a4f --- /dev/null +++ b/ironic/common/release_mappings.py @@ -0,0 +1,86 @@ +# Copyright 2016 Intel Corp. +# +# 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. + +# NOTE(xek): This decides the version cap of RPC messages sent to conductor +# and objects during rolling upgrades, when [DEFAULT]/pin_release_version +# configuration is set. +# +# Remember to add a new entry for the new version that is shipping in a new +# release. +# +# We support a rolling upgrade between adjacent named releases, as well as +# between a release and master, so old, unsupported releases can be removed, +# together with the supporting code, which is typically found in an object's +# make_compatible methods and RPC client code. + +# NOTE(xek): The format of this dict is: +# { '': { +# 'rpc': '', +# 'objects': { +# '': '', +# } +# }, +# } +# The list should contain all objects which are persisted in the database and +# sent over RPC. Notifications/Payloads are not being included here since we +# don't need to pin them during rolling upgrades. +# +# There should always be a 'master' entry that reflects the objects in the +# master branch. +# +# Just before doing a release, copy the 'master' entry, and rename the first +# 'master' entry to the (semver) version being released. +# +# Just after doing a named release, delete any entries associated with the +# oldest named release. +RELEASE_MAPPING = { + '7.0': { + 'rpc': '1.40', + 'objects': { + 'Node': '1.21', + 'Conductor': '1.2', + 'Chassis': '1.3', + 'Port': '1.6', + 'Portgroup': '1.3', + 'VolumeConnector': '1.0', + 'VolumeTarget': '1.0', + } + }, + 'master': { + 'rpc': '1.40', + 'objects': { + 'Node': '1.21', + 'Conductor': '1.2', + 'Chassis': '1.3', + 'Port': '1.6', + 'Portgroup': '1.3', + 'VolumeConnector': '1.0', + 'VolumeTarget': '1.0', + } + }, +} + +# NOTE(xek): Assign each named release to the appropriate semver. +# +# Just before we do a new named release, add a mapping for the new +# named release. +# +# Just after we do a new named release, delete the oldest named +# release (that we are no longer supporting for a rolling upgrade). +# +# There should be at most two named mappings here. +RELEASE_MAPPING['ocata'] = RELEASE_MAPPING['7.0'] + +# List of available versions with named versions first; 'master' is excluded. +RELEASE_VERSIONS = sorted(set(RELEASE_MAPPING) - {'master'}, reverse=True) diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index d559b0ed6..af7c286a4 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -25,6 +25,7 @@ import oslo_messaging as messaging from ironic.common import exception from ironic.common import hash_ring from ironic.common.i18n import _ +from ironic.common import release_mappings as versions from ironic.common import rpc from ironic.conductor import manager from ironic.conf import CONF @@ -103,8 +104,10 @@ class ConductorAPI(object): target = messaging.Target(topic=self.topic, version='1.0') serializer = objects_base.IronicObjectSerializer() - self.client = rpc.get_client(target, - version_cap=self.RPC_API_VERSION, + release_ver = versions.RELEASE_MAPPING.get(CONF.pin_release_version) + version_cap = (release_ver['rpc'] if release_ver + else self.RPC_API_VERSION) + self.client = rpc.get_client(target, version_cap=version_cap, serializer=serializer) # NOTE(deva): this is going to be buggy self.ring_manager = hash_ring.HashRingManager() diff --git a/ironic/conf/default.py b/ironic/conf/default.py index 0861c1fe7..79d1079eb 100644 --- a/ironic/conf/default.py +++ b/ironic/conf/default.py @@ -25,6 +25,7 @@ from oslo_config import cfg from oslo_utils import netutils from ironic.common.i18n import _ +from ironic.common import release_mappings as versions _ENABLED_IFACE_HELP = _('Specify the list of {0} interfaces to load during ' @@ -261,6 +262,15 @@ service_opts = [ 'However, the node name must be valid within ' 'an AMQP key, and if using ZeroMQ, a valid ' 'hostname, FQDN, or IP address.')), + cfg.StrOpt('pin_release_version', + choices=versions.RELEASE_VERSIONS, + # TODO(xek): mutable=True, + help=_('Used for rolling upgrades. Setting this option ' + 'downgrades 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 this to X. Defaults to ' + 'using the newest possible RPC behavior.')), ] utils_opts = [ diff --git a/ironic/objects/base.py b/ironic/objects/base.py index 84aed63d7..a366623c6 100644 --- a/ironic/objects/base.py +++ b/ironic/objects/base.py @@ -14,12 +14,17 @@ """Ironic common internal object model""" +from oslo_log import log from oslo_utils import versionutils from oslo_versionedobjects import base as object_base +from ironic.common import release_mappings as versions +from ironic.conf import CONF from ironic import objects from ironic.objects import fields as object_fields +LOG = log.getLogger(__name__) + class IronicObjectRegistry(object_base.VersionedObjectRegistry): def registration_hook(self, cls, index): @@ -103,7 +108,35 @@ class IronicObject(object_base.VersionedObject): return [cls._from_db_object(cls(context), db_obj) for db_obj in db_objects] + def _get_target_version(self): + """Returns the target version for this object. + + If pinned, returns the version of this object corresponding to + the pin. Otherwise, returns this (latest) version of the object. + """ + 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 + class IronicObjectSerializer(object_base.VersionedObjectSerializer): # Base class to use for object hydration OBJ_BASE_CLASS = IronicObject + + 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, (int, str, bool, float, type)) and entity: + LOG.warning("Entity %s was not serialized.", str(entity)) + return entity diff --git a/ironic/tests/unit/common/test_release_mappings.py b/ironic/tests/unit/common/test_release_mappings.py new file mode 100644 index 000000000..74d8d5853 --- /dev/null +++ b/ironic/tests/unit/common/test_release_mappings.py @@ -0,0 +1,95 @@ +# Copyright 2016 Intel Corp. +# +# 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 oslo_utils import versionutils +import six + +from ironic.common.release_mappings import RELEASE_MAPPING +from ironic.conductor import rpcapi +from ironic.db.sqlalchemy import models +from ironic.objects import base as obj_base +from ironic.tests import base +from ironic import version + + +def _check_versions_compatibility(conf_version, actual_version): + """Checks the configured version against the actual version. + + Returns True if the configured version is <= the actual version; + otherwise returns False. + + param conf_version: configured version, a string with dots + param actual_version: actual version, a string with dots + """ + conf_cap = versionutils.convert_version_to_tuple(conf_version) + actual_cap = versionutils.convert_version_to_tuple(actual_version) + return conf_cap <= actual_cap + + +class ReleaseMappingsTestCase(base.TestCase): + """Tests whether the dict RELEASE_MAPPING is correct, valid and consistent. + + """ + def test_structure(self): + for value in RELEASE_MAPPING.values(): + self.assertTrue(isinstance(value, dict)) + self.assertEqual({'rpc', 'objects'}, set(value)) + self.assertTrue(isinstance(value['rpc'], six.string_types)) + self.assertTrue(isinstance(value['objects'], dict)) + for obj_value in value['objects'].values(): + self.assertTrue(isinstance(obj_value, six.string_types)) + tuple_ver = versionutils.convert_version_to_tuple(obj_value) + self.assertEqual(2, len(tuple_ver)) + + def test_object_names_are_registered(self): + registered_objects = set(obj_base.IronicObjectRegistry.obj_classes()) + for mapping in RELEASE_MAPPING.values(): + objects = set(mapping['objects']) + self.assertTrue(objects.issubset(registered_objects)) + + def test_contains_current_release_entry(self): + version_info = str(version.version_info) + # NOTE(sborkows): We only need first two values from version (like 5.1) + # and assume version_info is of the form 'x.y.z'. + current_release = version_info[:version_info.rfind('.')] + self.assertIn(current_release, RELEASE_MAPPING) + + def test_current_rpc_version(self): + self.assertEqual(rpcapi.ConductorAPI.RPC_API_VERSION, + RELEASE_MAPPING['master']['rpc']) + + def test_current_object_versions(self): + registered_objects = obj_base.IronicObjectRegistry.obj_classes() + for obj, objver in RELEASE_MAPPING['master']['objects'].items(): + self.assertEqual(registered_objects[obj][0].VERSION, objver) + + def test_contains_all_db_objects(self): + self.assertIn('master', RELEASE_MAPPING) + model_names = set((s.__name__ for s in models.Base.__subclasses__())) + exceptions = set(['NodeTag', 'ConductorHardwareInterfaces']) + # NOTE(xek): As a rule, all models which can be changed between + # releases or are sent through RPC should have their counterpart + # versioned objects. This means all, but very simple models. + model_names -= exceptions + object_names = set(RELEASE_MAPPING['master']['objects']) + self.assertEqual(model_names, object_names) + + def test_rpc_and_objects_versions_supported(self): + registered_objects = obj_base.IronicObjectRegistry.obj_classes() + for versions in RELEASE_MAPPING.values(): + self.assertTrue(_check_versions_compatibility( + versions['rpc'], rpcapi.ConductorAPI.RPC_API_VERSION)) + for obj_name, obj_ver in versions['objects'].items(): + self.assertTrue(_check_versions_compatibility( + obj_ver, registered_objects[obj_name][0].VERSION)) diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py index 1feec62d1..e31b71003 100644 --- a/ironic/tests/unit/conductor/test_rpcapi.py +++ b/ironic/tests/unit/conductor/test_rpcapi.py @@ -27,6 +27,7 @@ from oslo_messaging import _utils as messaging_utils from ironic.common import boot_devices from ironic.common import exception +from ironic.common import release_mappings from ironic.common import states from ironic.conductor import manager as conductor_manager from ironic.conductor import rpcapi as conductor_rpcapi @@ -45,6 +46,22 @@ class ConductorRPCAPITestCase(tests_base.TestCase): conductor_manager.ConductorManager.RPC_API_VERSION, conductor_rpcapi.ConductorAPI.RPC_API_VERSION) + @mock.patch('ironic.common.rpc.get_client') + def test_version_cap(self, mock_get_client): + conductor_rpcapi.ConductorAPI() + self.assertEqual(conductor_rpcapi.ConductorAPI.RPC_API_VERSION, + mock_get_client.call_args[1]['version_cap']) + + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING') + @mock.patch('ironic.common.rpc.get_client') + def test_version_capped(self, mock_get_client, mock_release_mapping): + CONF.set_override('pin_release_version', + release_mappings.RELEASE_VERSIONS[0], + enforce_type=True) + mock_release_mapping.get.return_value = {'rpc': '3'} + conductor_rpcapi.ConductorAPI() + self.assertEqual('3', mock_get_client.call_args[1]['version_cap']) + class RPCAPITestCase(base.DbTestCase): diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index d3400b343..58c0396b9 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -23,6 +23,8 @@ from oslo_versionedobjects import fixture as object_fixture import six from ironic.common import context +from ironic.common import release_mappings +from ironic.conf import CONF from ironic.objects import base from ironic.objects import fields from ironic.tests import base as test_base @@ -37,6 +39,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!') @@ -518,6 +525,90 @@ 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.""" + serializer = base.IronicObjectSerializer() + obj = MyObj(self.context) + obj.foo = 1 + obj.bar = 'text' + obj.missing = 'textt' + primitive = serializer.serialize_entity(self.context, obj) + self.assertEqual('1.5', primitive['ironic_object.version']) + data = primitive['ironic_object.data'] + self.assertEqual(1, data['foo']) + self.assertEqual('text', data['bar']) + 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.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], + enforce_type=True) + 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' + 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) + changes = primitive['ironic_object.changes'] + self.assertEqual(set(['foo', 'bar']), set(changes)) + + @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], + enforce_type=True) + 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.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], + enforce_type=True) + mock_release_mapping.__getitem__.return_value = { + 'objects': {} + } + serializer = base.IronicObjectSerializer() + 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()} + + 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() + class TestRegistry(test_base.TestCase): @mock.patch('ironic.objects.base.objects')