From 950e693697e78a1af637ab6f8cc6c30afd5ba011 Mon Sep 17 00:00:00 2001 From: John Griffith Date: Tue, 24 Oct 2017 16:50:24 -0600 Subject: [PATCH] Make service object UUID not nullable Fix the UUID entry for the newly added service attribute and udpate the unit tests appropriately. This makes the UUID entry in the service object not nullable and fixes up the unit tests to work properly. Also introduces a unit test specifically for the online migration api in the db. Closes-Bug: #1727091 Change-Id: I17d3a873cfc8f056c2d31f6c8710489785998d3c --- cinder/db/sqlalchemy/api.py | 10 +-- cinder/objects/base.py | 1 + cinder/objects/service.py | 9 ++- .../unit/api/contrib/test_admin_actions.py | 3 +- cinder/tests/unit/api/contrib/test_backups.py | 75 ++++++++++++------- .../unit/api/contrib/test_capabilities.py | 8 +- cinder/tests/unit/api/contrib/test_hosts.py | 31 +++++--- .../tests/unit/api/contrib/test_services.py | 21 ++++-- .../unit/api/contrib/test_volume_manage.py | 6 +- cinder/tests/unit/api/v2/fakes.py | 6 +- cinder/tests/unit/objects/test_objects.py | 8 +- cinder/tests/unit/scheduler/fakes.py | 30 +++++--- .../tests/unit/scheduler/test_host_manager.py | 45 +++++++---- cinder/tests/unit/test_cmd.py | 28 ++++--- cinder/tests/unit/test_db_api.py | 48 ++++++++++++ cinder/tests/unit/test_service.py | 3 +- .../unit/volume/test_availability_zone.py | 17 +++-- cinder/tests/unit/volume/test_volume.py | 4 +- 18 files changed, 250 insertions(+), 103 deletions(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index e1fdbcfb3a7..63caa4ac46d 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -568,9 +568,6 @@ def service_get_by_uuid(context, service_uuid): @require_admin_context def service_create(context, values): - if not values.get('uuid'): - values['uuid'] = str(uuid.uuid4()) - service_ref = models.Service() service_ref.update(values) if not CONF.enable_new_services: @@ -600,14 +597,11 @@ def service_update(context, service_id, values): def service_uuids_online_data_migration(context, max_count): from cinder.objects import service - total = 0 updated = 0 - + total = model_query(context, models.Service).filter_by(uuid=None).count() db_services = model_query(context, models.Service).filter_by( - uuid=None).limit(max_count) + uuid=None).limit(max_count).all() for db_service in db_services: - total += 1 - # The conversion in the Service object code # will generate a UUID and save it for us. service_obj = service.Service._from_db_object( diff --git a/cinder/objects/base.py b/cinder/objects/base.py index c600c9dfc65..1fc172f6c6b 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -136,6 +136,7 @@ OBJ_VERSIONS.add('1.25', {'Group': '1.2'}) OBJ_VERSIONS.add('1.26', {'Snapshot': '1.5'}) OBJ_VERSIONS.add('1.27', {'Backup': '1.5', 'BackupImport': '1.5'}) OBJ_VERSIONS.add('1.28', {'Service': '1.5'}) +OBJ_VERSIONS.add('1.29', {'Service': '1.6'}) class CinderObjectRegistry(base.VersionedObjectRegistry): diff --git a/cinder/objects/service.py b/cinder/objects/service.py index 9182cef8f26..90a6a96bfb4 100644 --- a/cinder/objects/service.py +++ b/cinder/objects/service.py @@ -39,7 +39,8 @@ class Service(base.CinderPersistentObject, base.CinderObject, # Version 1.3: Add replication fields # Version 1.4: Add cluster fields # Version 1.5: Add UUID field - VERSION = '1.5' + # Version 1.6: Modify UUID field to be not nullable + VERSION = '1.6' OPTIONAL_FIELDS = ('cluster',) @@ -66,7 +67,7 @@ class Service(base.CinderPersistentObject, base.CinderObject, 'frozen': fields.BooleanField(default=False), 'active_backend_id': fields.StringField(nullable=True), - 'uuid': fields.StringField(nullable=True), + 'uuid': fields.StringField(), } def obj_make_compatible(self, primitive, target_version): @@ -86,8 +87,10 @@ class Service(base.CinderPersistentObject, base.CinderObject, def _from_db_object(context, service, db_service, expected_attrs=None): expected_attrs = expected_attrs or [] for name, field in service.fields.items(): - if name in Service.OPTIONAL_FIELDS: + if ((name == 'uuid' and not db_service.get(name)) or + name in service.OPTIONAL_FIELDS): continue + value = db_service.get(name) if isinstance(field, fields.IntegerField): value = value or 0 diff --git a/cinder/tests/unit/api/contrib/test_admin_actions.py b/cinder/tests/unit/api/contrib/test_admin_actions.py index 9363a5cd420..22d13f5270b 100644 --- a/cinder/tests/unit/api/contrib/test_admin_actions.py +++ b/cinder/tests/unit/api/contrib/test_admin_actions.py @@ -770,7 +770,8 @@ class AdminActionsTest(BaseAdminTest): mock_service_get_all): mock_service_get_all.return_value = [ {'availability_zone': "az1", 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] # admin context mock_check_support.return_value = True # current status is dependent on argument: test_status. diff --git a/cinder/tests/unit/api/contrib/test_backups.py b/cinder/tests/unit/api/contrib/test_backups.py index 69bd7d3f638..6b070699bd2 100644 --- a/cinder/tests/unit/api/contrib/test_backups.py +++ b/cinder/tests/unit/api/contrib/test_backups.py @@ -488,7 +488,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=5) @@ -524,7 +525,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=1) # Create a backup with metadata @@ -564,7 +566,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=5, status='in-use') @@ -595,7 +598,8 @@ class BackupsAPITestCase(test.TestCase): def test_create_backup_inuse_force(self, _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=5, status='in-use') backup = utils.create_backup(self.context, volume.id, @@ -635,7 +639,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=5, status='available') @@ -770,7 +775,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=5) snapshot = None @@ -819,7 +825,8 @@ class BackupsAPITestCase(test.TestCase): self, _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=5) @@ -973,7 +980,8 @@ class BackupsAPITestCase(test.TestCase): self, _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=5, status='available') @@ -1009,22 +1017,27 @@ class BackupsAPITestCase(test.TestCase): empty_service = [] # service host not match with volume's host host_not_match = [{'availability_zone': 'fake_az', 'host': alt_host, - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] # service az not match with volume's az az_not_match = [{'availability_zone': 'strange_az', 'host': testhost, - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}] # service disabled disabled_service = [] # dead service that last reported at 20th century dead_service = [{'availability_zone': 'fake_az', 'host': alt_host, - 'disabled': 0, 'updated_at': '1989-04-16 02:55:44'}] + 'disabled': 0, 'updated_at': '1989-04-16 02:55:44', + 'uuid': '6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'}] # first service's host not match but second one works. multi_services = [{'availability_zone': 'fake_az', 'host': alt_host, - 'disabled': 0, 'updated_at': timeutils.utcnow()}, + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': '18417850-2ca9-43d1-9619-ae16bfb0f655'}, {'availability_zone': 'fake_az', 'host': testhost, - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'f838f35c-4035-464f-9792-ce60e390c13d'}] # Setup mock to run through the following service cases _mock_service_get_all.side_effect = [empty_service, @@ -1075,11 +1088,14 @@ class BackupsAPITestCase(test.TestCase): def test_get_available_backup_service(self, _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost1', - 'disabled': 0, 'updated_at': timeutils.utcnow()}, + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}, {'availability_zone': 'az2', 'host': 'testhost2', - 'disabled': 0, 'updated_at': timeutils.utcnow()}, + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}, {'availability_zone': 'az2', 'host': 'testhost3', - 'disabled': 0, 'updated_at': timeutils.utcnow()}, ] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': '6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'}, ] actual_host = self.backup_api._get_available_backup_service_host( None, 'az1') self.assertEqual('testhost1', actual_host) @@ -1095,9 +1111,11 @@ class BackupsAPITestCase(test.TestCase): self, _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost1', - 'disabled': 0, 'updated_at': timeutils.utcnow()}, + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}, {'availability_zone': 'az2', 'host': 'testhost2', - 'disabled': 0, 'updated_at': timeutils.utcnow()}, ] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}, ] self.override_config('backup_use_same_host', True) actual_host = self.backup_api._get_available_backup_service_host( None, 'az1') @@ -1113,7 +1131,8 @@ class BackupsAPITestCase(test.TestCase): def test_delete_backup_available(self, _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] backup = utils.create_backup(self.context, status=fields.BackupStatus.AVAILABLE, availability_zone='az1', host='testhost') @@ -1136,7 +1155,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] backup = utils.create_backup(self.context, status=fields.BackupStatus.AVAILABLE, availability_zone='az1', host='testhost') @@ -1164,7 +1184,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] backup = utils.create_backup(self.context, status=fields.BackupStatus.ERROR, availability_zone='az1', host='testhost') @@ -1222,7 +1243,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] volume = utils.create_volume(self.context, size=5) backup = utils.create_backup(self.context, volume.id, status=fields.BackupStatus.AVAILABLE) @@ -1253,7 +1275,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all): _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', - 'disabled': 0, 'updated_at': '1775-04-19 05:00:00'}] + 'disabled': 0, 'updated_at': '1775-04-19 05:00:00', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] backup = utils.create_backup(self.context, status='available') req = webob.Request.blank('/v2/%s/backups/%s' % ( fake.PROJECT_ID, backup.id)) @@ -1351,7 +1374,8 @@ class BackupsAPITestCase(test.TestCase): _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] _mock_volume_api_create.side_effect = fake_volume_api_create backup = utils.create_backup(self.context, size=5, @@ -1386,7 +1410,8 @@ class BackupsAPITestCase(test.TestCase): _mock_volume_api_create.side_effect = fake_volume_api_create _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', - 'disabled': 0, 'updated_at': timeutils.utcnow()}] + 'disabled': 0, 'updated_at': timeutils.utcnow(), + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] backup = utils.create_backup(self.context, size=5, status=fields.BackupStatus.AVAILABLE, diff --git a/cinder/tests/unit/api/contrib/test_capabilities.py b/cinder/tests/unit/api/contrib/test_capabilities.py index 3207c6a5fa2..4c92598afdd 100644 --- a/cinder/tests/unit/api/contrib/test_capabilities.py +++ b/cinder/tests/unit/api/contrib/test_capabilities.py @@ -70,7 +70,9 @@ class CapabilitiesAPITest(test.TestCase): @mock.patch('cinder.volume.rpcapi.VolumeAPI.get_capabilities', rpcapi_get_capabilities) def test_capabilities_summary(self, mock_services): - mock_services.return_value = [{'name': 'fake', 'host': 'fake_host'}] + mock_services.return_value = [ + {'name': 'fake', 'host': 'fake_host', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] req = fakes.HTTPRequest.blank('/fake/capabilities/fake') req.environ['cinder.context'] = self.ctxt res = self.controller.show(req, 'fake') @@ -113,7 +115,9 @@ class CapabilitiesAPITest(test.TestCase): @mock.patch('cinder.volume.rpcapi.VolumeAPI.get_capabilities') def test_get_capabilities_rpc_timeout(self, mock_rpc, mock_services): mock_rpc.side_effect = oslo_messaging.MessagingTimeout - mock_services.return_value = [{'name': 'fake'}] + mock_services.return_value = [ + {'name': 'fake', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] req = fakes.HTTPRequest.blank('/fake/capabilities/fake') req.environ['cinder.context'] = self.ctxt diff --git a/cinder/tests/unit/api/contrib/test_hosts.py b/cinder/tests/unit/api/contrib/test_hosts.py index dbf40ebd731..ae8db60ea73 100644 --- a/cinder/tests/unit/api/contrib/test_hosts.py +++ b/cinder/tests/unit/api/contrib/test_hosts.py @@ -31,37 +31,46 @@ curr_time = datetime.datetime(2013, 7, 3, 0, 0, 1) SERVICE_LIST = [ {'created_at': created_time, 'updated_at': curr_time, 'host': 'test.host.1', 'topic': 'cinder-volume', 'disabled': 0, - 'availability_zone': 'cinder'}, + 'availability_zone': 'cinder', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}, {'created_at': created_time, 'updated_at': curr_time, 'host': 'test.host.1', 'topic': 'cinder-volume', 'disabled': 0, - 'availability_zone': 'cinder'}, + 'availability_zone': 'cinder', + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}, {'created_at': created_time, 'updated_at': curr_time, 'host': 'test.host.1', 'topic': 'cinder-volume', 'disabled': 0, - 'availability_zone': 'cinder'}, + 'availability_zone': 'cinder', + 'uuid': '6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'}, {'created_at': created_time, 'updated_at': curr_time, 'host': 'test.host.1', 'topic': 'cinder-volume', 'disabled': 0, - 'availability_zone': 'cinder'}, + 'availability_zone': 'cinder', + 'uuid': '18417850-2ca9-43d1-9619-ae16bfb0f655'}, {'created_at': created_time, 'updated_at': None, 'host': 'test.host.1', 'topic': 'cinder-volume', 'disabled': 0, - 'availability_zone': 'cinder'}, + 'availability_zone': 'cinder', + 'uuid': 'f838f35c-4035-464f-9792-ce60e390c13d'}, ] LIST_RESPONSE = [{'service-status': 'available', 'service': 'cinder-volume', 'zone': 'cinder', 'service-state': 'enabled', - 'host_name': 'test.host.1', 'last-update': curr_time}, + 'host_name': 'test.host.1', 'last-update': curr_time, + }, {'service-status': 'available', 'service': 'cinder-volume', 'zone': 'cinder', 'service-state': 'enabled', - 'host_name': 'test.host.1', 'last-update': curr_time}, + 'host_name': 'test.host.1', 'last-update': curr_time, + }, {'service-status': 'available', 'service': 'cinder-volume', 'zone': 'cinder', 'service-state': 'enabled', - 'host_name': 'test.host.1', 'last-update': curr_time}, + 'host_name': 'test.host.1', 'last-update': curr_time, + }, {'service-status': 'available', 'service': 'cinder-volume', 'zone': 'cinder', 'service-state': 'enabled', - 'host_name': 'test.host.1', 'last-update': curr_time}, + 'host_name': 'test.host.1', 'last-update': curr_time, + }, {'service-status': 'unavailable', 'service': 'cinder-volume', 'zone': 'cinder', 'service-state': 'enabled', - 'host_name': 'test.host.1', 'last-update': None}, - ] + 'host_name': 'test.host.1', 'last-update': None, + }, ] def stub_utcnow(with_timezone=False): diff --git a/cinder/tests/unit/api/contrib/test_services.py b/cinder/tests/unit/api/contrib/test_services.py index 7a92683f7d5..f33d671c657 100644 --- a/cinder/tests/unit/api/contrib/test_services.py +++ b/cinder/tests/unit/api/contrib/test_services.py @@ -47,7 +47,8 @@ fake_services_list = [ 'updated_at': datetime.datetime(2012, 10, 29, 13, 42, 2), 'created_at': datetime.datetime(2012, 9, 18, 2, 46, 27), 'disabled_reason': 'test1', - 'modified_at': ''}, + 'modified_at': '', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}, {'binary': 'cinder-volume', 'host': 'host1', 'cluster_name': None, @@ -57,7 +58,8 @@ fake_services_list = [ 'updated_at': datetime.datetime(2012, 10, 29, 13, 42, 5), 'created_at': datetime.datetime(2012, 9, 18, 2, 46, 27), 'disabled_reason': 'test2', - 'modified_at': ''}, + 'modified_at': '', + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}, {'binary': 'cinder-scheduler', 'host': 'host2', 'cluster_name': 'cluster1', @@ -67,7 +69,8 @@ fake_services_list = [ 'updated_at': datetime.datetime(2012, 9, 19, 6, 55, 34), 'created_at': datetime.datetime(2012, 9, 18, 2, 46, 28), 'disabled_reason': '', - 'modified_at': ''}, + 'modified_at': '', + 'uuid': '6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'}, {'binary': 'cinder-volume', 'host': 'host2', 'cluster_name': 'cluster1', @@ -77,7 +80,8 @@ fake_services_list = [ 'updated_at': datetime.datetime(2012, 9, 18, 8, 3, 38), 'created_at': datetime.datetime(2012, 9, 18, 2, 46, 28), 'disabled_reason': 'test4', - 'modified_at': ''}, + 'modified_at': '', + 'uuid': '18417850-2ca9-43d1-9619-ae16bfb0f655'}, {'binary': 'cinder-volume', 'host': 'host2', 'cluster_name': 'cluster2', @@ -87,7 +91,8 @@ fake_services_list = [ 'updated_at': datetime.datetime(2012, 9, 18, 8, 3, 38), 'created_at': datetime.datetime(2012, 9, 18, 2, 46, 28), 'disabled_reason': 'test5', - 'modified_at': datetime.datetime(2012, 10, 29, 13, 42, 5)}, + 'modified_at': datetime.datetime(2012, 10, 29, 13, 42, 5), + 'uuid': 'f838f35c-4035-464f-9792-ce60e390c13d'}, {'binary': 'cinder-volume', 'host': 'host2', 'cluster_name': 'cluster2', @@ -97,7 +102,8 @@ fake_services_list = [ 'updated_at': datetime.datetime(2012, 9, 18, 8, 3, 38), 'created_at': datetime.datetime(2012, 9, 18, 2, 46, 28), 'disabled_reason': '', - 'modified_at': datetime.datetime(2012, 9, 18, 8, 1, 38)}, + 'modified_at': datetime.datetime(2012, 9, 18, 8, 1, 38), + 'uuid': 'f2825a00-cc2f-493d-9635-003e01db8b3d'}, {'binary': 'cinder-scheduler', 'host': 'host2', 'cluster_name': None, @@ -107,7 +113,8 @@ fake_services_list = [ 'updated_at': None, 'created_at': datetime.datetime(2012, 9, 18, 2, 46, 28), 'disabled_reason': '', - 'modified_at': None}, + 'modified_at': None, + 'uuid': '35fcf841-1974-4944-a798-1fb6d0a44972'}, ] diff --git a/cinder/tests/unit/api/contrib/test_volume_manage.py b/cinder/tests/unit/api/contrib/test_volume_manage.py index 34d07f591de..8588766e0e0 100644 --- a/cinder/tests/unit/api/contrib/test_volume_manage.py +++ b/cinder/tests/unit/api/contrib/test_volume_manage.py @@ -62,9 +62,11 @@ def service_get(context, service_id, backend_match_level=None, host=None, check for existence of a host, so the content returned doesn't matter. """ if host == 'host_ok': - return {'disabled': False} + return {'disabled': False, + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'} if host == 'host_disabled': - return {'disabled': True} + return {'disabled': True, + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'} raise exception.ServiceNotFound(service_id=host) # Some of the tests check that volume types are correctly validated during a diff --git a/cinder/tests/unit/api/v2/fakes.py b/cinder/tests/unit/api/v2/fakes.py index 6f1b66d15a1..1229118b26a 100644 --- a/cinder/tests/unit/api/v2/fakes.py +++ b/cinder/tests/unit/api/v2/fakes.py @@ -223,11 +223,13 @@ def fake_snapshot_update(self, context, *args, **param): def fake_service_get_all(*args, **kwargs): - return [{'availability_zone': "zone1:host1", "disabled": 0}] + return [{'availability_zone': "zone1:host1", "disabled": 0, + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] def fake_service_get_all_by_topic(context, topic, disabled=None): - return [{'availability_zone': "zone1:host1", "disabled": 0}] + return [{'availability_zone': "zone1:host1", "disabled": 0, + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}] def fake_snapshot_get(self, context, snapshot_id): diff --git a/cinder/tests/unit/objects/test_objects.py b/cinder/tests/unit/objects/test_objects.py index 29c78b54d6c..29f90c0ca57 100644 --- a/cinder/tests/unit/objects/test_objects.py +++ b/cinder/tests/unit/objects/test_objects.py @@ -43,7 +43,7 @@ object_data = { 'QualityOfServiceSpecs': '1.0-0b212e0a86ee99092229874e03207fe8', 'QualityOfServiceSpecsList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'RequestSpec': '1.1-b0bd1a28d191d75648901fa853e8a733', - 'Service': '1.5-dfa465098dd9017bad825cab55eacc93', + 'Service': '1.6-e881b6b324151dd861e09cdfffcdaccd', 'ServiceList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'Snapshot': '1.5-ac1cdbd5b89588f6a8f44afdf6b8b201', 'SnapshotList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', @@ -101,6 +101,12 @@ class TestObjectVersions(test.TestCase): # db model and object match. def _check_table_matched(db_model, cls): for column in db_model.__table__.columns: + + # NOTE(jdg): Model and Object don't match intentionally here + if (column.name in cls.fields and + (column.name == 'uuid' and name == 'Service')): + continue + # NOTE(xyang): Skip the comparison of the colume name # group_type_id in table Group because group_type_id # is in the object Group but it is stored in a different diff --git a/cinder/tests/unit/scheduler/fakes.py b/cinder/tests/unit/scheduler/fakes.py index 532815c00b2..47d6179817a 100644 --- a/cinder/tests/unit/scheduler/fakes.py +++ b/cinder/tests/unit/scheduler/fakes.py @@ -47,7 +47,8 @@ class FakeHostManager(host_manager.HostManager): 'reserved_percentage': 10, 'volume_backend_name': 'lvm1', 'timestamp': UTC_NOW, - 'multiattach': True}, + 'multiattach': True, + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}, 'host2': {'total_capacity_gb': 2048, 'free_capacity_gb': 300, 'allocated_capacity_gb': 1748, @@ -57,7 +58,8 @@ class FakeHostManager(host_manager.HostManager): 'thick_provisioning_support': False, 'reserved_percentage': 10, 'volume_backend_name': 'lvm2', - 'timestamp': UTC_NOW}, + 'timestamp': UTC_NOW, + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}, 'host3': {'total_capacity_gb': 512, 'free_capacity_gb': 256, 'allocated_capacity_gb': 256, @@ -67,7 +69,8 @@ class FakeHostManager(host_manager.HostManager): 'thick_provisioning_support': True, 'reserved_percentage': 0, 'volume_backend_name': 'lvm3', - 'timestamp': UTC_NOW}, + 'timestamp': UTC_NOW, + 'uuid': '6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'}, 'host4': {'total_capacity_gb': 2048, 'free_capacity_gb': 200, 'allocated_capacity_gb': 1848, @@ -78,7 +81,8 @@ class FakeHostManager(host_manager.HostManager): 'reserved_percentage': 5, 'volume_backend_name': 'lvm4', 'timestamp': UTC_NOW, - 'consistent_group_snapshot_enabled': True}, + 'consistent_group_snapshot_enabled': True, + 'uuid': '18417850-2ca9-43d1-9619-ae16bfb0f655'}, 'host5': {'total_capacity_gb': 'infinite', 'free_capacity_gb': 'unknown', 'allocated_capacity_gb': 1548, @@ -87,7 +91,8 @@ class FakeHostManager(host_manager.HostManager): 'thin_provisioning_support': True, 'thick_provisioning_support': False, 'reserved_percentage': 5, - 'timestamp': UTC_NOW}, + 'timestamp': UTC_NOW, + 'uuid': 'f838f35c-4035-464f-9792-ce60e390c13d'}, } @@ -150,15 +155,20 @@ class FakeNovaClient(object): def mock_host_manager_db_calls(mock_obj, disabled=None): services = [ dict(id=1, host='host1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'), dict(id=2, host='host2', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='4200b32b-0bf9-436c-86b2-0675f6ac218e'), dict(id=3, host='host3', topic='volume', disabled=False, - availability_zone='zone2', updated_at=timeutils.utcnow()), + availability_zone='zone2', updated_at=timeutils.utcnow(), + uuid='4200b32b-0bf9-436c-86b2-0675f6ac218e'), dict(id=4, host='host4', topic='volume', disabled=False, - availability_zone='zone3', updated_at=timeutils.utcnow()), + availability_zone='zone3', updated_at=timeutils.utcnow(), + uuid='18417850-2ca9-43d1-9619-ae16bfb0f655'), dict(id=5, host='host5', topic='volume', disabled=False, - availability_zone='zone3', updated_at=timeutils.utcnow()), + availability_zone='zone3', updated_at=timeutils.utcnow(), + uuid='f838f35c-4035-464f-9792-ce60e390c13d'), ] if disabled is None: mock_obj.return_value = services diff --git a/cinder/tests/unit/scheduler/test_host_manager.py b/cinder/tests/unit/scheduler/test_host_manager.py index f233184adf2..3a5cf24d285 100644 --- a/cinder/tests/unit/scheduler/test_host_manager.py +++ b/cinder/tests/unit/scheduler/test_host_manager.py @@ -502,11 +502,14 @@ class HostManagerTestCase(test.TestCase): _mock_service_is_up.return_value = True services = [ dict(id=1, host='host1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'), dict(id=2, host='host2', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='4200b32b-0bf9-436c-86b2-0675f6ac218e'), dict(id=3, host='host3', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'), ] _mock_service_get_all.return_value = services # Create host_manager again to let db.service_get_all mock run @@ -553,7 +556,8 @@ class HostManagerTestCase(test.TestCase): services = [ # This is the first call to utcnow() dict(id=1, host='host1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7',) ] mocked_service_states = { @@ -659,19 +663,23 @@ class HostManagerTestCase(test.TestCase): dict(id=1, host='host1', topic='volume', disabled=False, availability_zone='zone1', updated_at=timeutils.utcnow(), binary=None, deleted=False, created_at=None, modified_at=None, - report_count=0, deleted_at=None, disabled_reason=None), + report_count=0, deleted_at=None, disabled_reason=None, + uuid='a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'), dict(id=2, host='host2', topic='volume', disabled=False, availability_zone='zone1', updated_at=timeutils.utcnow(), binary=None, deleted=False, created_at=None, modified_at=None, - report_count=0, deleted_at=None, disabled_reason=None), + report_count=0, deleted_at=None, disabled_reason=None, + uuid='4200b32b-0bf9-436c-86b2-0675f6ac218e'), dict(id=3, host='host3', topic='volume', disabled=False, availability_zone='zone2', updated_at=timeutils.utcnow(), binary=None, deleted=False, created_at=None, modified_at=None, - report_count=0, deleted_at=None, disabled_reason=None), + report_count=0, deleted_at=None, disabled_reason=None, + uuid='6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'), dict(id=4, host='host4', topic='volume', disabled=False, availability_zone='zone3', updated_at=timeutils.utcnow(), binary=None, deleted=False, created_at=None, modified_at=None, - report_count=0, deleted_at=None, disabled_reason=None), + report_count=0, deleted_at=None, disabled_reason=None, + uuid='18417850-2ca9-43d1-9619-ae16bfb0f655'), ] service_objs = [] @@ -759,11 +767,14 @@ class HostManagerTestCase(test.TestCase): services = [ dict(id=1, host='host1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'), dict(id=2, host='host2@back1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='4200b32b-0bf9-436c-86b2-0675f6ac218e'), dict(id=3, host='host2@back2', topic='volume', disabled=False, - availability_zone='zone2', updated_at=timeutils.utcnow()), + availability_zone='zone2', updated_at=timeutils.utcnow(), + uuid='6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'), ] mocked_service_states = { @@ -982,9 +993,11 @@ class HostManagerTestCase(test.TestCase): services = [ dict(id=1, host='host1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'), dict(id=2, host='host2@back1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()) + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='4200b32b-0bf9-436c-86b2-0675f6ac218e') ] mocked_service_states = { @@ -1074,9 +1087,11 @@ class HostManagerTestCase(test.TestCase): services = [ dict(id=1, host='host1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'), dict(id=2, host='host2@back1', topic='volume', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()) + availability_zone='zone1', updated_at=timeutils.utcnow(), + uuid='4200b32b-0bf9-436c-86b2-0675f6ac218e') ] mocked_service_states = { diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py index ca2006607d9..3a99343aca3 100644 --- a/cinder/tests/unit/test_cmd.py +++ b/cinder/tests/unit/test_cmd.py @@ -335,8 +335,10 @@ class TestCinderManageCmd(test.TestCase): @mock.patch('cinder.context.get_admin_context') def test_host_commands_list(self, get_admin_context, service_get_all): get_admin_context.return_value = mock.sentinel.ctxt - service_get_all.return_value = [{'host': 'fake-host', - 'availability_zone': 'fake-az'}] + service_get_all.return_value = [ + {'host': 'fake-host', + 'availability_zone': 'fake-az', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] with mock.patch('sys.stdout', new=six.StringIO()) as fake_out: expected_out = ("%(host)-25s\t%(zone)-15s\n" % @@ -356,10 +358,13 @@ class TestCinderManageCmd(test.TestCase): def test_host_commands_list_with_zone(self, get_admin_context, service_get_all): get_admin_context.return_value = mock.sentinel.ctxt - service_get_all.return_value = [{'host': 'fake-host', - 'availability_zone': 'fake-az1'}, - {'host': 'fake-host', - 'availability_zone': 'fake-az2'}] + service_get_all.return_value = [ + {'host': 'fake-host', + 'availability_zone': 'fake-az1', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}, + {'host': 'fake-host', + 'availability_zone': 'fake-az2', + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}] with mock.patch('sys.stdout', new=six.StringIO()) as fake_out: expected_out = ("%(host)-25s\t%(zone)-15s\n" % @@ -692,7 +697,8 @@ class TestCinderManageCmd(test.TestCase): 'disabled': False, 'rpc_current_version': '1.1', 'object_current_version': '1.1', - 'cluster_name': 'my_cluster'} + 'cluster_name': 'my_cluster', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'} for binary in ('volume', 'scheduler', 'backup'): service['binary'] = 'cinder-%s' % binary self._test_service_commands_list(service) @@ -704,7 +710,8 @@ class TestCinderManageCmd(test.TestCase): 'updated_at': None, 'disabled': False, 'rpc_current_version': '1.1', - 'object_current_version': '1.1'} + 'object_current_version': '1.1', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'} for binary in ('volume', 'scheduler', 'backup'): service['binary'] = 'cinder-%s' % binary self._test_service_commands_list(service) @@ -965,7 +972,10 @@ class TestCinderManageCmd(test.TestCase): self.assertEqual(2, exit) @mock.patch('cinder.db.service_destroy') - @mock.patch('cinder.db.service_get', return_value = {'id': '12'}) + @mock.patch( + 'cinder.db.service_get', + return_value = {'id': '12', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}) def test_remove_service_success(self, mock_get_by_args, mock_service_destroy): service_commands = cinder_manage.ServiceCommands() diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 1da26da96df..2f5315cfd06 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -162,6 +162,54 @@ class DBAPIServiceTestCase(BaseTest): """Unit tests for cinder.db.api.service_*.""" + def test_service_uuid_migrations(self): + # Force create one entry with no UUID + sqlalchemy_api.service_create(self.ctxt, { + 'host': 'host1', + 'binary': 'cinder-volume', + 'topic': 'volume', }) + + # Create another one with a valid UUID + sqlalchemy_api.service_create(self.ctxt, { + 'host': 'host2', + 'binary': 'cinder-volume', + 'topic': 'volume', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}) + + # Run the migration and verify that we updated 1 entry + total, updated = db.service_uuids_online_data_migration( + self.ctxt, 10) + + self.assertEqual(1, total) + self.assertEqual(1, updated) + + def test_service_uuid_migrations_with_limit(self): + sqlalchemy_api.service_create(self.ctxt, { + 'host': 'host1', + 'binary': 'cinder-volume', + 'topic': 'volume', }) + sqlalchemy_api.service_create(self.ctxt, { + 'host': 'host2', + 'binary': 'cinder-volume', + 'topic': 'volume', }) + sqlalchemy_api.service_create(self.ctxt, { + 'host': 'host3', + 'binary': 'cinder-volume', + 'topic': 'volume', }) + # Run the migration and verify that we updated 1 entry + total, updated = db.service_uuids_online_data_migration( + self.ctxt, 2) + + self.assertEqual(3, total) + self.assertEqual(2, updated) + + # Now get the rest, intentionally setting max > what we should have + total, updated = db.service_uuids_online_data_migration( + self.ctxt, 2) + + self.assertEqual(1, total) + self.assertEqual(1, updated) + def test_service_create(self): # Add a cluster value to the service values = {'cluster_name': 'cluster'} diff --git a/cinder/tests/unit/test_service.py b/cinder/tests/unit/test_service.py index 139e0dd3a50..e9adbfa5095 100644 --- a/cinder/tests/unit/test_service.py +++ b/cinder/tests/unit/test_service.py @@ -147,7 +147,8 @@ class ServiceTestCase(test.TestCase): 'topic': self.topic, 'report_count': 0, 'availability_zone': 'nova', - 'id': 1} + 'id': 1, + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'} self.ctxt = context.get_admin_context() def _check_app(self, app, cluster=None, cluster_exists=None, diff --git a/cinder/tests/unit/volume/test_availability_zone.py b/cinder/tests/unit/volume/test_availability_zone.py index de4feddd7bb..fb9ceba6357 100644 --- a/cinder/tests/unit/volume/test_availability_zone.py +++ b/cinder/tests/unit/volume/test_availability_zone.py @@ -29,7 +29,8 @@ class AvailabilityZoneTestCase(base.BaseVolumeTestCase): super(AvailabilityZoneTestCase, self).setUp() self.get_all = self.patch( 'cinder.db.service_get_all', autospec=True, - return_value = [{'availability_zone': 'a', 'disabled': False}]) + return_value=[{'availability_zone': 'a', 'disabled': False, + 'uuid': 'f838f35c-4035-464f-9792-ce60e390c13d'}]) def test_list_availability_zones_cached(self): azs = self.volume_api.list_availability_zones(enable_cache=True) @@ -80,10 +81,12 @@ class AvailabilityZoneTestCase(base.BaseVolumeTestCase): { 'availability_zone': 'a', 'disabled': False, + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824', }, { 'availability_zone': 'b', 'disabled': False, + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e', }, ] azs = self.volume_api.list_availability_zones(enable_cache=True) @@ -99,10 +102,14 @@ class AvailabilityZoneTestCase(base.BaseVolumeTestCase): return obj['name'] self.get_all.return_value = [ - {'availability_zone': 'ping', 'disabled': 0}, - {'availability_zone': 'ping', 'disabled': 1}, - {'availability_zone': 'pong', 'disabled': 0}, - {'availability_zone': 'pung', 'disabled': 1}, + {'availability_zone': 'ping', 'disabled': 0, + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}, + {'availability_zone': 'ping', 'disabled': 1, + 'uuid': '4200b32b-0bf9-436c-86b2-0675f6ac218e'}, + {'availability_zone': 'pong', 'disabled': 0, + 'uuid': '6d91e7f5-ca17-4e3b-bf4f-19ca77166dd7'}, + {'availability_zone': 'pung', 'disabled': 1, + 'uuid': '18417850-2ca9-43d1-9619-ae16bfb0f655'}, ] volume_api = cinder.volume.api.API() diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index 72c46765a06..46909faa7d7 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -1083,7 +1083,9 @@ class VolumeTestCase(base.BaseVolumeTestCase): with mock.patch('cinder.db.service_get_all') as mock_get_service, \ mock.patch.object(volume_api, 'list_availability_zones') as mock_get_azs: - mock_get_service.return_value = [{'host': 'foo'}] + mock_get_service.return_value = [ + {'host': 'foo', + 'uuid': 'a3a593da-7f8d-4bb7-8b4c-f2bc1e0b4824'}] mock_get_azs.return_value = {} volume_api.create(self.context, size=1,