Skip to content

Commit b88a6cb

Browse files
Locharla, SandeepLocharla, Sandeep
Locharla, Sandeep
authored and
Locharla, Sandeep
committed
CSTACKEX-46: Added UTs and variables to pass maxRetries and delay to job polling method
1 parent 892706a commit b88a6cb

File tree

8 files changed

+2461
-54
lines changed

8 files changed

+2461
-54
lines changed

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,6 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
115115
if (dataStore == null) {
116116
throw new InvalidParameterValueException("createAsync: dataStore should not be null");
117117
}
118-
if (dataObject == null) {
119-
throw new InvalidParameterValueException("createAsync: dataObject should not be null");
120-
}
121118
if (callback == null) {
122119
throw new InvalidParameterValueException("createAsync: callback should not be null");
123120
}
@@ -154,11 +151,6 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
154151
throw new CloudRuntimeException("createAsync: Missing LUN name for volume " + volInfo.getId());
155152
}
156153

157-
// Determine scope ID based on storage pool scope (cluster or zone level igroup)
158-
long scopeId = (storagePool.getScope() == ScopeType.CLUSTER)
159-
? storagePool.getClusterId()
160-
: storagePool.getDataCenterId();
161-
162154
// Persist LUN details for future operations (delete, grant/revoke access)
163155
volumeDetailsDao.addDetail(volInfo.getId(), Constants.LUN_DOT_UUID, created.getLun().getUuid(), false);
164156
volumeDetailsDao.addDetail(volInfo.getId(), Constants.LUN_DOT_NAME, lunName, false);
@@ -348,7 +340,6 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
348340
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
349341
String svmName = details.get(Constants.SVM_NAME);
350342
String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue();
351-
long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId();
352343

353344
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
354345
UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details);
@@ -455,7 +446,6 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
455446
StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details);
456447
String svmName = details.get(Constants.SVM_NAME);
457448
String storagePoolUuid = storagePool.getUuid();
458-
long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId();
459449

460450
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
461451
String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid);

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ public Volume createStorageVolume(String volumeName, Long size) {
232232
String jobUUID = jobResponse.getJob().getUuid();
233233

234234
//Create URI for GET Job API
235-
Boolean jobSucceeded = jobPollForSuccess(jobUUID);
235+
Boolean jobSucceeded = jobPollForSuccess(jobUUID, 10, 1);
236236
if (!jobSucceeded) {
237237
s_logger.error("Volume creation job failed for volume: " + volumeName);
238238
throw new CloudRuntimeException("Volume creation job failed for volume: " + volumeName);
@@ -318,7 +318,7 @@ public void deleteStorageVolume(Volume volume) {
318318
try {
319319
// TODO: Implement lun and file deletion, if any, before deleting the volume
320320
JobResponse jobResponse = volumeFeignClient.deleteVolume(authHeader, volume.getUuid());
321-
Boolean jobSucceeded = jobPollForSuccess(jobResponse.getJob().getUuid());
321+
Boolean jobSucceeded = jobPollForSuccess(jobResponse.getJob().getUuid(), 10, 1);
322322
if (!jobSucceeded) {
323323
s_logger.error("Volume deletion job failed for volume: " + volume.getName());
324324
throw new CloudRuntimeException("Volume deletion job failed for volume: " + volume.getName());
@@ -569,14 +569,14 @@ public String getNetworkInterface() {
569569
*/
570570
abstract public Map<String, String> getLogicalAccess(Map<String, String> values);
571571

572-
private Boolean jobPollForSuccess(String jobUUID) {
572+
private Boolean jobPollForSuccess(String jobUUID, int maxRetries, int sleepTimeInSecs) {
573573
//Create URI for GET Job API
574574
int jobRetryCount = 0;
575575
Job jobResp = null;
576576
try {
577577
String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword());
578578
while (jobResp == null || !jobResp.getState().equals(Constants.JOB_SUCCESS)) {
579-
if (jobRetryCount >= Constants.JOB_MAX_RETRIES) {
579+
if (jobRetryCount >= maxRetries) {
580580
s_logger.error("Job did not complete within expected time.");
581581
throw new CloudRuntimeException("Job did not complete within expected time.");
582582
}
@@ -593,7 +593,7 @@ private Boolean jobPollForSuccess(String jobUUID) {
593593
}
594594

595595
jobRetryCount++;
596-
Thread.sleep(Constants.CREATE_VOLUME_CHECK_SLEEP_TIME); // Sleep for 2 seconds before polling again
596+
Thread.sleep(sleepTimeInSecs * 1000);
597597
}
598598
if (jobResp == null || !jobResp.getState().equals(Constants.JOB_SUCCESS)) {
599599
return false;

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,11 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {
132132

133133
@Override
134134
public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) {
135-
s_logger.debug("copyCloudStackVolume: Creating clone of the cloudstack volume: {}", cloudstackVolume.getLun().getName());
136135
if (cloudstackVolume == null || cloudstackVolume.getLun() == null) {
137136
s_logger.error("copyCloudStackVolume: Lun clone creation failed. Invalid request: {}", cloudstackVolume);
138137
throw new CloudRuntimeException("copyCloudStackVolume : Failed to create Lun clone, invalid request");
139138
}
139+
s_logger.debug("copyCloudStackVolume: Creating clone of the cloudstack volume: {}", cloudstackVolume.getLun().getName());
140140

141141
try {
142142
// Get AuthHeader
@@ -451,6 +451,10 @@ public Map<String, String> enableLogicalAccess(Map<String, String> values) {
451451
s_logger.info("enableLogicalAccess : Create LunMap");
452452
s_logger.debug("enableLogicalAccess : Creating LunMap with values {} ", values);
453453
Map<String, String> response = null;
454+
if (values == null) {
455+
s_logger.error("enableLogicalAccess: LunMap creation failed. Invalid request values: null");
456+
throw new CloudRuntimeException("enableLogicalAccess : Failed to create LunMap, invalid request");
457+
}
454458
String svmName = values.get(Constants.SVM_DOT_NAME);
455459
String lunName = values.get(Constants.LUN_DOT_NAME);
456460
String igroupName = values.get(Constants.IGROUP_DOT_NAME);
@@ -514,6 +518,10 @@ public Map<String, String> enableLogicalAccess(Map<String, String> values) {
514518
public void disableLogicalAccess(Map<String, String> values) {
515519
s_logger.info("disableLogicalAccess : Delete LunMap");
516520
s_logger.debug("disableLogicalAccess : Deleting LunMap with values {} ", values);
521+
if (values == null) {
522+
s_logger.error("disableLogicalAccess: LunMap deletion failed. Invalid request values: null");
523+
throw new CloudRuntimeException("disableLogicalAccess : Failed to delete LunMap, invalid request");
524+
}
517525
String lunUUID = values.get(Constants.LUN_DOT_UUID);
518526
String igroupUUID = values.get(Constants.IGROUP_DOT_UUID);
519527
if (lunUUID == null || igroupUUID == null || lunUUID.isEmpty() || igroupUUID.isEmpty()) {
@@ -541,6 +549,10 @@ public void disableLogicalAccess(Map<String, String> values) {
541549
public Map<String, String> getLogicalAccess(Map<String, String> values) {
542550
s_logger.info("getLogicalAccess : Fetch LunMap");
543551
s_logger.debug("getLogicalAccess : Fetching LunMap with values {} ", values);
552+
if (values == null) {
553+
s_logger.error("getLogicalAccess: Invalid request values: null");
554+
throw new CloudRuntimeException("getLogicalAccess : Invalid request");
555+
}
544556
String svmName = values.get(Constants.SVM_DOT_NAME);
545557
String lunName = values.get(Constants.LUN_DOT_NAME);
546558
String igroupName = values.get(Constants.IGROUP_DOT_NAME);

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,6 @@ public class Constants {
6666
public static final String SERVICES = "services";
6767
public static final String RETURN_RECORDS = "return_records";
6868

69-
public static final int JOB_MAX_RETRIES = 100;
70-
public static final int CREATE_VOLUME_CHECK_SLEEP_TIME = 2000;
71-
7269
public static final String SLASH = "/";
7370
public static final String EQUALS = "=";
7471
public static final String SEMICOLON = ";";

0 commit comments

Comments
 (0)