From 82b261f1d5283391477411d469bc6c019455fbbf Mon Sep 17 00:00:00 2001 From: sandeeplocharla Date: Mon, 9 Nov 2026 16:50:34 +0530 Subject: [PATCH] CSTACKEX-367: Remove LUN-map logical unit number dead code Since the managed iSCSI path became // (PR #114), nothing consumes the LUN-map logical unit number. Stop fetching and returning it: - enableLogicalAccess/ensureLunMapped now return void; the follow-up lun-maps GET that only read the number is removed - getLogicalAccess returns boolean (mapping exists) - drop OntapStorageConstants.LOGICAL_UNIT_NUMBER and LUN-number logging in OntapPrimaryDatastoreDriver - update unit tests, asserting a new mapping needs one lookup and one create --- .../driver/OntapPrimaryDatastoreDriver.java | 13 +- .../storage/service/SANStrategy.java | 6 +- .../storage/service/StorageStrategy.java | 7 +- .../storage/service/UnifiedNASStrategy.java | 7 +- .../storage/service/UnifiedSANStrategy.java | 69 ++------- .../storage/utils/OntapStorageConstants.java | 1 - .../OntapPrimaryDatastoreDriverTest.java | 12 +- .../storage/service/StorageStrategyTest.java | 7 +- .../service/UnifiedSANStrategyTest.java | 138 +++++------------- 9 files changed, 74 insertions(+), 186 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 2bc06c12ca6d..3d8b1d5aab29 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java @@ -1325,10 +1325,10 @@ private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map ensureAccessGroupForHost(sanStrategy, host, storagePool, svmName, accessGroupName); // Create or retrieve existing LUN mapping. - String lunNumber = sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName); + sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName); String lunWwid = resolveVolumeLunWwid(sanStrategy, svmName, cloudStackVolumeName, volumeVO.getId()); - logger.debug("grantAccessIscsi: LUN [{}] mapped to igroup [{}] as logical unit number [{}], WWID [{}]", - cloudStackVolumeName, accessGroupName, lunNumber, lunWwid); + logger.debug("grantAccessIscsi: LUN [{}] mapped to igroup [{}], WWID [{}]", + cloudStackVolumeName, accessGroupName, lunWwid); // Update volume path if changed (e.g. the WWID was not yet known for a pre-existing volume) String iscsiPath = buildIscsiPath(storagePool, lunWwid); @@ -1343,7 +1343,7 @@ private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map * *

Called by the framework from {@code copyTemplateToManagedTemplateVolume} just before it * issues the {@code CopyCommand}. That method reads {@code managedStoreTarget} from the pool - * details before this call, when the LUN number does not exist yet, so the stale + * details before this call, when the LUN WWID is not yet known, so the stale * value is corrected here. The datastore details are re-read when the command is built, so the * update lands in time.

*/ @@ -1365,13 +1365,12 @@ private void grantAccessTemplate(TemplateInfo templateInfo, Host host, DataStore boolean lunMapped = false; try { - String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName); + sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName); lunMapped = true; // The template cache LUN has no volume_details row to cache the WWID in, and this runs // once per template per pool, so it is fetched from the array each time. String lunWwid = fetchLunWwid(sanStrategy, svmName, lunName); - logger.debug("grantAccessTemplate: template cache LUN [{}] mapped as logical unit number [{}], WWID [{}]", - lunName, lunNumber, lunWwid); + logger.debug("grantAccessTemplate: template cache LUN [{}] mapped, WWID [{}]", lunName, lunWwid); String iscsiPath = buildIscsiPath(storagePool, lunWwid); templatePoolRef.setInstallPath(iscsiPath); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java index 4b1bca00f95c..7713708e450f 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java @@ -33,15 +33,13 @@ public SANStrategy(OntapStorage ontapStorage) { /** * Ensures the LUN is mapped to the specified access group (igroup). - * If a mapping already exists, returns the existing LUN number. - * If not, creates a new mapping and returns the assigned LUN number. + * If a mapping already exists, nothing is done; otherwise a new mapping is created. * * @param svmName the SVM name * @param lunName the LUN name * @param accessGroupName the igroup name - * @return the logical unit number as a String */ - public abstract String ensureLunMapped(String svmName, String lunName, String accessGroupName); + public abstract void ensureLunMapped(String svmName, String lunName, String accessGroupName); /** * Validates that the host initiator is present in the access group (igroup). diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index a19a90c59641..ed2948a22932 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -1035,9 +1035,8 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam * //TODO for NFS 3.0 and NFS 4.1 protocols (e.g., export rule management) * //TODO for Nvme/TCP and Nvme/FC protocols * @param values map including SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS - * @return String containing logical unit number for the new/existing mapping (SAN) or relevant info for NAS */ - abstract public String enableLogicalAccess(Map values); + abstract public void enableLogicalAccess(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses @@ -1050,9 +1049,9 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam * Method encapsulates the behavior based on the opted protocol in subclasses * lunMap lookup for iSCSI/FC protocols (GET-only, no side-effects) * @param values map with SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS - * @return String containing logical unit number if mapping exists; otherwise null + * @return true if the mapping exists; otherwise false */ - abstract public String getLogicalAccess(Map values); + abstract public boolean getLogicalAccess(Map values); // ── FlexVolume Snapshot accessors ──────────────────────────────────────── diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java index 7973412feb5e..7d6b39ed393a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java @@ -500,8 +500,7 @@ public AccessGroup getAccessGroup(Map values) { } @Override - public String enableLogicalAccess(Map values) { - return null; + public void enableLogicalAccess(Map values) { } @Override @@ -509,8 +508,8 @@ public void disableLogicalAccess(Map values) { } @Override - public String getLogicalAccess(Map values) { - return null; + public boolean getLogicalAccess(Map values) { + return false; } private ExportPolicy createExportPolicy(String svmName, ExportPolicy policy) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index c6310352b468..7846779666e1 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -652,9 +652,8 @@ public AccessGroup getAccessGroup(Map values) { } @Override - public String enableLogicalAccess(Map values) { + public void enableLogicalAccess(Map values) { logger.trace("enableLogicalAccess : Creating LunMap with values {} ", values); - String lunNumber = null; if (values == null) { logger.error("enableLogicalAccess: LunMap creation failed. Invalid request values: null"); throw new CloudRuntimeException("Failed to create LunMap, invalid request"); @@ -693,34 +692,11 @@ public String enableLogicalAccess(Map values) { throw feignEx; } } - // Get the LunMap poolDetails - OntapResponse lunMapResponse = null; - try { - lunMapResponse = sanFeignClient.getLunMapResponse(authHeader, - Map.of( - OntapStorageConstants.SVM_DOT_NAME, svmName, - OntapStorageConstants.LUN_DOT_NAME, lunName, - OntapStorageConstants.IGROUP_DOT_NAME, igroupName, - OntapStorageConstants.FIELDS, OntapStorageConstants.LOGICAL_UNIT_NUMBER - )); - if(lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty() - && lunMapResponse.getRecords().get(0).getLogicalUnitNumber() != null) { - lunNumber = lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString(); - - } else { - logger.error("enableLogicalAccess: Failed to fetch LunMap poolDetails for Lun: {} and igroup: {}. LunMap response is null or empty.", lunName, igroupName); - throw new CloudRuntimeException("Failed to fetch LunMap poolDetails for Lun: " + lunName + " and igroup: " + igroupName); - } - } catch (Exception e) { - logger.error("enableLogicalAccess: Failed to fetch LunMap poolDetails for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e); - throw new CloudRuntimeException("Failed to fetch LunMap poolDetails for Lun: " + lunName + " and igroup: " + igroupName); - } - logger.trace("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMapResponse.getRecords().get(0)); + logger.trace("enableLogicalAccess: LunMap ensured for Lun: {} and igroup: {}", lunName, igroupName); } catch (Exception e) { logger.error("Exception occurred while creating LunMap", e); throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage()); } - return lunNumber; } public void disableLogicalAccess(Map values) { @@ -752,10 +728,9 @@ public void disableLogicalAccess(Map values) { } } - // GET-only helper: fetch LUN-map and return logical unit number if it exists; otherwise return null + // GET-only helper: returns true if the LUN-map exists; otherwise false @Override - public String getLogicalAccess(Map values) { - String lunNumber = null; + public boolean getLogicalAccess(Map values) { logger.trace("getLogicalAccess : Fetching LunMap with values {} ", values); if (values == null) { logger.error("getLogicalAccess: Invalid request values: null"); @@ -774,47 +749,31 @@ public String getLogicalAccess(Map values) { Map.of( OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.LUN_DOT_NAME, lunName, - OntapStorageConstants.IGROUP_DOT_NAME, igroupName, - OntapStorageConstants.FIELDS, OntapStorageConstants.LOGICAL_UNIT_NUMBER + OntapStorageConstants.IGROUP_DOT_NAME, igroupName )); - if (lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty()) { - Integer lunLogicalUnitNum = lunMapResponse.getRecords().get(0).getLogicalUnitNumber(); - lunNumber = lunLogicalUnitNum != null ? lunLogicalUnitNum.toString() : null; - } + return lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty(); } catch (Exception e) { logger.warn("getLogicalAccess: LunMap not found for Lun: {} and igroup: {} ({}).", lunName, igroupName, e.getMessage()); + return false; } - return lunNumber; } @Override - public String ensureLunMapped(String svmName, String lunName, String accessGroupName) { + public void ensureLunMapped(String svmName, String lunName, String accessGroupName) { logger.trace("ensureLunMapped: Ensuring LUN [{}] is mapped to igroup [{}] on SVM [{}]", lunName, accessGroupName, svmName); - // Check existing map first - Map getMap = Map.of( + Map mapRequest = Map.of( OntapStorageConstants.LUN_DOT_NAME, lunName, OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName ); - String lunNumber = getLogicalAccess(getMap); - if (lunNumber != null) { - logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}] with LUN number [{}]", lunName, accessGroupName, lunNumber); - return lunNumber; + if (getLogicalAccess(mapRequest)) { + logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}]", lunName, accessGroupName); + return; } - // Create if not exists - Map enableMap = Map.of( - OntapStorageConstants.LUN_DOT_NAME, lunName, - OntapStorageConstants.SVM_DOT_NAME, svmName, - OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName - ); - String response = enableLogicalAccess(enableMap); - if (response == null ) { - throw new CloudRuntimeException("Failed to map LUN [" + lunName + "] to iGroup [" + accessGroupName + "]"); - } - logger.trace("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response); - return response; + enableLogicalAccess(mapRequest); + logger.trace("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}]", lunName, accessGroupName); } /** * Reverts a LUN to a snapshot using the ONTAP CLI-based snapshot file restore API. diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index c12fe7325761..68c527f4ee6a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -136,7 +136,6 @@ public class OntapStorageConstants { public static final String QOS_POLICY_NAME_IOPS = "iops_"; public static final String UUID = "uuid"; public static final String QOS_POLICY_OBJECT_COUNT_FIELDS = "uuid,name,object_count"; - public static final String LOGICAL_UNIT_NUMBER = "logical_unit_number"; public static final String IGROUP_DOT_NAME = "igroup.name"; public static final String IGROUP_DOT_UUID = "igroup.uuid"; public static final String UNDERSCORE = "_"; diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index b675618ae2a8..9f2e28dff944 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -577,7 +577,7 @@ void testGrantAccess_ClusterScope_Success() { .thenReturn("igroup1"); when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup); - when(sanStrategy.ensureLunMapped(anyString(), anyString(), anyString())).thenReturn("0"); + doNothing().when(sanStrategy).ensureLunMapped(anyString(), anyString(), anyString()); when(sanStrategy.getLunSerialNumber(anyString(), anyString())).thenReturn(LUN_SERIAL_NUMBER); // Execute @@ -631,7 +631,7 @@ void testGrantAccess_IgroupNotFound_CreatesNewIgroup() { when(sanStrategy.getAccessGroup(any())).thenReturn(null); when(sanStrategy.createAccessGroup(any())).thenReturn(createdAccessGroup); - when(sanStrategy.ensureLunMapped(anyString(), anyString(), anyString())).thenReturn("0"); + doNothing().when(sanStrategy).ensureLunMapped(anyString(), anyString(), anyString()); when(sanStrategy.getLunSerialNumber(anyString(), anyString())).thenReturn(LUN_SERIAL_NUMBER); // Execute @@ -1519,7 +1519,7 @@ void testGrantAccess_Template_WritesInstallPathAndRefreshesStoreTarget() { utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup); - when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); + doNothing().when(sanStrategy).ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1")); when(sanStrategy.getLunSerialNumber(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"))).thenReturn(LUN_SERIAL_NUMBER); assertTrue(driver.grantAccess(templateInfo, host, primaryDataStore)); @@ -1575,7 +1575,7 @@ void testGrantAccess_Template_UnmapsWhenInstallPathUpdateFails() { utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup); - when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); + doNothing().when(sanStrategy).ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1")); when(sanStrategy.getLunSerialNumber(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"))).thenReturn(LUN_SERIAL_NUMBER); when(sanStrategy.getCloudStackVolume(argThat(map -> map != null && "/vol/vol1/cs_tmpl_50".equals(map.get("name"))))) @@ -2874,7 +2874,7 @@ void testGrantAccess_Template_CreatesIgroupWhenMissing() { when(sanStrategy.getAccessGroup(any())).thenReturn(null); when(sanStrategy.createAccessGroup(any())).thenReturn(createdAccessGroup); - when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); + doNothing().when(sanStrategy).ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1")); when(sanStrategy.getLunSerialNumber(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"))).thenReturn(LUN_SERIAL_NUMBER); assertTrue(driver.grantAccess(templateInfo, hostVo, primaryDataStore)); @@ -3749,7 +3749,7 @@ void testGrantAccess_TemporarySnapshotCopy_SyncsIscsiPathOnInMemoryVolume() { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup); - when(sanStrategy.ensureLunMapped(anyString(), anyString(), anyString())).thenReturn("0"); + doNothing().when(sanStrategy).ensureLunMapped(anyString(), anyString(), anyString()); when(sanStrategy.getLunSerialNumber(anyString(), anyString())).thenReturn(LUN_SERIAL_NUMBER); assertTrue(driver.grantAccess(volumeInfo, host, dataStore)); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java index 17d553951637..5855009daaff 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java @@ -203,8 +203,7 @@ public AccessGroup getAccessGroup(Map values) { } @Override - public String enableLogicalAccess(Map values) { - return null; + public void enableLogicalAccess(Map values) { } @Override @@ -212,8 +211,8 @@ public void disableLogicalAccess(Map values) { } @Override - public String getLogicalAccess(Map values) { - return null; + public boolean getLogicalAccess(Map values) { + return false; } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index 3a37936009b1..fc34b2e73d99 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -64,6 +64,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.mockito.junit.jupiter.MockitoExtension; @@ -710,58 +711,40 @@ void testEnableLogicalAccess_Success() { values.put(OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1"); values.put(OntapStorageConstants.IGROUP_DOT_NAME, "igroup1"); - LunMap lunMap = new LunMap(); - lunMap.setLogicalUnitNumber(0); - - OntapResponse response = new OntapResponse<>(); - response.setRecords(List.of(lunMap)); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) .thenReturn(authHeader); when(sanFeignClient.createLunMap(eq(authHeader), eq(true), any(LunMap.class))) .thenReturn(new OntapResponse<>()); - when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - String result = unifiedSANStrategy.enableLogicalAccess(values); + unifiedSANStrategy.enableLogicalAccess(values); - // Verify - assertNotNull(result); - assertEquals("0", result); + // Verify: map is created and no follow-up lookup is made verify(sanFeignClient).createLunMap(eq(authHeader), eq(true), any(LunMap.class)); + verify(sanFeignClient, never()).getLunMapResponse(any(), anyMap()); } } @Test - void testEnableLogicalAccess_AlreadyMapped_ReturnsLunNumber() { + void testEnableLogicalAccess_AlreadyMapped_DoesNotThrow() { // Setup Map values = new HashMap<>(); values.put(OntapStorageConstants.SVM_DOT_NAME, "svm1"); values.put(OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1"); values.put(OntapStorageConstants.IGROUP_DOT_NAME, "igroup1"); - LunMap lunMap = new LunMap(); - lunMap.setLogicalUnitNumber(5); - - OntapResponse response = new OntapResponse<>(); - response.setRecords(List.of(lunMap)); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) .thenReturn(authHeader); Exception exception = new RuntimeException("LUN already mapped to this group"); doThrow(exception).when(sanFeignClient).createLunMap(eq(authHeader), eq(true), any(LunMap.class)); - when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); - // Execute - String result = unifiedSANStrategy.enableLogicalAccess(values); - - // Verify - assertNotNull(result); - assertEquals("5", result); + // Execute / Verify + assertDoesNotThrow(() -> unifiedSANStrategy.enableLogicalAccess(values)); + verify(sanFeignClient, never()).getLunMapResponse(any(), anyMap()); } } @@ -815,11 +798,8 @@ void testGetLogicalAccess_Success() { values.put(OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1"); values.put(OntapStorageConstants.IGROUP_DOT_NAME, "igroup1"); - LunMap lunMap = new LunMap(); - lunMap.setLogicalUnitNumber(3); - OntapResponse response = new OntapResponse<>(); - response.setRecords(List.of(lunMap)); + response.setRecords(List.of(new LunMap())); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) @@ -827,17 +807,13 @@ void testGetLogicalAccess_Success() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); - // Execute - String result = unifiedSANStrategy.getLogicalAccess(values); - - // Verify - assertNotNull(result); - assertEquals("3", result); + // Execute / Verify + assertTrue(unifiedSANStrategy.getLogicalAccess(values)); } } @Test - void testGetLogicalAccess_NotFound_ReturnsNull() { + void testGetLogicalAccess_NotFound_ReturnsFalse() { // Setup Map values = new HashMap<>(); values.put(OntapStorageConstants.SVM_DOT_NAME, "svm1"); @@ -851,26 +827,20 @@ void testGetLogicalAccess_NotFound_ReturnsNull() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())) .thenThrow(new RuntimeException("Not found")); - // Execute - String result = unifiedSANStrategy.getLogicalAccess(values); - - // Verify - assertNull(result); + // Execute / Verify + assertFalse(unifiedSANStrategy.getLogicalAccess(values)); } } @Test - void testEnsureLunMapped_ExistingMapping_ReturnsLunNumber() { + void testEnsureLunMapped_ExistingMapping_SkipsCreate() { // Setup String svmName = "svm1"; String lunName = "/vol/vol1/lun1"; String accessGroupName = "igroup1"; - LunMap lunMap = new LunMap(); - lunMap.setLogicalUnitNumber(2); - OntapResponse response = new OntapResponse<>(); - response.setRecords(List.of(lunMap)); + response.setRecords(List.of(new LunMap())); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) @@ -879,47 +849,36 @@ void testEnsureLunMapped_ExistingMapping_ReturnsLunNumber() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - String result = unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName); + unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName); // Verify - assertEquals("2", result); verify(sanFeignClient, never()).createLunMap(any(), anyBoolean(), any()); } } @Test - void testEnsureLunMapped_CreatesNewMapping_ReturnsLunNumber() { + void testEnsureLunMapped_CreatesNewMapping_SingleLookup() { // Setup String svmName = "svm1"; String lunName = "/vol/vol1/lun1"; String accessGroupName = "igroup1"; - LunMap lunMap = new LunMap(); - lunMap.setLogicalUnitNumber(4); - OntapResponse emptyResponse = new OntapResponse<>(); emptyResponse.setRecords(new ArrayList<>()); - OntapResponse response = new OntapResponse<>(); - response.setRecords(List.of(lunMap)); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) .thenReturn(authHeader); - // First call returns empty (no existing mapping), second call returns the new mapping - when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())) - .thenReturn(emptyResponse) // First call - no records - .thenReturn(response); // Second call after creation - + when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(emptyResponse); when(sanFeignClient.createLunMap(eq(authHeader), eq(true), any(LunMap.class))) .thenReturn(new OntapResponse<>()); // Execute - String result = unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName); + unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName); - // Verify - assertEquals("4", result); + // Verify: one existence lookup and one create, no follow-up lookup + verify(sanFeignClient, times(1)).getLunMapResponse(eq(authHeader), anyMap()); verify(sanFeignClient).createLunMap(eq(authHeader), eq(true), any(LunMap.class)); } } @@ -1829,7 +1788,7 @@ void testEnableLogicalAccess_MissingIgroupName_ThrowsException() { } @Test - void testEnableLogicalAccess_FetchLunMapFails_ThrowsException() { + void testEnableLogicalAccess_CreateLunMapFails_ThrowsException() { Map values = new HashMap<>(); values.put(OntapStorageConstants.SVM_DOT_NAME, "svm1"); values.put(OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1"); @@ -1840,9 +1799,7 @@ void testEnableLogicalAccess_FetchLunMapFails_ThrowsException() { .thenReturn(authHeader); when(sanFeignClient.createLunMap(eq(authHeader), eq(true), any(LunMap.class))) - .thenReturn(new OntapResponse<>()); - when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())) - .thenThrow(new RuntimeException("Failed to fetch LunMap")); + .thenThrow(new RuntimeException("Failed to create LunMap")); assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.enableLogicalAccess(values)); @@ -1913,30 +1870,21 @@ void testEnsureLunMapped_CreateNewMapping_Success() { String lunName = "/vol/vol1/lun1"; String accessGroupName = "igroup1"; - LunMap lunMap = new LunMap(); - lunMap.setLogicalUnitNumber(4); - OntapResponse emptyResponse = new OntapResponse<>(); emptyResponse.setRecords(new ArrayList<>()); - OntapResponse response = new OntapResponse<>(); - response.setRecords(List.of(lunMap)); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) .thenReturn(authHeader); - // First call returns empty (no existing mapping) - when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())) - .thenReturn(emptyResponse) // First call - check existing - .thenReturn(response); // Second call - after creation - + // Single lookup returns empty (no existing mapping), then the map is created + when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(emptyResponse); when(sanFeignClient.createLunMap(eq(authHeader), eq(true), any(LunMap.class))) .thenReturn(new OntapResponse<>()); - String result = unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName); + unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName); - assertEquals("4", result); + verify(sanFeignClient, times(1)).getLunMapResponse(eq(authHeader), anyMap()); verify(sanFeignClient).createLunMap(eq(authHeader), eq(true), any(LunMap.class)); } } @@ -1954,12 +1902,9 @@ void testEnsureLunMapped_FailedToMap_ThrowsException() { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) .thenReturn(authHeader); - when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())) - .thenReturn(emptyResponse) // First call - no existing - .thenReturn(emptyResponse); // Second call - still empty after creation - + when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(emptyResponse); when(sanFeignClient.createLunMap(eq(authHeader), eq(true), any(LunMap.class))) - .thenReturn(new OntapResponse<>()); + .thenThrow(new RuntimeException("ONTAP rejected lun-map create")); assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName)); @@ -2330,7 +2275,7 @@ void testGetLogicalAccess_MissingIgroupName_ThrowsException() { } @Test - void testGetLogicalAccess_EmptyResponse_ReturnsNull() { + void testGetLogicalAccess_EmptyResponse_ReturnsFalse() { Map values = new HashMap<>(); values.put(OntapStorageConstants.SVM_DOT_NAME, "svm1"); values.put(OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1"); @@ -2345,14 +2290,12 @@ void testGetLogicalAccess_EmptyResponse_ReturnsNull() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(emptyResponse); - String result = unifiedSANStrategy.getLogicalAccess(values); - - assertNull(result); + assertFalse(unifiedSANStrategy.getLogicalAccess(values)); } } @Test - void testGetLogicalAccess_ExceptionThrown_ReturnsNull() { + void testGetLogicalAccess_ExceptionThrown_ReturnsFalse() { Map values = new HashMap<>(); values.put(OntapStorageConstants.SVM_DOT_NAME, "svm1"); values.put(OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1"); @@ -2365,9 +2308,7 @@ void testGetLogicalAccess_ExceptionThrown_ReturnsNull() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())) .thenThrow(new RuntimeException("Connection failed")); - String result = unifiedSANStrategy.getLogicalAccess(values); - - assertNull(result); + assertFalse(unifiedSANStrategy.getLogicalAccess(values)); } } @@ -2451,17 +2392,14 @@ void testDeleteAccessGroup_NotFoundInResponse_SkipsDeletion() { } @Test - void testEnsureLunMapped_ExistingMapping_ReturnsExistingNumber() { + void testEnsureLunMapped_ExistingMapping_DoesNotCreateAgain() { // Setup String svmName = "svm1"; String lunName = "/vol/vol1/lun1"; String accessGroupName = "igroup1"; - LunMap lunMap = new LunMap(); - lunMap.setLogicalUnitNumber(3); - OntapResponse response = new OntapResponse<>(); - response.setRecords(List.of(lunMap)); + response.setRecords(List.of(new LunMap())); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) @@ -2470,10 +2408,8 @@ void testEnsureLunMapped_ExistingMapping_ReturnsExistingNumber() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - String result = unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName); + unifiedSANStrategy.ensureLunMapped(svmName, lunName, accessGroupName); - // Verify - assertEquals("3", result); // Verify createLunMap was NOT called verify(sanFeignClient, never()).createLunMap(any(), anyBoolean(), any(LunMap.class)); }