Add version counter to Service object

This implements blueprint service-version-number. It adds a monotonic
version number to the service object and data model. Existing non-versioned
service objects are said to be version 0, and anything running this patch
or later is version 1 (or later, when changed).

This doesn't affect anything specific yet, but can be used later to drive
policy (such as online data migrations, RPC version pins, etc) after this
is merged.

Change-Id: I12f3be255e37965270f9dd183a8a77b0774ce929

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))

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):

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):

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)

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 = []

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
 

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,

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',

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'})