Skip to content

CSTACKEX-114: Delete volume or qcow2 file NFS #32

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

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

CSTACKEX-114: Delete volume or qcow2 file NFS #32

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
19afcd1
CSTACKEX-114: Delete volume or qcow2 file NFS
Feb 10, 2026
87bf0f9
CSTACKEX-114: Updated UT and review comments
Feb 11, 2026
7a3fdda
CSTACKEX-114: latest main
Feb 11, 2026
40e8d87
CSTACKEX-114: resolved UT failure
Feb 11, 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
71 changes: 39 additions & 32 deletions ...ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,39 +234,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());
Expand Down Expand Up @@ -610,6 +586,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;

}
}
46 changes: 43 additions & 3 deletions .../volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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
Expand Down Expand Up @@ -508,4 +521,31 @@ private Answer createVolumeOnKVMHost(DataObject volumeInfo) {
return new Answer(null, false, e.toString());
}
}

private Answer deleteVolumeOnKVMHost(DataObject volumeInfo) {
Copy link
Collaborator

@sandeeplocharla sandeeplocharla Feb 11, 2026

Choose a reason for hiding this comment

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

Is there a way to include this in cloudstack core code, instead of having it here?
This looks like a good candidate for contributing to the community.

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());
}
}
}
1 change: 0 additions & 1 deletion ...p/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
71 changes: 71 additions & 0 deletions ...ume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
});
}
}
Loading