From 60070421a3aa6e946d47e3ec278eb90ecfe534e7 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Thu, 13 Aug 2026 11:45:55 +0530 Subject: [PATCH 1/5] CSTACKEX-246: setting volume format based on protocol --- .../cloudstack/storage/volume/VolumeServiceImpl.java | 2 +- .../storage/driver/OntapPrimaryDatastoreDriver.java | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 00b5148a6e4d..f36fa0f1bbc1 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -737,7 +737,7 @@ protected Void managedCopyBaseImageCallback(AsyncCallbackDispatcher Date: Thu, 13 Aug 2026 17:46:21 +0530 Subject: [PATCH 2/5] CSTACKEX-246: fixing a small error --- .../org/apache/cloudstack/storage/volume/VolumeServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index f36fa0f1bbc1..fc5dc6dcc9b9 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -737,7 +737,7 @@ protected Void managedCopyBaseImageCallback(AsyncCallbackDispatcher Date: Fri, 14 Aug 2026 17:26:23 +0530 Subject: [PATCH 3/5] CSTACKEX-246: Added provider check with the null check --- .../cloudstack/storage/volume/VolumeServiceImpl.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index fc5dc6dcc9b9..50c2169b1d8a 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -737,8 +737,12 @@ protected Void managedCopyBaseImageCallback(AsyncCallbackDispatcher Date: Tue, 18 Aug 2026 17:48:29 +0530 Subject: [PATCH 4/5] CSTACKEX-246: UT's and resolving comments --- .../driver/OntapPrimaryDatastoreDriver.java | 19 +++--- .../OntapPrimaryDatastoreDriverTest.java | 68 +++++++++++++++++-- 2 files changed, 75 insertions(+), 12 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 0c7d07e8fed1..eea96d5b2626 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 @@ -160,8 +160,8 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet volumeVO.setPoolType(storagePool.getPoolType()); volumeVO.setPoolId(storagePool.getId()); - volumeVO.setFormat(getImageFormatByHypervisor(storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL))); - logger.info("createAsync: Volume format set to [{}] for hypervisor [{}]", volumeVO.getFormat(), storagePool.getHypervisor()); + volumeVO.setFormat(getImageFormatByHypervisorAndProtocol(storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL))); + logger.info("createAsync: Volume format set to [{}] for hypervisor [{}] and protocol [{}]", volumeVO.getFormat(), storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL)); if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null; @@ -988,14 +988,17 @@ private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId) } - private Storage.ImageFormat getImageFormatByHypervisor(HypervisorType hypervisorType, String protocol) { + private Storage.ImageFormat getImageFormatByHypervisorAndProtocol(HypervisorType hypervisorType, String protocol) { if (HypervisorType.KVM.equals(hypervisorType)) { - if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { - return Storage.ImageFormat.QCOW2; - } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { - return Storage.ImageFormat.RAW; + ProtocolType protocolType = ProtocolType.valueOf(protocol.toUpperCase()); + switch (protocolType) { + case NFS3: + return Storage.ImageFormat.QCOW2; + case ISCSI: + return Storage.ImageFormat.RAW; + default: + throw new CloudRuntimeException("Unsupported protocol [" + protocol + "] for ONTAP image format resolution"); } - throw new CloudRuntimeException("Unsupported protocol [" + protocol + "] for ONTAP image format resolution"); } throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution"); } 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 bad8168ba86d..11f2a88f20e9 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 @@ -39,6 +39,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.service.UnifiedNASStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -108,6 +109,9 @@ class OntapPrimaryDatastoreDriverTest { @Mock private UnifiedSANStrategy sanStrategy; + @Mock + private UnifiedNASStrategy nasStrategy; + @Mock private AsyncCompletionCallback createCallback; @@ -167,7 +171,7 @@ void testCreateAsync_VolumeWithISCSI_Success() { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); - when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.Iscsi); when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); @@ -204,7 +208,7 @@ void testCreateAsync_VolumeWithISCSI_Success() { verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_UUID), eq("lun-uuid-123"), eq(false)); verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_NAME), eq("/vol/vol1/lun1"), eq(false)); - verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2); + verify(volumeVO).setFormat(Storage.ImageFormat.RAW); verify(volumeDao).update(eq(100L), any(VolumeVO.class)); } } @@ -232,11 +236,11 @@ void testCreateAsync_VolumeWithNFS_Success() { try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) - .thenReturn(sanStrategy); + .thenReturn(nasStrategy); utilityMock.when(() -> OntapStorageUtils.createCloudStackVolumeRequestByProtocol( any(), any(), any())).thenReturn(mockCloudStackVolume); - when(sanStrategy.createCloudStackVolume(any())).thenReturn(mockCloudStackVolume); + when(nasStrategy.createCloudStackVolume(any())).thenReturn(mockCloudStackVolume); // Execute driver.createAsync(dataStore, volumeInfo, createCallback); @@ -253,6 +257,62 @@ void testCreateAsync_VolumeWithNFS_Success() { } } + @Test + void testCreateAsync_UnsupportedHypervisor_FailsWithError() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.ISCSI.name()); + + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(volumeInfo.getType()).thenReturn(VOLUME); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getName()).thenReturn("test-volume"); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.VMware); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Unsupported hypervisor [VMware]")); + } + } + + @Test + void testCreateAsync_KvmUnsupportedProtocol_FailsWithError() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, "FC"); + + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(volumeInfo.getType()).thenReturn(VOLUME); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getName()).thenReturn("test-volume"); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("No enum constant")); + } + } + @Test void testDeleteAsync_NullStore_ThrowsException() { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); From d9de68683f0bc059ccc4b69bbb074e207b8b7634 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Tue, 18 Aug 2026 23:16:15 +0530 Subject: [PATCH 5/5] CSTACKEX-246: small fix --- .../cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 eea96d5b2626..89a937689c28 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 @@ -990,7 +990,7 @@ private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId) private Storage.ImageFormat getImageFormatByHypervisorAndProtocol(HypervisorType hypervisorType, String protocol) { if (HypervisorType.KVM.equals(hypervisorType)) { - ProtocolType protocolType = ProtocolType.valueOf(protocol.toUpperCase()); + ProtocolType protocolType = ProtocolType.valueOf(protocol); switch (protocolType) { case NFS3: return Storage.ImageFormat.QCOW2;