Skip to content

Commit cc6af4a

Browse files
Gupta, SuryaGupta, Surya
Gupta, Surya
authored and
Gupta, Surya
committed
CSTACKEX-35 Resolved review comments
1 parent c968ccc commit cc6af4a

File tree

3 files changed

+33
-25
lines changed

3 files changed

+33
-25
lines changed

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

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@
4444
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
4545
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
4646
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
47-
import org.apache.cloudstack.storage.feign.model.OntapStorage;
48-
import org.apache.cloudstack.storage.provider.StorageProviderFactory;
4947
import org.apache.cloudstack.storage.service.StorageStrategy;
5048
import org.apache.cloudstack.storage.service.model.CloudStackVolume;
5149
import org.apache.cloudstack.storage.service.model.ProtocolType;
@@ -101,7 +99,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
10199
throw new InvalidParameterValueException("createAsync: callback should not be null");
102100
}
103101
try {
104-
s_logger.info("createAsync: Volume creation starting for data store [{}] and data object [{}] of type [{}]",
102+
s_logger.info("createAsync: Started for data store [{}] and data object [{}] of type [{}]",
105103
dataStore, dataObject, dataObject.getType());
106104
if (dataObject.getType() == DataObjectType.VOLUME) {
107105
path = createCloudStackVolumeForTypeVolume(dataStore, dataObject);
@@ -113,7 +111,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
113111
}
114112
} catch (Exception e) {
115113
errMsg = e.getMessage();
116-
s_logger.error("createAsync: Volume creation failed for dataObject [{}]: {}", dataObject, errMsg);
114+
s_logger.error("createAsync: Failed for dataObject [{}]: {}", dataObject, errMsg);
117115
createCmdResult = new CreateCmdResult(null, new Answer(null, false, errMsg));
118116
createCmdResult.setResult(e.toString());
119117
} finally {
@@ -124,31 +122,22 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
124122
private String createCloudStackVolumeForTypeVolume(DataStore dataStore, DataObject dataObject) {
125123
StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId());
126124
if(storagePool == null) {
125+
s_logger.error("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId());
127126
throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId());
128127
}
129128
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId());
130129
if(details == null || details.isEmpty()) {
130+
s_logger.error("createCloudStackVolume : Storage Details not found for id: " + dataStore.getId());
131131
throw new CloudRuntimeException("createCloudStackVolume : Storage Details not found for id: " + dataStore.getId());
132132
}
133-
String protocol = details.get(Constants.PROTOCOL);
134-
OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD),
135-
details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol),
136-
Boolean.parseBoolean(details.get(Constants.IS_DISAGGREGATED)));
137-
StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage);
138-
boolean isValid = storageStrategy.connect();
139-
if (isValid) {
140-
s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME));
141-
CloudStackVolume cloudStackVolumeRequest = utils.createCloudStackVolumeRequestByProtocol(storagePool, details, dataObject);
142-
CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest);
143-
if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) {
144-
return cloudStackVolume.getLun().getName();
145-
} else {
146-
String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + dataObject;
147-
s_logger.error(errMsg);
148-
throw new CloudRuntimeException(errMsg);
149-
}
133+
StorageStrategy storageStrategy = utils.getStrategyByStoragePoolDetails(details);
134+
s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME));
135+
CloudStackVolume cloudStackVolumeRequest = utils.createCloudStackVolumeRequestByProtocol(storagePool, details, dataObject);
136+
CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest);
137+
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL)) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) {
138+
return cloudStackVolume.getLun().getName();
150139
} else {
151-
String errMsg = "createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed";
140+
String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + dataObject;
152141
s_logger.error(errMsg);
153142
throw new CloudRuntimeException(errMsg);
154143
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,15 @@ public UnifiedSANStrategy(OntapStorage ontapStorage) {
4848
public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume) {
4949
s_logger.info("createCloudStackVolume : Creating Lun with cloudstackVolume request {} ", cloudstackVolume);
5050
if (cloudstackVolume == null || cloudstackVolume.getLun() == null) {
51-
s_logger.error("createCloudStackVolume: LUN creation failed. Invalid cloudstackVolume request: {}", cloudstackVolume);
52-
throw new CloudRuntimeException("createCloudStackVolume : Failed to create Lun, invalid cloudstackVolume request");
51+
s_logger.error("createCloudStackVolume: LUN creation failed. Invalid request: {}", cloudstackVolume);
52+
throw new CloudRuntimeException("createCloudStackVolume : Failed to create Lun, invalid request");
5353
}
5454
try {
5555
// Get AuthHeader
5656
String authHeader = utils.generateAuthHeader(storage.getUsername(), storage.getPassword());
5757
// Create URI for lun creation
5858
URI url = utils.generateURI(Constants.CREATE_LUN);
59+
//TODO: there is possible that Lun creation will take time and we may need to handle through async job.
5960
OntapResponse<Lun> createdLun = sanFeignClient.createLun(url, authHeader, true, cloudstackVolume.getLun());
6061
if (createdLun == null || createdLun.getRecords() == null || createdLun.getRecords().size() == 0) {
6162
s_logger.error("createCloudStackVolume: LUN creation failed for Lun {}", cloudstackVolume.getLun().getName());

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.apache.cloudstack.storage.feign.model.LunSpace;
3030
import org.apache.cloudstack.storage.feign.model.OntapStorage;
3131
import org.apache.cloudstack.storage.feign.model.Svm;
32+
import org.apache.cloudstack.storage.provider.StorageProviderFactory;
33+
import org.apache.cloudstack.storage.service.StorageStrategy;
3234
import org.apache.cloudstack.storage.service.model.CloudStackVolume;
3335
import org.apache.cloudstack.storage.service.model.ProtocolType;
3436
import org.apache.logging.log4j.LogManager;
@@ -82,7 +84,7 @@ public CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO st
8284
LunSpace lunSpace = new LunSpace();
8385
lunSpace.setSize(dataObject.getSize());
8486
lunRequest.setSpace(lunSpace);
85-
87+
//Lun name is full path like in unified "/vol/VolumeName/LunName"
8688
String lunFullName = Constants.VOLUME_PATH_PREFIX + storagePool.getName() + Constants.PATH_SEPARATOR + dataObject.getName();
8789
lunRequest.setName(lunFullName);
8890

@@ -105,4 +107,20 @@ public CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO st
105107
throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol);
106108
}
107109
}
110+
111+
public StorageStrategy getStrategyByStoragePoolDetails(Map<String, String> details) {
112+
String protocol = details.get(Constants.PROTOCOL);
113+
OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD),
114+
details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol),
115+
Boolean.parseBoolean(details.get(Constants.IS_DISAGGREGATED)));
116+
StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage);
117+
boolean isValid = storageStrategy.connect();
118+
if (isValid) {
119+
s_logger.info("Connection to Ontap SVM [{}] successful", details.get(Constants.SVM_NAME));
120+
return storageStrategy;
121+
} else {
122+
s_logger.error("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed");
123+
throw new CloudRuntimeException("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed");
124+
}
125+
}
108126
}

0 commit comments

Comments
 (0)