CSTACKEX-114: Delete volume or qcow2 file NFS

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

@@ -168,16 +168,15 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
                         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));
 
                     } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
-                        // For NFS, the hypervisor handles file creation; we only track pool association
+                        createCmdResult = new CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null));
                         s_logger.info("createAsync: Managed NFS volume [{}] associated with pool {}",
                                 volumeVO.getId(), storagePool.getId());
                     }
-
                     volumeDao.update(volumeVO.getId(), volumeVO);
                 }
-                createCmdResult = new CreateCmdResult(null, new Answer(null, true, null));
             } else {
                 errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync";
                 s_logger.error(errMsg);
@@ -234,39 +233,15 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac
                     s_logger.error("deleteAsync: Storage Pool not found for id: " + store.getId());
                     throw new CloudRuntimeException("deleteAsync: Storage Pool not found for id: " + store.getId());
                 }
-
                 Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(store.getId());
-
-                if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
-                    // NFS file deletion is handled by the hypervisor; no ONTAP REST call needed
-                    s_logger.info("deleteAsync: ManagedNFS volume {} - file deletion handled by hypervisor", data.getId());
-
-                } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
-                    StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details);
-                    VolumeInfo volumeObject = (VolumeInfo) data;
-                    s_logger.info("deleteAsync: Deleting LUN for volume id [{}]", volumeObject.getId());
-
-                    // Retrieve LUN identifiers stored during volume creation
-                    String lunName = volumeDetailsDao.findDetail(volumeObject.getId(), Constants.LUN_DOT_NAME).getValue();
-                    String lunUUID = volumeDetailsDao.findDetail(volumeObject.getId(), Constants.LUN_DOT_UUID).getValue();
-                    if (lunName == null) {
-                        throw new CloudRuntimeException("deleteAsync: Missing LUN name for volume " + volumeObject.getId());
-                    }
-
-                    CloudStackVolume delRequest = new CloudStackVolume();
-                    Lun lun = new Lun();
-                    lun.setName(lunName);
-                    lun.setUuid(lunUUID);
-                    delRequest.setLun(lun);
-                    storageStrategy.deleteCloudStackVolume(delRequest);
-
-                    commandResult.setResult(null);
-                    commandResult.setSuccess(true);
-                    s_logger.info("deleteAsync: LUN [{}] deleted successfully", lunName);
-
-                } else {
-                    throw new CloudRuntimeException("deleteAsync: Unsupported protocol: " + details.get(Constants.PROTOCOL));
-                }
+                StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details);
+                s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME));
+                VolumeInfo volumeInfo = (VolumeInfo) data;
+                CloudStackVolume cloudStackVolumeRequest = createDeleteCloudStackVolumeRequest(storagePool,details,volumeInfo);
+                storageStrategy.deleteCloudStackVolume(cloudStackVolumeRequest);
+                s_logger.error("deleteAsync : Volume deleted: " + volumeInfo.getId());
+                commandResult.setResult(null);
+                commandResult.setSuccess(true);
             }
         } catch (Exception e) {
             s_logger.error("deleteAsync: Failed for data object [{}]: {}", data, e.getMessage());
@@ -339,9 +314,10 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
 
                 Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
                 String svmName = details.get(Constants.SVM_NAME);
-                String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue();
 
                 if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
+                    // 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);
 
@@ -360,8 +336,11 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
                         volumeVO.set_iScsiName(iscsiPath);
                         volumeVO.setPath(iscsiPath);
                     }
+                } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
+                    // For NFS, no access grant needed - file is accessible via mount
+                    s_logger.debug("grantAccess: NFS volume [{}], no igroup mapping required", volumeVO.getUuid());
+                    return true;
                 }
-
                 volumeVO.setPoolType(storagePool.getPoolType());
                 volumeVO.setPoolId(storagePool.getId());
                 volumeDao.update(volumeVO.getId(), volumeVO);
@@ -610,6 +589,37 @@ public boolean isStorageSupportHA(Storage.StoragePoolType type) {
 
     @Override
     public void detachVolumeFromAllStorageNodes(Volume volume) {
+    }
+
+    private CloudStackVolume createDeleteCloudStackVolumeRequest(StoragePool storagePool, Map<String, String> details, VolumeInfo volumeInfo) {
+        CloudStackVolume cloudStackVolumeDeleteRequest = null;
+
+        String protocol = details.get(Constants.PROTOCOL);
+        ProtocolType protocolType = ProtocolType.valueOf(protocol);
+        switch (protocolType) {
+            case NFS3:
+                cloudStackVolumeDeleteRequest = new CloudStackVolume();
+                cloudStackVolumeDeleteRequest.setDatastoreId(String.valueOf(storagePool.getId()));
+                cloudStackVolumeDeleteRequest.setVolumeInfo(volumeInfo);
+                break;
+            case ISCSI:
+                // Retrieve LUN identifiers stored during volume creation
+                String lunName = volumeDetailsDao.findDetail(volumeInfo.getId(), Constants.LUN_DOT_NAME).getValue();
+                String lunUUID = volumeDetailsDao.findDetail(volumeInfo.getId(), Constants.LUN_DOT_UUID).getValue();
+                if (lunName == null) {
+                    throw new CloudRuntimeException("deleteAsync: Missing LUN name for volume " + volumeInfo.getId());
+                }
+                cloudStackVolumeDeleteRequest = new CloudStackVolume();
+                Lun lun = new Lun();
+                lun.setName(lunName);
+                lun.setUuid(lunUUID);
+                cloudStackVolumeDeleteRequest.setLun(lun);
+                break;
+            default:
+                throw new CloudRuntimeException("createDeleteCloudStackVolumeRequest: Unsupported protocol " + protocol);
+
+        }
+        return cloudStackVolumeDeleteRequest;
 
     }
 }

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

@@ -230,7 +230,9 @@ public DataStore initialize(Map<String, Object> dsInfos) {
                 parameters.setType(Storage.StoragePoolType.NetworkFilesystem);
                 path = Constants.SLASH + storagePoolName;
                 port = Constants.NFS3_PORT;
-                s_logger.info("Setting NFS path for storage pool: " + path + ", port: " + port);
+                // Force NFSv3 for ONTAP managed storage to avoid NFSv4 ID mapping issues
+                details.put("nfsmountopts", "vers=3");
+                s_logger.info("Setting NFS path for storage pool: " + path + ", port: " + port + " with mount option: vers=3");
                 break;
             case ISCSI:
                 parameters.setType(Storage.StoragePoolType.Iscsi);

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java

@@ -37,9 +37,12 @@
 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());
 
@@ -51,7 +54,10 @@ public class OntapHostListener implements HypervisorHostListener {
     private PrimaryDataStoreDao _storagePoolDao;
     @Inject
     private HostDao _hostDao;
-    @Inject private StoragePoolHostDao storagePoolHostDao;
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+    @Inject
+    private StoragePoolDetailsDao _storagePoolDetailsDao;
 
 
     @Override
@@ -71,10 +77,12 @@ public boolean hostConnect(long hostId, long poolId)  {
         }
         logger.info("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);
             // 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.
-            ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool);
+            ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool, detailsMap);
 
             Answer answer = _agentMgr.easySend(hostId, cmd);
 

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

@@ -31,6 +31,7 @@
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
 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.FeignClientFactory;
 import org.apache.cloudstack.storage.feign.client.JobFeignClient;
@@ -93,12 +94,12 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume
             Answer answer = createVolumeOnKVMHost(cloudstackVolume.getVolumeInfo());
             if (answer == null || !answer.getResult()) {
                 String errMsg = answer != null ? answer.getDetails() : "Failed to create qcow2 on KVM host";
-                s_logger.error("createCloudStackVolumeForTypeVolume: " + errMsg);
+                s_logger.error("createCloudStackVolume: " + errMsg);
                 throw new CloudRuntimeException(errMsg);
             }
             return cloudstackVolume;
         }catch (Exception e) {
-            s_logger.error("createCloudStackVolumeForTypeVolume: error occured " + e);
+            s_logger.error("createCloudStackVolume: error occured " + e);
             throw new CloudRuntimeException(e);
         }
     }
@@ -111,7 +112,19 @@ CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {
 
     @Override
     public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {
-        //TODO
+        s_logger.info("deleteCloudStackVolume: Delete cloudstack volume " + cloudstackVolume);
+        try {
+            // Step 1: Send command to KVM host to delete qcow2 file using qemu-img
+            Answer answer = deleteVolumeOnKVMHost(cloudstackVolume.getVolumeInfo());
+            if (answer == null || !answer.getResult()) {
+                String errMsg = answer != null ? answer.getDetails() : "Failed to delete qcow2 on KVM host";
+                s_logger.error("deleteCloudStackVolume: " + errMsg);
+                throw new CloudRuntimeException(errMsg);
+            }
+        }catch (Exception e) {
+            s_logger.error("deleteCloudStackVolume: error occured " + e);
+            throw new CloudRuntimeException(e);
+        }
     }
 
     @Override
@@ -445,7 +458,7 @@ private ExportPolicy createExportPolicyRequest(AccessGroup accessGroup,String sv
             exportClients.add(exportClient);
         }
         exportRule.setClients(exportClients);
-        exportRule.setProtocols(List.of(ExportRule.ProtocolsEnum.any));
+        exportRule.setProtocols(List.of(ExportRule.ProtocolsEnum.nfs3));
         exportRule.setRoRule(List.of("sys"));
         exportRule.setRwRule(List.of("sys"));
         exportRule.setSuperuser(List.of("sys"));
@@ -508,4 +521,31 @@ private Answer createVolumeOnKVMHost(DataObject volumeInfo) {
             return new Answer(null, false, e.toString());
         }
     }
+
+    private Answer deleteVolumeOnKVMHost(DataObject volumeInfo) {
+        s_logger.info("deleteVolumeOnKVMHost called with volumeInfo: {} ", volumeInfo);
+
+        try {
+            s_logger.info("deleteVolumeOnKVMHost: Sending DeleteCommand to KVM agent for volume: {}", volumeInfo.getUuid());
+            DeleteCommand cmd = new DeleteCommand(volumeInfo.getTO());
+            EndPoint ep = epSelector.select(volumeInfo);
+            if (ep == null) {
+                String errMsg = "No remote endpoint to send DeleteCommand, check if host is up";
+                s_logger.error(errMsg);
+                return new Answer(cmd, false, errMsg);
+            }
+            s_logger.info("deleteVolumeOnKVMHost: Sending command to endpoint: {}", ep.getHostAddr());
+            Answer answer = ep.sendMessage(cmd);
+            if (answer != null && answer.getResult()) {
+                s_logger.info("deleteVolumeOnKVMHost: Successfully deleted qcow2 file on KVM host");
+            } else {
+                s_logger.error("deleteVolumeOnKVMHost: Failed to delete qcow2 file: {}",
+                        answer != null ? answer.getDetails() : "null answer");
+            }
+            return answer;
+        } catch (Exception e) {
+            s_logger.error("deleteVolumeOnKVMHost: Exception sending DeleteCommand", e);
+            return new Answer(null, false, e.toString());
+        }
+    }
 }

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java

@@ -308,7 +308,6 @@ void testDeleteAsync_NFSVolume_Success() {
 
         when(dataStore.getId()).thenReturn(1L);
         when(volumeInfo.getType()).thenReturn(VOLUME);
-        when(volumeInfo.getId()).thenReturn(100L);
 
         when(storagePoolDao.findById(1L)).thenReturn(storagePool);
         when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java

@@ -27,6 +27,7 @@
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.storage.command.CreateObjectCommand;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.feign.client.JobFeignClient;
@@ -533,4 +534,74 @@ public void testDeleteAccessGroup_Failed() {
             strategy.deleteAccessGroup(accessGroup);
         });
     }
+
+    // Test deleteCloudStackVolume - Success
+    @Test
+    public void testDeleteCloudStackVolume_Success() throws Exception {
+        CloudStackVolume cloudStackVolume = mock(CloudStackVolume.class);
+        VolumeInfo volumeInfo = mock(VolumeInfo.class);
+        EndPoint endpoint = mock(EndPoint.class);
+        Answer answer = mock(Answer.class);
+
+        when(cloudStackVolume.getVolumeInfo()).thenReturn(volumeInfo);
+        when(epSelector.select(volumeInfo)).thenReturn(endpoint);
+        when(endpoint.sendMessage(any())).thenReturn(answer);
+        when(answer.getResult()).thenReturn(true);
+
+        // Execute - should not throw exception
+        strategy.deleteCloudStackVolume(cloudStackVolume);
+
+        // Verify endpoint was selected and message sent
+        verify(epSelector).select(volumeInfo);
+        verify(endpoint).sendMessage(any());
+    }
+
+    // Test deleteCloudStackVolume - Endpoint Not Found
+    @Test
+    public void testDeleteCloudStackVolume_EndpointNotFound() {
+        CloudStackVolume cloudStackVolume = mock(CloudStackVolume.class);
+        VolumeInfo volumeInfo = mock(VolumeInfo.class);
+
+        when(cloudStackVolume.getVolumeInfo()).thenReturn(volumeInfo);
+        when(epSelector.select(volumeInfo)).thenReturn(null);
+
+        assertThrows(CloudRuntimeException.class, () -> {
+            strategy.deleteCloudStackVolume(cloudStackVolume);
+        });
+    }
+
+    // Test deleteCloudStackVolume - Answer Result False
+    @Test
+    public void testDeleteCloudStackVolume_AnswerResultFalse() throws Exception {
+        CloudStackVolume cloudStackVolume = mock(CloudStackVolume.class);
+        VolumeInfo volumeInfo = mock(VolumeInfo.class);
+        EndPoint endpoint = mock(EndPoint.class);
+        Answer answer = mock(Answer.class);
+
+        when(cloudStackVolume.getVolumeInfo()).thenReturn(volumeInfo);
+        when(epSelector.select(volumeInfo)).thenReturn(endpoint);
+        when(endpoint.sendMessage(any())).thenReturn(answer);
+        when(answer.getResult()).thenReturn(false);
+        when(answer.getDetails()).thenReturn("Failed to delete volume file");
+
+        assertThrows(CloudRuntimeException.class, () -> {
+            strategy.deleteCloudStackVolume(cloudStackVolume);
+        });
+    }
+
+    // Test deleteCloudStackVolume - Answer is Null
+    @Test
+    public void testDeleteCloudStackVolume_AnswerNull() throws Exception {
+        CloudStackVolume cloudStackVolume = mock(CloudStackVolume.class);
+        VolumeInfo volumeInfo = mock(VolumeInfo.class);
+        EndPoint endpoint = mock(EndPoint.class);
+
+        when(cloudStackVolume.getVolumeInfo()).thenReturn(volumeInfo);
+        when(epSelector.select(volumeInfo)).thenReturn(endpoint);
+        when(endpoint.sendMessage(any())).thenReturn(null);
+
+        assertThrows(CloudRuntimeException.class, () -> {
+            strategy.deleteCloudStackVolume(cloudStackVolume);
+        });
+    }
 }