Skip to content

Commit 54a7eef

Browse files
Gupta, SuryaGupta, Surya
authored andcommitted
CSTACKEX-46 Resolve review comments
1 parent ad4e526 commit 54a7eef

File tree

4 files changed

+103
-100
lines changed

4 files changed

+103
-100
lines changed

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

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@
4747
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
4848
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
4949
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
50-
import org.apache.cloudstack.storage.feign.model.Igroup;
51-
import org.apache.cloudstack.storage.feign.model.Initiator;
50+
import org.apache.cloudstack.storage.feign.model.*;
5251
import org.apache.cloudstack.storage.service.StorageStrategy;
5352
import org.apache.cloudstack.storage.service.model.AccessGroup;
5453
import org.apache.cloudstack.storage.service.model.CloudStackVolume;
@@ -113,7 +112,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
113112
throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId());
114113
}
115114
if (dataObject.getType() == DataObjectType.VOLUME) {
116-
path = createCloudStackVolumeForTypeVolume(storagePool, dataObject);
115+
path = createCloudStackVolumeForTypeVolume(storagePool, (VolumeInfo)dataObject);
117116
createCmdResult = new CreateCmdResult(path, new Answer(null, true, null));
118117
} else {
119118
errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync";
@@ -130,21 +129,59 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
130129
}
131130
}
132131

133-
private String createCloudStackVolumeForTypeVolume(StoragePoolVO storagePool, DataObject dataObject) {
132+
private String createCloudStackVolumeForTypeVolume(StoragePoolVO storagePool, VolumeInfo volumeInfo) {
134133
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
135134
StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details);
136135
s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME));
137-
CloudStackVolume cloudStackVolumeRequest = Utility.createCloudStackVolumeRequestByProtocol(storagePool, details, dataObject);
136+
CloudStackVolume cloudStackVolumeRequest = createCloudStackVolumeRequestByProtocol(storagePool, details, volumeInfo);
138137
CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest);
139138
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL)) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) {
140139
return cloudStackVolume.getLun().getName();
141140
} else {
142-
String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + dataObject;
141+
String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + volumeInfo;
143142
s_logger.error(errMsg);
144143
throw new CloudRuntimeException(errMsg);
145144
}
146145
}
147146

147+
private CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map<String, String> details, VolumeInfo volumeInfo) {
148+
CloudStackVolume cloudStackVolumeRequest = null;
149+
150+
String protocol = details.get(Constants.PROTOCOL);
151+
if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) {
152+
cloudStackVolumeRequest = new CloudStackVolume();
153+
Lun lunRequest = new Lun();
154+
Svm svm = new Svm();
155+
svm.setName(details.get(Constants.SVM_NAME));
156+
lunRequest.setSvm(svm);
157+
158+
LunSpace lunSpace = new LunSpace();
159+
lunSpace.setSize(volumeInfo.getSize());
160+
lunRequest.setSpace(lunSpace);
161+
//Lun name is full path like in unified "/vol/VolumeName/LunName"
162+
String lunFullName = Utility.getLunName(storagePool.getName(), volumeInfo.getName());
163+
lunRequest.setName(lunFullName);
164+
165+
String hypervisorType = storagePool.getHypervisor().name();
166+
String osType = null;
167+
switch (hypervisorType) {
168+
case Constants.KVM:
169+
osType = Lun.OsTypeEnum.LINUX.getValue();
170+
break;
171+
default:
172+
String errMsg = "createCloudStackVolume : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage";
173+
s_logger.error(errMsg);
174+
throw new CloudRuntimeException(errMsg);
175+
}
176+
lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType));
177+
178+
cloudStackVolumeRequest.setLun(lunRequest);
179+
return cloudStackVolumeRequest;
180+
} else {
181+
throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol);
182+
}
183+
}
184+
148185
@Override
149186
public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallback<CommandResult> callback) {
150187

@@ -219,6 +256,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
219256
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
220257
StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details);
221258
String svmName = details.get(Constants.SVM_NAME);
259+
//TODO- verify scopeId DatacenterId is zoneId
222260
long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId();
223261

224262
if(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
@@ -305,7 +343,7 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
305343
//TODO check if initiator does exits in igroup, will throw the error ?
306344
if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) {
307345
s_logger.error("revokeAccessForVolume: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName);
308-
throw new CloudRuntimeException("revokeAccessForVolume: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName);
346+
return;
309347
}
310348

311349
Map<String, String> disableLogicalAccessMap = new HashMap<>();

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@
3333
public interface SANFeignClient {
3434

3535
// LUN Operation APIs
36-
@RequestLine("POST /")
36+
@RequestLine("POST /api/storage/luns")
3737
@Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"})
3838
OntapResponse<Lun> createLun(@Param("authHeader") String authHeader,
3939
@Param("returnRecords") boolean returnRecords,
4040
Lun lun);
4141

42-
@RequestLine("GET /")
42+
@RequestLine("GET /api/storage/luns")
4343
@Headers({"Authorization: {authHeader}"})
4444
OntapResponse<Lun> getLunResponse(@Param("authHeader") String authHeader, @QueryMap Map<String, Object> queryMap);
4545

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@
4343
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
4444
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
4545
import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl;
46+
import org.apache.cloudstack.storage.feign.model.Igroup;
47+
import org.apache.cloudstack.storage.feign.model.Initiator;
4648
import org.apache.cloudstack.storage.feign.model.OntapStorage;
49+
import org.apache.cloudstack.storage.feign.model.Svm;
4750
import org.apache.cloudstack.storage.provider.StorageProviderFactory;
4851
import org.apache.cloudstack.storage.service.StorageStrategy;
4952
import org.apache.cloudstack.storage.service.model.AccessGroup;
@@ -271,13 +274,13 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
271274
StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details);
272275
ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL));
273276
//TODO- Check if we have to handle heterogeneous host within the cluster
274-
if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) {
277+
if (!validateProtocolSupportAndFetchHostsIndentifier(hostsToConnect, protocol, hostsIdentifier)) {
275278
s_logger.error("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name());
276279
throw new CloudRuntimeException("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name());
277280
}
278281
//TODO - check if no host to connect then also need to create access group without initiators
279282
if (hostsIdentifier != null && hostsIdentifier.size() > 0) {
280-
AccessGroup accessGroupRequest = Utility.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier);
283+
AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier);
281284
strategy.createAccessGroup(accessGroupRequest);
282285
}
283286
logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId());
@@ -320,12 +323,12 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
320323
StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details);
321324
ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL));
322325
//TODO- Check if we have to handle heterogeneous host within the zone
323-
if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) {
326+
if (!validateProtocolSupportAndFetchHostsIndentifier(hostsToConnect, protocol, hostsIdentifier)) {
324327
s_logger.error("attachZone: Not all hosts in the zone support the protocol: " + protocol.name());
325328
throw new CloudRuntimeException("attachZone: Not all hosts in the zone support the protocol: " + protocol.name());
326329
}
327330
if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) {
328-
AccessGroup accessGroupRequest = Utility.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier);
331+
AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier);
329332
strategy.createAccessGroup(accessGroupRequest);
330333
}
331334
for (HostVO host : hostsToConnect) {
@@ -339,7 +342,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
339342
return true;
340343
}
341344

342-
private boolean isProtocolSupportedByAllHosts(List<HostVO> hosts, ProtocolType protocolType, List<String> hostIdentifiers) {
345+
private boolean validateProtocolSupportAndFetchHostsIndentifier(List<HostVO> hosts, ProtocolType protocolType, List<String> hostIdentifiers) {
343346
switch (protocolType) {
344347
case ISCSI:
345348
String protocolPrefix = Constants.IQN;
@@ -357,6 +360,53 @@ private boolean isProtocolSupportedByAllHosts(List<HostVO> hosts, ProtocolType p
357360
return true;
358361
}
359362

363+
private AccessGroup createAccessGroupRequestByProtocol(StoragePoolVO storagePool, long scopeId, Map<String, String> details, List<String> hostsIdentifier) {
364+
ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toLowerCase());
365+
String svmName = details.get(Constants.SVM_NAME);
366+
switch (protocol) {
367+
case ISCSI:
368+
// Access group name format: cs_svmName_scopeId
369+
String igroupName = Utility.getIgroupName(svmName, scopeId);
370+
Hypervisor.HypervisorType hypervisorType = storagePool.getHypervisor();
371+
return createSANAccessGroupRequest(svmName, igroupName, hypervisorType, hostsIdentifier);
372+
default:
373+
s_logger.error("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol);
374+
throw new CloudRuntimeException("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol);
375+
}
376+
}
377+
378+
private AccessGroup createSANAccessGroupRequest(String svmName, String igroupName, Hypervisor.HypervisorType hypervisorType, List<String> hostsIdentifier) {
379+
AccessGroup accessGroupRequest = new AccessGroup();
380+
Igroup igroup = new Igroup();
381+
382+
if (svmName != null && !svmName.isEmpty()) {
383+
Svm svm = new Svm();
384+
svm.setName(svmName);
385+
igroup.setSvm(svm);
386+
}
387+
388+
if (igroupName != null && !igroupName.isEmpty()) {
389+
igroup.setName(igroupName);
390+
}
391+
392+
if (hypervisorType != null) {
393+
String hypervisorName = hypervisorType.name();
394+
igroup.setOsType(Igroup.OsTypeEnum.valueOf(Utility.getOSTypeFromHypervisor(hypervisorName)));
395+
}
396+
397+
if (hostsIdentifier != null && hostsIdentifier.size() > 0) {
398+
List<Initiator> initiators = new ArrayList<>();
399+
for (String hostIdentifier : hostsIdentifier) {
400+
Initiator initiator = new Initiator();
401+
initiator.setName(hostIdentifier);
402+
initiators.add(initiator);
403+
}
404+
igroup.setInitiators(initiators);
405+
}
406+
accessGroupRequest.setIgroup(igroup);
407+
return accessGroupRequest;
408+
}
409+
360410
@Override
361411
public boolean maintain(DataStore store) {
362412
return true;

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

Lines changed: 1 addition & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -62,44 +62,6 @@ public static String generateAuthHeader (String username, String password) {
6262
return BASIC + StringUtils.SPACE + new String(encodedBytes);
6363
}
6464

65-
public static CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map<String, String> details, DataObject dataObject) {
66-
CloudStackVolume cloudStackVolumeRequest = null;
67-
68-
String protocol = details.get(Constants.PROTOCOL);
69-
if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) {
70-
cloudStackVolumeRequest = new CloudStackVolume();
71-
Lun lunRequest = new Lun();
72-
Svm svm = new Svm();
73-
svm.setName(details.get(Constants.SVM_NAME));
74-
lunRequest.setSvm(svm);
75-
76-
LunSpace lunSpace = new LunSpace();
77-
lunSpace.setSize(dataObject.getSize());
78-
lunRequest.setSpace(lunSpace);
79-
//Lun name is full path like in unified "/vol/VolumeName/LunName"
80-
String lunFullName = getLunName(storagePool.getName(), dataObject.getName());
81-
lunRequest.setName(lunFullName);
82-
83-
String hypervisorType = storagePool.getHypervisor().name();
84-
String osType = null;
85-
switch (hypervisorType) {
86-
case Constants.KVM:
87-
osType = Lun.OsTypeEnum.LINUX.getValue();
88-
break;
89-
default:
90-
String errMsg = "createCloudStackVolume : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage";
91-
s_logger.error(errMsg);
92-
throw new CloudRuntimeException(errMsg);
93-
}
94-
lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType));
95-
96-
cloudStackVolumeRequest.setLun(lunRequest);
97-
return cloudStackVolumeRequest;
98-
} else {
99-
throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol);
100-
}
101-
}
102-
10365
public static String getOSTypeFromHypervisor(String hypervisorType){
10466
switch (hypervisorType) {
10567
case Constants.KVM:
@@ -131,54 +93,7 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map<String, String
13193
}
13294
}
13395

134-
public static AccessGroup createAccessGroupRequestByProtocol(StoragePoolVO storagePool, long scopeId, Map<String, String> details, List<String> hostsIdentifier) {
135-
ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toLowerCase());
136-
String svmName = details.get(Constants.SVM_NAME);
137-
switch (protocol) {
138-
case ISCSI:
139-
// Access group name format: cs_svmName_scopeId
140-
String igroupName = getIgroupName(svmName, scopeId);
141-
Hypervisor.HypervisorType hypervisorType = storagePool.getHypervisor();
142-
return createSANAccessGroupRequest(svmName, igroupName, hypervisorType, hostsIdentifier);
143-
default:
144-
s_logger.error("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol);
145-
throw new CloudRuntimeException("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol);
146-
}
147-
}
148-
149-
public static AccessGroup createSANAccessGroupRequest(String svmName, String igroupName, Hypervisor.HypervisorType hypervisorType, List<String> hostsIdentifier) {
150-
AccessGroup accessGroupRequest = new AccessGroup();
151-
Igroup igroup = new Igroup();
152-
153-
if (svmName != null && !svmName.isEmpty()) {
154-
Svm svm = new Svm();
155-
svm.setName(svmName);
156-
igroup.setSvm(svm);
157-
}
158-
159-
if (igroupName != null && !igroupName.isEmpty()) {
160-
igroup.setName(igroupName);
161-
}
162-
163-
if (hypervisorType != null) {
164-
String hypervisorName = hypervisorType.name();
165-
igroup.setOsType(Igroup.OsTypeEnum.valueOf(getOSTypeFromHypervisor(hypervisorName)));
166-
}
167-
168-
if (hostsIdentifier != null && hostsIdentifier.size() > 0) {
169-
List<Initiator> initiators = new ArrayList<>();
170-
for (String hostIdentifier : hostsIdentifier) {
171-
Initiator initiator = new Initiator();
172-
initiator.setName(hostIdentifier);
173-
initiators.add(initiator);
174-
}
175-
igroup.setInitiators(initiators);
176-
}
177-
accessGroupRequest.setIgroup(igroup);
178-
return accessGroupRequest;
179-
}
180-
181-
public static String getLunName(String volName, String lunName) {
96+
public static String getLunName(String volName, String lunName) {
18297
//Lun name in unified "/vol/VolumeName/LunName"
18398
return Constants.VOLUME_PATH_PREFIX + volName + Constants.SLASH + lunName;
18499
}

0 commit comments

Comments
 (0)