diff --git a/nova/db/sqlalchemy/migrate_repo/versions/299_service_version_number.py b/nova/db/sqlalchemy/migrate_repo/versions/299_service_version_number.py new file mode 100644 index 00000000000..f5f905f8aa6 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/299_service_version_number.py @@ -0,0 +1,24 @@ +# 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 Integer, Column, MetaData, Table + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + services = Table('services', meta, autoload=True) + shadow_services = Table('shadow_services', meta, autoload=True) + services.create_column(Column('version', Integer, default=0)) + shadow_services.create_column(Column('version', Integer, + default=0)) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 91e6ca65593..92758d1b5de 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -102,6 +102,7 @@ class Service(BASE, NovaBase): disabled_reason = Column(String(255)) last_seen_up = Column(DateTime, nullable=True) forced_down = Column(Boolean, default=False) + version = Column(Integer, default=0) class ComputeNode(BASE, NovaBase): diff --git a/nova/objects/service.py b/nova/objects/service.py index ff822bd8ca3..fed08484069 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -26,6 +26,36 @@ LOG = logging.getLogger(__name__) +# NOTE(danms): This is the global service version counter +SERVICE_VERSION = 1 + + +# NOTE(danms): This is our SERVICE_VERSION history. The idea is that any +# time we bump the version, we will put an entry here to record the change, +# along with any pertinent data. For things that we can programatically +# detect that need a bump, we put something in _collect_things() below to +# assemble a dict of things we can check. For example, we pretty much always +# want to consider the compute RPC API version a thing that requires a service +# bump so that we can drive version pins from it. We could include other +# service RPC versions at some point, minimum object versions, etc. +# +# The TestServiceVersion test will fail if the calculated set of +# things differs from the value in the last item of the list below, +# indicating that a version bump is needed. +# +# Also note that there are other reasons we may want to bump this, +# which will not be caught by the test. An example of this would be +# triggering (or disabling) an online data migration once all services +# in the cluster are at the same level. +SERVICE_VERSION_HISTORY = ( + # Version 0: Pre-history + None, + + # Version 1: Introduction of SERVICE_VERSION + {'compute_rpc': '4.4'}, +) + + # TODO(berrange): Remove NovaObjectDictCompat @base.NovaObjectRegistry.register class Service(base.NovaPersistentObject, base.NovaObject, @@ -46,7 +76,8 @@ class Service(base.NovaPersistentObject, base.NovaObject, # Version 1.13: Added last_seen_up # Version 1.14: Added forced_down # Version 1.15: ComputeNode version 1.12 - VERSION = '1.15' + # Version 1.16: Added version + VERSION = '1.16' fields = { 'id': fields.IntegerField(read_only=True), @@ -60,6 +91,7 @@ class Service(base.NovaPersistentObject, base.NovaObject, 'compute_node': fields.ObjectField('ComputeNode'), 'last_seen_up': fields.DateTimeField(nullable=True), 'forced_down': fields.BooleanField(), + 'version': fields.IntegerField(), } obj_relationships = { @@ -68,8 +100,28 @@ class Service(base.NovaPersistentObject, base.NovaObject, ('1.12', '1.11'), ('1.15', '1.12')], } + def __init__(self, *args, **kwargs): + # NOTE(danms): We're going against the rules here and overriding + # init. The reason is that we want to *ensure* that we're always + # setting the current service version on our objects, overriding + # whatever else might be set in the database, or otherwise (which + # is the normal reason not to override init). + # + # We also need to do this here so that it's set on the client side + # all the time, such that create() and save() operations will + # include the current service version. + if 'version' in kwargs: + raise exception.ObjectActionError( + action='init', + reason='Version field is immutable') + + super(Service, self).__init__(*args, **kwargs) + self.version = SERVICE_VERSION + def obj_make_compatible(self, primitive, target_version): _target_version = utils.convert_version_to_tuple(target_version) + if _target_version < (1, 16) and 'version' in primitive: + del primitive['version'] if _target_version < (1, 14) and 'forced_down' in primitive: del primitive['forced_down'] if _target_version < (1, 13) and 'last_seen_up' in primitive: @@ -104,6 +156,10 @@ def _from_db_object(context, service, db_service): if key == 'compute_node': # NOTE(sbauza); We want to only lazy-load compute_node continue + elif key == 'version': + # NOTE(danms): Special handling of the version field, since + # it is read_only and set in our init. + setattr(service, base.get_attrname(key), db_service[key]) else: service[key] = db_service[key] service._context = context @@ -181,6 +237,11 @@ def create(self): def save(self): updates = self.obj_get_changes() updates.pop('id', None) + if list(updates.keys()) == ['version']: + # NOTE(danms): Since we set/dirty version in init, don't + # do a save if that's all that has changed. This keeps the + # "save is a no-op if nothing has changed" behavior. + return db_service = db.service_update(self._context, self.id, updates) self._from_db_object(self._context, self, db_service) @@ -206,7 +267,8 @@ class ServiceList(base.ObjectListBase, base.NovaObject): # Version 1.11: Service version 1.13 # Version 1.12: Service version 1.14 # Version 1.13: Service version 1.15 - VERSION = '1.13' + # Version 1.14: Service version 1.16 + VERSION = '1.14' fields = { 'objects': fields.ListOfObjectsField('Service'), @@ -217,8 +279,8 @@ class ServiceList(base.ObjectListBase, base.NovaObject): ('1.3', '1.5'), ('1.4', '1.6'), ('1.5', '1.7'), ('1.6', '1.8'), ('1.7', '1.9'), ('1.8', '1.10'), ('1.9', '1.11'), ('1.10', '1.12'), ('1.11', '1.13'), - ('1.12', '1.14'), ('1.13', '1.15')], - } + ('1.12', '1.14'), ('1.13', '1.15'), ('1.14', '1.16')], + } @base.remotable_classmethod def get_by_topic(cls, context, topic): diff --git a/nova/tests/unit/api/openstack/compute/contrib/test_services.py b/nova/tests/unit/api/openstack/compute/contrib/test_services.py index 336fb451fc7..1b24c74bf03 100644 --- a/nova/tests/unit/api/openstack/compute/contrib/test_services.py +++ b/nova/tests/unit/api/openstack/compute/contrib/test_services.py @@ -894,6 +894,7 @@ def setUp(self): services_list = [] for service in fake_services_list: service = service.copy() + del service['version'] service_obj = objects.Service(**service) service_proxy = cells_utils.ServiceProxy(service_obj, 'cell1') services_list.append(service_proxy) diff --git a/nova/tests/unit/compute/test_host_api.py b/nova/tests/unit/compute/test_host_api.py index 98a7705ebde..555f20e0b35 100644 --- a/nova/tests/unit/compute/test_host_api.py +++ b/nova/tests/unit/compute/test_host_api.py @@ -366,13 +366,15 @@ def test_service_get_all_no_zones(self): self.assertEqual(services, result) def _test_service_get_all(self, fake_filters, **kwargs): + service_attrs = dict(test_service.fake_service) + del service_attrs['version'] services = [ cells_utils.ServiceProxy( - objects.Service(**dict(test_service.fake_service, id=1, + objects.Service(**dict(service_attrs, id=1, topic='compute', host='host1')), 'cell1'), cells_utils.ServiceProxy( - objects.Service(**dict(test_service.fake_service, id=2, + objects.Service(**dict(service_attrs, id=2, topic='compute', host='host2')), 'cell1')] exp_services = [] diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index 9a408b40e04..902974dcd03 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -294,7 +294,8 @@ def _create_service(self, host="fakehost", compute=None): 'deleted_at': None, 'deleted': False, 'last_seen_up': None, - 'forced_down': False + 'forced_down': False, + 'version': 0, } return service diff --git a/nova/tests/unit/db/test_migrations.py b/nova/tests/unit/db/test_migrations.py index 0733a0afc2c..2f0036c1184 100644 --- a/nova/tests/unit/db/test_migrations.py +++ b/nova/tests/unit/db/test_migrations.py @@ -788,6 +788,9 @@ def removed_column(element): if not removed_column(element) ] + def _check_299(self, engine, data): + self.assertColumnExists(engine, 'services', 'version') + class TestNovaMigrationsSQLite(NovaMigrationsCheckers, test_base.DbTestCase, diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index ae71dbf8051..e53b5e2c3f9 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1156,8 +1156,8 @@ def test_serialize_args(self): 'SecurityGroupList': '1.0-dc8bbea01ba09a2edb6e5233eae85cbc', 'SecurityGroupRule': '1.1-ae1da17b79970012e8536f88cb3c6b29', 'SecurityGroupRuleList': '1.1-674b323c9ccea02e93b1b40e7fd2091a', - 'Service': '1.15-1d5c9a16f47da93e82082c4fce31588a', - 'ServiceList': '1.13-b767102cba7cbed290e396114c3f86b3', + 'Service': '1.16-f1c6e82b5479f63e35970fe7625c3878', + 'ServiceList': '1.14-b767102cba7cbed290e396114c3f86b3', 'TaskLog': '1.0-78b0534366f29aa3eebb01860fbe18fe', 'TaskLogList': '1.0-cc8cce1af8a283b9d28b55fcd682e777', 'Tag': '1.1-8b8d7d5b48887651a0e01241672e2963', diff --git a/nova/tests/unit/objects/test_service.py b/nova/tests/unit/objects/test_service.py index 687b3578f20..2a508c8e91b 100644 --- a/nova/tests/unit/objects/test_service.py +++ b/nova/tests/unit/objects/test_service.py @@ -16,11 +16,13 @@ from oslo_utils import timeutils from oslo_versionedobjects import exception as ovo_exc +from nova.compute import manager as compute_manager from nova import db from nova import exception from nova import objects from nova.objects import aggregate from nova.objects import service +from nova import test from nova.tests.unit.objects import test_compute_node from nova.tests.unit.objects import test_objects @@ -39,6 +41,7 @@ 'disabled': False, 'disabled_reason': None, 'last_seen_up': None, + 'version': service.SERVICE_VERSION, } OPTIONAL = ['availability_zone', 'compute_node'] @@ -107,17 +110,20 @@ def test_get_by_args(self): def test_create(self): self.mox.StubOutWithMock(db, 'service_create') - db.service_create(self.context, {'host': 'fake-host'}).AndReturn( + db.service_create(self.context, {'host': 'fake-host', + 'version': 1}).AndReturn( fake_service) self.mox.ReplayAll() service_obj = service.Service(context=self.context) service_obj.host = 'fake-host' service_obj.create() self.assertEqual(fake_service['id'], service_obj.id) + self.assertEqual(service.SERVICE_VERSION, service_obj.version) def test_recreate_fails(self): self.mox.StubOutWithMock(db, 'service_create') - db.service_create(self.context, {'host': 'fake-host'}).AndReturn( + db.service_create(self.context, {'host': 'fake-host', + 'version': 1}).AndReturn( fake_service) self.mox.ReplayAll() service_obj = service.Service(context=self.context) @@ -127,13 +133,15 @@ def test_recreate_fails(self): def test_save(self): self.mox.StubOutWithMock(db, 'service_update') - db.service_update(self.context, 123, {'host': 'fake-host'}).AndReturn( + db.service_update(self.context, 123, {'host': 'fake-host', + 'version': 1}).AndReturn( fake_service) self.mox.ReplayAll() service_obj = service.Service(context=self.context) service_obj.id = 123 service_obj.host = 'fake-host' service_obj.save() + self.assertEqual(service.SERVICE_VERSION, service_obj.version) @mock.patch.object(db, 'service_create', return_value=fake_service) @@ -250,8 +258,10 @@ def test_obj_make_compatible_for_compute_node(self, get_all_by_host): @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host') def test_obj_make_compatible_with_juno_computes(self, get_all_by_host): + service_attrs = dict(fake_service) + del service_attrs['version'] service_obj = objects.Service( - context=self.context, **fake_service) + context=self.context, **service_attrs) service_obj.binary = 'nova-compute' fake_service_dict = fake_service.copy() fake_service_dict['binary'] = 'nova-compute' @@ -275,3 +285,58 @@ class TestServiceObject(test_objects._LocalTest, class TestRemoteServiceObject(test_objects._RemoteTest, _TestServiceObject): pass + + +class TestServiceVersion(test.TestCase): + def _collect_things(self): + data = { + 'compute_rpc': compute_manager.ComputeManager.target.version, + } + return data + + def test_version(self): + calculated = self._collect_things() + self.assertEqual( + len(service.SERVICE_VERSION_HISTORY), service.SERVICE_VERSION + 1, + 'Service version %i has no history. Please update ' + 'nova.objects.service.SERVICE_VERSION_HISTORY ' + 'and add %s to it' % (service.SERVICE_VERSION, repr(calculated))) + current = service.SERVICE_VERSION_HISTORY[service.SERVICE_VERSION] + self.assertEqual( + current, calculated, + 'Changes detected that require a SERVICE_VERSION change. Please ' + 'increment nova.objects.service.SERVICE_VERSION') + + def test_version_in_init(self): + self.assertRaises(exception.ObjectActionError, + objects.Service, + version=123) + + def test_version_set_on_init(self): + self.assertEqual(service.SERVICE_VERSION, + objects.Service().version) + + def test_version_loaded_from_db(self): + fake_version = fake_service['version'] + 1 + fake_different_service = dict(fake_service) + fake_different_service['version'] = fake_version + obj = objects.Service() + obj._from_db_object(mock.sentinel.context, obj, fake_different_service) + self.assertEqual(fake_version, obj.version) + + def test_save_noop_with_only_version(self): + o = objects.Service(context=mock.sentinel.context, + id=fake_service['id']) + o.obj_reset_changes(['id']) + self.assertEqual(set(['version']), o.obj_what_changed()) + with mock.patch('nova.db.service_update') as mock_update: + o.save() + self.assertFalse(mock_update.called) + o.host = 'foo' + with mock.patch('nova.db.service_update') as mock_update: + mock_update.return_value = fake_service + o.save() + mock_update.assert_called_once_with( + mock.sentinel.context, fake_service['id'], + {'version': service.SERVICE_VERSION, + 'host': 'foo'})