Skip to content

Commit 7a0a03b

Browse files
piyush5netappSrivastava, Piyush
authored and
rajiv-jain-netapp
committed
feature/CSTACKEX-122: Per host Igroup changes (#37)
Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
1 parent a1edb10 commit 7a0a03b

File tree

10 files changed

+330
-316
lines changed

10 files changed

+330
-316
lines changed

‎engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,6 +1340,15 @@ private void createManagedVolumeCopyTemplateAsync(VolumeInfo volumeInfo, Primary
13401340
primaryDataStore.setDetails(details);
13411341

13421342
grantAccess(volumeInfo, destHost, primaryDataStore);
1343+
volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore);
1344+
// For Netapp ONTAP iscsiName or Lun path is available only after grantAccess
1345+
String managedStoreTarget = volumeInfo.get_iScsiName() != null ? volumeInfo.get_iScsiName() : volumeInfo.getUuid();
1346+
details.put(PrimaryDataStore.MANAGED_STORE_TARGET, managedStoreTarget);
1347+
primaryDataStore.setDetails(details);
1348+
// Update destTemplateInfo with the iSCSI path from volumeInfo
1349+
if (destTemplateInfo instanceof TemplateObject) {
1350+
((TemplateObject)destTemplateInfo).setInstallPath(volumeInfo.getPath());
1351+
}
13431352

13441353
try {
13451354
motionSrv.copyAsync(srcTemplateInfo, destTemplateInfo, destHost, caller);

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

Lines changed: 45 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.cloud.agent.api.to.DataTO;
2626
import com.cloud.exception.InvalidParameterValueException;
2727
import com.cloud.host.Host;
28+
import com.cloud.host.HostVO;
2829
import com.cloud.storage.Storage;
2930
import com.cloud.storage.StoragePool;
3031
import com.cloud.storage.Volume;
@@ -36,8 +37,6 @@
3637
import com.cloud.storage.dao.VolumeDetailsDao;
3738
import com.cloud.utils.Pair;
3839
import com.cloud.utils.exception.CloudRuntimeException;
39-
import com.cloud.vm.VirtualMachine;
40-
import com.cloud.vm.dao.VMInstanceDao;
4140
import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
4241
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
4342
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
@@ -54,6 +53,7 @@
5453
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
5554
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
5655
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
56+
import org.apache.cloudstack.storage.feign.model.Igroup;
5757
import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient;
5858
import org.apache.cloudstack.storage.feign.model.FlexVolSnapshot;
5959
import org.apache.cloudstack.storage.feign.model.Lun;
@@ -73,8 +73,9 @@
7373
import org.apache.logging.log4j.Logger;
7474

7575
import javax.inject.Inject;
76-
import java.util.Arrays;
76+
import java.util.ArrayList;
7777
import java.util.HashMap;
78+
import java.util.List;
7879
import java.util.Map;
7980

8081
/**
@@ -87,7 +88,6 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver {
8788

8889
@Inject private StoragePoolDetailsDao storagePoolDetailsDao;
8990
@Inject private PrimaryDataStoreDao storagePoolDao;
90-
@Inject private VMInstanceDao vmDao;
9191
@Inject private VolumeDao volumeDao;
9292
@Inject private VolumeDetailsDao volumeDetailsDao;
9393
@Inject private SnapshotDetailsDao snapshotDetailsDao;
@@ -111,6 +111,12 @@ public DataStoreTO getStoreTO(DataStore store) {
111111
return null;
112112
}
113113

114+
@Override
115+
public boolean volumesRequireGrantAccessWhenUsed(){
116+
s_logger.info("OntapPrimaryDatastoreDriver: volumesRequireGrantAccessWhenUsed: Called");
117+
return true;
118+
}
119+
114120
/**
115121
* Creates a volume on the ONTAP storage system.
116122
*/
@@ -155,7 +161,6 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
155161
volumeVO.setPoolId(storagePool.getId());
156162

157163
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
158-
String svmName = details.get(Constants.SVM_NAME);
159164
String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null;
160165
if (lunName == null) {
161166
throw new CloudRuntimeException("Missing LUN name for volume " + volInfo.getId());
@@ -168,22 +173,13 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
168173
volumeVO.setFolder(created.getLun().getUuid());
169174
}
170175

171-
// Create LUN-to-igroup mapping and retrieve the assigned LUN ID
172-
UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details);
173-
String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid);
174-
String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName);
175-
176-
// Construct iSCSI path: /<iqn>/<lun_id> format for KVM/libvirt attachment
177-
String iscsiPath = Constants.SLASH + storagePool.getPath() + Constants.SLASH + lunNumber;
178-
volumeVO.set_iScsiName(iscsiPath);
179-
volumeVO.setPath(iscsiPath);
180-
s_logger.info("createAsync: Volume [{}] iSCSI path set to {}", volumeVO.getId(), iscsiPath);
181-
createCmdResult = new CreateCmdResult(null, new Answer(null, true, null));
182-
176+
s_logger.info("createAsync: Created LUN [{}] for volume [{}]. LUN mapping will occur during grantAccess() to per-host igroup.",
177+
lunName, volumeVO.getId());
178+
createCmdResult = new CreateCmdResult(lunName, new Answer(null, true, null));
183179
} else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
184180
createCmdResult = new CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null));
185-
s_logger.info("createAsync: Managed NFS volume [{}] associated with pool {}",
186-
volumeVO.getId(), storagePool.getId());
181+
s_logger.info("createAsync: Managed NFS volume [{}] with path [{}] associated with pool {}",
182+
volumeVO.getId(), volInfo.getUuid(), storagePool.getId());
187183
}
188184
volumeDao.update(volumeVO.getId(), volumeVO);
189185
}
@@ -413,14 +409,35 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
413409
// Only retrieve LUN name for iSCSI volumes
414410
String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue();
415411
UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details);
416-
String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid);
417-
418-
// Verify host initiator is registered in the igroup before allowing access
419-
if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroupName)) {
420-
throw new CloudRuntimeException("Host initiator [" + host.getStorageUrl() +
421-
"] is not present in iGroup [" + accessGroupName + "]");
412+
String accessGroupName = Utility.getIgroupName(svmName, host.getName());
413+
414+
// Validate if Igroup exist ONTAP for this host as we may be using delete_on_unmap= true and igroup may be deleted by ONTAP automatically
415+
Map<String, String> getAccessGroupMap = Map.of(
416+
Constants.NAME, accessGroupName,
417+
Constants.SVM_DOT_NAME, svmName
418+
);
419+
Igroup igroup = new Igroup();
420+
AccessGroup accessGroup = sanStrategy.getAccessGroup(getAccessGroupMap);
421+
if(accessGroup == null || accessGroup.getIgroup() == null) {
422+
s_logger.info("grantAccess: Igroup {} does not exist for the host {} : Need to create Igroup for the host ", accessGroupName, host.getName());
423+
// create the igroup for the host and perform lun-mapping
424+
accessGroup = new AccessGroup();
425+
List<HostVO> hosts = new ArrayList<>();
426+
hosts.add((HostVO) host);
427+
accessGroup.setHostsToConnect(hosts);
428+
accessGroup.setStoragePoolId(storagePool.getId());
429+
accessGroup = sanStrategy.createAccessGroup(accessGroup);
430+
}else{
431+
s_logger.info("grantAccess: Igroup {} already exist for the host {}: ", accessGroup.getIgroup().getName() ,host.getName());
432+
igroup = accessGroup.getIgroup();
433+
/* TODO Below cases will be covered later, for now they will be a pre-requisite on customer side
434+
1. Igroup exist with the same name but host initiator has been rempved
435+
2. Igroup exist with the same name but host initiator has been changed may be due to new NIC or new adapter
436+
In both cases we need to verify current host initiator is registered in the igroup before allowing access
437+
Incase it is not , add it and proceed for lun-mapping
438+
*/
422439
}
423-
440+
s_logger.info("grantAccess: Igroup {} is present now with initiators {} ", accessGroup.getIgroup().getName(), accessGroup.getIgroup().getInitiators());
424441
// Create or retrieve existing LUN mapping
425442
String lunNumber = sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName);
426443

@@ -465,22 +482,6 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
465482
throw new InvalidParameterValueException("host should not be null");
466483
}
467484

468-
// Safety check: don't revoke access if volume is still attached to an active VM
469-
if (dataObject.getType() == DataObjectType.VOLUME) {
470-
Volume volume = volumeDao.findById(dataObject.getId());
471-
if (volume.getInstanceId() != null) {
472-
VirtualMachine vm = vmDao.findById(volume.getInstanceId());
473-
if (vm != null && !Arrays.asList(
474-
VirtualMachine.State.Destroyed,
475-
VirtualMachine.State.Expunging,
476-
VirtualMachine.State.Error).contains(vm.getState())) {
477-
s_logger.warn("revokeAccess: Volume [{}] is still attached to VM [{}] in state [{}], skipping revokeAccess",
478-
dataObject.getId(), vm.getInstanceName(), vm.getState());
479-
return;
480-
}
481-
}
482-
}
483-
484485
StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId());
485486
if (storagePool == null) {
486487
s_logger.error("revokeAccess: Storage Pool not found for id: " + dataStore.getId());
@@ -518,10 +519,9 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
518519
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
519520
StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details);
520521
String svmName = details.get(Constants.SVM_NAME);
521-
String storagePoolUuid = storagePool.getUuid();
522522

523523
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
524-
String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid);
524+
String accessGroupName = Utility.getIgroupName(svmName, host.getName());
525525

526526
// Retrieve LUN name from volume details; if missing, volume may not have been fully created
527527
String lunName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME) != null ?
@@ -547,7 +547,7 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,
547547

548548
// Verify host initiator is in the igroup before attempting to remove mapping
549549
SANStrategy sanStrategy = (UnifiedSANStrategy) storageStrategy;
550-
if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup().getName())) {
550+
if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup())) {
551551
s_logger.warn("revokeAccessForVolume: Initiator [{}] is not in iGroup [{}], skipping revoke",
552552
host.getStorageUrl(), accessGroupName);
553553
return;
@@ -916,7 +916,6 @@ public boolean isVmInfoNeeded() {
916916

917917
@Override
918918
public void provideVmInfo(long vmId, long volumeId) {
919-
920919
}
921920

922921
@Override

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@
1919

2020
package org.apache.cloudstack.storage.service;
2121

22+
import org.apache.cloudstack.storage.feign.model.Igroup;
23+
import org.apache.cloudstack.storage.feign.model.Initiator;
2224
import org.apache.cloudstack.storage.feign.model.OntapStorage;
25+
import org.apache.logging.log4j.LogManager;
26+
import org.apache.logging.log4j.Logger;
2327

2428
public abstract class SANStrategy extends StorageStrategy {
29+
private static final Logger s_logger = LogManager.getLogger(SANStrategy.class);
2530
public SANStrategy(OntapStorage ontapStorage) {
2631
super(ontapStorage);
2732
}
@@ -43,8 +48,25 @@ public SANStrategy(OntapStorage ontapStorage) {
4348
*
4449
* @param hostInitiator the host initiator IQN
4550
* @param svmName the SVM name
46-
* @param accessGroupName the igroup name
51+
* @param igroup the igroup
4752
* @return true if the initiator is found in the igroup, false otherwise
4853
*/
49-
public abstract boolean validateInitiatorInAccessGroup(String hostInitiator, String svmName, String accessGroupName);
54+
public boolean validateInitiatorInAccessGroup(String hostInitiator, String svmName, Igroup igroup) {
55+
s_logger.info("validateInitiatorInAccessGroup: Validating initiator [{}] is in igroup [{}] on SVM [{}]", hostInitiator, igroup, svmName);
56+
57+
if (hostInitiator == null || hostInitiator.isEmpty()) {
58+
s_logger.warn("validateInitiatorInAccessGroup: host initiator is null or empty");
59+
return false;
60+
}
61+
if (igroup.getInitiators() != null) {
62+
for (Initiator initiator : igroup.getInitiators()) {
63+
if (initiator.getName().equalsIgnoreCase(hostInitiator)) {
64+
s_logger.info("validateInitiatorInAccessGroup: Initiator [{}] validated successfully in igroup [{}]", hostInitiator, igroup);
65+
return true;
66+
}
67+
}
68+
}
69+
s_logger.warn("validateInitiatorInAccessGroup: Initiator [{}] NOT found in igroup [{}]", hostInitiator, igroup);
70+
return false;
71+
}
5072
}

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

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -271,23 +271,10 @@ public Volume createStorageVolume(String volumeName, Long size) {
271271
OntapResponse<Volume> ontapVolume = volumeFeignClient.getVolume(authHeader, queryParams);
272272
s_logger.debug("Feign call completed. Processing response...");
273273

274-
if (ontapVolume == null) {
274+
if (ontapVolume == null || ontapVolume.getRecords() == null || ontapVolume.getRecords().isEmpty()) {
275275
s_logger.error("OntapResponse is null for volume: " + volumeName);
276-
throw new CloudRuntimeException("Failed to fetch volume " + volumeName + ": Response is null");
276+
throw new CloudRuntimeException("Failed to fetch volume " + volumeName + ": no records found");
277277
}
278-
s_logger.debug("OntapResponse is not null. Checking records field...");
279-
280-
if (ontapVolume.getRecords() == null) {
281-
s_logger.error("OntapResponse.records is null for volume: " + volumeName);
282-
throw new CloudRuntimeException("Failed to fetch volume " + volumeName + ": Records list is null");
283-
}
284-
s_logger.debug("Records field is not null. Size: " + ontapVolume.getRecords().size());
285-
286-
if (ontapVolume.getRecords().isEmpty()) {
287-
s_logger.error("OntapResponse.records is empty for volume: " + volumeName);
288-
throw new CloudRuntimeException("Failed to fetch volume " + volumeName + ": No records found");
289-
}
290-
291278
Volume volume = ontapVolume.getRecords().get(0);
292279
s_logger.info("Volume retrieved successfully: " + volumeName + ", UUID: " + volume.getUuid());
293280
return volume;
@@ -297,7 +284,7 @@ public Volume createStorageVolume(String volumeName, Long size) {
297284
}
298285
}
299286

300-
/**
287+
/**
301288
* Updates ONTAP Flex-Volume
302289
* Eligible only for Unified ONTAP storage
303290
* throw exception in case of disaggregated ONTAP storage
@@ -336,7 +323,6 @@ public void deleteStorageVolume(Volume volume) {
336323
}
337324
s_logger.info("ONTAP volume deletion process completed for volume: " + volume.getName());
338325
}
339-
340326
/**
341327
* Gets ONTAP Flex-Volume
342328
* Eligible only for Unified ONTAP storage

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
package org.apache.cloudstack.storage.service.model;
2121

2222
import com.cloud.host.HostVO;
23-
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
2423
import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
2524
import org.apache.cloudstack.storage.feign.model.ExportPolicy;
2625
import org.apache.cloudstack.storage.feign.model.Igroup;
@@ -33,7 +32,7 @@ public class AccessGroup {
3332
private ExportPolicy exportPolicy;
3433

3534
private List<HostVO> hostsToConnect;
36-
private PrimaryDataStoreInfo primaryDataStoreInfo;
35+
private Long storagePoolId;
3736
private Scope scope;
3837

3938

@@ -58,12 +57,15 @@ public List<HostVO> getHostsToConnect() {
5857
public void setHostsToConnect(List<HostVO> hostsToConnect) {
5958
this.hostsToConnect = hostsToConnect;
6059
}
61-
public PrimaryDataStoreInfo getPrimaryDataStoreInfo() {
62-
return primaryDataStoreInfo;
60+
61+
public Long getStoragePoolId() {
62+
return storagePoolId;
6363
}
64-
public void setPrimaryDataStoreInfo(PrimaryDataStoreInfo primaryDataStoreInfo) {
65-
this.primaryDataStoreInfo = primaryDataStoreInfo;
64+
65+
public void setStoragePoolId(Long storagePoolId) {
66+
this.storagePoolId = storagePoolId;
6667
}
68+
6769
public Scope getScope() {
6870
return scope;
6971
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ public class Constants {
4242
public static final String IS_DISAGGREGATED = "isDisaggregated";
4343
public static final String RUNNING = "running";
4444
public static final String EXPORT = "export";
45+
public static final String NFS_MOUNT_OPTIONS = "nfsmountopts";
46+
public static final String NFS3_MOUNT_OPTIONS_VER_3 = "vers=3";
4547

4648
public static final int ONTAP_PORT = 443;
4749

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,14 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map<String, String
139139
}
140140
}
141141

142-
public static String getIgroupName(String svmName, String poolUuid) {
143-
//Igroup name format: cs_svmName_poolUuid
144-
return Constants.CS + Constants.UNDERSCORE + svmName + Constants.UNDERSCORE + poolUuid;
142+
public static String getIgroupName(String svmName, String hostName) {
143+
//Igroup name format: cs_svmName_hostName
144+
String sanitizedHostName = hostName.split("\\.")[0].replaceAll("[^a-zA-Z0-9_-]", "_");
145+
return Constants.CS + Constants.UNDERSCORE + svmName + Constants.UNDERSCORE + sanitizedHostName;
145146
}
146147

147148
public static String generateExportPolicyName(String svmName, String volumeName){
148-
return Constants.EXPORT + Constants.HYPHEN + svmName + Constants.HYPHEN + volumeName;
149+
return Constants.CS + Constants.HYPHEN + svmName + Constants.HYPHEN + volumeName;
149150
}
150151

151152
public static String getLunName(String volName, String lunName) {

0 commit comments

Comments
 (0)