From 742a066e33b2bfab25ffd80ca0a9e07a3bb58b76 Mon Sep 17 00:00:00 2001 From: agireesh Date: Tue, 3 Feb 2026 07:48:14 -0500 Subject: [PATCH] NetApp - Fix for LUN and namespace name handling in cache for ASAr2 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. Change-Id: Ia7e904ced97bef8f2668cf12b88089c77daba24a Closes-Bug: #2139272 Signed-off-by: agireesh --- .../volume/drivers/netapp/dataontap/fakes.py | 8 ++ .../netapp/dataontap/test_block_base.py | 84 ++++++++++++++++++- .../netapp/dataontap/test_nvme_library.py | 64 ++++++++++++++ .../tests/unit/volume/drivers/netapp/fakes.py | 2 + .../unit/volume/drivers/netapp/test_utils.py | 21 +++++ .../drivers/netapp/dataontap/block_base.py | 16 +++- .../drivers/netapp/dataontap/nvme_library.py | 17 +++- cinder/volume/drivers/netapp/utils.py | 8 ++ ...handling-cache-asar2-5e821e2d862d3669.yaml | 12 +++ 9 files changed, 225 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/bug-fix-2139272-handling-cache-asar2-5e821e2d862d3669.yaml 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.