diff --git a/manila/share/driver.py b/manila/share/driver.py index 6926119f61..461844f9f1 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -449,11 +449,11 @@ class ShareDriver(object): :param share_server: Share server model or None. :param destination_share_server: Destination Share server model or None. - :return: If the migration changes the export locations or snapshot - provider locations, this method should return a dictionary with - the relevant info. In such case, a dictionary containing a list of - export locations and a list of model updates for each snapshot - indexed by their IDs. + :return: If the migration changes the share export locations, snapshot + provider locations or snapshot export locations, this method should + return a dictionary with the relevant info. In such case, a + dictionary containing a list of export locations and a list of + model updates for each snapshot indexed by their IDs. Example:: @@ -475,11 +475,33 @@ class ShareDriver(object): { 'bc4e3b28-0832-4168-b688-67fdc3e9d408': { - 'provider_location': '/snapshots/foo/bar_1' + 'provider_location': '/snapshots/foo/bar_1', + 'export_locations': + [ + { + 'path': '1.2.3.4:/snapshots/foo/bar_1', + 'is_admin_only': False, + }, + { + 'path': '5.6.7.8:/snapshots/foo/bar_1', + 'is_admin_only': True, + }, + ], }, '2e62b7ea-4e30-445f-bc05-fd523ca62941': { - 'provider_location': '/snapshots/foo/bar_2' + 'provider_location': '/snapshots/foo/bar_2', + 'export_locations': + [ + { + 'path': '1.2.3.4:/snapshots/foo/bar_2', + 'is_admin_only': False, + }, + { + 'path': '5.6.7.8:/snapshots/foo/bar_2', + 'is_admin_only': True, + }, + ], }, }, } diff --git a/manila/share/manager.py b/manila/share/manager.py index 7fb1244469..735b15cd0f 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -1207,13 +1207,31 @@ class ShareManager(manager.SchedulerDependentManager): data_updates['export_locations']) snapshot_updates = data_updates.get('snapshot_updates') or {} + + dest_extra_specs = self._get_extra_specs_from_share_type( + context, dest_share_instance['share_type_id']) + for src_snap_ins, dest_snap_ins in snapshot_mappings.items(): model_update = snapshot_updates.get(dest_snap_ins['id']) or {} + snapshot_export_locations = model_update.pop( + 'export_locations', []) + model_update['status'] = constants.STATUS_AVAILABLE model_update['progress'] = '100%' self.db.share_snapshot_instance_update( context, dest_snap_ins['id'], model_update) + if dest_extra_specs['mount_snapshot_support']: + + for el in snapshot_export_locations: + values = { + 'share_snapshot_instance_id': dest_snap_ins['id'], + 'path': el['path'], + 'is_admin_only': el['is_admin_only'], + } + self.db.share_snapshot_instance_export_location_create( + context, values) + helper = migration.ShareMigrationHelper( context, self.db, share_ref, self.access_helper) @@ -1324,15 +1342,19 @@ class ShareManager(manager.SchedulerDependentManager): LOG.info(_LI("Share Migration for share %s" " completed successfully."), share_ref['id']) - def _update_migrated_share_model( - self, context, share_id, dest_share_instance): + def _get_extra_specs_from_share_type(self, context, share_type_id): - share_type = share_types.get_share_type( - context, dest_share_instance['share_type_id']) + share_type = share_types.get_share_type(context, share_type_id) share_api = api.API() - update = share_api.get_share_attributes_from_share_type(share_type) + return share_api.get_share_attributes_from_share_type(share_type) + + def _update_migrated_share_model( + self, context, share_id, dest_share_instance): + + update = self._get_extra_specs_from_share_type( + context, dest_share_instance['share_type_id']) self.db.share_update(context, share_id, update) diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index 3e8ec8a107..485c7e0094 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -4535,20 +4535,29 @@ class ShareManagerTestCase(test.TestCase): new_instance) self.assertTrue(manager.LOG.exception.called) - def test__migration_complete_driver(self): + @ddt.data({'mount_snapshot_support': True, 'snapshot_els': False}, + {'mount_snapshot_support': True, 'snapshot_els': True}, + {'mount_snapshot_support': False, 'snapshot_els': False}, + {'mount_snapshot_support': False, 'snapshot_els': True},) + @ddt.unpack + def test__migration_complete_driver( + self, mount_snapshot_support, snapshot_els): fake_src_host = 'src_host' fake_dest_host = 'dest_host' fake_rules = 'fake_rules' src_server = db_utils.create_share_server() dest_server = db_utils.create_share_server() + share_type = db_utils.create_share_type( + extra_specs={'mount_snapshot_support': mount_snapshot_support}) share = db_utils.create_share( share_server_id='fake_src_server_id', host=fake_src_host) dest_instance = db_utils.create_share_instance( share_id=share['id'], share_server_id='fake_dest_server_id', - host=fake_dest_host) + host=fake_dest_host, + share_type_id=share_type['id']) src_instance = share.instance snapshot = db_utils.create_snapshot(share_id=share['id']) dest_snap_instance = db_utils.create_snapshot_instance( @@ -4557,11 +4566,14 @@ class ShareManagerTestCase(test.TestCase): snapshot_mappings = {snapshot.instance['id']: dest_snap_instance} + model_update = {'fake_keys': 'fake_values'} + if snapshot_els: + el = {'path': 'fake_path', 'is_admin_only': False} + model_update['export_locations'] = [el] + fake_return_data = { 'export_locations': 'fake_export_locations', - 'snapshot_updates': {dest_snap_instance['id']: { - 'fake_keys': 'fake_values'}, - }, + 'snapshot_updates': {dest_snap_instance['id']: model_update}, } # mocks @@ -4589,6 +4601,9 @@ class ShareManagerTestCase(test.TestCase): [snapshot.instance]])) self.mock_object( self.share_manager.db, 'share_snapshot_instance_update') + el_create = self.mock_object( + self.share_manager.db, + 'share_snapshot_instance_export_location_create') # run self.share_manager._migration_complete_driver( @@ -4635,6 +4650,11 @@ class ShareManagerTestCase(test.TestCase): (self.share_manager.db.share_snapshot_instance_update. assert_called_once_with(self.context, dest_snap_instance['id'], snap_data_update)) + if mount_snapshot_support and snapshot_els: + el['share_snapshot_instance_id'] = dest_snap_instance['id'] + el_create.assert_called_once_with(self.context, el) + else: + el_create.assert_not_called() def test__migration_complete_host_assisted(self): diff --git a/releasenotes/notes/bug-1661381-migration-snapshot-export-locations-169786dcec386402.yaml b/releasenotes/notes/bug-1661381-migration-snapshot-export-locations-169786dcec386402.yaml new file mode 100644 index 0000000000..9ec0d0d93d --- /dev/null +++ b/releasenotes/notes/bug-1661381-migration-snapshot-export-locations-169786dcec386402.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Fixed error in driver-assisted share migration of mountable + snapshots. +