diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py index 00c0a516839..efa2cc2f93a 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py @@ -70,6 +70,14 @@ LUN_METADATA = { 'Qtree': None, 'Volume': POOL_NAME, } +LUN_METADATA_ASAR2 = { + 'OsType': None, + 'SpaceReserved': 'true', + 'SpaceAllocated': 'false', + 'Path': LUN_NAME, + 'Qtree': None, + 'Volume': LUN_NAME, +} LUN_METADATA_WITH_SPACE_ALLOCATION = { 'OsType': None, 'SpaceReserved': 'true', diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py index b400e2d70ab..df14991ea75 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py @@ -143,6 +143,35 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): 0, self.library. _mark_qos_policy_group_for_deletion.call_count) self.assertEqual(0, block_base.LOG.error.call_count) + def test_create_volume_asar2(self): + self.library.configuration.netapp_disaggregated_platform = True + volume_size_in_bytes = int(fake.SIZE) * units.Gi + self.mock_object(na_utils, 'get_volume_extra_specs', + return_value={}) + self.mock_object(na_utils, 'log_extra_spec_warnings') + self.mock_object(block_base, 'LOG') + self.mock_object(volume_utils, 'extract_host', + return_value=fake.POOL_NAME) + self.mock_object(self.library, '_setup_qos_for_volume', + return_value=fake.QOS_POLICY_GROUP_INFO) + self.mock_object(self.library, '_create_lun') + self.mock_object(self.library, '_create_lun_handle') + self.mock_object(self.library, '_add_lun_to_table') + self.mock_object(self.library, '_mark_qos_policy_group_for_deletion') + self.mock_object(self.library, '_get_volume_model_update') + + self.library.create_volume(fake.VOLUME) + + self.library._create_lun.assert_called_once_with( + fake.POOL_NAME, fake.LUN_NAME, volume_size_in_bytes, + fake.LUN_METADATA_ASAR2, + fake.QOS_POLICY_GROUP_NAME, False) + self.library._get_volume_model_update.assert_called_once_with( + fake.VOLUME) + self.assertEqual( + 0, self.library. _mark_qos_policy_group_for_deletion.call_count) + self.assertEqual(0, block_base.LOG.error.call_count) + def test_create_volume_space_allocation_extra_spec_false(self): volume_size_in_bytes = int(fake.SIZE) * units.Gi self.mock_object(na_utils, 'get_volume_extra_specs', @@ -927,7 +956,6 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): def test_initialize_connection_iscsi(self): target_details_list = fake.ISCSI_TARGET_DETAILS_LIST - self.library.configuration.netapp_disaggregated_platform = False volume = fake.ISCSI_VOLUME connector = fake.ISCSI_CONNECTOR self.mock_object(block_base.NetAppBlockStorageLibrary, '_map_lun', @@ -1182,6 +1210,24 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): fake.LUN_NAME, 'metadata') self.library.zapi_client.destroy_lun.assert_called_once_with(fake.PATH) + def test_delete_lun_asar2(self): + mock_get_lun_attr = self.mock_object(self.library, '_get_lun_attr') + self.library.configuration.netapp_disaggregated_platform = True + mock_get_lun_attr.return_value = fake.LUN_METADATA_ASAR2 + self.library.zapi_client = mock.Mock() + + lun_backend_name = na_utils.get_backend_lun_or_ns_name( + fake.LUN_NAME, self.library.configuration) + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, fake.LUN_ID, + fake.LUN_SIZE, + fake.LUN_METADATA_ASAR2) + self.library.lun_table = {lun_backend_name: fake_lun} + self.library._delete_lun(lun_backend_name) + + mock_get_lun_attr.assert_called_once_with(fake.LUN_NAME, 'metadata') + self.library.zapi_client.destroy_lun.assert_called_once_with( + fake.LUN_NAME) + def test_delete_lun_no_metadata(self): self.mock_object(self.library, '_get_lun_attr', return_value=None) self.library.zapi_client = mock.Mock() @@ -1379,6 +1425,28 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): new_size, fake.QOS_POLICY_GROUP_NAME) + def test_extend_volume_asar2(self): + self.library.configuration.netapp_disaggregated_platform = True + new_size = 100 + volume_copy = copy.copy(fake.VOLUME) + volume_copy['size'] = new_size + + mock_get_volume_extra_specs = self.mock_object( + na_utils, 'get_volume_extra_specs', return_value=fake.EXTRA_SPECS) + mock_setup_qos_for_volume = self.mock_object( + self.library, '_setup_qos_for_volume', + return_value=fake.QOS_POLICY_GROUP_INFO) + mock_extend_volume = self.mock_object(self.library, '_extend_volume') + + self.library.extend_volume(fake.VOLUME, new_size) + + mock_get_volume_extra_specs.assert_called_once_with(fake.VOLUME) + mock_setup_qos_for_volume.assert_called_once_with(volume_copy, + fake.EXTRA_SPECS) + mock_extend_volume.assert_called_once_with(fake.VOLUME, + new_size, + fake.QOS_POLICY_GROUP_NAME) + def test_extend_volume_api_error(self): new_size = 100 @@ -1869,7 +1937,6 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): def test_add_looping_tasks(self): mock_add_task = self.mock_object(self.library.loopingcalls, 'add_task') - self.library.configuration.netapp_disaggregated_platform = False mock_call_snap_cleanup = self.mock_object( self.library, '_delete_snapshots_marked_for_deletion') mock_call_ems_logging = self.mock_object( @@ -1906,6 +1973,18 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): self.library._delete_lun_from_table(fake_lun.name) self.assertEqual({}, self.library.lun_table) + def test_delete_lun_from_table_asar2(self): + self.library.configuration.netapp_disaggregated_platform = True + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, fake.LUN_ID, + fake.LUN_SIZE, fake.LUN_METADATA_ASAR2) + backend_name = na_utils.get_backend_lun_or_ns_name( + fake_lun.name, + self.library.configuration + ) + self.library.lun_table = {backend_name: fake_lun} + self.library._delete_lun_from_table(fake_lun.name) + self.assertEqual({}, self.library.lun_table) + def test_delete_lun_from_table_not_found(self): fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, fake.LUN_ID, fake.LUN_SIZE, fake.LUN_METADATA) @@ -1970,7 +2049,6 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): def test_add_looping_tasks_traditional_platform(self): """Test _add_looping_tasks with AFF platform""" mock_add_task = self.mock_object(self.library.loopingcalls, 'add_task') - self.library.configuration.netapp_disaggregated_platform = False mock_call_snap_cleanup = self.mock_object( self.library, '_delete_snapshots_marked_for_deletion') mock_call_ems_logging = self.mock_object( diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nvme_library.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nvme_library.py index c5ebda5e0d2..3b80f825f3c 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nvme_library.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nvme_library.py @@ -204,6 +204,30 @@ class NetAppNVMeStorageLibraryTestCase(test.TestCase): self.library._create_namespace_handle.assert_called_once_with( fake_metadata) + def test_create_volume_asar2(self): + self.library.configuration.netapp_disaggregated_platform = True + volume_size_in_bytes = int(fake.SIZE) * units.Gi + self.mock_object(volume_utils, 'extract_host', + return_value=fake.POOL_NAME) + self.mock_object(self.library.client, 'create_namespace') + self.mock_object(self.library, '_create_namespace_handle') + self.mock_object(self.library, '_add_namespace_to_table') + + volume1 = copy.deepcopy(fake.test_volume) + self.library.create_volume(volume1) + + fake_metadata = { + 'OsType': self.library.namespace_ostype, + 'Path': 'fakename', + 'Volume': 'aggr1', + 'Qtree': None + } + self.library.client.create_namespace.assert_called_once_with( + fake.POOL_NAME, 'fakename', volume_size_in_bytes, fake_metadata) + self.library._create_namespace_handle.assert_called_once_with( + fake_metadata) + self.library.configuration.netapp_disaggregated_platform = False + def test_create_namespace_handle(self): self.library.vserver = fake.VSERVER_NAME res = self.library._create_namespace_handle(fake.NAMESPACE_METADATA) @@ -270,6 +294,13 @@ class NetAppNVMeStorageLibraryTestCase(test.TestCase): self.assertEqual(self.fake_namespace, res) + def test__get_namespace_from_table_asar2(self): + self.library.configuration.netapp_disaggregated_platform = True + res = self.library._get_namespace_from_table(fake.NAMESPACE_NAME) + + self.assertEqual(self.fake_namespace, res) + self.library.configuration.netapp_disaggregated_platform = False + @ddt.data(exception.VolumeNotFound, netapp_api.NaApiError) def test__get_namespace_attr_error(self, error_obj): self.mock_object(self.library, '_get_namespace_from_table', @@ -371,6 +402,23 @@ class NetAppNVMeStorageLibraryTestCase(test.TestCase): has_namespace = fake.NAMESPACE_NAME in self.library.namespace_table self.assertFalse(has_namespace) + def test__delete_namespace_asar2(self): + self.library.configuration.netapp_disaggregated_platform = True + namespace = copy.deepcopy(fake.NAMESPACE_WITH_METADATA) + self.mock_object(self.library, '_get_namespace_attr', + return_value=namespace['metadata']) + self.mock_object(self.library.client, 'destroy_namespace') + + self.library._delete_namespace(fake.NAMESPACE_NAME) + + self.library._get_namespace_attr.assert_called_once_with( + fake.NAMESPACE_NAME, 'metadata') + self.library.client.destroy_namespace.assert_called_once_with( + namespace['metadata']['Path']) + has_namespace = fake.NAMESPACE_NAME in self.library.namespace_table + self.assertFalse(has_namespace) + self.library.configuration.netapp_disaggregated_platform = False + def test__delete_namespace_not_found(self): namespace = copy.deepcopy(fake.NAMESPACE_WITH_METADATA) self.mock_object(self.library, '_get_namespace_attr', @@ -844,6 +892,22 @@ class NetAppNVMeStorageLibraryTestCase(test.TestCase): self.library.client.namespace_resize.assert_called_once_with( fake.PATH_NAMESPACE, new_bytes) + def test__extend_volume_asar2(self): + self.library.configuration.netapp_disaggregated_platform = True + self.mock_object(self.library, '_get_namespace_from_table', + return_value=self.fake_namespace) + self.mock_object(self.library.client, 'namespace_resize') + + self.library._extend_volume(fake.NAMESPACE_VOLUME, fake.SIZE) + + new_bytes = str(int(fake.SIZE) * units.Gi) + self.assertEqual(new_bytes, self.fake_namespace.size) + self.library._get_namespace_from_table.assert_called_once_with( + fake.NAMESPACE_NAME) + self.library.client.namespace_resize.assert_called_once_with( + fake.PATH_NAMESPACE, new_bytes) + self.library.configuration.netapp_disaggregated_platform = False + def test__map_namespace(self): self.library.host_type = 'win' fake_namespace_metadata = [{ diff --git a/cinder/tests/unit/volume/drivers/netapp/fakes.py b/cinder/tests/unit/volume/drivers/netapp/fakes.py index e3fdfde7a9f..24b59596c9f 100644 --- a/cinder/tests/unit/volume/drivers/netapp/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/fakes.py @@ -178,6 +178,8 @@ def create_configuration(): config.append_config_values(na_opts.netapp_basicauth_opts) config.append_config_values(na_opts.netapp_certificateauth_opts) config.append_config_values(na_opts.netapp_provisioning_opts) + config.append_config_values(na_opts.netapp_cluster_opts) + return config diff --git a/cinder/tests/unit/volume/drivers/netapp/test_utils.py b/cinder/tests/unit/volume/drivers/netapp/test_utils.py index 9ff3364354c..bf19e179931 100644 --- a/cinder/tests/unit/volume/drivers/netapp/test_utils.py +++ b/cinder/tests/unit/volume/drivers/netapp/test_utils.py @@ -1266,3 +1266,24 @@ class BitSetTestCase(test.TestCase): actual >>= 16 self.assertEqual(na_utils.BitSet(1), actual) + + def test_get_backend_lun_or_ns_name_disaggregated(self): + config = mock.Mock() + config.netapp_disaggregated_platform = True + volume_names = [ + ('vol-1234-5678-90ab-cdef', 'vol_1234_5678_90ab_cdef'), + ('test-0000-1111-2222-3333', 'test_0000_1111_2222_3333'), + ('simple', 'simple'), + ('with-dash-', 'with_dash_'), + ] + for volume_name, expected in volume_names: + result = na_utils.get_backend_lun_or_ns_name(volume_name, config) + self.assertEqual(expected, result) + + def test_get_backend_lun_or_ns_name_non_disaggregated(self): + config = mock.Mock() + config.netapp_disaggregated_platform = False + volume_name = 'vol-1234-5678-90ab-cdef' + expected = volume_name + result = na_utils.get_backend_lun_or_ns_name(volume_name, config) + self.assertEqual(expected, result) diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index dce0b472a47..32213a299c1 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -278,6 +278,14 @@ class NetAppBlockStorageLibrary( metadata['Volume'] = pool_name metadata['Qtree'] = None + if self.configuration.netapp_disaggregated_platform: + # For ASAr2 path name is same as namespace name + metadata['Path'] = na_utils.get_backend_lun_or_ns_name( + lun_name, + self.configuration, + ) + metadata['Volume'] = metadata['Path'] + handle = self._create_lun_handle(metadata) self._add_lun_to_table(NetAppLun(handle, lun_name, size, metadata)) @@ -301,7 +309,8 @@ class NetAppBlockStorageLibrary( def _delete_lun(self, lun_name): """Helper method to delete LUN backing a volume or snapshot.""" - + lun_name = na_utils.get_backend_lun_or_ns_name(lun_name, + self.configuration) metadata = self._get_lun_attr(lun_name, 'metadata') if metadata: try: @@ -582,6 +591,7 @@ class NetAppBlockStorageLibrary( def _delete_lun_from_table(self, name): """Deletes LUN from cache table.""" + name = na_utils.get_backend_lun_or_ns_name(name, self.configuration) if self.lun_table.get(name, None): self.lun_table.pop(name) @@ -597,6 +607,7 @@ class NetAppBlockStorageLibrary( Refreshes cache if LUN not found in cache. """ + name = na_utils.get_backend_lun_or_ns_name(name, self.configuration) lun = self.lun_table.get(name) if lun is None: lun_list = self.zapi_client.get_lun_list() @@ -674,7 +685,8 @@ class NetAppBlockStorageLibrary( def _extend_volume(self, volume, new_size, qos_policy_group_name): """Extend an existing volume to the new size.""" - name = volume['name'] + name = na_utils.get_backend_lun_or_ns_name(volume['name'], + self.configuration) lun = self._get_lun_from_table(name) path = lun.metadata['Path'] curr_size_bytes = str(lun.size) diff --git a/cinder/volume/drivers/netapp/dataontap/nvme_library.py b/cinder/volume/drivers/netapp/dataontap/nvme_library.py index a4335bff7e2..fc567a49997 100644 --- a/cinder/volume/drivers/netapp/dataontap/nvme_library.py +++ b/cinder/volume/drivers/netapp/dataontap/nvme_library.py @@ -280,6 +280,14 @@ class NetAppNVMeStorageLibrary( metadata = {'OsType': self.namespace_ostype, 'Path': '/vol/%s/%s' % (pool_name, namespace)} + if self.configuration.netapp_disaggregated_platform: + namespace = na_utils.get_backend_lun_or_ns_name( + namespace, + self.configuration, + ) + # For ASAr2 path name is same as namespace name + metadata = {'OsType': self.namespace_ostype, + 'Path': namespace} try: self.client.create_namespace(pool_name, namespace, size, metadata) except Exception: @@ -303,7 +311,10 @@ class NetAppNVMeStorageLibrary( def _delete_namespace(self, namespace_name): """Helper method to delete namespace backing a volume or snapshot.""" - + namespace_name = na_utils.get_backend_lun_or_ns_name( + namespace_name, + self.configuration, + ) metadata = self._get_namespace_attr(namespace_name, 'metadata') if metadata: try: @@ -455,6 +466,7 @@ class NetAppNVMeStorageLibrary( Refreshes cache if namespace not found in cache. """ + name = na_utils.get_backend_lun_or_ns_name(name, self.configuration) namespace = self.namespace_table.get(name) if namespace is None: namespace_list = self.client.get_namespace_list() @@ -618,7 +630,8 @@ class NetAppNVMeStorageLibrary( def _extend_volume(self, volume, new_size): """Extend an existing volume to the new size.""" - name = volume['name'] + name = na_utils.get_backend_lun_or_ns_name(volume['name'], + self.configuration) namespace = self._get_namespace_from_table(name) path = namespace.metadata['Path'] curr_size_bytes = str(namespace.size) diff --git a/cinder/volume/drivers/netapp/utils.py b/cinder/volume/drivers/netapp/utils.py index 18ee76a94da..5730a39dcf9 100644 --- a/cinder/volume/drivers/netapp/utils.py +++ b/cinder/volume/drivers/netapp/utils.py @@ -582,6 +582,14 @@ def is_multiattach_to_host(volume, connector): return len(attachment) > 1 +def get_backend_lun_or_ns_name(volume_name, config): + """Get backend LUN or NameSpace name""" + if config.netapp_disaggregated_platform: + return volume_name.replace("-", "_") + else: + return volume_name + + class hashabledict(dict): """A hashable dictionary that is comparable (i.e. in unit tests, etc.)""" def __hash__(self): diff --git a/releasenotes/notes/bug-fix-2139272-handling-cache-asar2-5e821e2d862d3669.yaml b/releasenotes/notes/bug-fix-2139272-handling-cache-asar2-5e821e2d862d3669.yaml new file mode 100644 index 00000000000..c5bf4b4bedb --- /dev/null +++ b/releasenotes/notes/bug-fix-2139272-handling-cache-asar2-5e821e2d862d3669.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + NetApp Driver `bug #2139272 + `_: After a service + restart, LUN and Namespace information is refreshed in + the Cinder volume cache. In the ASAr2 cluster, these names must use + underscores (_); however, the Cinder database contained volume names + with hyphens (-), which caused some Cinder volume operation failures. + This fix ensures that LUN and Namespace names are correctly set during + volume creation and properly passed when accessing cache data, preventing + failures after Cinder service restarts.