diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py index 667f10c39e7..c3fa715f39f 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py @@ -439,6 +439,7 @@ class NetAppNfsDriverTestCase(test.TestCase): self.mock_object(os.path, 'exists', mock.Mock(return_value=path_exists)) self.mock_object(self.driver, '_clone_backing_file_for_volume') + self.mock_object(os, 'utime') retval = self.driver._do_clone_rel_img_cache( fake.CLONE_SOURCE_NAME, fake.CLONE_DESTINATION_NAME, @@ -450,8 +451,12 @@ class NetAppNfsDriverTestCase(test.TestCase): self.driver._clone_backing_file_for_volume.assert_called_once_with( fake.CLONE_SOURCE_NAME, fake.CLONE_DESTINATION_NAME, share=fake.NFS_SHARE, volume_id=None) + os.utime.assert_called_once_with( + 'dir/' + fake.CLONE_SOURCE_NAME, None) else: self.driver._clone_backing_file_for_volume.assert_not_called() + os.utime.assert_not_called() + os.path.exists.assert_called_once_with( 'dir/' + fake.CLONE_DESTINATION_NAME) diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/fakes.py index e104f9098ed..adb4bc25052 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/fakes.py @@ -39,8 +39,8 @@ SSC = { 'netapp_dedup': 'true', 'netapp_mirrored': 'false', 'netapp_raid_type': 'raid_dp', - 'netapp_disk_type': 'SSD', - 'netapp_hybrid_aggregate': False, + 'netapp_disk_type': ['SSD'], + 'netapp_hybrid_aggregate': 'false', 'pool_name': 'volume1', }, 'volume2': { @@ -52,8 +52,8 @@ SSC = { 'netapp_dedup': 'true', 'netapp_mirrored': 'true', 'netapp_raid_type': 'raid_dp', - 'netapp_disk_type': 'FCAL', - 'netapp_hybrid_aggregate': True, + 'netapp_disk_type': ['FCAL', 'SSD'], + 'netapp_hybrid_aggregate': 'true', 'pool_name': 'volume2', }, } @@ -95,14 +95,14 @@ SSC_MIRROR_INFO = { SSC_AGGREGATE_INFO = { 'volume1': { - 'netapp_disk_type': 'SSD', + 'netapp_disk_type': ['SSD'], 'netapp_raid_type': 'raid_dp', - 'netapp_hybrid_aggregate': False, + 'netapp_hybrid_aggregate': 'false', }, 'volume2': { - 'netapp_disk_type': 'FCAL', + 'netapp_disk_type': ['FCAL', 'SSD'], 'netapp_raid_type': 'raid_dp', - 'netapp_hybrid_aggregate': True, + 'netapp_hybrid_aggregate': 'true', }, } diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_capabilities.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_capabilities.py index d8df5833af2..b99c5b77620 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_capabilities.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_capabilities.py @@ -91,6 +91,12 @@ class CapabilitiesLibraryTestCase(test.TestCase): self.assertEqual(fake.SSC, result) self.assertIsNot(fake.SSC, result) + def test_get_ssc_flexvol_names(self): + + result = self.ssc_library.get_ssc_flexvol_names() + + self.assertItemsEqual(fake.SSC_VOLUMES, result) + def test_get_ssc_for_flexvol(self): result = self.ssc_library.get_ssc_for_flexvol(fake.SSC_VOLUMES[0]) @@ -350,12 +356,98 @@ class CapabilitiesLibraryTestCase(test.TestCase): 'netapp_mirrored': 'true', 'netapp_raid_type': 'raid_dp', 'netapp_disk_type': 'FCAL', + 'non_ssc_key': 'fake_value', } result = self.ssc_library.get_matching_flexvols_for_extra_specs(specs) self.assertEqual(['volume2'], result) + @ddt.data( + { + 'flexvol_info': { + 'netapp_dedup': 'true', + }, + 'extra_specs': { + 'netapp_dedup': 'true', + 'non_ssc_key': 'fake_value', + } + }, + { + 'flexvol_info': fake.SSC['volume1'], + 'extra_specs': { + 'netapp_disk_type': 'SSD', + 'pool_name': 'volume1', + } + }, + { + 'flexvol_info': fake.SSC['volume2'], + 'extra_specs': { + 'netapp_disk_type': 'SSD', + 'netapp_hybrid_aggregate': 'true', + } + } + ) + @ddt.unpack + def test_flexvol_matches_extra_specs(self, flexvol_info, extra_specs): + + result = self.ssc_library._flexvol_matches_extra_specs(flexvol_info, + extra_specs) + + self.assertTrue(result) + + @ddt.data( + { + 'flexvol_info': { + 'netapp_dedup': 'true', + }, + 'extra_specs': { + 'netapp_dedup': 'false', + 'non_ssc_key': 'fake_value', + } + }, + { + 'flexvol_info': fake.SSC['volume2'], + 'extra_specs': { + 'netapp_disk_type': 'SSD', + 'pool_name': 'volume1', + } + }, + { + 'flexvol_info': fake.SSC['volume2'], + 'extra_specs': { + 'netapp_disk_type': 'SATA', + } + } + ) + @ddt.unpack + def test_flexvol_matches_extra_specs_no_match(self, flexvol_info, + extra_specs): + + result = self.ssc_library._flexvol_matches_extra_specs(flexvol_info, + extra_specs) + + self.assertFalse(result) + + @ddt.data(('SSD', 'SSD'), ('SSD', ['SSD', 'FCAL'])) + @ddt.unpack + def test_extra_spec_matches(self, extra_spec_value, ssc_flexvol_value): + + result = self.ssc_library._extra_spec_matches(extra_spec_value, + ssc_flexvol_value) + + self.assertTrue(result) + + @ddt.data(('SSD', 'FCAL'), ('SSD', ['FCAL'])) + @ddt.unpack + def test_extra_spec_matches_no_match(self, extra_spec_value, + ssc_flexvol_value): + + result = self.ssc_library._extra_spec_matches(extra_spec_value, + ssc_flexvol_value) + + self.assertFalse(result) + def test_modify_extra_specs_for_comparison(self): specs = { diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index e0a317fe29b..694289e6785 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -403,6 +403,8 @@ class NetAppNfsDriver(driver.ManageableVD, LOG.info(_LI('Cloning from cache to destination %s'), dst) self._clone_backing_file_for_volume(src, dst, volume_id=None, share=share) + src_path = '%s/%s' % (dir, src) + os.utime(src_path, None) _do_clone() @utils.synchronized('clean_cache') diff --git a/cinder/volume/drivers/netapp/dataontap/utils/capabilities.py b/cinder/volume/drivers/netapp/dataontap/utils/capabilities.py index 8447b8dc9a4..3df17f331c7 100644 --- a/cinder/volume/drivers/netapp/dataontap/utils/capabilities.py +++ b/cinder/volume/drivers/netapp/dataontap/utils/capabilities.py @@ -215,20 +215,49 @@ class CapabilitiesLibrary(object): """Return a list of flexvol names that match a set of extra specs.""" extra_specs = self._modify_extra_specs_for_comparison(extra_specs) - extra_specs_set = set(extra_specs.items()) - ssc = self.get_ssc() matching_flexvols = [] - for flexvol_name, flexvol_info in ssc.items(): - if extra_specs_set.issubset(set(flexvol_info.items())): + for flexvol_name, flexvol_info in self.get_ssc().items(): + + if self._flexvol_matches_extra_specs(flexvol_info, extra_specs): matching_flexvols.append(flexvol_name) return matching_flexvols + def _flexvol_matches_extra_specs(self, flexvol_info, extra_specs): + """Check whether the SSC data for a FlexVol matches extra specs. + + A set of extra specs is considered a match for a FlexVol if, for each + extra spec, the value matches what is in SSC or the key is unknown to + SSC. + """ + + for extra_spec_key, extra_spec_value in extra_specs.items(): + + if extra_spec_key in flexvol_info: + if not self._extra_spec_matches(extra_spec_value, + flexvol_info[extra_spec_key]): + return False + + return True + + def _extra_spec_matches(self, extra_spec_value, ssc_flexvol_value): + """Check whether an extra spec value matches something in the SSC. + + The SSC values may be scalars or lists, so the extra spec value must be + compared to the SSC value if it is a scalar, or it must be sought in + the list. + """ + + if isinstance(ssc_flexvol_value, list): + return extra_spec_value in ssc_flexvol_value + else: + return extra_spec_value == ssc_flexvol_value + def _modify_extra_specs_for_comparison(self, extra_specs): """Adjust extra spec values for simple comparison to SSC values. - Most extra-spec key-value tuples may be directly compared. But the + Most extra-spec key-value tuples may be directly compared. But the boolean values that take the form ' True' or ' False' must be modified to allow comparison with the values we keep in the SSC and report to the scheduler. @@ -237,9 +266,11 @@ class CapabilitiesLibrary(object): modified_extra_specs = copy.deepcopy(extra_specs) for key, value in extra_specs.items(): - if re.match('\s+True', value, re.I): - modified_extra_specs[key] = True - elif re.match('\s+False', value, re.I): - modified_extra_specs[key] = False + + if isinstance(value, six.string_types): + if re.match('\s+True', value, re.I): + modified_extra_specs[key] = True + elif re.match('\s+False', value, re.I): + modified_extra_specs[key] = False return modified_extra_specs diff --git a/releasenotes/notes/bug-1634203-netapp-cdot-fix-clone-from-nfs-image-cache-2218fb402783bc20.yaml b/releasenotes/notes/bug-1634203-netapp-cdot-fix-clone-from-nfs-image-cache-2218fb402783bc20.yaml new file mode 100644 index 00000000000..aefc36b1829 --- /dev/null +++ b/releasenotes/notes/bug-1634203-netapp-cdot-fix-clone-from-nfs-image-cache-2218fb402783bc20.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Fixed an issue where the NetApp cDOT NFS driver failed to clone new + volumes from the image cache.