Skip to content

Commit eace4ee

Browse files
piyush5netappSrivastava, Piyush
piyush5netapp
and
Srivastava, Piyush
authored
CSTACKEX-114: Delete volume or qcow2 file NFS (#32)
Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
1 parent 763aa3b commit eace4ee

File tree

6 files changed

+175
-45
lines changed

6 files changed

+175
-45
lines changed

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

100644100755
Lines changed: 47 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,15 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
168168
volumeVO.set_iScsiName(iscsiPath);
169169
volumeVO.setPath(iscsiPath);
170170
s_logger.info("createAsync: Volume [{}] iSCSI path set to {}", volumeVO.getId(), iscsiPath);
171+
createCmdResult = new CreateCmdResult(null, new Answer(null, true, null));
171172

172173
} else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
173-
// For NFS, the hypervisor handles file creation; we only track pool association
174+
createCmdResult = new CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null));
174175
s_logger.info("createAsync: Managed NFS volume [{}] associated with pool {}",
175176
volumeVO.getId(), storagePool.getId());
176177
}
177-
178178
volumeDao.update(volumeVO.getId(), volumeVO);
179179
}
180-
createCmdResult = new CreateCmdResult(null, new Answer(null, true, null));
181180
} else {
182181
errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync";
183182
s_logger.error(errMsg);
@@ -234,39 +233,15 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac
234233
s_logger.error("deleteAsync: Storage Pool not found for id: " + store.getId());
235234
throw new CloudRuntimeException("deleteAsync: Storage Pool not found for id: " + store.getId());
236235
}
237-
238236
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(store.getId());
239-
240-
if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
241-
// NFS file deletion is handled by the hypervisor; no ONTAP REST call needed
242-
s_logger.info("deleteAsync: ManagedNFS volume {} - file deletion handled by hypervisor", data.getId());
243-
244-
} else if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
245-
StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details);
246-
VolumeInfo volumeObject = (VolumeInfo) data;
247-
s_logger.info("deleteAsync: Deleting LUN for volume id [{}]", volumeObject.getId());
248-
249-
// Retrieve LUN identifiers stored during volume creation
250-
String lunName = volumeDetailsDao.findDetail(volumeObject.getId(), Constants.LUN_DOT_NAME).getValue();
251-
String lunUUID = volumeDetailsDao.findDetail(volumeObject.getId(), Constants.LUN_DOT_UUID).getValue();
252-
if (lunName == null) {
253-
throw new CloudRuntimeException("deleteAsync: Missing LUN name for volume " + volumeObject.getId());
254-
}
255-
256-
CloudStackVolume delRequest = new CloudStackVolume();
257-
Lun lun = new Lun();
258-
lun.setName(lunName);
259-
lun.setUuid(lunUUID);
260-
delRequest.setLun(lun);
261-
storageStrategy.deleteCloudStackVolume(delRequest);
262-
263-
commandResult.setResult(null);
264-
commandResult.setSuccess(true);
265-
s_logger.info("deleteAsync: LUN [{}] deleted successfully", lunName);
266-
267-
} else {
268-
throw new CloudRuntimeException("deleteAsync: Unsupported protocol: " + details.get(Constants.PROTOCOL));
269-
}
237+
StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details);
238+
s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME));
239+
VolumeInfo volumeInfo = (VolumeInfo) data;
240+
CloudStackVolume cloudStackVolumeRequest = createDeleteCloudStackVolumeRequest(storagePool,details,volumeInfo);
241+
storageStrategy.deleteCloudStackVolume(cloudStackVolumeRequest);
242+
s_logger.error("deleteAsync : Volume deleted: " + volumeInfo.getId());
243+
commandResult.setResult(null);
244+
commandResult.setSuccess(true);
270245
}
271246
} catch (Exception e) {
272247
s_logger.error("deleteAsync: Failed for data object [{}]: {}", data, e.getMessage());
@@ -339,9 +314,10 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
339314

340315
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
341316
String svmName = details.get(Constants.SVM_NAME);
342-
String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue();
343317

344318
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
319+
// Only retrieve LUN name for iSCSI volumes
320+
String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue();
345321
UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details);
346322
String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid);
347323

@@ -360,8 +336,11 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
360336
volumeVO.set_iScsiName(iscsiPath);
361337
volumeVO.setPath(iscsiPath);
362338
}
339+
} else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
340+
// For NFS, no access grant needed - file is accessible via mount
341+
s_logger.debug("grantAccess: NFS volume [{}], no igroup mapping required", volumeVO.getUuid());
342+
return true;
363343
}
364-
365344
volumeVO.setPoolType(storagePool.getPoolType());
366345
volumeVO.setPoolId(storagePool.getId());
367346
volumeDao.update(volumeVO.getId(), volumeVO);
@@ -610,6 +589,37 @@ public boolean isStorageSupportHA(Storage.StoragePoolType type) {
610589

611590
@Override
612591
public void detachVolumeFromAllStorageNodes(Volume volume) {
592+
}
593+
594+
private CloudStackVolume createDeleteCloudStackVolumeRequest(StoragePool storagePool, Map<String, String> details, VolumeInfo volumeInfo) {
595+
CloudStackVolume cloudStackVolumeDeleteRequest = null;
596+
597+
String protocol = details.get(Constants.PROTOCOL);
598+
ProtocolType protocolType = ProtocolType.valueOf(protocol);
599+
switch (protocolType) {
600+
case NFS3:
601+
cloudStackVolumeDeleteRequest = new CloudStackVolume();
602+
cloudStackVolumeDeleteRequest.setDatastoreId(String.valueOf(storagePool.getId()));
603+
cloudStackVolumeDeleteRequest.setVolumeInfo(volumeInfo);
604+
break;
605+
case ISCSI:
606+
// Retrieve LUN identifiers stored during volume creation
607+
String lunName = volumeDetailsDao.findDetail(volumeInfo.getId(), Constants.LUN_DOT_NAME).getValue();
608+
String lunUUID = volumeDetailsDao.findDetail(volumeInfo.getId(), Constants.LUN_DOT_UUID).getValue();
609+
if (lunName == null) {
610+
throw new CloudRuntimeException("deleteAsync: Missing LUN name for volume " + volumeInfo.getId());
611+
}
612+
cloudStackVolumeDeleteRequest = new CloudStackVolume();
613+
Lun lun = new Lun();
614+
lun.setName(lunName);
615+
lun.setUuid(lunUUID);
616+
cloudStackVolumeDeleteRequest.setLun(lun);
617+
break;
618+
default:
619+
throw new CloudRuntimeException("createDeleteCloudStackVolumeRequest: Unsupported protocol " + protocol);
620+
621+
}
622+
return cloudStackVolumeDeleteRequest;
613623

614624
}
615625
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,9 @@ public DataStore initialize(Map<String, Object> dsInfos) {
230230
parameters.setType(Storage.StoragePoolType.NetworkFilesystem);
231231
path = Constants.SLASH + storagePoolName;
232232
port = Constants.NFS3_PORT;
233-
s_logger.info("Setting NFS path for storage pool: " + path + ", port: " + port);
233+
// Force NFSv3 for ONTAP managed storage to avoid NFSv4 ID mapping issues
234+
details.put("nfsmountopts", "vers=3");
235+
s_logger.info("Setting NFS path for storage pool: " + path + ", port: " + port + " with mount option: vers=3");
234236
break;
235237
case ISCSI:
236238
parameters.setType(Storage.StoragePoolType.Iscsi);

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,12 @@
3737
import com.cloud.utils.exception.CloudRuntimeException;
3838
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
3939
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
40+
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
4041
import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
4142
import com.cloud.host.dao.HostDao;
4243

44+
import java.util.Map;
45+
4346
public class OntapHostListener implements HypervisorHostListener {
4447
protected Logger logger = LogManager.getLogger(getClass());
4548

@@ -51,7 +54,10 @@ public class OntapHostListener implements HypervisorHostListener {
5154
private PrimaryDataStoreDao _storagePoolDao;
5255
@Inject
5356
private HostDao _hostDao;
54-
@Inject private StoragePoolHostDao storagePoolHostDao;
57+
@Inject
58+
private StoragePoolHostDao storagePoolHostDao;
59+
@Inject
60+
private StoragePoolDetailsDao _storagePoolDetailsDao;
5561

5662

5763
@Override
@@ -71,10 +77,12 @@ public boolean hostConnect(long hostId, long poolId) {
7177
}
7278
logger.info("Connecting host {} to ONTAP storage pool {}", host.getName(), pool.getName());
7379
try {
80+
// Load storage pool details from database to pass mount options and other config to agent
81+
Map<String, String> detailsMap = _storagePoolDetailsDao.listDetailsKeyPairs(poolId);
7482
// Create the ModifyStoragePoolCommand to send to the agent
7583
// Note: Always send command even if database entry exists, because agent may have restarted
7684
// and lost in-memory pool registration. The command handler is idempotent.
77-
ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool);
85+
ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool, detailsMap);
7886

7987
Answer answer = _agentMgr.easySend(hostId, cmd);
8088

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

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
3232
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
3333
import org.apache.cloudstack.storage.command.CreateObjectCommand;
34+
import org.apache.cloudstack.storage.command.DeleteCommand;
3435
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
3536
import org.apache.cloudstack.storage.feign.FeignClientFactory;
3637
import org.apache.cloudstack.storage.feign.client.JobFeignClient;
@@ -93,12 +94,12 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume
9394
Answer answer = createVolumeOnKVMHost(cloudstackVolume.getVolumeInfo());
9495
if (answer == null || !answer.getResult()) {
9596
String errMsg = answer != null ? answer.getDetails() : "Failed to create qcow2 on KVM host";
96-
s_logger.error("createCloudStackVolumeForTypeVolume: " + errMsg);
97+
s_logger.error("createCloudStackVolume: " + errMsg);
9798
throw new CloudRuntimeException(errMsg);
9899
}
99100
return cloudstackVolume;
100101
}catch (Exception e) {
101-
s_logger.error("createCloudStackVolumeForTypeVolume: error occured " + e);
102+
s_logger.error("createCloudStackVolume: error occured " + e);
102103
throw new CloudRuntimeException(e);
103104
}
104105
}
@@ -111,7 +112,19 @@ CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) {
111112

112113
@Override
113114
public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) {
114-
//TODO
115+
s_logger.info("deleteCloudStackVolume: Delete cloudstack volume " + cloudstackVolume);
116+
try {
117+
// Step 1: Send command to KVM host to delete qcow2 file using qemu-img
118+
Answer answer = deleteVolumeOnKVMHost(cloudstackVolume.getVolumeInfo());
119+
if (answer == null || !answer.getResult()) {
120+
String errMsg = answer != null ? answer.getDetails() : "Failed to delete qcow2 on KVM host";
121+
s_logger.error("deleteCloudStackVolume: " + errMsg);
122+
throw new CloudRuntimeException(errMsg);
123+
}
124+
}catch (Exception e) {
125+
s_logger.error("deleteCloudStackVolume: error occured " + e);
126+
throw new CloudRuntimeException(e);
127+
}
115128
}
116129

117130
@Override
@@ -445,7 +458,7 @@ private ExportPolicy createExportPolicyRequest(AccessGroup accessGroup,String sv
445458
exportClients.add(exportClient);
446459
}
447460
exportRule.setClients(exportClients);
448-
exportRule.setProtocols(List.of(ExportRule.ProtocolsEnum.any));
461+
exportRule.setProtocols(List.of(ExportRule.ProtocolsEnum.nfs3));
449462
exportRule.setRoRule(List.of("sys"));
450463
exportRule.setRwRule(List.of("sys"));
451464
exportRule.setSuperuser(List.of("sys"));
@@ -508,4 +521,31 @@ private Answer createVolumeOnKVMHost(DataObject volumeInfo) {
508521
return new Answer(null, false, e.toString());
509522
}
510523
}
524+
525+
private Answer deleteVolumeOnKVMHost(DataObject volumeInfo) {
526+
s_logger.info("deleteVolumeOnKVMHost called with volumeInfo: {} ", volumeInfo);
527+
528+
try {
529+
s_logger.info("deleteVolumeOnKVMHost: Sending DeleteCommand to KVM agent for volume: {}", volumeInfo.getUuid());
530+
DeleteCommand cmd = new DeleteCommand(volumeInfo.getTO());
531+
EndPoint ep = epSelector.select(volumeInfo);
532+
if (ep == null) {
533+
String errMsg = "No remote endpoint to send DeleteCommand, check if host is up";
534+
s_logger.error(errMsg);
535+
return new Answer(cmd, false, errMsg);
536+
}
537+
s_logger.info("deleteVolumeOnKVMHost: Sending command to endpoint: {}", ep.getHostAddr());
538+
Answer answer = ep.sendMessage(cmd);
539+
if (answer != null && answer.getResult()) {
540+
s_logger.info("deleteVolumeOnKVMHost: Successfully deleted qcow2 file on KVM host");
541+
} else {
542+
s_logger.error("deleteVolumeOnKVMHost: Failed to delete qcow2 file: {}",
543+
answer != null ? answer.getDetails() : "null answer");
544+
}
545+
return answer;
546+
} catch (Exception e) {
547+
s_logger.error("deleteVolumeOnKVMHost: Exception sending DeleteCommand", e);
548+
return new Answer(null, false, e.toString());
549+
}
550+
}
511551
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,6 @@ void testDeleteAsync_NFSVolume_Success() {
308308

309309
when(dataStore.getId()).thenReturn(1L);
310310
when(volumeInfo.getType()).thenReturn(VOLUME);
311-
when(volumeInfo.getId()).thenReturn(100L);
312311

313312
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
314313
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);

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

100644100755
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
2828
import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
2929
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
30+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
3031
import org.apache.cloudstack.storage.command.CreateObjectCommand;
3132
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
3233
import org.apache.cloudstack.storage.feign.client.JobFeignClient;
@@ -533,4 +534,74 @@ public void testDeleteAccessGroup_Failed() {
533534
strategy.deleteAccessGroup(accessGroup);
534535
});
535536
}
537+
538+
// Test deleteCloudStackVolume - Success
539+
@Test
540+
public void testDeleteCloudStackVolume_Success() throws Exception {
541+
CloudStackVolume cloudStackVolume = mock(CloudStackVolume.class);
542+
VolumeInfo volumeInfo = mock(VolumeInfo.class);
543+
EndPoint endpoint = mock(EndPoint.class);
544+
Answer answer = mock(Answer.class);
545+
546+
when(cloudStackVolume.getVolumeInfo()).thenReturn(volumeInfo);
547+
when(epSelector.select(volumeInfo)).thenReturn(endpoint);
548+
when(endpoint.sendMessage(any())).thenReturn(answer);
549+
when(answer.getResult()).thenReturn(true);
550+
551+
// Execute - should not throw exception
552+
strategy.deleteCloudStackVolume(cloudStackVolume);
553+
554+
// Verify endpoint was selected and message sent
555+
verify(epSelector).select(volumeInfo);
556+
verify(endpoint).sendMessage(any());
557+
}
558+
559+
// Test deleteCloudStackVolume - Endpoint Not Found
560+
@Test
561+
public void testDeleteCloudStackVolume_EndpointNotFound() {
562+
CloudStackVolume cloudStackVolume = mock(CloudStackVolume.class);
563+
VolumeInfo volumeInfo = mock(VolumeInfo.class);
564+
565+
when(cloudStackVolume.getVolumeInfo()).thenReturn(volumeInfo);
566+
when(epSelector.select(volumeInfo)).thenReturn(null);
567+
568+
assertThrows(CloudRuntimeException.class, () -> {
569+
strategy.deleteCloudStackVolume(cloudStackVolume);
570+
});
571+
}
572+
573+
// Test deleteCloudStackVolume - Answer Result False
574+
@Test
575+
public void testDeleteCloudStackVolume_AnswerResultFalse() throws Exception {
576+
CloudStackVolume cloudStackVolume = mock(CloudStackVolume.class);
577+
VolumeInfo volumeInfo = mock(VolumeInfo.class);
578+
EndPoint endpoint = mock(EndPoint.class);
579+
Answer answer = mock(Answer.class);
580+
581+
when(cloudStackVolume.getVolumeInfo()).thenReturn(volumeInfo);
582+
when(epSelector.select(volumeInfo)).thenReturn(endpoint);
583+
when(endpoint.sendMessage(any())).thenReturn(answer);
584+
when(answer.getResult()).thenReturn(false);
585+
when(answer.getDetails()).thenReturn("Failed to delete volume file");
586+
587+
assertThrows(CloudRuntimeException.class, () -> {
588+
strategy.deleteCloudStackVolume(cloudStackVolume);
589+
});
590+
}
591+
592+
// Test deleteCloudStackVolume - Answer is Null
593+
@Test
594+
public void testDeleteCloudStackVolume_AnswerNull() throws Exception {
595+
CloudStackVolume cloudStackVolume = mock(CloudStackVolume.class);
596+
VolumeInfo volumeInfo = mock(VolumeInfo.class);
597+
EndPoint endpoint = mock(EndPoint.class);
598+
599+
when(cloudStackVolume.getVolumeInfo()).thenReturn(volumeInfo);
600+
when(epSelector.select(volumeInfo)).thenReturn(endpoint);
601+
when(endpoint.sendMessage(any())).thenReturn(null);
602+
603+
assertThrows(CloudRuntimeException.class, () -> {
604+
strategy.deleteCloudStackVolume(cloudStackVolume);
605+
});
606+
}
536607
}

0 commit comments

Comments
 (0)