Skip to content

Commit ad4e526

Browse files
Gupta, SuryaGupta, Surya
Gupta, Surya
authored and
Gupta, Surya
committed
CSTACKEX-46 Resolve Copilot review comments
1 parent f43783c commit ad4e526

File tree

4 files changed

+55
-13
lines changed

4 files changed

+55
-13
lines changed

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
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;
5052
import org.apache.cloudstack.storage.service.StorageStrategy;
5153
import org.apache.cloudstack.storage.service.model.AccessGroup;
5254
import org.apache.cloudstack.storage.service.model.CloudStackVolume;
@@ -224,7 +226,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
224226
CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath());
225227
s_logger.info("grantAccessForVolume: Retrieved LUN [{}] details for volume [{}]", cloudStackVolume.getLun().getName(), volumeVO.getName());
226228
AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName);
227-
if(accessGroup.getIgroup().getInitiators() == null || accessGroup.getIgroup().getInitiators().size() == 0 || !accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) {
229+
if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) {
228230
s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName);
229231
throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName);
230232
}
@@ -240,6 +242,16 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
240242
throw new CloudRuntimeException(errMsg);
241243
}
242244
}
245+
private boolean hostInitiatorFoundInIgroup(String hostInitiator, Igroup igroup) {
246+
if(igroup != null || igroup.getInitiators() != null || hostInitiator != null || !hostInitiator.isEmpty()) {
247+
for(Initiator initiator : igroup.getInitiators()) {
248+
if(initiator.getName().equalsIgnoreCase(hostInitiator)) {
249+
return true;
250+
}
251+
}
252+
}
253+
return false;
254+
}
243255

244256
@Override
245257
public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) {
@@ -291,9 +303,9 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
291303
CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath());
292304
AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName);
293305
//TODO check if initiator does exits in igroup, will throw the error ?
294-
if(!accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) {
295-
s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName);
296-
throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName);
306+
if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) {
307+
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);
297309
}
298310

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ OntapResponse<Lun> createLun(@Param("authHeader") String authHeader,
4141

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

4646
@RequestLine("GET /{uuid}")
4747
@Headers({"Authorization: {authHeader}"})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
321321
ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL));
322322
//TODO- Check if we have to handle heterogeneous host within the zone
323323
if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) {
324-
s_logger.error("attachZone: Not all hosts in the cluster support the protocol: " + protocol.name());
324+
s_logger.error("attachZone: Not all hosts in the zone support the protocol: " + protocol.name());
325325
throw new CloudRuntimeException("attachZone: Not all hosts in the zone support the protocol: " + protocol.name());
326326
}
327327
if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) {

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

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,39 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {
9797
}
9898

9999
@Override
100-
public CloudStackVolume getCloudStackVolume(Map<String, String> cloudStackVolumeMap) {
101-
//TODO
102-
return null;
100+
public CloudStackVolume getCloudStackVolume(Map<String, String> values) {
101+
s_logger.info("getCloudStackVolume : fetching Igroup with params {} ", values);
102+
if (values == null || values.isEmpty()) {
103+
s_logger.error("getCloudStackVolume: get Igroup failed. Invalid request: {}", values);
104+
throw new CloudRuntimeException("getCloudStackVolume : get Igroup Failed, invalid request");
105+
}
106+
String svmName = values.get(Constants.SVM_DOT_NAME);
107+
String lunName = values.get(Constants.NAME);
108+
if(svmName == null || lunName == null || svmName.isEmpty() || lunName.isEmpty()) {
109+
s_logger.error("getCloudStackVolume: get Igroup failed. Invalid svm:{} or igroup name: {}", svmName, lunName);
110+
throw new CloudRuntimeException("getCloudStackVolume : Fget Igroup failed, invalid request");
111+
}
112+
try {
113+
// Get AuthHeader
114+
String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword());
115+
// get Igroup
116+
Map<String, Object> queryParams = Map.of(Constants.SVM_DOT_NAME, svmName, Constants.NAME, lunName);
117+
OntapResponse<Lun> lunResponse = sanFeignClient.getLunResponse(authHeader, queryParams);
118+
if (lunResponse == null || lunResponse.getRecords() == null || lunResponse.getRecords().size() == 0) {
119+
s_logger.error("getCloudStackVolume: Failed to fetch Igroup");
120+
throw new CloudRuntimeException("getCloudStackVolume: Failed to fetch Igroup");
121+
}
122+
Lun lun = lunResponse.getRecords().get(0);
123+
s_logger.debug("getCloudStackVolume: Lun Details : {}", lun);
124+
s_logger.info("getCloudStackVolume: Fetched the Lun successfully. LunName: {}", lun.getName());
125+
126+
CloudStackVolume cloudStackVolume = new CloudStackVolume();
127+
cloudStackVolume.setLun(lun);
128+
return cloudStackVolume;
129+
} catch (Exception e) {
130+
s_logger.error("Exception occurred while fetching Lun. Exception: {}", e.getMessage());
131+
throw new CloudRuntimeException("Failed to fetch Lun details: " + e.getMessage());
132+
}
103133
}
104134

105135
@Override
@@ -152,7 +182,7 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
152182
String igroupName = values.get(Constants.IGROUP_DOT_NAME);
153183
if(svmName == null || igroupName == null || svmName.isEmpty() || igroupName.isEmpty()) {
154184
s_logger.error("getAccessGroup: get Igroup failed. Invalid svm:{} or igroup name: {}", svmName, igroupName);
155-
throw new CloudRuntimeException("getAccessGroup : Fget Igroup failed, invalid request");
185+
throw new CloudRuntimeException("getAccessGroup : Failed to get Igroup, invalid request");
156186
}
157187
try {
158188
// Get AuthHeader
@@ -166,7 +196,7 @@ public AccessGroup getAccessGroup(Map<String, String> values) {
166196
}
167197
Igroup igroup = igroupResponse.getRecords().get(0);
168198
s_logger.debug("getAccessGroup: Igroup Details : {}", igroup);
169-
s_logger.info("getAccessGroup: Fetched the Igroup successfully. LunName: {}", igroup.getName());
199+
s_logger.info("getAccessGroup: Fetched the Igroup successfully. IgroupName: {}", igroup.getName());
170200

171201
AccessGroup accessGroup = new AccessGroup();
172202
accessGroup.setIgroup(igroup);
@@ -200,7 +230,7 @@ public void enableLogicalAccess(Map<String, String> values) {
200230
s_logger.debug("enableLogicalAccess: LunMap created successfully. LunMap: {}", lunMap);
201231
s_logger.info("enableLogicalAccess: LunMap created successfully.");
202232
} catch (Exception e) {
203-
s_logger.error("Exception occurred while creating LunMap: {}. Exception: {}", e.getMessage());
233+
s_logger.error("Exception occurred while creating LunMap: {}. Exception: {}", e.getMessage(), e);
204234
throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage());
205235
}
206236
}
@@ -220,7 +250,7 @@ public void disableLogicalAccess(Map<String, String> values) {
220250
sanFeignClient.deleteLunMap(authHeader, lunUUID, igroupUUID);
221251
s_logger.info("disableLogicalAccess: LunMap deleted successfully.");
222252
} catch (Exception e) {
223-
s_logger.error("Exception occurred while deleting LunMap: {}. Exception: {}", e.getMessage());
253+
s_logger.error("Exception occurred while deleting LunMap: {}. Exception: {}", e.getMessage(), e);
224254
throw new CloudRuntimeException("Failed to delete LunMap: " + e.getMessage());
225255
}
226256
}

0 commit comments

Comments
 (0)