Skip to content

CSTACKEX-191: logical access methods can have return type as primitive #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rajiv-jain-netapp wants to merge 1 commit into
base: main
Choose a base branch
from
Open

CSTACKEX-191: logical access methods can have return type as primitive #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...e/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ public class VolumeConcise {
private String uuid;

@JsonProperty("name")

private String name;

public String getUuid() {
Expand All @@ -46,5 +45,6 @@ public String getName() {
}

public void setName(String name) {
this.name = name; }
this.name = name;
}
}
2 changes: 1 addition & 1 deletion ...src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust
throw new CloudRuntimeException("Storage pool name is null or empty, cannot create primary storage");
}

if (StringUtils.isBlank(providerName )) {
if (StringUtils.isBlank(providerName)) {
throw new CloudRuntimeException("Provider name is null or empty, cannot create primary storage");
}

Expand Down
4 changes: 2 additions & 2 deletions ...age/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam
* @param values map including SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS
* @return map containing logical unit number for the new/existing mapping (SAN) or relevant info for NAS
*/
abstract public Map<String,String> enableLogicalAccess(Map<String,String> values);
abstract public String enableLogicalAccess(Map<String,String> values);

/**
* Method encapsulates the behavior based on the opted protocol in subclasses
Expand All @@ -610,7 +610,7 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam
* @param values map with SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS
* @return map containing logical unit number if mapping exists; otherwise null
*/
abstract public Map<String, String> getLogicalAccess(Map<String, String> values);
abstract public String getLogicalAccess(Map<String, String> values);

// ── FlexVolume Snapshot accessors ────────────────────────────────────────

Expand Down
6 changes: 3 additions & 3 deletions .../volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
}

@Override
public Map <String, String> enableLogicalAccess(Map<String, String> values) {
public String enableLogicalAccess(Map<String, String> values) {
return null;
}

Expand All @@ -211,8 +211,8 @@ public void disableLogicalAccess(Map<String, String> values) {
}

@Override
public Map<String, String> getLogicalAccess(Map<String, String> values) {
return Map.of();
public String getLogicalAccess(Map<String, String> values) {
return null;
}

private ExportPolicy createExportPolicy(String svmName, ExportPolicy policy) {
Expand Down
31 changes: 14 additions & 17 deletions .../volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,10 +383,10 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
}
}

public Map<String, String> enableLogicalAccess(Map<String, String> values) {
public String enableLogicalAccess(Map<String, String> values) {
logger.info("enableLogicalAccess : Create LunMap");
logger.debug("enableLogicalAccess : Creating LunMap with values {} ", values);
Map<String, String> response = null;
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");
Expand Down Expand Up @@ -435,9 +435,7 @@ public Map<String, String> enableLogicalAccess(Map<String, String> values) {
OntapStorageConstants.IGROUP_DOT_NAME, igroupName,
OntapStorageConstants.FIELDS, OntapStorageConstants.LOGICAL_UNIT_NUMBER
));
response = Map.of(
OntapStorageConstants.LOGICAL_UNIT_NUMBER, lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString()
);
lunNumber = lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString();
} catch (Exception e) {
logger.error("enableLogicalAccess: Failed to fetch LunMap details for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e);
throw new CloudRuntimeException("Failed to fetch LunMap details for Lun: " + lunName + " and igroup: " + igroupName);
Expand All @@ -448,7 +446,7 @@ public Map<String, String> enableLogicalAccess(Map<String, String> values) {
logger.error("Exception occurred while creating LunMap", e);
throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage());
}
return response;
return lunNumber;
}

public void disableLogicalAccess(Map<String, String> values) {
Expand Down Expand Up @@ -482,8 +480,9 @@ public void disableLogicalAccess(Map<String, String> values) {
}

// GET-only helper: fetch LUN-map and return logical unit number if it exists; otherwise return null
public Map<String, String> getLogicalAccess(Map<String, String> values) {
public String getLogicalAccess(Map<String, String> values) {
logger.info("getLogicalAccess : Fetch LunMap");
String lunNumber = null;
logger.debug("getLogicalAccess : Fetching LunMap with values {} ", values);
if (values == null) {
logger.error("getLogicalAccess: Invalid request values: null");
Expand All @@ -507,13 +506,12 @@ public Map<String, String> getLogicalAccess(Map<String, String> values) {
));
if (lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty()) {
Integer lunLogicalUnitNum = lunMapResponse.getRecords().get(0).getLogicalUnitNumber();
String lunNumber = lunLogicalUnitNum != null ? lunLogicalUnitNum.toString() : null;
return lunNumber != null ? Map.of(OntapStorageConstants.LOGICAL_UNIT_NUMBER, lunNumber) : null;
lunNumber = lunLogicalUnitNum != null ? lunLogicalUnitNum.toString() : null;
}
} catch (Exception e) {
logger.warn("getLogicalAccess: LunMap not found for Lun: {} and igroup: {} ({}).", lunName, igroupName, e.getMessage());
}
return null;
return lunNumber;
}

@Override
Expand All @@ -526,9 +524,8 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup
OntapStorageConstants.SVM_DOT_NAME, svmName,
OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName
);
Map<String, String> mapResp = getLogicalAccess(getMap);
if (mapResp != null && mapResp.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER)) {
String lunNumber = mapResp.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER);
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;
}
Expand All @@ -539,12 +536,12 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup
OntapStorageConstants.SVM_DOT_NAME, svmName,
OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName
);
Map<String, String> response = enableLogicalAccess(enableMap);
if (response == null || !response.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER)) {
String response = enableLogicalAccess(enableMap);
if (response == null ) {
throw new CloudRuntimeException("Failed to map LUN [" + lunName + "] to iGroup [" + accessGroupName + "]");
}
logger.info("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER));
return response.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER);
logger.info("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response);
return response;
}
/**
* Reverts a LUN to a snapshot using the ONTAP CLI-based snapshot file restore API.
Expand Down
4 changes: 2 additions & 2 deletions ...volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
}

@Override
public Map<String, String> enableLogicalAccess(Map<String, String> values) {
public String enableLogicalAccess(Map<String, String> values) {
return null;
}

Expand All @@ -178,7 +178,7 @@ public void disableLogicalAccess(Map<String, String> values) {
}

@Override
public Map<String, String> getLogicalAccess(Map<String, String> values) {
public String getLogicalAccess(Map<String, String> values) {
return null;
}
}
Expand Down
17 changes: 7 additions & 10 deletions ...ume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,12 +516,10 @@ void testEnableLogicalAccess_Success() {
when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response);

// Execute
Map<String, String> result = unifiedSANStrategy.enableLogicalAccess(values);
String result = unifiedSANStrategy.enableLogicalAccess(values);

// Verify
assertNotNull(result);
assertTrue(result.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER));
assertEquals("0", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER));

verify(sanFeignClient).createLunMap(eq(authHeader), eq(true), any(LunMap.class));
}
Expand Down Expand Up @@ -550,11 +548,10 @@ void testEnableLogicalAccess_AlreadyMapped_ReturnsLunNumber() {
when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response);

// Execute
Map<String, String> result = unifiedSANStrategy.enableLogicalAccess(values);
String result = unifiedSANStrategy.enableLogicalAccess(values);

// Verify
assertNotNull(result);
assertEquals("5", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER));
}
}

Expand Down Expand Up @@ -621,11 +618,11 @@ void testGetLogicalAccess_Success() {
when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response);

// Execute
Map<String, String> result = unifiedSANStrategy.getLogicalAccess(values);
String result = unifiedSANStrategy.getLogicalAccess(values);

// Verify
assertNotNull(result);
assertEquals("3", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER));
assertEquals("3", result);
}
}

Expand All @@ -645,7 +642,7 @@ void testGetLogicalAccess_NotFound_ReturnsNull() {
.thenThrow(new RuntimeException("Not found"));

// Execute
Map<String, String> result = unifiedSANStrategy.getLogicalAccess(values);
String result = unifiedSANStrategy.getLogicalAccess(values);

// Verify
assertNull(result);
Expand Down Expand Up @@ -1671,7 +1668,7 @@ void testGetLogicalAccess_EmptyResponse_ReturnsNull() {

when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(emptyResponse);

Map<String, String> result = unifiedSANStrategy.getLogicalAccess(values);
String result = unifiedSANStrategy.getLogicalAccess(values);

assertNull(result);
}
Expand All @@ -1691,7 +1688,7 @@ void testGetLogicalAccess_ExceptionThrown_ReturnsNull() {
when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap()))
.thenThrow(new RuntimeException("Connection failed"));

Map<String, String> result = unifiedSANStrategy.getLogicalAccess(values);
String result = unifiedSANStrategy.getLogicalAccess(values);

assertNull(result);
}
Expand Down
Loading