diff --git a/nova/db/sqlalchemy/migrate_repo/versions/359_add_service_uuid.py b/nova/db/sqlalchemy/migrate_repo/versions/359_add_service_uuid.py new file mode 100644 index 000000000000..c16a89544266 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/359_add_service_uuid.py @@ -0,0 +1,33 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +from sqlalchemy import Column +from sqlalchemy.engine.reflection import Inspector +from sqlalchemy import Index +from sqlalchemy import MetaData +from sqlalchemy import String +from sqlalchemy import Table + + +def upgrade(migrate_engine): + meta = MetaData(bind=migrate_engine) + for prefix in ('', 'shadow_'): + services = Table(prefix + 'services', meta, autoload=True) + if not hasattr(services.c, 'uuid'): + services.create_column(Column('uuid', String(36), nullable=True)) + + uuid_index_name = 'services_uuid_idx' + indexes = Inspector(migrate_engine).get_indexes('services') + if uuid_index_name not in (i['name'] for i in indexes): + services = Table('services', meta, autoload=True) + Index(uuid_index_name, services.c.uuid, unique=True).create() diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 776b2fdf1552..0e8894267dc6 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -81,10 +81,12 @@ class Service(BASE, NovaBase, models.SoftDeleteMixin): schema.UniqueConstraint("host", "topic", "deleted", name="uniq_services0host0topic0deleted"), schema.UniqueConstraint("host", "binary", "deleted", - name="uniq_services0host0binary0deleted") - ) + name="uniq_services0host0binary0deleted"), + Index('services_uuid_idx', 'uuid', unique=True), + ) id = Column(Integer, primary_key=True) + uuid = Column(String(36), nullable=True) host = Column(String(255)) # , ForeignKey('hosts.id')) binary = Column(String(255)) topic = Column(String(255)) diff --git a/nova/objects/service.py b/nova/objects/service.py index f863d6fb71b1..42088b7bfcea 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -13,6 +13,7 @@ # under the License. from oslo_log import log as logging +from oslo_utils import uuidutils from oslo_utils import versionutils from nova import availability_zones @@ -128,10 +129,12 @@ class Service(base.NovaPersistentObject, base.NovaObject, # Version 1.18: ComputeNode version 1.14 # Version 1.19: Added get_minimum_version() # Version 1.20: Added get_minimum_version_multi() - VERSION = '1.20' + # Version 1.21: Added uuid + VERSION = '1.21' fields = { 'id': fields.IntegerField(read_only=True), + 'uuid': fields.UUIDField(), 'host': fields.StringField(nullable=True), 'binary': fields.StringField(nullable=True), 'topic': fields.StringField(nullable=True), @@ -171,6 +174,8 @@ class Service(base.NovaPersistentObject, base.NovaObject, super(Service, self).obj_make_compatible_from_manifest( primitive, target_version, version_manifest) _target_version = versionutils.convert_version_to_tuple(target_version) + if _target_version < (1, 21) and 'uuid' in primitive: + del primitive['uuid'] if _target_version < (1, 16) and 'version' in primitive: del primitive['version'] if _target_version < (1, 14) and 'forced_down' in primitive: @@ -210,10 +215,23 @@ class Service(base.NovaPersistentObject, base.NovaObject, # 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]) + elif key == 'uuid' and not db_service.get(key): + # Leave uuid off the object if undefined in the database + # so that it will be generated below. + continue else: service[key] = db_service[key] + service._context = context service.obj_reset_changes() + + # TODO(dpeschman): Drop this once all services have uuids in database + if 'uuid' not in service: + service.uuid = uuidutils.generate_uuid() + LOG.debug('Generated UUID %(uuid)s for service %(id)i', + dict(uuid=service.uuid, id=service.id)) + service.save() + return service def obj_load_attr(self, attrname): @@ -311,6 +329,11 @@ class Service(base.NovaPersistentObject, base.NovaObject, reason='already created') self._check_minimum_version() updates = self.obj_get_changes() + + if 'uuid' not in updates: + updates['uuid'] = uuidutils.generate_uuid() + self.uuid = updates['uuid'] + db_service = db.service_create(self._context, updates) self._from_db_object(self._context, self, db_service) diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 70e70e07be48..2b1ac7128951 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -3471,6 +3471,7 @@ class ServiceTestCase(test.TestCase, ModelsObjectComparatorMixin): def _get_base_values(self): return { + 'uuid': None, 'host': 'fake_host', 'binary': 'fake_binary', 'topic': 'fake_topic', @@ -3514,6 +3515,7 @@ class ServiceTestCase(test.TestCase, ModelsObjectComparatorMixin): def test_service_update(self): service = self._create_service({}) new_values = { + 'uuid': uuidsentinel.service, 'host': 'fake_host1', 'binary': 'fake_binary1', 'topic': 'fake_topic1', diff --git a/nova/tests/unit/db/test_migrations.py b/nova/tests/unit/db/test_migrations.py index 8f910ee22f89..5580df3419bd 100644 --- a/nova/tests/unit/db/test_migrations.py +++ b/nova/tests/unit/db/test_migrations.py @@ -947,6 +947,11 @@ class NovaMigrationsCheckers(test_migrations.ModelsMigrationsSync, self.assertColumnExists(engine, 'block_device_mapping', 'attachment_id') + def _check_359(self, engine, data): + self.assertColumnExists(engine, 'services', 'uuid') + self.assertIndexMembers(engine, 'services', 'services_uuid_idx', + ['uuid']) + class TestNovaMigrationsSQLite(NovaMigrationsCheckers, test_base.DbTestCase, diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index 293d021c7b72..ddf0b4501d4c 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -102,6 +102,7 @@ class TestNotificationBase(test.NoDBTestCase): 'deleted_at': None, 'deleted': False, 'id': 123, + 'uuid': uuids.service, 'host': 'fake-host', 'binary': 'nova-fake', 'topic': 'fake-service-topic', diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py index 13f59e9eac5b..bbc4625dc4a4 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -176,7 +176,7 @@ class _TestInstanceObject(object): 'topic': 'fake-service-topic', 'report_count': 1, 'forced_down': False, 'disabled': False, 'disabled_reason': None, 'last_seen_up': None, - 'version': 1, + 'version': 1, 'uuid': uuids.service, } fake_instance = dict(self.fake_instance, services=[fake_service], diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 899c7cf30276..870e14289611 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1157,7 +1157,7 @@ object_data = { 'SecurityGroupList': '1.0-dc8bbea01ba09a2edb6e5233eae85cbc', 'SecurityGroupRule': '1.1-ae1da17b79970012e8536f88cb3c6b29', 'SecurityGroupRuleList': '1.2-0005c47fcd0fb78dd6d7fd32a1409f5b', - 'Service': '1.20-0f9c0bf701e68640b78638fd09e2cddc', + 'Service': '1.21-4ff88e4cd40f3b3ce923805f29d84ee0', 'ServiceList': '1.19-5325bce13eebcbf22edc9678285270cc', 'TaskLog': '1.0-78b0534366f29aa3eebb01860fbe18fe', 'TaskLogList': '1.0-cc8cce1af8a283b9d28b55fcd682e777', diff --git a/nova/tests/unit/objects/test_service.py b/nova/tests/unit/objects/test_service.py index 05ec5b2365cb..6632e5273a68 100644 --- a/nova/tests/unit/objects/test_service.py +++ b/nova/tests/unit/objects/test_service.py @@ -27,6 +27,7 @@ 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 +from nova.tests import uuidsentinel NOW = timeutils.utcnow().replace(microsecond=0) @@ -38,6 +39,7 @@ def _fake_service(**kwargs): 'deleted_at': None, 'deleted': False, 'id': 123, + 'uuid': uuidsentinel.service, 'host': 'fake-host', 'binary': 'nova-fake', 'topic': 'fake-service-topic', @@ -123,13 +125,25 @@ class _TestServiceObject(object): def test_create(self, mock_service_create): service_obj = service.Service(context=self.context) service_obj.host = 'fake-host' + service_obj.uuid = uuidsentinel.service2 service_obj.create() self.assertEqual(fake_service['id'], service_obj.id) self.assertEqual(service.SERVICE_VERSION, service_obj.version) mock_service_create.assert_called_once_with( self.context, {'host': 'fake-host', + 'uuid': uuidsentinel.service2, 'version': fake_service['version']}) + @mock.patch('nova.objects.service.uuidutils.generate_uuid', + return_value=uuidsentinel.service3) + @mock.patch.object(db, 'service_create', return_value=fake_service) + def test_create_without_uuid_generates_one( + self, mock_service_create, generate_uuid): + service_obj = service.Service(context=self.context) + service_obj.create() + create_args = mock_service_create.call_args[0][1] + self.assertEqual(generate_uuid.return_value, create_args['uuid']) + @mock.patch.object(db, 'service_create', return_value=fake_service) def test_recreate_fails(self, mock_service_create): service_obj = service.Service(context=self.context) @@ -392,6 +406,28 @@ class _TestServiceObject(object): binary='nova-compute', ).create) + @mock.patch('nova.objects.base.NovaObject' + '.obj_make_compatible_from_manifest', new=mock.Mock()) + def test_obj_make_compatible_from_manifest_strips_uuid(self): + s = service.Service() + primitive = {'uuid': uuidsentinel.service} + s.obj_make_compatible_from_manifest(primitive, '1.20', mock.Mock()) + self.assertNotIn('uuid', primitive) + + @mock.patch('nova.objects.service.uuidutils.generate_uuid', + return_value=uuidsentinel.service4) + def test_from_db_object_without_uuid_generates_one(self, generate_uuid): + values = _fake_service(uuid=None, id=None) + db_service = db.api.service_create(self.context, values) + + s = service.Service() + service.Service._from_db_object(self.context, s, db_service) + self.assertEqual(uuidsentinel.service4, s.uuid) + + # Check the DB too + db_service2 = db.api.service_get(self.context, s.id) + self.assertEqual(s.uuid, db_service2['uuid']) + class TestServiceObject(test_objects._LocalTest, _TestServiceObject): diff --git a/nova/tests/unit/test_availability_zones.py b/nova/tests/unit/test_availability_zones.py index 63831a4c2ee3..25b15fdc9d53 100644 --- a/nova/tests/unit/test_availability_zones.py +++ b/nova/tests/unit/test_availability_zones.py @@ -61,7 +61,7 @@ class AvailabilityZoneTestCases(test.TestCase): def _create_service_with_topic(self, topic, host, disabled=False): values = { - 'binary': 'bin', + 'binary': 'nova-bin', 'host': host, 'topic': topic, 'disabled': disabled,