Skip to content

feature/CSTACKEX-122: Per host Igroup changes #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
piyush5netapp wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

feature/CSTACKEX-122: Per host Igroup changes #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4781b3f
feature/CSTACKEX-122: Per host Igroup changes
Mar 3, 2026
7f470fd
feature/CSTACKEX-122: Per host Igroup changes 2
Mar 5, 2026
c1535f3
feature/CSTACKEX-122: Per host Igroup changes 3
Mar 5, 2026
25cd46c
feature/CSTACKEX-122: Per host Igroup changes 4
Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion .../storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,16 @@ protected Void managedCopyBaseImageCallback(AsyncCallbackDispatcher<VolumeServic
CopyCmdAnswer answer = (CopyCmdAnswer)result.getAnswer();
TemplateObjectTO templateObjectTo = (TemplateObjectTO)answer.getNewData();

volume.setPath(templateObjectTo.getPath());
// For NFS managed storage, preserve the volume UUID path to avoid file collision
Copy link

@rajiv-jain-netapp rajiv-jain-netapp Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common path. Please add the plugin check as well, since we want this applied to the ONTAP flow rather than all the vendors.

Copy link
Collaborator

@sandeeplocharla sandeeplocharla Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if plugin name would be available in this method. If its not available, we can check if volume.getPath is null and set it if there's new info available.

// For iSCSI, update path as before (iSCSI uses _iScsiName field for actual LUN access)
PrimaryDataStore primaryDataStore = (PrimaryDataStore) volumeInfo.getDataStore();
if (primaryDataStore != null && primaryDataStore.getPoolType() == StoragePoolType.NetworkFilesystem) {
if (logger.isDebugEnabled()) {
logger.debug("NFS managed storage - preserving volume path: " + volume.getPath() + " (not overwriting with template path: " + templateObjectTo.getPath() + ")");
Copy link
Collaborator

@sandeeplocharla sandeeplocharla Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it already in Debug mode? Do we need to check for it specifically?

}
} else {
volume.setPath(templateObjectTo.getPath());
}

if (templateObjectTo.getFormat() != null) {
volume.setFormat(templateObjectTo.getFormat());
Expand Down Expand Up @@ -1337,6 +1346,14 @@ private void createManagedVolumeCopyTemplateAsync(VolumeInfo volumeInfo, Primary
primaryDataStore.setDetails(details);

grantAccess(volumeInfo, destHost, primaryDataStore);
volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore);
details.put(PrimaryDataStore.MANAGED_STORE_TARGET, volumeInfo.get_iScsiName());
primaryDataStore.setDetails(details);

// Update destTemplateInfo with the iSCSI path from volumeInfo
if (destTemplateInfo instanceof TemplateObject) {
((TemplateObject)destTemplateInfo).setInstallPath(volumeInfo.getPath());
}

try {
motionSrv.copyAsync(srcTemplateInfo, destTemplateInfo, destHost, caller);
Expand Down
67 changes: 44 additions & 23 deletions ...ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.cloud.agent.api.to.DataTO;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.storage.Storage;
import com.cloud.storage.StoragePool;
import com.cloud.storage.Volume;
Expand All @@ -50,6 +51,7 @@
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.feign.model.Igroup;
import org.apache.cloudstack.storage.feign.model.Lun;
import org.apache.cloudstack.storage.service.SANStrategy;
import org.apache.cloudstack.storage.service.StorageStrategy;
Expand All @@ -63,8 +65,10 @@
import org.apache.logging.log4j.Logger;

import javax.inject.Inject;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
Expand Down Expand Up @@ -145,7 +149,6 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
volumeVO.setPoolId(storagePool.getId());

if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
String svmName = details.get(Constants.SVM_NAME);
String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null;
if (lunName == null) {
throw new CloudRuntimeException("createAsync: Missing LUN name for volume " + volInfo.getId());
Expand All @@ -158,22 +161,21 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
volumeVO.setFolder(created.getLun().getUuid());
}

// Create LUN-to-igroup mapping and retrieve the assigned LUN ID
UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details);
String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid);
String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName);

// Construct iSCSI path: /<iqn>/<lun_id> format for KVM/libvirt attachment
String iscsiPath = Constants.SLASH + storagePool.getPath() + Constants.SLASH + lunNumber;
volumeVO.set_iScsiName(iscsiPath);
volumeVO.setPath(iscsiPath);
s_logger.info("createAsync: Volume [{}] iSCSI path set to {}", volumeVO.getId(), iscsiPath);
createCmdResult = new CreateCmdResult(null, new Answer(null, true, null));
s_logger.info("createAsync: Created LUN [{}] for volume [{}]. LUN mapping will occur during grantAccess() to per-host igroup.",
lunName, volumeVO.getId());

// Path will be set during grantAccess when LUN is mapped and we get the LUN ID
// Return LUN name as identifier for CloudStack tracking
volumeVO.set_iScsiName(null);
volumeVO.setPath(null);
createCmdResult = new CreateCmdResult(lunName, new Answer(null, true, null));
} else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
// For NFS, set path to volume UUID to ensure uniqueness
// This prevents multiple VMs from using the same template file path
volumeVO.setPath(volInfo.getUuid());
createCmdResult = new CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null));
s_logger.info("createAsync: Managed NFS volume [{}] associated with pool {}",
volumeVO.getId(), storagePool.getId());
s_logger.info("createAsync: Managed NFS volume [{}] with path [{}] associated with pool {}",
volumeVO.getId(), volInfo.getUuid(), storagePool.getId());
}
volumeDao.update(volumeVO.getId(), volumeVO);
}
Expand Down Expand Up @@ -319,12 +321,33 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
// Only retrieve LUN name for iSCSI volumes
String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue();
UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details);
String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid);

// Verify host initiator is registered in the igroup before allowing access
if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroupName)) {
throw new CloudRuntimeException("grantAccess: Host initiator [" + host.getStorageUrl() +
"] is not present in iGroup [" + accessGroupName + "]");
String accessGroupName = Utility.getIgroupName(svmName, host.getName());

// 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
Map<String, String> getAccessGroupMap = Map.of(
Constants.NAME, accessGroupName,
Constants.SVM_DOT_NAME, svmName
);
Igroup igroup = new Igroup();
AccessGroup accessGroup = sanStrategy.getAccessGroup(getAccessGroupMap);
if(accessGroup == null || accessGroup.getIgroup() == null) {
s_logger.info("grantAccess: Igroup does not exist for the host: Need to create Igroup " + host.getName());
// create the igroup for the host and perform lun-mapping
accessGroup = new AccessGroup();
List<HostVO> hosts = new ArrayList<>();
hosts.add((HostVO) host);
accessGroup.setHostsToConnect(hosts);
accessGroup.setStoragePoolId(storagePool.getId());
accessGroup = sanStrategy.createAccessGroup(accessGroup);
}else{
s_logger.info("grantAccess: Igroup {} already exist for the host {}: ", accessGroup.getIgroup().getName() ,host.getName());
igroup = accessGroup.getIgroup();
// TODO
// Verify host initiator is registered in the igroup before allowing access
// if (sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup())) {
// // add host initiator to the igroup ? or fail here ?
Copy link
Collaborator

@sandeeplocharla sandeeplocharla Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 corner cases that I see this happening:

  1. User deleberately deleting the accessGroup from ONTAP
  2. There's a change in the host but the corresponsing HostListener operation did not go through
    In both cases, I feel instead of creation of an accessGroup on fly, it would be better to inform the user about the same and let him intervene in taking corrective action.

// }
// Use the existing igroup and perform lun-mapping
}

// Create or retrieve existing LUN mapping
Expand Down Expand Up @@ -453,7 +476,7 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO,

// Verify host initiator is in the igroup before attempting to remove mapping
SANStrategy sanStrategy = (UnifiedSANStrategy) storageStrategy;
if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup().getName())) {
if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup())) {
s_logger.warn("revokeAccessForVolume: Initiator [{}] is not in iGroup [{}], skipping revoke",
host.getStorageUrl(), accessGroupName);
return;
Expand Down Expand Up @@ -524,7 +547,6 @@ public long getUsedIops(StoragePool storagePool) {

@Override
public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCmdResult> callback) {

}

@Override
Expand Down Expand Up @@ -569,7 +591,6 @@ public boolean isVmInfoNeeded() {

@Override
public void provideVmInfo(long vmId, long volumeId) {

}

@Override
Expand Down
71 changes: 41 additions & 30 deletions ...src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
throw new CloudRuntimeException("Cluster Id or Pod Id is null, cannot create primary storage");
}

if (podId == null && clusterId == null) {
if (podId == null) {
if (zoneId != null) {
s_logger.info("Both Pod Id and Cluster Id are null, Primary storage pool will be associated with a Zone");
} else {
Expand Down Expand Up @@ -231,7 +231,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
path = Constants.SLASH + storagePoolName;
port = Constants.NFS3_PORT;
// Force NFSv3 for ONTAP managed storage to avoid NFSv4 ID mapping issues
details.put("nfsmountopts", "vers=3");
details.put(Constants.NFS_MOUNT_OPTIONS,Constants.NFS3_MOUNT_OPTIONS_VER_3);
s_logger.info("Setting NFS path for storage pool: " + path + ", port: " + port + " with mount option: vers=3");
break;
case ISCSI:
Expand Down Expand Up @@ -281,8 +281,11 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
}
PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore;
List<HostVO> hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(primaryStore);
// TODO- need to check if no host to connect then throw exception or just continue?
logger.debug("attachCluster: Eligible Up and Enabled hosts: {} in cluster {}", hostsToConnect, primaryStore.getClusterId());
if(hostsToConnect.isEmpty()) {
Copy link
Collaborator

@sandeeplocharla sandeeplocharla Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to enforce having hosts before creating a pool? Anyways hostlistener will handle the necessary operations in case if new hosts got added right?
I think both export-policy and igroup can be created without either host IP's or iqn's, can't they?

s_logger.info("attachCluster: No hosts found for primary storage");
throw new CloudRuntimeException("attachCluster: No hosts found for primary storage");
}

Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(primaryStore.getId());
StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details);
Expand All @@ -294,22 +297,24 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
s_logger.error(errMsg);
throw new CloudRuntimeException(errMsg);
}

logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId());
//TODO - check if no host to connect then also need to create access group without initiators
if (hostsIdentifier != null && hostsIdentifier.size() > 0) {
try {
AccessGroup accessGroupRequest = new AccessGroup();
accessGroupRequest.setHostsToConnect(hostsToConnect);
accessGroupRequest.setScope(scope);
primaryStore.setDetails(details);// setting details as it does not come from cloudstack
accessGroupRequest.setPrimaryDataStoreInfo(primaryStore);
strategy.createAccessGroup(accessGroupRequest);
} catch (Exception e) {
s_logger.error("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage());
throw new CloudRuntimeException("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage());
// We need to create export policy at pool level and igroup at host level(in grantAccess)
if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
if (!hostsIdentifier.isEmpty()) {
try {
AccessGroup accessGroupRequest = new AccessGroup();
accessGroupRequest.setHostsToConnect(hostsToConnect);
accessGroupRequest.setScope(scope);
primaryStore.setDetails(details);// setting details as it does not come from cloudstack
accessGroupRequest.setStoragePoolId(storagePool.getId());
strategy.createAccessGroup(accessGroupRequest);
} catch (Exception e) {
s_logger.error("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage());
throw new CloudRuntimeException("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage());
}
}
}

logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId());
for (HostVO host : hostsToConnect) {
try {
Expand Down Expand Up @@ -347,30 +352,36 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore;
List<HostVO> hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), Hypervisor.HypervisorType.KVM);
logger.debug(String.format("In createPool. Attaching the pool to each of the hosts in %s.", hostsToConnect));
if(hostsToConnect.isEmpty()) {
s_logger.info("attachCluster: No hosts found for primary storage");
throw new CloudRuntimeException("attachCluster: No hosts found for primary storage");
}

Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(primaryStore.getId());
StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details);

// TODO- need to check if no host to connect then throw exception or just continue
logger.debug("attachZone: Eligible Up and Enabled hosts: {}", hostsToConnect);
ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL));
//TODO- Check if we have to handle heterogeneous host within the zone
if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) {
String errMsg = "attachZone: Not all hosts in the zone support the protocol: " + protocol.name();
s_logger.error(errMsg);
throw new CloudRuntimeException(errMsg);
}
if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) {
try {
AccessGroup accessGroupRequest = new AccessGroup();
accessGroupRequest.setHostsToConnect(hostsToConnect);
accessGroupRequest.setScope(scope);
primaryStore.setDetails(details); // setting details as it does not come from cloudstack
accessGroupRequest.setPrimaryDataStoreInfo(primaryStore);
strategy.createAccessGroup(accessGroupRequest);
} catch (Exception e) {
s_logger.error("attachZone: Failed to create access group on storage system for zone with Exception: " + e.getMessage());
throw new CloudRuntimeException("attachZone: Failed to create access group on storage system for zone with Exception: " + e.getMessage());

// We need to create export policy at pool level and igroup at host level
if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
if (!hostsIdentifier.isEmpty()) {
try {
AccessGroup accessGroupRequest = new AccessGroup();
accessGroupRequest.setHostsToConnect(hostsToConnect);
accessGroupRequest.setScope(scope);
primaryStore.setDetails(details); // setting details as it does not come from cloudstack
accessGroupRequest.setStoragePoolId(storagePool.getId());
strategy.createAccessGroup(accessGroupRequest);
} catch (Exception e) {
s_logger.error("attachZone: Failed to create access group on storage system for zone with Exception: " + e.getMessage());
throw new CloudRuntimeException("attachZone: Failed to create access group on storage system for zone with Exception: " + e.getMessage());
}
}
}
for (HostVO host : hostsToConnect) {
Expand Down Expand Up @@ -485,7 +496,7 @@ public boolean deleteDataStore(DataStore store) {
storagePoolId, e.getMessage(), e);
}
AccessGroup accessGroup = new AccessGroup();
accessGroup.setPrimaryDataStoreInfo(primaryDataStoreInfo);
accessGroup.setStoragePoolId(storagePoolId);
// Delete access groups associated with this storage pool
storageStrategy.deleteAccessGroup(accessGroup);
s_logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName());
Expand Down
Loading
Loading