From 19afcd16874cfcd342f204d574b786f0453904a2 Mon Sep 17 00:00:00 2001 From: "Srivastava, Piyush" Date: Tue, 10 Feb 2026 10:32:38 +0530 Subject: [PATCH 1/3] CSTACKEX-114: Delete volume or qcow2 file NFS --- .../driver/OntapPrimaryDatastoreDriver.java | 28 +++++++++-- .../storage/service/StorageStrategy.java | 2 +- .../storage/service/UnifiedNASStrategy.java | 48 +++++++++++++++++-- .../storage/service/UnifiedSANStrategy.java | 2 +- .../storage/service/StorageStrategyTest.java | 2 +- 5 files changed, 71 insertions(+), 11 deletions(-) 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 5e79aa2298da..1542ca1e7569 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 @@ -158,10 +158,12 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac throw new CloudRuntimeException("deleteAsync : Storage Pool not found for id: " + store.getId()); } Map details = storagePoolDetailsDao.listDetailsKeyPairs(store.getId()); - if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - // ManagedNFS qcow2 backing file deletion handled by KVM host/libvirt; nothing to do via ONTAP REST. - s_logger.info("deleteAsync: ManagedNFS volume {} no-op ONTAP deletion", data.getId()); - } + 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()); } } catch (Exception e) { commandResult.setResult(e.getMessage()); @@ -291,6 +293,24 @@ public boolean isStorageSupportHA(Storage.StoragePoolType type) { @Override public void detachVolumeFromAllStorageNodes(Volume volume) { + } + + private CloudStackVolume createDeleteCloudStackVolumeRequest(StoragePool storagePool, Map details, VolumeInfo volumeInfo) { + CloudStackVolume cloudStackVolumeRequest = null; + + String protocol = details.get(Constants.PROTOCOL); + ProtocolType protocolType = ProtocolType.valueOf(protocol); + switch (protocolType) { + case NFS3: + cloudStackVolumeRequest = new CloudStackVolume(); + cloudStackVolumeRequest.setDatastoreId(String.valueOf(storagePool.getId())); + cloudStackVolumeRequest.setVolumeInfo(volumeInfo); + break; + default: + throw new CloudRuntimeException("createDeleteCloudStackVolumeRequest: Unsupported protocol " + protocol); + + } + return cloudStackVolumeRequest; } } 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 822e09851f39..e2849053997e 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 @@ -472,7 +472,7 @@ public String getNetworkInterface() { * * @param cloudstackVolume the CloudStack volume to delete */ - abstract void deleteCloudStackVolume(CloudStackVolume cloudstackVolume); + abstract public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume); /** * 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 d75f2e695b10..287c7d721d7b 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 @@ -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); } } @@ -110,8 +111,20 @@ CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { - //TODO + public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { + 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 @@ -493,4 +506,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()); + } + } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 7b5372c69bdd..ff9b2248b1b8 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -99,7 +99,7 @@ CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { + public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { //TODO } 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 d298a6afe937..6728c8a85170 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 @@ -134,7 +134,7 @@ org.apache.cloudstack.storage.service.model.CloudStackVolume updateCloudStackVol } @Override - void deleteCloudStackVolume(org.apache.cloudstack.storage.service.model.CloudStackVolume cloudstackVolume) { + public void deleteCloudStackVolume(org.apache.cloudstack.storage.service.model.CloudStackVolume cloudstackVolume) { } @Override From 87bf0f99432e448b55c36572fffea421c1b39784 Mon Sep 17 00:00:00 2001 From: "Srivastava, Piyush" Date: Wed, 11 Feb 2026 14:16:58 +0530 Subject: [PATCH 2/3] CSTACKEX-114: Updated UT and review comments --- .../driver/OntapPrimaryDatastoreDriver.java | 1 + .../service/UnifiedNASStrategyTest.java | 71 +++++++++++++++++++ 2 files changed, 72 insertions(+) mode change 100644 => 100755 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java mode change 100644 => 100755 plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java 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 old mode 100644 new mode 100755 index 1542ca1e7569..ee9ae67a0949 --- 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 @@ -162,6 +162,7 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac 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); + // NFS qcow2 backing file deletion handled by KVM host/libvirt; nothing to do via ONTAP REST. storageStrategy.deleteCloudStackVolume(cloudStackVolumeRequest); s_logger.error("deleteAsync : Volume deleted: " + volumeInfo.getId()); } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java old mode 100644 new mode 100755 index 1310ceafab7a..cb0e440f1bc4 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java +++ b/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; @@ -534,4 +535,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); + }); + } } From 40e8d87e427278b8c7026421719df3f7d41fbe60 Mon Sep 17 00:00:00 2001 From: "Srivastava, Piyush" Date: Wed, 11 Feb 2026 14:48:32 +0530 Subject: [PATCH 3/3] CSTACKEX-114: resolved UT failure --- .../storage/driver/OntapPrimaryDatastoreDriverTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index b9fe71ec199c..d050d379563c 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/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);