diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 04996b74d2a5..53af00990237 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java @@ -18,26 +18,13 @@ */ package org.apache.cloudstack.storage.driver; -import org.apache.cloudstack.storage.utils.OntapStorageConstants; -import com.cloud.agent.api.Answer; -import com.cloud.agent.api.to.DataObjectType; -import com.cloud.agent.api.to.DataStoreTO; -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; -import com.cloud.storage.VolumeDetailVO; -import com.cloud.storage.VolumeVO; -import com.cloud.storage.ScopeType; -import com.cloud.storage.dao.SnapshotDetailsDao; -import com.cloud.storage.dao.SnapshotDetailsVO; -import com.cloud.storage.dao.VolumeDao; -import com.cloud.storage.dao.VolumeDetailsDao; -import com.cloud.utils.Pair; -import com.cloud.utils.exception.CloudRuntimeException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import javax.inject.Inject; + import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; @@ -66,17 +53,36 @@ import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; -import javax.inject.Inject; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.to.DataObjectType; +import com.cloud.agent.api.to.DataStoreTO; +import com.cloud.agent.api.to.DataTO; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.StorageConflictException; +import com.cloud.exception.StorageUnavailableException; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.storage.ScopeType; +import com.cloud.storage.Storage; +import com.cloud.storage.StorageManager; +import com.cloud.storage.StoragePool; +import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.SnapshotDetailsDao; +import com.cloud.storage.dao.SnapshotDetailsVO; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; /** * Primary datastore driver for NetApp ONTAP storage systems. @@ -91,6 +97,7 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { @Inject private VolumeDao volumeDao; @Inject private VolumeDetailsDao volumeDetailsDao; @Inject private SnapshotDetailsDao snapshotDetailsDao; + @Inject private StorageManager storageManager; @Override public Map getCapabilities() { @@ -392,8 +399,9 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore // Only retrieve LUN name for iSCSI volumes grantAccessIscsi(host, volumeVO, details, svmName, storagePool); } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { - // For NFS, no access grant needed - file is accessible via mount - logger.debug("grantAccess: NFS volume [{}], no igroup mapping required", volumeVO.getUuid()); + // For NFS, ensure export policy has host rule and host is connected to pool. + ensureNfsHostAccess(host, storagePool, details); + logger.debug("grantAccess: NFS volume [{}], ensured host access to storage pool [{}]", volumeVO.getUuid(), storagePool.getId()); return true; } volumeVO.setPoolType(storagePool.getPoolType()); @@ -410,6 +418,43 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore } } + private void ensureNfsHostAccess(Host host, StoragePoolVO storagePool, Map details) { + boolean hostConnectedToPool = false; + List connectedPools = storageManager.findStoragePoolsConnectedToHost(host.getId()); + if (connectedPools != null) { + for (StoragePoolHostVO connectedPoolRef : connectedPools) { + if (connectedPoolRef.getPoolId() == storagePool.getId()) { + hostConnectedToPool = true; + break; + } + } + } + + if (!hostConnectedToPool) { + try { + logger.info("grantAccess: Host [{}] is not connected to NFS pool [{}], reconnecting host to pool", host.getId(), storagePool.getId()); + boolean connected = storageManager.connectHostToSharedPool(host, storagePool.getId()); + if (!connected) { + throw new CloudRuntimeException("Failed to connect host " + host.getId() + " to NFS pool " + storagePool.getId()); + } + } catch (StorageUnavailableException | StorageConflictException e) { + throw new CloudRuntimeException("Unable to connect host " + host.getId() + " to NFS pool " + storagePool.getId(), e); + } + return; + } + + if (!(host instanceof HostVO)) { + throw new CloudRuntimeException("Host object is not of type HostVO for host id: " + host.getId()); + } + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(storagePool.getId()); + accessGroup.setHostsToConnect(List.of((HostVO) host)); + + StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); + strategy.updateAccessGroup(accessGroup); + } + private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map details, String svmName, StoragePoolVO storagePool) { String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME).getValue(); UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ExportRule.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ExportRule.java index 087e9aa681b4..c515fe60e9aa 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ExportRule.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ExportRule.java @@ -19,10 +19,13 @@ package org.apache.cloudstack.storage.feign.model; +import java.util.List; + +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; -import java.util.List; +import com.fasterxml.jackson.annotation.JsonValue; /** * ExportRule @@ -54,6 +57,7 @@ public enum ProtocolsEnum { this.value = value; } + @JsonValue public String getValue() { return value; } @@ -63,13 +67,17 @@ public String toString() { return String.valueOf(value); } + @JsonCreator public static ProtocolsEnum fromValue(String text) { + if (text == null) { + return null; + } for (ProtocolsEnum b : ProtocolsEnum.values()) { - if (String.valueOf(b.value).equals(text)) { + if (String.valueOf(b.value).equalsIgnoreCase(text) || b.name().equalsIgnoreCase(text)) { return b; } } - return null; + throw new IllegalArgumentException("Unexpected protocol value '" + text + "'"); } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java index ecdd3efd2c5c..823e8b354ef5 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java @@ -19,30 +19,38 @@ package org.apache.cloudstack.storage.listener; +import java.util.List; +import java.util.Map; + import javax.inject.Inject; -import com.cloud.agent.api.ModifyStoragePoolCommand; +import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener; +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.service.StorageStrategy; +import org.apache.cloudstack.storage.service.model.AccessGroup; +import org.apache.cloudstack.storage.service.model.ProtocolType; +import org.apache.cloudstack.storage.utils.OntapStorageConstants; +import org.apache.cloudstack.storage.utils.OntapStorageUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; import com.cloud.agent.api.ModifyStoragePoolAnswer; +import com.cloud.agent.api.ModifyStoragePoolCommand; import com.cloud.agent.api.StoragePoolInfo; import com.cloud.alert.AlertManager; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; +import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.dao.StoragePoolHostDao; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; -import com.cloud.agent.AgentManager; -import com.cloud.agent.api.Answer; -import com.cloud.agent.api.DeleteStoragePoolCommand; -import com.cloud.host.Host; -import com.cloud.storage.StoragePool; import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; -import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener; -import com.cloud.host.dao.HostDao; - -import java.util.Map; public class OntapHostListener implements HypervisorHostListener { protected Logger logger = LogManager.getLogger(getClass()); @@ -83,6 +91,19 @@ public boolean hostConnect(long hostId, long poolId) { try { // Load storage pool details from database to pass mount options and other config to agent Map detailsMap = _storagePoolDetailsDao.listDetailsKeyPairs(poolId); + if (detailsMap == null || detailsMap.isEmpty()) { + logger.error("Failed to load storage pool details for pool id: {}", poolId); + return false; + } + + if (detailsMap.get(OntapStorageConstants.PROTOCOL) == null) { + logger.error("Storage pool details missing required protocol type for pool id: {}", poolId); + return false; + } + + // Update NFS export policy for this connected host when the pool protocol is NFS3. + updateNfsExportPolicyForConnectedHostIfNeeded(poolId, hostId, host, detailsMap); + // Create the ModifyStoragePoolCommand to send to the agent // Note: Always send command even if database entry exists, because agent may have restarted // and lost in-memory pool registration. The command handler is idempotent. @@ -151,43 +172,86 @@ public boolean hostConnect(long hostId, long poolId) { return true; } + private void updateNfsExportPolicyForConnectedHostIfNeeded(long poolId, long hostId, Host host, Map detailsMap) { + if (!ProtocolType.NFS3.name().equalsIgnoreCase(detailsMap.get(OntapStorageConstants.PROTOCOL))) { + return; + } + + if (host == null) { + throw new CloudRuntimeException("Host was not found with id: " + hostId); + } + + if (!(host instanceof HostVO)) { + throw new CloudRuntimeException("Host object is not of type HostVO for hostId: " + hostId); + } + + HostVO hostVO = (HostVO) host; + if (!isNfs3EnabledOnHost(hostVO)) { + throw new CloudRuntimeException("NFS protocol is not enabled on host with id: " + hostId); + } + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(poolId); + accessGroup.setHostsToConnect(List.of(hostVO)); + + StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(detailsMap); + strategy.updateAccessGroup(accessGroup); + logger.info("Updated NFS export policy rules for host {} on storage pool {}", host.getName(), poolId); + } + + private boolean isNfs3EnabledOnHost(HostVO host) { + if (host == null) { + return false; + } + + String storageIp = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : ""; + if (storageIp.isEmpty() && StringUtils.isBlank(host.getPrivateIpAddress())) { + logger.warn("Host {} is not eligible for NFS3 protocol: both storage IP and private IP are empty", + host.getId()); + return false; + } + + return true; + } + @Override public boolean hostDisconnected(long hostId, long poolId) { logger.info("Disconnect from host " + hostId + " from pool " + poolId); + // Note: This is currently only called for PowerFlex storage pools. + // For ONTAP NFS pools, export policy cleanup is handled in hostAboutToBeRemoved(). + return false; + } + + @Override + public boolean hostAboutToBeRemoved(long hostId) { + logger.info("Host {} is about to be removed", hostId); - Host hostToremove = _hostDao.findById(hostId); - if (hostToremove == null) { - logger.error("Failed to add host by HostListener as host was not found with id : {}", hostId); + Host host = _hostDao.findById(hostId); + if (host == null) { + logger.warn("Host not found with id: {}", hostId); return false; } - StoragePool pool = _storagePoolDao.findById(poolId); - if (pool == null) { - logger.error("Failed to disconnect host - storage pool not found with id: {}", poolId); + if (!host.getHypervisorType().equals(Hypervisor.HypervisorType.KVM)) { + logger.debug("ONTAP plugin does not support {} type host, skipping cleanup", host.getHypervisorType()); return false; } - logger.info("Disconnecting host {} from ONTAP storage pool {}", hostToremove.getName(), pool.getName()); - try { - DeleteStoragePoolCommand cmd = new DeleteStoragePoolCommand(pool); - Answer answer = _agentMgr.easySend(hostId, cmd); - if (answer != null && answer.getResult()) { - logger.info("Successfully disconnected host {} from ONTAP storage pool {}", hostToremove.getName(), pool.getName()); - return true; - } else { - String errMsg = (answer != null) ? answer.getDetails() : "Unknown error"; - logger.warn("Failed to disconnect host {} from storage pool {}. Error: {}", hostToremove.getName(), pool.getName(), errMsg); - return false; + List poolHostRefs = storagePoolHostDao.listByHostId(hostId); + if (poolHostRefs == null || poolHostRefs.isEmpty()) { + logger.debug("No storage pool associations found for host {}", hostId); + return true; + } + + for (StoragePoolHostVO ref : poolHostRefs) { + StoragePoolVO pool = _storagePoolDao.findById(ref.getPoolId()); + if (pool != null) { + removeHostFromOntapPoolIfNeeded(hostId, pool, host); } - } catch (Exception e) { - logger.error("Exception while disconnecting host {} from storage pool {}", hostToremove.getName(), pool.getName(), e); - return false; } - } - @Override - public boolean hostAboutToBeRemoved(long hostId) { - return false; + logger.info("Cleaned up ONTAP export policies for host {} about to be removed", hostId); + return true; } @Override @@ -195,6 +259,45 @@ public boolean hostRemoved(long hostId, long clusterId) { return false; } + private void removeHostFromOntapPoolIfNeeded(long hostId, StoragePoolVO pool, Host host) { + try { + Map detailsMap = _storagePoolDetailsDao.listDetailsKeyPairs(pool.getId()); + if (detailsMap == null || detailsMap.isEmpty()) { + logger.debug("No pool details found for pool id: {}", pool.getId()); + return; + } + + // Skip non-NFS3 pools + if (!ProtocolType.NFS3.name().equalsIgnoreCase(detailsMap.get(OntapStorageConstants.PROTOCOL))) { + return; + } + + if (!(host instanceof HostVO)) { + logger.warn("Host object is not of type HostVO for hostId: {}", hostId); + return; + } + + logger.info("Removing export policy rule for host {} from storage pool {}", host.getName(), pool.getName()); + HostVO hostVO = (HostVO) host; + if (!isNfs3EnabledOnHost(hostVO)) { + logger.warn("Skipping NFS export policy removal for host {} on pool {} as host is not NFS-enabled", + hostId, pool.getId()); + return; + } + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(pool.getId()); + accessGroup.setHostsToConnect(List.of(hostVO)); + accessGroup.setHostRuleAction(AccessGroup.HostRuleAction.REMOVE); + + StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(detailsMap); + strategy.updateAccessGroup(accessGroup); + logger.info("Removed NFS export policy rules for removed host {} from storage pool {}", host.getName(), pool.getName()); + } catch (Exception e) { + logger.warn("Failed to remove NFS export policy rule for host {} from pool {}: {}", hostId, pool.getName(), e.getMessage()); + // Continue processing other pools even if one fails + } + } + @Override public boolean hostEnabled(long hostId) { return false; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index bd808a26d6f8..45d3edd5ca0f 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -19,13 +19,16 @@ package org.apache.cloudstack.storage.service; -import com.cloud.utils.exception.CloudRuntimeException; -import feign.FeignException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + import org.apache.cloudstack.storage.feign.FeignClientFactory; import org.apache.cloudstack.storage.feign.client.AggregateFeignClient; import org.apache.cloudstack.storage.feign.client.JobFeignClient; -import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.NASFeignClient; +import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; import org.apache.cloudstack.storage.feign.client.SvmFeignClient; @@ -48,10 +51,9 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; +import com.cloud.utils.exception.CloudRuntimeException; + +import feign.FeignException; /** * Storage Strategy represents the communication path for all the ONTAP storage options @@ -577,7 +579,7 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam * @param accessGroup the access group to update * @return the updated AccessGroup object */ - abstract AccessGroup updateAccessGroup(AccessGroup accessGroup); + public abstract AccessGroup updateAccessGroup(AccessGroup accessGroup); /** * Method encapsulates the behavior based on the opted protocol in subclasses diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java index 477e92630387..1bd82fa0eabd 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java @@ -19,19 +19,21 @@ package org.apache.cloudstack.storage.service; -import com.cloud.agent.api.Answer; -import com.cloud.host.HostVO; -import com.cloud.storage.Storage; -import com.cloud.storage.VolumeVO; -import com.cloud.storage.dao.VolumeDao; -import com.cloud.utils.exception.CloudRuntimeException; -import feign.FeignException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import javax.inject.Inject; + import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.storage.command.CreateObjectCommand; import org.apache.cloudstack.storage.command.DeleteCommand; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.feign.model.ExportPolicy; import org.apache.cloudstack.storage.feign.model.ExportRule; import org.apache.cloudstack.storage.feign.model.FileInfo; @@ -42,19 +44,22 @@ import org.apache.cloudstack.storage.feign.model.Volume; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; -import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; -import org.apache.cloudstack.storage.volume.VolumeObject; import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; +import org.apache.cloudstack.storage.volume.VolumeObject; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import javax.inject.Inject; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; +import com.cloud.agent.api.Answer; +import com.cloud.host.HostVO; +import com.cloud.storage.Storage; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.utils.exception.CloudRuntimeException; + +import feign.FeignException; public class UnifiedNASStrategy extends NASStrategy { private static final Logger logger = LogManager.getLogger(UnifiedNASStrategy.class); @@ -193,7 +198,114 @@ public void deleteAccessGroup(AccessGroup accessGroup) { @Override public AccessGroup updateAccessGroup(AccessGroup accessGroup) { - return null; + if (accessGroup == null) { + throw new CloudRuntimeException("Invalid accessGroup object - accessGroup is null"); + } + if (accessGroup.getStoragePoolId() == null) { + throw new CloudRuntimeException("Invalid accessGroup object - storagePoolId is null"); + } + if (accessGroup.getHostsToConnect() == null || accessGroup.getHostsToConnect().isEmpty()) { + throw new CloudRuntimeException("Invalid accessGroup object - hostsToConnect is null or empty"); + } + + Map details = storagePoolDetailsDao.listDetailsKeyPairs(accessGroup.getStoragePoolId()); + String exportPolicyId = details.get(OntapStorageConstants.EXPORT_POLICY_ID); + + // No policy exists yet (e.g. pool was created before any eligible hosts came up), create it now. + if (exportPolicyId == null || exportPolicyId.isEmpty()) { + logger.info("updateAccessGroup: No export policy found for pool {}. Creating one now.", accessGroup.getStoragePoolId()); + return createAccessGroup(accessGroup); + } + + try { + String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + ExportPolicy existingPolicy = nasFeignClient.getExportPolicyById(authHeader, exportPolicyId); + if (existingPolicy == null) { + throw new CloudRuntimeException("Failed to fetch existing export policy with id: " + exportPolicyId); + } + + List rules = existingPolicy.getRules(); + if (rules == null || rules.isEmpty()) { + rules = new ArrayList<>(); + ExportRule newRule = new ExportRule(); + newRule.setProtocols(List.of(ExportRule.ProtocolsEnum.NFS3)); + newRule.setRoRule(List.of("sys")); + newRule.setRwRule(List.of("sys")); + newRule.setSuperuser(List.of("sys")); + newRule.setClients(new ArrayList<>()); + rules.add(newRule); + } + + ExportRule exportRule = rules.get(0); + List exportClients = exportRule.getClients(); + if (exportClients == null) { + exportClients = new ArrayList<>(); + exportRule.setClients(exportClients); + } + + Set hostMatches = new HashSet<>(); + for (HostVO host : accessGroup.getHostsToConnect()) { + String hostStorageIp = host.getStorageIpAddress(); + String ip = (hostStorageIp != null && !hostStorageIp.isEmpty()) ? hostStorageIp : host.getPrivateIpAddress(); + if (ip == null || ip.isEmpty()) { + logger.warn("updateAccessGroup: Host {} has no storage/private IP, skipping export rule update", host.getName()); + continue; + } + hostMatches.add(ip + "/32"); + } + + if (hostMatches.isEmpty()) { + accessGroup.setPolicy(existingPolicy); + return accessGroup; + } + + boolean updated = false; + if (AccessGroup.HostRuleAction.REMOVE.equals(accessGroup.getHostRuleAction())) { + updated = exportClients.removeIf(c -> c != null && c.getMatch() != null && hostMatches.contains(c.getMatch())); + if (!updated) { + logger.info("updateAccessGroup: No matching host IPs found in export policy {} for removal", existingPolicy.getName()); + } + } else { + Set existingMatches = new HashSet<>(); + for (ExportRule.ExportClient exportClient : exportClients) { + if (exportClient != null && exportClient.getMatch() != null) { + existingMatches.add(exportClient.getMatch()); + } + } + + for (String match : hostMatches) { + if (existingMatches.add(match)) { + ExportRule.ExportClient exportClient = new ExportRule.ExportClient(); + exportClient.setMatch(match); + exportClients.add(exportClient); + updated = true; + } + } + } + + if (!updated) { + if (!AccessGroup.HostRuleAction.REMOVE.equals(accessGroup.getHostRuleAction())) { + logger.info("updateAccessGroup: No new host IPs to add to export policy {}", existingPolicy.getName()); + } + } + + if (!updated) { + accessGroup.setPolicy(existingPolicy); + return accessGroup; + } + + ExportPolicy updateRequest = new ExportPolicy(); + updateRequest.setRules(rules); + nasFeignClient.updateExportPolicy(authHeader, exportPolicyId, updateRequest); + + existingPolicy.setRules(rules); + accessGroup.setPolicy(existingPolicy); + logger.info("updateAccessGroup: Successfully updated export policy {} with new host client rules", existingPolicy.getName()); + return accessGroup; + } catch (Exception e) { + logger.error("updateAccessGroup: Failed to update export policy for pool {}", accessGroup.getStoragePoolId(), e); + throw new CloudRuntimeException("Failed to update export policy for NFS host connection: " + e.getMessage(), e); + } } @Override diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java index 9815724fc1aa..8b1aa24fe5d9 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java @@ -19,21 +19,28 @@ package org.apache.cloudstack.storage.service.model; -import com.cloud.host.HostVO; +import java.util.List; + import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.storage.feign.model.ExportPolicy; import org.apache.cloudstack.storage.feign.model.Igroup; -import java.util.List; +import com.cloud.host.HostVO; public class AccessGroup { + public enum HostRuleAction { + ADD, + REMOVE + } + private Igroup igroup; private ExportPolicy exportPolicy; private List hostsToConnect; private Long storagePoolId; private Scope scope; + private HostRuleAction hostRuleAction = HostRuleAction.ADD; public Igroup getIgroup() { @@ -74,4 +81,12 @@ public Scope getScope() { public void setScope(Scope scope) { this.scope = scope; } + + public HostRuleAction getHostRuleAction() { + return hostRuleAction; + } + + public void setHostRuleAction(HostRuleAction hostRuleAction) { + this.hostRuleAction = hostRuleAction; + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java index 86ef1d7c79b6..63d98dec3dd4 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java @@ -18,8 +18,11 @@ */ package org.apache.cloudstack.storage.service; -import com.cloud.utils.exception.CloudRuntimeException; -import feign.FeignException; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + import org.apache.cloudstack.storage.feign.client.AggregateFeignClient; import org.apache.cloudstack.storage.feign.client.JobFeignClient; import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; @@ -39,32 +42,30 @@ import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.utils.OntapStorageConstants; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.mockito.junit.jupiter.MockitoSettings; -import org.mockito.quality.Strictness; - -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import org.mockito.Mock; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; + +import com.cloud.utils.exception.CloudRuntimeException; + +import feign.FeignException; @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) @@ -158,7 +159,7 @@ public void deleteAccessGroup(AccessGroup accessGroup) { } @Override - AccessGroup updateAccessGroup(AccessGroup accessGroup) { + public AccessGroup updateAccessGroup(AccessGroup accessGroup) { return null; }