GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — CSTACKEX-367: Remove LUN-map logical unit number dead code · NetApp/cloudstack@11be34b

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • plugins/storage/volume/ontap/src
    • main/java/org/apache/cloudstack/storage
      • driver
        • OntapPrimaryDatastoreDriver.java
      • service
        • SANStrategy.java
        • StorageStrategy.java
        • UnifiedNASStrategy.java
        • UnifiedSANStrategy.java
      • utils
        • OntapStorageConstants.java
    • test/java/org/apache/cloudstack/storage
      • driver
        • OntapPrimaryDatastoreDriverTest.java
      • service
        • StorageStrategyTest.java
        • UnifiedSANStrategyTest.java
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java‎

Copy file name to clipboardExpand all lines: plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
+6-7Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,10 +1325,10 @@ private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map<String, String>
13251325
ensureAccessGroupForHost(sanStrategy, host, storagePool, svmName, accessGroupName);
13261326

13271327
// Create or retrieve existing LUN mapping.
1328-
String lunNumber = sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName);
1328+
sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName);
13291329
String lunWwid = resolveVolumeLunWwid(sanStrategy, svmName, cloudStackVolumeName, volumeVO.getId());
1330-
logger.debug("grantAccessIscsi: LUN [{}] mapped to igroup [{}] as logical unit number [{}], WWID [{}]",
1331-
cloudStackVolumeName, accessGroupName, lunNumber, lunWwid);
1330+
logger.debug("grantAccessIscsi: LUN [{}] mapped to igroup [{}], WWID [{}]",
1331+
cloudStackVolumeName, accessGroupName, lunWwid);
13321332

13331333
// Update volume path if changed (e.g. the WWID was not yet known for a pre-existing volume)
13341334
String iscsiPath = buildIscsiPath(storagePool, lunWwid);
@@ -1343,7 +1343,7 @@ private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map<String, String>
13431343
*
13441344
* <p>Called by the framework from {@code copyTemplateToManagedTemplateVolume} just before it
13451345
* issues the {@code CopyCommand}. That method reads {@code managedStoreTarget} from the pool
1346-
* details <em>before</em> this call, when the LUN number does not exist yet, so the stale
1346+
* details <em>before</em> this call, when the LUN WWID is not yet known, so the stale
13471347
* value is corrected here. The datastore details are re-read when the command is built, so the
13481348
* update lands in time.</p>
13491349
*/
@@ -1365,13 +1365,12 @@ private void grantAccessTemplate(TemplateInfo templateInfo, Host host, DataStore
13651365

13661366
boolean lunMapped = false;
13671367
try {
1368-
String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName);
1368+
sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName);
13691369
lunMapped = true;
13701370
// The template cache LUN has no volume_details row to cache the WWID in, and this runs
13711371
// once per template per pool, so it is fetched from the array each time.
13721372
String lunWwid = fetchLunWwid(sanStrategy, svmName, lunName);
1373-
logger.debug("grantAccessTemplate: template cache LUN [{}] mapped as logical unit number [{}], WWID [{}]",
1374-
lunName, lunNumber, lunWwid);
1373+
logger.debug("grantAccessTemplate: template cache LUN [{}] mapped, WWID [{}]", lunName, lunWwid);
13751374
String iscsiPath = buildIscsiPath(storagePool, lunWwid);
13761375

13771376
templatePoolRef.setInstallPath(iscsiPath);
Collapse file

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java‎

Copy file name to clipboardExpand all lines: plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java
+2-4Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,13 @@ public SANStrategy(OntapStorage ontapStorage) {
3333

3434
/**
3535
* Ensures the LUN is mapped to the specified access group (igroup).
36-
* If a mapping already exists, returns the existing LUN number.
37-
* If not, creates a new mapping and returns the assigned LUN number.
36+
* If a mapping already exists, nothing is done; otherwise a new mapping is created.
3837
*
3938
* @param svmName the SVM name
4039
* @param lunName the LUN name
4140
* @param accessGroupName the igroup name
42-
* @return the logical unit number as a String
4341
*/
44-
public abstract String ensureLunMapped(String svmName, String lunName, String accessGroupName);
42+
public abstract void ensureLunMapped(String svmName, String lunName, String accessGroupName);
4543

4644
/**
4745
* Validates that the host initiator is present in the access group (igroup).
Collapse file

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java‎

Copy file name to clipboardExpand all lines: plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
+3-4Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,9 +1035,8 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam
10351035
* //TODO for NFS 3.0 and NFS 4.1 protocols (e.g., export rule management)
10361036
* //TODO for Nvme/TCP and Nvme/FC protocols
10371037
* @param values map including SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS
1038-
* @return String containing logical unit number for the new/existing mapping (SAN) or relevant info for NAS
10391038
*/
1040-
abstract public String enableLogicalAccess(Map<String,String> values);
1039+
abstract public void enableLogicalAccess(Map<String,String> values);
10411040

10421041
/**
10431042
* Method encapsulates the behavior based on the opted protocol in subclasses
@@ -1050,9 +1049,9 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam
10501049
* Method encapsulates the behavior based on the opted protocol in subclasses
10511050
* lunMap lookup for iSCSI/FC protocols (GET-only, no side-effects)
10521051
* @param values map with SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS
1053-
* @return String containing logical unit number if mapping exists; otherwise null
1052+
* @return true if the mapping exists; otherwise false
10541053
*/
1055-
abstract public String getLogicalAccess(Map<String, String> values);
1054+
abstract public boolean getLogicalAccess(Map<String, String> values);
10561055

10571056
// ── FlexVolume Snapshot accessors ────────────────────────────────────────
10581057

Collapse file

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java‎

Copy file name to clipboardExpand all lines: plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java
+3-4Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -500,17 +500,16 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
500500
}
501501

502502
@Override
503-
public String enableLogicalAccess(Map<String, String> values) {
504-
return null;
503+
public void enableLogicalAccess(Map<String, String> values) {
505504
}
506505

507506
@Override
508507
public void disableLogicalAccess(Map<String, String> values) {
509508
}
510509

511510
@Override
512-
public String getLogicalAccess(Map<String, String> values) {
513-
return null;
511+
public boolean getLogicalAccess(Map<String, String> values) {
512+
return false;
514513
}
515514

516515
private ExportPolicy createExportPolicy(String svmName, ExportPolicy policy) {
Collapse file

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java‎

Copy file name to clipboardExpand all lines: plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java
+14-55Lines changed: 14 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -652,9 +652,8 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
652652
}
653653

654654
@Override
655-
public String enableLogicalAccess(Map<String, String> values) {
655+
public void enableLogicalAccess(Map<String, String> values) {
656656
logger.trace("enableLogicalAccess : Creating LunMap with values {} ", values);
657-
String lunNumber = null;
658657
if (values == null) {
659658
logger.error("enableLogicalAccess: LunMap creation failed. Invalid request values: null");
660659
throw new CloudRuntimeException("Failed to create LunMap, invalid request");
@@ -693,34 +692,11 @@ public String enableLogicalAccess(Map<String, String> values) {
693692
throw feignEx;
694693
}
695694
}
696-
// Get the LunMap poolDetails
697-
OntapResponse<LunMap> lunMapResponse = null;
698-
try {
699-
lunMapResponse = sanFeignClient.getLunMapResponse(authHeader,
700-
Map.of(
701-
OntapStorageConstants.SVM_DOT_NAME, svmName,
702-
OntapStorageConstants.LUN_DOT_NAME, lunName,
703-
OntapStorageConstants.IGROUP_DOT_NAME, igroupName,
704-
OntapStorageConstants.FIELDS, OntapStorageConstants.LOGICAL_UNIT_NUMBER
705-
));
706-
if(lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty()
707-
&& lunMapResponse.getRecords().get(0).getLogicalUnitNumber() != null) {
708-
lunNumber = lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString();
709-
710-
} else {
711-
logger.error("enableLogicalAccess: Failed to fetch LunMap poolDetails for Lun: {} and igroup: {}. LunMap response is null or empty.", lunName, igroupName);
712-
throw new CloudRuntimeException("Failed to fetch LunMap poolDetails for Lun: " + lunName + " and igroup: " + igroupName);
713-
}
714-
} catch (Exception e) {
715-
logger.error("enableLogicalAccess: Failed to fetch LunMap poolDetails for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e);
716-
throw new CloudRuntimeException("Failed to fetch LunMap poolDetails for Lun: " + lunName + " and igroup: " + igroupName);
717-
}
718-
logger.trace("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMapResponse.getRecords().get(0));
695+
logger.trace("enableLogicalAccess: LunMap ensured for Lun: {} and igroup: {}", lunName, igroupName);
719696
} catch (Exception e) {
720697
logger.error("Exception occurred while creating LunMap", e);
721698
throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage());
722699
}
723-
return lunNumber;
724700
}
725701

726702
public void disableLogicalAccess(Map<String, String> values) {
@@ -752,10 +728,9 @@ public void disableLogicalAccess(Map<String, String> values) {
752728
}
753729
}
754730

755-
// GET-only helper: fetch LUN-map and return logical unit number if it exists; otherwise return null
731+
// GET-only helper: returns true if the LUN-map exists; otherwise false
756732
@Override
757-
public String getLogicalAccess(Map<String, String> values) {
758-
String lunNumber = null;
733+
public boolean getLogicalAccess(Map<String, String> values) {
759734
logger.trace("getLogicalAccess : Fetching LunMap with values {} ", values);
760735
if (values == null) {
761736
logger.error("getLogicalAccess: Invalid request values: null");
@@ -774,47 +749,31 @@ public String getLogicalAccess(Map<String, String> values) {
774749
Map.of(
775750
OntapStorageConstants.SVM_DOT_NAME, svmName,
776751
OntapStorageConstants.LUN_DOT_NAME, lunName,
777-
OntapStorageConstants.IGROUP_DOT_NAME, igroupName,
778-
OntapStorageConstants.FIELDS, OntapStorageConstants.LOGICAL_UNIT_NUMBER
752+
OntapStorageConstants.IGROUP_DOT_NAME, igroupName
779753
));
780-
if (lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty()) {
781-
Integer lunLogicalUnitNum = lunMapResponse.getRecords().get(0).getLogicalUnitNumber();
782-
lunNumber = lunLogicalUnitNum != null ? lunLogicalUnitNum.toString() : null;
783-
}
754+
return lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty();
784755
} catch (Exception e) {
785756
logger.warn("getLogicalAccess: LunMap not found for Lun: {} and igroup: {} ({}).", lunName, igroupName, e.getMessage());
757+
return false;
786758
}
787-
return lunNumber;
788759
}
789760

790761
@Override
791-
public String ensureLunMapped(String svmName, String lunName, String accessGroupName) {
762+
public void ensureLunMapped(String svmName, String lunName, String accessGroupName) {
792763
logger.trace("ensureLunMapped: Ensuring LUN [{}] is mapped to igroup [{}] on SVM [{}]", lunName, accessGroupName, svmName);
793764

794-
// Check existing map first
795-
Map<String, String> getMap = Map.of(
765+
Map<String, String> mapRequest = Map.of(
796766
OntapStorageConstants.LUN_DOT_NAME, lunName,
797767
OntapStorageConstants.SVM_DOT_NAME, svmName,
798768
OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName
799769
);
800-
String lunNumber = getLogicalAccess(getMap);
801-
if (lunNumber != null) {
802-
logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}] with LUN number [{}]", lunName, accessGroupName, lunNumber);
803-
return lunNumber;
770+
if (getLogicalAccess(mapRequest)) {
771+
logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}]", lunName, accessGroupName);
772+
return;
804773
}
805774

806-
// Create if not exists
807-
Map<String, String> enableMap = Map.of(
808-
OntapStorageConstants.LUN_DOT_NAME, lunName,
809-
OntapStorageConstants.SVM_DOT_NAME, svmName,
810-
OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName
811-
);
812-
String response = enableLogicalAccess(enableMap);
813-
if (response == null ) {
814-
throw new CloudRuntimeException("Failed to map LUN [" + lunName + "] to iGroup [" + accessGroupName + "]");
815-
}
816-
logger.trace("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response);
817-
return response;
775+
enableLogicalAccess(mapRequest);
776+
logger.trace("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}]", lunName, accessGroupName);
818777
}
819778
/**
820779
* Reverts a LUN to a snapshot using the ONTAP CLI-based snapshot file restore API.
Collapse file

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java‎

Copy file name to clipboardExpand all lines: plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ public class OntapStorageConstants {
136136
public static final String QOS_POLICY_NAME_IOPS = "iops_";
137137
public static final String UUID = "uuid";
138138
public static final String QOS_POLICY_OBJECT_COUNT_FIELDS = "uuid,name,object_count";
139-
public static final String LOGICAL_UNIT_NUMBER = "logical_unit_number";
140139
public static final String IGROUP_DOT_NAME = "igroup.name";
141140
public static final String IGROUP_DOT_UUID = "igroup.uuid";
142141
public static final String UNDERSCORE = "_";
Collapse file

‎plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java‎

Copy file name to clipboardExpand all lines: plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ void testGrantAccess_ClusterScope_Success() {
577577
.thenReturn("igroup1");
578578

579579
when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup);
580-
when(sanStrategy.ensureLunMapped(anyString(), anyString(), anyString())).thenReturn("0");
580+
doNothing().when(sanStrategy).ensureLunMapped(anyString(), anyString(), anyString());
581581
when(sanStrategy.getLunSerialNumber(anyString(), anyString())).thenReturn(LUN_SERIAL_NUMBER);
582582

583583
// Execute
@@ -631,7 +631,7 @@ void testGrantAccess_IgroupNotFound_CreatesNewIgroup() {
631631

632632
when(sanStrategy.getAccessGroup(any())).thenReturn(null);
633633
when(sanStrategy.createAccessGroup(any())).thenReturn(createdAccessGroup);
634-
when(sanStrategy.ensureLunMapped(anyString(), anyString(), anyString())).thenReturn("0");
634+
doNothing().when(sanStrategy).ensureLunMapped(anyString(), anyString(), anyString());
635635
when(sanStrategy.getLunSerialNumber(anyString(), anyString())).thenReturn(LUN_SERIAL_NUMBER);
636636

637637
// Execute
@@ -1519,7 +1519,7 @@ void testGrantAccess_Template_WritesInstallPathAndRefreshesStoreTarget() {
15191519
utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1");
15201520

15211521
when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup);
1522-
when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3");
1522+
doNothing().when(sanStrategy).ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"));
15231523
when(sanStrategy.getLunSerialNumber(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"))).thenReturn(LUN_SERIAL_NUMBER);
15241524

15251525
assertTrue(driver.grantAccess(templateInfo, host, primaryDataStore));
@@ -1575,7 +1575,7 @@ void testGrantAccess_Template_UnmapsWhenInstallPathUpdateFails() {
15751575
utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1");
15761576

15771577
when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup);
1578-
when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3");
1578+
doNothing().when(sanStrategy).ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"));
15791579
when(sanStrategy.getLunSerialNumber(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"))).thenReturn(LUN_SERIAL_NUMBER);
15801580
when(sanStrategy.getCloudStackVolume(argThat(map ->
15811581
map != null && "/vol/vol1/cs_tmpl_50".equals(map.get("name")))))
@@ -2874,7 +2874,7 @@ void testGrantAccess_Template_CreatesIgroupWhenMissing() {
28742874

28752875
when(sanStrategy.getAccessGroup(any())).thenReturn(null);
28762876
when(sanStrategy.createAccessGroup(any())).thenReturn(createdAccessGroup);
2877-
when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3");
2877+
doNothing().when(sanStrategy).ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"));
28782878
when(sanStrategy.getLunSerialNumber(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"))).thenReturn(LUN_SERIAL_NUMBER);
28792879

28802880
assertTrue(driver.grantAccess(templateInfo, hostVo, primaryDataStore));
@@ -3749,7 +3749,7 @@ void testGrantAccess_TemporarySnapshotCopy_SyncsIscsiPathOnInMemoryVolume() {
37493749
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy);
37503750
utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1");
37513751
when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup);
3752-
when(sanStrategy.ensureLunMapped(anyString(), anyString(), anyString())).thenReturn("0");
3752+
doNothing().when(sanStrategy).ensureLunMapped(anyString(), anyString(), anyString());
37533753
when(sanStrategy.getLunSerialNumber(anyString(), anyString())).thenReturn(LUN_SERIAL_NUMBER);
37543754

37553755
assertTrue(driver.grantAccess(volumeInfo, host, dataStore));
Collapse file

‎plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java‎

Copy file name to clipboardExpand all lines: plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java
+3-4Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,17 +203,16 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
203203
}
204204

205205
@Override
206-
public String enableLogicalAccess(Map<String, String> values) {
207-
return null;
206+
public void enableLogicalAccess(Map<String, String> values) {
208207
}
209208

210209
@Override
211210
public void disableLogicalAccess(Map<String, String> values) {
212211
}
213212

214213
@Override
215-
public String getLogicalAccess(Map<String, String> values) {
216-
return null;
214+
public boolean getLogicalAccess(Map<String, String> values) {
215+
return false;
217216
}
218217
}
219218

0 commit comments

Comments
0 (0)
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.