Merge NetApp main into feature/CSTACKEX-228

Bring in CSTACKEX-158 (idempotent ONTAP snapshot delete) and
multi-host NFS/iSCSI support (#64) on top of the apache/main sync.

Author: rajiv-jain-netapp

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

@@ -316,7 +316,7 @@ private void deleteCloudStackVolumeSnapshot(SnapshotInfo snapshotInfo, CommandRe
             commandResult.setSuccess(true);
             commandResult.setResult(null);
         } catch (Exception e) {
-            if (isSnapshotNotFoundError(e)) {
+            if (OntapStorageUtils.isOntapObjectNotFoundError(e)) {
                 logger.warn("deleteCloudStackVolumeSnapshot: ONTAP snapshot for CloudStack snapshot [{}] "
                         + "already absent (idempotent success): {}", snapshotId, e.getMessage());
                 commandResult.setSuccess(true);
@@ -330,25 +330,6 @@ private void deleteCloudStackVolumeSnapshot(SnapshotInfo snapshotInfo, CommandRe
         }
     }
 
-    /**
-     * Returns true when the exception indicates the ONTAP snapshot was already removed.
-     * Delete is idempotent: a missing backend snapshot is treated as success.
-     */
-    private boolean isSnapshotNotFoundError(Throwable error) {
-        if (error == null) {
-            return false;
-        }
-        String message = error.getMessage();
-        if (message != null) {
-            String lower = message.toLowerCase();
-            if (lower.contains("404") || lower.contains("not found") || lower.contains("does not exist")
-                    || lower.contains("entry doesn't exist")) {
-                return true;
-            }
-        }
-        return isSnapshotNotFoundError(error.getCause());
-    }
-
     private long resolveSnapshotPoolId(String poolIdStr, long snapshotId) {
         if (poolIdStr != null && !poolIdStr.isEmpty()) {
             return Long.parseLong(poolIdStr);

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,9 +67,13 @@ 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)) {
                     return 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());
@@ -63,26 +71,39 @@ public class OntapHostListener implements HypervisorHostListener {
 
     @Override
     public boolean hostConnect(long hostId, long poolId)  {
-        logger.info("Connect to host " + hostId + " from pool " + poolId);
+        logger.info("hostConnect: Connecting host {} to pool {}", hostId, poolId);
         Host host = _hostDao.findById(hostId);
         if (host == null) {
-            logger.error("host was not found with id : {}", hostId);
+            logger.error("hostConnect: Host was not found with id: {}", hostId);
             return false;
         }
         if (!host.getHypervisorType().equals(Hypervisor.HypervisorType.KVM)) {
-            logger.error("ONTAP plugin does not support {} type host currently ", host.getHypervisorType());
+            logger.error("hostConnect: ONTAP plugin does not support {} type host currently", host.getHypervisorType());
             return false;
         }
 
         StoragePool pool = _storagePoolDao.findById(poolId);
         if (pool == null) {
-            logger.error("Failed to connect host - storage pool not found with id: {}", poolId);
+            logger.error("hostConnect: Failed to connect host - storage pool not found with id: {}", poolId);
             return false;
         }
-        logger.info("Connecting host {} to ONTAP storage pool {}", host.getName(), pool.getName());
+        logger.info("hostConnect: Connecting host {} to ONTAP storage pool {}", host.getName(), pool.getName());
         try {
             // Load storage pool details from database to pass mount options and other config to agent
             Map<String, String> detailsMap = _storagePoolDetailsDao.listDetailsKeyPairs(poolId);
+            if (detailsMap == null || detailsMap.isEmpty()) {
+                logger.error("hostConnect: Failed to load storage pool details for pool id: {}", poolId);
+                return false;
+            }
+
+            if (detailsMap.get(OntapStorageConstants.PROTOCOL) == null) {
+                logger.error("hostConnect: 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.
@@ -118,19 +139,19 @@ public boolean hostConnect(long hostId, long poolId)  {
             }
 
             String localPath = poolInfo.getLocalPath();
-            logger.info("Storage pool {} successfully mounted at: {}", pool.getName(), localPath);
+            logger.info("hostConnect: Storage pool {} successfully mounted at: {}", pool.getName(), localPath);
 
             // Update or create the storage_pool_host_ref entry with the correct local_path
             StoragePoolHostVO storagePoolHost = storagePoolHostDao.findByPoolHost(poolId, hostId);
 
             if (storagePoolHost == null) {
                 storagePoolHost = new StoragePoolHostVO(poolId, hostId, localPath);
                 storagePoolHostDao.persist(storagePoolHost);
-                logger.info("Created storage_pool_host_ref entry for pool {} and host {}", pool.getName(), host.getName());
+                logger.info("hostConnect: Created storage_pool_host_ref entry for pool {} and host {}", pool.getName(), host.getName());
             } else {
                 storagePoolHost.setLocalPath(localPath);
                 storagePoolHostDao.update(storagePoolHost.getId(), storagePoolHost);
-                logger.info("Updated storage_pool_host_ref entry with local_path: {}", localPath);
+                logger.info("hostConnect: Updated storage_pool_host_ref entry with local_path: {}", localPath);
             }
 
             // Update pool capacity/usage information
@@ -139,62 +160,127 @@ public boolean hostConnect(long hostId, long poolId)  {
                 poolVO.setCapacityBytes(poolInfo.getCapacityBytes());
                 poolVO.setUsedBytes(poolInfo.getCapacityBytes() - poolInfo.getAvailableBytes());
                 _storagePoolDao.update(poolVO.getId(), poolVO);
-                logger.info("Updated storage pool capacity: {} GB, used: {} GB", poolInfo.getCapacityBytes() / (1024 * 1024 * 1024), (poolInfo.getCapacityBytes() - poolInfo.getAvailableBytes()) / (1024 * 1024 * 1024));
+                logger.info("hostConnect: Updated storage pool capacity: {} GB, used: {} GB", poolInfo.getCapacityBytes() / (1024 * 1024 * 1024), (poolInfo.getCapacityBytes() - poolInfo.getAvailableBytes()) / (1024 * 1024 * 1024));
             }
 
         } catch (Exception e) {
-            logger.error("Exception while connecting host {} to storage pool {}", host.getName(), pool.getName(), e);
+            logger.error("hostConnect: Exception while connecting host {} to storage pool {}", host.getName(), pool.getName(), e);
             // CRITICAL: Don't throw exception - it crashes the agent and causes restart loops
             // Return false to indicate failure without crashing
             return false;
         }
         return true;
     }
 
-    @Override
-    public boolean hostDisconnected(long hostId, long poolId) {
-        logger.info("Disconnect from host " + hostId + " from pool " + poolId);
+    private void updateNfsExportPolicyForConnectedHostIfNeeded(long poolId, long hostId, Host host, Map<String, String> detailsMap) {
+        if (!ProtocolType.NFS3.name().equalsIgnoreCase(detailsMap.get(OntapStorageConstants.PROTOCOL))) {
+            return;
+        }
 
-        Host hostToremove = _hostDao.findById(hostId);
-        if (hostToremove == null) {
-            logger.error("Failed to add host by HostListener as host was not found with id : {}", hostId);
-            return false;
+        if (host == null) {
+            throw new CloudRuntimeException("Host was not found with id: " + hostId);
         }
 
-        StoragePool pool = _storagePoolDao.findById(poolId);
-        if (pool == null) {
-            logger.error("Failed to disconnect host - storage pool not found with id: {}", poolId);
+        if (!isNfs3EnabledOnHost(host)) {
+            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) host));
+
+        StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(detailsMap);
+        strategy.updateAccessGroup(accessGroup);
+        logger.info("hostConnect: updateNfsExportPolicyForConnectedHostIfNeeded: Updated NFS export policy rules for host {} on storage pool {}", host.getName(), poolId);
+    }
+
+    private boolean isNfs3EnabledOnHost(Host host) {
+        if (host == null) {
             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;
-            }
-        } catch (Exception e) {
-            logger.error("Exception while disconnecting host {} from storage pool {}", hostToremove.getName(), pool.getName(), e);
+        String storageIp = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : "";
+        if (storageIp.isEmpty() && StringUtils.isBlank(host.getPrivateIpAddress())) {
+            logger.warn("isNfs3EnabledOnHost: Host {} is not eligible for NFS3 protocol: both storage IP and private IP are empty",
+                    host.getId());
             return false;
         }
+
+        return true;
     }
 
     @Override
-    public boolean hostAboutToBeRemoved(long hostId) {
+    public boolean hostDisconnected(long hostId, long poolId) {
+        logger.info("hostDisconnected: Disconnecting host {} from pool {}", hostId, poolId);
+        // Note: This is not currently being called for NetApp ONTAP storage plugin.
         return false;
     }
 
+    @Override
+    public boolean hostAboutToBeRemoved(long hostId) {
+        logger.info("hostAboutToBeRemoved: Host {} is about to be removed", hostId);
+
+        Host host = _hostDao.findById(hostId);
+        if (host == null) {
+            logger.warn("hostAboutToBeRemoved: Host not found with id: {}, considering it as no-op", hostId);
+            return true;
+        }
+
+        List<StoragePoolHostVO> poolHostRefs = storagePoolHostDao.listByHostId(hostId);
+        if (poolHostRefs == null || poolHostRefs.isEmpty()) {
+            logger.debug("hostAboutToBeRemoved: No storage pool associations found for host {}", hostId);
+            return true;
+        }
+
+        for (StoragePoolHostVO ref : poolHostRefs) {
+            StoragePoolVO pool = _storagePoolDao.findById(ref.getPoolId());
+            if (pool != null) {
+                removeHostFromOntapPoolIfNeeded(pool, host);
+            }
+        }
+
+        logger.info("hostAboutToBeRemoved: Cleaned up ONTAP export policies for host {} about to be removed", hostId);
+        return true;
+    }
+
     @Override
     public boolean hostRemoved(long hostId, long clusterId) {
         return false;
     }
 
+    private void removeHostFromOntapPoolIfNeeded(StoragePoolVO pool, Host host) {
+        try {
+            Map<String, String> detailsMap = _storagePoolDetailsDao.listDetailsKeyPairs(pool.getId());
+            if (detailsMap == null || detailsMap.isEmpty()) {
+                logger.debug("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: No pool details found for pool id: {}", pool.getId());
+                return;
+            }
+
+            // Skip non-NFS3 pools; Currently, for iSCSI type, iGroup rules are being handled as part of revokeAccess in OntapPrimaryDataStoreDriver, so no need to handle here.
+            if (!ProtocolType.NFS3.name().equalsIgnoreCase(detailsMap.get(OntapStorageConstants.PROTOCOL))) {
+                return;
+            }
+
+            logger.info("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: Removing export policy rule for host {} from storage pool {}", host.getName(), pool.getName());
+            if (!isNfs3EnabledOnHost(host)) {
+                logger.warn("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: Skipping NFS export policy removal for host {} on pool {} as host is not NFS-enabled",
+                        host.getId(), pool.getId());
+                return;
+            }
+            AccessGroup accessGroup = new AccessGroup();
+            accessGroup.setStoragePoolId(pool.getId());
+            accessGroup.setHostsToConnect(List.of((HostVO) host));
+            accessGroup.setHostRuleAction(AccessGroup.HostRuleAction.REMOVE);
+
+            StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(detailsMap);
+            strategy.updateAccessGroup(accessGroup);
+            logger.info("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: Removed NFS export policy rules for removed host {} from storage pool {}", host.getName(), pool.getName());
+        } catch (Exception e) {
+            logger.warn("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: Failed to remove NFS export policy rule for host {} from pool {}: {}", host.getId(), pool.getName(), e.getMessage());
+            // Continue processing other pools even if one fails
+        }
+    }
+
     @Override
     public boolean hostEnabled(long hostId) {
         return false;