From 96a037fb6702fb89fdc64bc9af4683e296cd3fd5 Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Sun, 16 Jul 2017 05:15:19 -0400 Subject: [PATCH] NetApp cDOT: Fix share specs on migration Administrators may request to change the share type during a migration, and if optimized migration is possible, the driver must set the required specs on the destination. Change-Id: I11498058a3c80c8bd26be9d0bbf0459b4b2b2108 Closes-Bug: #1704622 --- .../netapp/dataontap/cluster_mode/lib_base.py | 25 +++++++- .../dataontap/cluster_mode/test_lib_base.py | 57 +++++++++++++++++++ ...e-specs-on-migration-bfbbebec26533652.yaml | 5 ++ 3 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/bug-1704622-netapp-cdot-fix-share-specs-on-migration-bfbbebec26533652.yaml diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index 3ddf85e2..ff8aebb2 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -1719,14 +1719,23 @@ class NetAppCmodeFileStorageLibrary(object): try: backend = share_utils.extract_host( destination_host, level='backend_name') + destination_aggregate = share_utils.extract_host( + destination_host, level='pool') + # Validate new share type extra-specs are valid on the + # destination + extra_specs = share_types.get_extra_specs_from_share( + destination_share) + self._check_extra_specs_validity( + destination_share, extra_specs) + self._check_aggregate_extra_specs_validity( + destination_aggregate, extra_specs) + data_motion.get_backend_configuration(backend) source_vserver, __ = self._get_vserver( share_server=share_server) share_volume = self._get_backend_share_name( source_share['id']) - destination_aggregate = share_utils.extract_host( - destination_host, level='pool') self._check_destination_vserver_for_vol_move( source_share, source_vserver, destination_share_server) @@ -1903,6 +1912,18 @@ class NetAppCmodeFileStorageLibrary(object): destination_share['id']) vserver_client.set_volume_name(share_volume, new_share_volume_name) + # Modify volume properties per share type extra-specs + extra_specs = share_types.get_extra_specs_from_share( + destination_share) + provisioning_options = self._get_provisioning_options(extra_specs) + destination_aggregate = share_utils.extract_host( + destination_share['host'], level='pool') + + # Modify volume to match extra specs + vserver_client.manage_volume(destination_aggregate, + new_share_volume_name, + **provisioning_options) + msg = ("Volume move operation for share %(shr)s has completed " "successfully. Share has been moved from %(src)s to " "%(dest)s.") diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index 510ad016..bd269dc6 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -3894,6 +3894,39 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): mock_warning_log.assert_called_once() self.assertFalse(data_motion.get_backend_configuration.called) + @ddt.data((None, exception.NetAppException), + (exception.Invalid, None)) + @ddt.unpack + def test_migration_check_compatibility_extra_specs_invalid( + self, side_effect_1, side_effect_2): + self.library._have_cluster_creds = True + self.mock_object(self.library, '_get_backend_share_name', + mock.Mock(return_value=fake.SHARE_NAME)) + mock_exception_log = self.mock_object(lib_base.LOG, 'exception') + self.mock_object(share_types, 'get_extra_specs_from_share') + self.mock_object(self.library, '_check_extra_specs_validity', + mock.Mock(side_effect=side_effect_1)) + self.mock_object(self.library, + '_check_aggregate_extra_specs_validity', + mock.Mock(side_effect=side_effect_2)) + self.mock_object(data_motion, 'get_backend_configuration') + + migration_compatibility = self.library.migration_check_compatibility( + self.context, fake_share.fake_share_instance(), + fake_share.fake_share_instance(), share_server=fake.SHARE_SERVER, + destination_share_server=None) + + expected_compatibility = { + 'compatible': False, + 'writable': False, + 'nondisruptive': False, + 'preserve_metadata': False, + 'preserve_snapshots': False, + } + self.assertDictMatch(expected_compatibility, migration_compatibility) + mock_exception_log.assert_called_once() + self.assertFalse(data_motion.get_backend_configuration.called) + def test_migration_check_compatibility_destination_not_configured(self): self.library._have_cluster_creds = True self.mock_object(self.library, '_get_backend_share_name', @@ -3905,6 +3938,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): mock_exception_log = self.mock_object(lib_base.LOG, 'exception') self.mock_object(share_utils, 'extract_host', mock.Mock( return_value='destination_backend')) + self.mock_object(share_types, 'get_extra_specs_from_share') + self.mock_object(self.library, '_check_extra_specs_validity') + self.mock_object(self.library, '_check_aggregate_extra_specs_validity') mock_vserver_compatibility_check = self.mock_object( self.library, '_check_destination_vserver_for_vol_move') @@ -3936,6 +3972,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): (exception.InvalidParameterValue, ('dest_vserver', None)))) def test_migration_check_compatibility_errors(self, side_effects): self.library._have_cluster_creds = True + self.mock_object(share_types, 'get_extra_specs_from_share') + self.mock_object(self.library, '_check_extra_specs_validity') + self.mock_object(self.library, '_check_aggregate_extra_specs_validity') self.mock_object(self.library, '_get_backend_share_name', mock.Mock(return_value=fake.SHARE_NAME)) self.mock_object(data_motion, 'get_backend_configuration') @@ -3967,6 +4006,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): def test_migration_check_compatibility_incompatible_vservers(self): self.library._have_cluster_creds = True + self.mock_object(share_types, 'get_extra_specs_from_share') + self.mock_object(self.library, '_check_extra_specs_validity') + self.mock_object(self.library, '_check_aggregate_extra_specs_validity') self.mock_object(self.library, '_get_backend_share_name', mock.Mock(return_value=fake.SHARE_NAME)) self.mock_object(data_motion, 'get_backend_configuration') @@ -4004,6 +4046,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): def test_migration_check_compatibility_client_error(self): self.library._have_cluster_creds = True + self.mock_object(share_types, 'get_extra_specs_from_share') + self.mock_object(self.library, '_check_extra_specs_validity') + self.mock_object(self.library, '_check_aggregate_extra_specs_validity') self.mock_object(self.library, '_get_backend_share_name', mock.Mock(return_value=fake.SHARE_NAME)) mock_exception_log = self.mock_object(lib_base.LOG, 'exception') @@ -4040,6 +4085,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): def test_migration_check_compatibility(self): self.library._have_cluster_creds = True + self.mock_object(share_types, 'get_extra_specs_from_share') + self.mock_object(self.library, '_check_extra_specs_validity') + self.mock_object(self.library, '_check_aggregate_extra_specs_validity') self.mock_object(self.library, '_get_backend_share_name', mock.Mock(return_value=fake.SHARE_NAME)) self.mock_object(data_motion, 'get_backend_configuration') @@ -4303,8 +4351,15 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): mock_move_status_check = self.mock_object( self.library, '_get_volume_move_status', mock.Mock(side_effect=vol_move_side_effects)) + self.mock_object(share_types, 'get_extra_specs_from_share', + mock.Mock(return_value=fake.EXTRA_SPEC)) + self.mock_object(self.library, '_get_provisioning_options', + mock.Mock(return_value=fake.PROVISIONING_OPTIONS)) + self.mock_object(vserver_client, 'manage_volume') + src_share = fake_share.fake_share_instance(id='source-share-instance') dest_share = fake_share.fake_share_instance(id='dest-share-instance') + dest_aggr = share_utils.extract_host(dest_share['host'], level='pool') data_updates = self.library.migration_complete( self.context, src_share, dest_share, source_snapshots, @@ -4323,6 +4378,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.library._create_export.assert_called_once_with( dest_share, fake.SHARE_SERVER, fake.VSERVER1, vserver_client, clear_current_export_policy=False) + vserver_client.manage_volume.assert_called_once_with( + dest_aggr, 'new_share_name', **fake.PROVISIONING_OPTIONS) mock_info_log.assert_called_once() if phase != 'completed': self.assertEqual(2, mock_warning_log.call_count) diff --git a/releasenotes/notes/bug-1704622-netapp-cdot-fix-share-specs-on-migration-bfbbebec26533652.yaml b/releasenotes/notes/bug-1704622-netapp-cdot-fix-share-specs-on-migration-bfbbebec26533652.yaml new file mode 100644 index 00000000..5a586cd2 --- /dev/null +++ b/releasenotes/notes/bug-1704622-netapp-cdot-fix-share-specs-on-migration-bfbbebec26533652.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - The NetApp driver has been fixed to ensure that share type changes + during driver optimized share migration will result in correction + of share properties as per the requested extra-specs.