From 531c80acab5cc3b8a4292bd7e612ad41fd2a3b87 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Tue, 15 Sep 2026 10:48:11 +0530 Subject: [PATCH 1/7] CSTACKEX-261: enabling cloudstack volume resize (GROW) for iscsi and nfs --- .../command/user/volume/ResizeVolumeCmd.java | 6 +- .../driver/OntapPrimaryDatastoreDriver.java | 163 ++++++-- .../storage/service/StorageStrategy.java | 19 +- .../storage/service/UnifiedNASStrategy.java | 12 +- .../OntapPrimaryDatastoreDriverTest.java | 394 ++++++++++++++++-- .../service/UnifiedNASStrategyTest.java | 114 ++++- .../service/UnifiedSANStrategyTest.java | 33 +- 7 files changed, 657 insertions(+), 84 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java index f8f744285c04..60f4f308503d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java @@ -15,14 +15,14 @@ // specific language governing permissions and limitations // under the License. package org.apache.cloudstack.api.command.user.volume; -import org.apache.cloudstack.api.BaseAsyncCmd; - import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiArgValidator; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseAsyncCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ResponseObject.ResponseView; import org.apache.cloudstack.api.ServerApiException; @@ -60,7 +60,7 @@ public class ResizeVolumeCmd extends BaseAsyncCmd implements UserCmd { @Parameter(name = ApiConstants.MAX_IOPS, type = CommandType.LONG, required = false, description = "New maximum number of IOPS") private Long maxIops; - @Parameter(name = ApiConstants.SIZE, type = CommandType.LONG, required = false, description = "New volume size in GB") + @Parameter(name = ApiConstants.SIZE, type = CommandType.LONG, required = false, description = "New volume size in GB",validations = {ApiArgValidator.PositiveNumber}) private Long size; @Parameter(name = ApiConstants.SHRINK_OK, type = CommandType.BOOLEAN, required = false, description = "Verify OK to Shrink") 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 ed942b438e16..5e4c0559ac30 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 @@ -18,31 +18,14 @@ */ package org.apache.cloudstack.storage.driver; -import org.apache.cloudstack.storage.utils.OntapStorageConstants; -import com.cloud.agent.api.Answer; -import com.cloud.agent.api.to.DataObjectType; -import com.cloud.agent.api.to.DataStoreTO; -import com.cloud.agent.api.to.DataTO; -import com.cloud.exception.InvalidParameterValueException; -import com.cloud.host.Host; -import com.cloud.host.HostVO; -import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.storage.Storage; -import com.cloud.storage.StoragePool; -import com.cloud.storage.Volume; -import com.cloud.storage.VolumeDetailVO; -import com.cloud.storage.VolumeVO; -import com.cloud.storage.ScopeType; -import com.cloud.storage.SnapshotVO; -import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.dao.SnapshotDao; -import com.cloud.storage.dao.SnapshotDetailsDao; -import com.cloud.storage.dao.SnapshotDetailsVO; -import com.cloud.storage.dao.VMTemplatePoolDao; -import com.cloud.storage.dao.VolumeDao; -import com.cloud.storage.dao.VolumeDetailsDao; -import com.cloud.utils.Pair; -import com.cloud.utils.exception.CloudRuntimeException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import javax.inject.Inject; + import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; @@ -55,7 +38,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; -import org.apache.commons.lang3.StringUtils; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.command.CommandResult; import org.apache.cloudstack.storage.command.CreateObjectAnswer; @@ -78,17 +60,38 @@ import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; -import javax.inject.Inject; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.to.DataObjectType; +import com.cloud.agent.api.to.DataStoreTO; +import com.cloud.agent.api.to.DataTO; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.ResizeVolumePayload; +import com.cloud.storage.ScopeType; +import com.cloud.storage.SnapshotVO; +import com.cloud.storage.Storage; +import com.cloud.storage.StoragePool; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.SnapshotDetailsDao; +import com.cloud.storage.dao.SnapshotDetailsVO; +import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; /** * Primary datastore driver for NetApp ONTAP storage systems. @@ -596,7 +599,62 @@ public boolean canCopy(DataObject srcData, DataObject destData) { } @Override - public void resize(DataObject data, AsyncCompletionCallback callback) {} + public void resize(DataObject data, AsyncCompletionCallback callback) { + CreateCmdResult result = null; + try { + VolumeInfo volumeInfo = (VolumeInfo) data; + ResizeVolumePayload payload = (ResizeVolumePayload) volumeInfo.getpayload(); + if (payload == null || payload.newSize == null) { + throw new CloudRuntimeException("Invalid resize payload for volume " + volumeInfo.getId()); + } + + StoragePoolVO storagePool = storagePoolDao.findById(volumeInfo.getDataStore().getId()); + if (storagePool == null) { + throw new CloudRuntimeException("Storage pool not found for volume " + volumeInfo.getId()); + } + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + + StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); + VolumeVO volumeVO = volumeDao.findById(volumeInfo.getId()); + if (volumeVO == null) { + throw new CloudRuntimeException("Volume not found for id " + volumeInfo.getId()); + } + if (payload.newSize < volumeVO.getSize()) { + throw new CloudRuntimeException(String.format( + "Storage pool %s does not support shrinking a volume.", storagePool.getName())); + } + + CloudStackVolume cloudStackVolume = new CloudStackVolume(); + cloudStackVolume.setVolumeInfo(volumeInfo); + + if (ProtocolType.ISCSI.name().equals(details.get(OntapStorageConstants.PROTOCOL))) { + VolumeDetailVO lunUuidDetail = volumeDetailsDao.findDetail(volumeInfo.getId(), OntapStorageConstants.LUN_DOT_UUID); + if (lunUuidDetail == null || lunUuidDetail.getValue() == null) { + throw new CloudRuntimeException("LUN UUID not found in volume details for volume " + volumeInfo.getId()); + } + Lun lun = new Lun(); + lun.setUuid(lunUuidDetail.getValue()); + cloudStackVolume.setLun(lun); + } + + // delegates to UnifiedSANStrategy (PATCH /api/storage/luns/{uuid}) for iSCSI + // or to UnifiedNASStrategy (ResizeVolumeCommand to KVM agent) for NFS3 + storageStrategy.resizeCloudStackVolume(cloudStackVolume, payload.newSize); + + volumeVO.setSize(payload.newSize); + volumeDao.update(volumeVO.getId(), volumeVO); + + result = new CreateCmdResult(null, new Answer(null, true, null)); + logger.info("resize: Successfully resized volume [{}] to [{}] bytes", volumeInfo.getId(), payload.newSize); + } catch (Exception e) { + String errMsg = e.getMessage(); + logger.error("resize: Failed for volume [{}]: {}", data.getId(), errMsg, e); + result = new CreateCmdResult(null, new Answer(null, false, errMsg)); + result.setResult(errMsg); + } finally { + callback.complete(result); + } + } @Override public ChapInfo getChapInfo(DataObject dataObject) { @@ -1018,9 +1076,46 @@ private boolean isTemplateCachedOnPool(VMTemplateStoragePoolVO templatePoolRef, return StringUtils.isNotBlank(templatePoolRef.getInstallPath()); } + /** + * Returns the bytes available on the FlexVolume backing this pool, read directly from ONTAP + * ({@code space.available}). + * + *

Returns {@code 0} if the FlexVolume UUID is not yet recorded in pool details, or if the + * ONTAP REST call fails for any reason (array unreachable, auth error, etc.).

+ */ @Override public long getUsedBytes(StoragePool storagePool) { - return 0; + if (storagePool == null) { + return 0; + } + + try { + Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + String flexVolUuid = poolDetails != null ? poolDetails.get(OntapStorageConstants.VOLUME_UUID) : null; + + if (StringUtils.isBlank(flexVolUuid)) { + logger.warn("getUsedBytes: No FlexVolume UUID recorded for pool [{}]; returning 0", + storagePool.getId()); + return 0; + } + + StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); + var flexVol = strategy.getStorageVolume(flexVolUuid); + + if (flexVol == null || flexVol.getSpace() == null) { + logger.warn("getUsedBytes: FlexVolume [{}] not found or has no space info for pool [{}]; returning 0", + flexVolUuid, storagePool.getId()); + return 0; + } + + logger.debug("getUsedBytes: FlexVolume [{}] backing pool [{}] reports {} bytes used", + flexVolUuid, storagePool.getId(), flexVol.getSpace().getUsed()); + return flexVol.getSpace().getUsed(); + } catch (Exception e) { + logger.warn("getUsedBytes: Could not read used space from ONTAP for pool [{}]; returning 0", + storagePool.getId(), e); + return 0; + } } @Override 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 6f79c4061419..a15cf9d52135 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 @@ -446,7 +446,24 @@ public void deleteStorageVolume(Volume volume) { * @return the retrieved Volume object */ public Volume getStorageVolume(Volume volume) { - return null; + return getStorageVolume(volume.getUuid()); + } + + public Volume getStorageVolume(String uuid) { + logger.info("getStorageVolume: Fetching ONTAP volume by UUID: {}", uuid); + String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + try { + Volume fetchedVolume = volumeFeignClient.getVolumeByUUID(authHeader, uuid); + logger.info("getStorageVolume: Volume [{}] fetched successfully", uuid); + return fetchedVolume; + } catch (FeignException e) { + if (OntapStorageUtils.isOntapObjectNotFoundError(e)) { + logger.warn("getStorageVolume: Volume [{}] not found in ONTAP", uuid); + return null; + } + logger.error("getStorageVolume: Exception while fetching volume [{}]: ", uuid, e); + throw new CloudRuntimeException("Failed to fetch volume: " + e.getMessage()); + } } /** 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 4a9f45f7301e..f9b409afa650 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 @@ -60,6 +60,7 @@ import com.cloud.agent.api.storage.ResizeVolumeCommand; import com.cloud.agent.api.to.StorageFilerTO; import com.cloud.host.HostVO; +import com.cloud.storage.ResizeVolumePayload; import com.cloud.storage.Storage; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VolumeDao; @@ -223,8 +224,15 @@ private Answer resizeVolumeOnKVMHost(DataObject volumeInfo, long sizeInBytes) { throw new CloudRuntimeException("Storage Pool not found for id: " + volume.getPoolId()); } - ResizeVolumeCommand cmd = new ResizeVolumeCommand(volume.getPath(), new StorageFilerTO(storagePool), - volume.getSize(), sizeInBytes, false, null); + // instanceName is set by VolumeApiServiceImpl.orchestrateResizeVolume() before calling the + // driver — it is the VM instance name when attached, or "none" when the volume is detached. + ResizeVolumePayload resizePayload = volumeObject.getpayload() instanceof ResizeVolumePayload + ? (ResizeVolumePayload) volumeObject.getpayload() + : null; + String instanceName = resizePayload != null ? resizePayload.instanceName : "none"; + ResizeVolumeCommand cmd = new ResizeVolumeCommand(volume.getPath(), new StorageFilerTO(storagePool), + volume.getSize(), sizeInBytes, false, instanceName); + EndPoint ep = epSelector.select(volumeInfo); if (ep == null) { String errMsg = "No remote endpoint to send ResizeVolumeCommand, check if host is up"; 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 db1806c8473e..49026361d321 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 @@ -18,19 +18,9 @@ */ package org.apache.cloudstack.storage.driver; -import com.cloud.exception.InvalidParameterValueException; -import com.cloud.host.Host; -import com.cloud.host.HostVO; -import com.cloud.hypervisor.Hypervisor; -import com.cloud.storage.ScopeType; -import com.cloud.storage.Storage; -import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.VolumeVO; -import com.cloud.storage.VolumeDetailVO; -import com.cloud.storage.dao.VMTemplatePoolDao; -import com.cloud.storage.dao.VolumeDao; -import com.cloud.storage.dao.VolumeDetailsDao; -import com.cloud.utils.exception.CloudRuntimeException; +import java.util.HashMap; +import java.util.Map; + import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; @@ -44,6 +34,8 @@ 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.feign.model.Volume; +import org.apache.cloudstack.storage.feign.model.VolumeSpace; import org.apache.cloudstack.storage.service.UnifiedNASStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; @@ -51,31 +43,24 @@ import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.ArgumentCaptor; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.MockedStatic; -import org.mockito.junit.jupiter.MockitoExtension; - -import java.util.HashMap; -import java.util.Map; - -import static com.cloud.agent.api.to.DataObjectType.TEMPLATE; -import static com.cloud.agent.api.to.DataObjectType.VOLUME; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockedStatic; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; @@ -85,6 +70,24 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import org.mockito.junit.jupiter.MockitoExtension; + +import static com.cloud.agent.api.to.DataObjectType.TEMPLATE; +import static com.cloud.agent.api.to.DataObjectType.VOLUME; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.hypervisor.Hypervisor; +import com.cloud.storage.ScopeType; +import com.cloud.storage.Storage; +import com.cloud.storage.ResizeVolumePayload; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.utils.exception.CloudRuntimeException; @ExtendWith(MockitoExtension.class) class OntapPrimaryDatastoreDriverTest { @@ -1415,4 +1418,341 @@ void testCreateAsync_VolumeClonedFromTemplate_MissingSpoolRef_Fails() { verify(sanStrategy, never()).cloneCloudStackVolume(any()); } } + + // ========================================================================= + // resize() tests + // ========================================================================= + + private void stubResizeCommon(long currentSize, long newSize) { + ResizeVolumePayload payload = new ResizeVolumePayload(newSize, null, null, null, false, "i-2-VM", null, false); + when(volumeInfo.getDataStore()).thenReturn(dataStore); + when(dataStore.getId()).thenReturn(1L); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getpayload()).thenReturn(payload); + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + // storagePool.getName() is only needed for the shrink-error message path + lenient().when(storagePool.getName()).thenReturn("test-pool"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getSize()).thenReturn(currentSize); + // volumeVO.getId() is only needed in the success path (volumeDao.update call) + lenient().when(volumeVO.getId()).thenReturn(100L); + } + + @Test + void testResize_iSCSI_Success() { + long currentSize = 10737418240L; // 10 GB + long newSize = 21474836480L; // 20 GB + stubResizeCommon(currentSize, newSize); + + VolumeDetailVO lunUuidDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, "lun-uuid-123", false); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(lunUuidDetail); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + doNothing().when(sanStrategy).resizeCloudStackVolume(any(), eq(newSize)); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + // verify LUN UUID was wired into the request and ONTAP was called + ArgumentCaptor volumeCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); + verify(sanStrategy).resizeCloudStackVolume(volumeCaptor.capture(), eq(newSize)); + assertEquals("lun-uuid-123", volumeCaptor.getValue().getLun().getUuid()); + + verify(volumeVO).setSize(newSize); + verify(volumeDao).update(eq(100L), any(VolumeVO.class)); + } + } + + @Test + void testResize_NFS_Success() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + long currentSize = 10737418240L; + long newSize = 21474836480L; + stubResizeCommon(currentSize, newSize); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(nasStrategy); + doNothing().when(nasStrategy).resizeCloudStackVolume(any(), eq(newSize)); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + verify(nasStrategy).resizeCloudStackVolume(any(CloudStackVolume.class), eq(newSize)); + // NFS has no LUN – volumeDetailsDao must never be queried for a LUN UUID + verify(volumeDetailsDao, never()).findDetail(anyLong(), eq(OntapStorageConstants.LUN_DOT_UUID)); + verify(volumeVO).setSize(newSize); + verify(volumeDao).update(eq(100L), any(VolumeVO.class)); + } + } + + @Test + void testResize_NullPayload_Fails() { + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getpayload()).thenReturn(null); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Invalid resize payload")); + } + + @Test + void testResize_NullNewSize_Fails() { + // payload.newSize is null + ResizeVolumePayload payload = new ResizeVolumePayload(null, null, null, null, false, "i-2-VM", null, false); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getpayload()).thenReturn(payload); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Invalid resize payload")); + } + + @Test + void testResize_StoragePoolNotFound_Fails() { + ResizeVolumePayload payload = new ResizeVolumePayload(21474836480L, null, null, null, false, "i-2-VM", null, false); + when(volumeInfo.getDataStore()).thenReturn(dataStore); + when(dataStore.getId()).thenReturn(1L); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getpayload()).thenReturn(payload); + when(storagePoolDao.findById(1L)).thenReturn(null); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Storage pool not found")); + } + + @Test + void testResize_VolumeVONotFound_Fails() { + long newSize = 21474836480L; + ResizeVolumePayload payload = new ResizeVolumePayload(newSize, null, null, null, false, "i-2-VM", null, false); + when(volumeInfo.getDataStore()).thenReturn(dataStore); + when(dataStore.getId()).thenReturn(1L); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getpayload()).thenReturn(payload); + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDao.findById(100L)).thenReturn(null); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Volume not found")); + } + } + + @Test + void testResize_ShrinkAttempt_Fails() { + long currentSize = 21474836480L; // 20 GB + long newSize = 10737418240L; // 10 GB – smaller than current + stubResizeCommon(currentSize, newSize); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("does not support shrinking")); + verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); + } + } + + @Test + void testResize_iSCSI_LunUuidNotFound_Fails() { + long currentSize = 10737418240L; + long newSize = 21474836480L; + stubResizeCommon(currentSize, newSize); + + // LUN UUID detail is missing + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(null); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("LUN UUID not found")); + verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); + } + } + + @Test + void testResize_iSCSI_LunUuidValueNull_Fails() { + long currentSize = 10737418240L; + long newSize = 21474836480L; + stubResizeCommon(currentSize, newSize); + + // Detail exists but value is null + VolumeDetailVO blankDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, null, false); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(blankDetail); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("LUN UUID not found")); + } + } + + @Test + void testResize_StrategyThrows_Fails() { + long currentSize = 10737418240L; + long newSize = 21474836480L; + stubResizeCommon(currentSize, newSize); + + VolumeDetailVO lunUuidDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, "lun-uuid-123", false); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(lunUuidDetail); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + doThrow(new com.cloud.utils.exception.CloudRuntimeException("ONTAP resize failed")) + .when(sanStrategy).resizeCloudStackVolume(any(), eq(newSize)); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("ONTAP resize failed")); + // volumeVO size must NOT be updated on failure + verify(volumeVO, never()).setSize(anyLong()); + } + } + + // ========================================================================= + // getUsedBytes() tests + // ========================================================================= + + @Test + void testGetUsedBytes_NullPool_ReturnsZero() { + assertEquals(0L, driver.getUsedBytes(null)); + } + + @Test + void testGetUsedBytes_NoFlexVolUuid_ReturnsZero() { + // VOLUME_UUID key is absent from pool details + storagePoolDetails.remove(OntapStorageConstants.VOLUME_UUID); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + assertEquals(0L, driver.getUsedBytes(storagePool)); + } + + @Test + void testGetUsedBytes_BlankFlexVolUuid_ReturnsZero() { + storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, " "); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + assertEquals(0L, driver.getUsedBytes(storagePool)); + } + + @Test + void testGetUsedBytes_Success_ReturnsUsedBytes() { + storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-123"); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + Volume flexVol = new Volume(); + VolumeSpace space = new VolumeSpace(); + space.setUsed(10737418240L); // 10 GB + flexVol.setSpace(space); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + when(sanStrategy.getStorageVolume("flexvol-uuid-123")).thenReturn(flexVol); + + assertEquals(10737418240L, driver.getUsedBytes(storagePool)); + } + } + + @Test + void testGetUsedBytes_FlexVolNotFound_ReturnsZero() { + storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-123"); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + when(sanStrategy.getStorageVolume("flexvol-uuid-123")).thenReturn(null); + + assertEquals(0L, driver.getUsedBytes(storagePool)); + } + } + + @Test + void testGetUsedBytes_FlexVolNullSpace_ReturnsZero() { + storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-123"); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + Volume flexVol = new Volume(); + // space is intentionally left null + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + when(sanStrategy.getStorageVolume("flexvol-uuid-123")).thenReturn(flexVol); + + assertEquals(0L, driver.getUsedBytes(storagePool)); + } + } + + @Test + void testGetUsedBytes_OntapException_ReturnsZero() { + storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-123"); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + when(sanStrategy.getStorageVolume("flexvol-uuid-123")) + .thenThrow(new com.cloud.utils.exception.CloudRuntimeException("ONTAP unreachable")); + + assertEquals(0L, driver.getUsedBytes(storagePool)); + } + } } 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 index dd90363af045..c79f3640e71c 100755 --- 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 @@ -22,6 +22,7 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.storage.ResizeVolumeCommand; import com.cloud.host.HostVO; +import com.cloud.storage.ResizeVolumePayload; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VolumeDao; import com.cloud.utils.exception.CloudRuntimeException; @@ -1028,9 +1029,12 @@ public void testResizeCloudStackVolume_SendsResizeCommand() { VolumeVO volumeVO = mock(VolumeVO.class); StoragePoolVO storagePool = mock(StoragePoolVO.class); EndPoint endPoint = mock(EndPoint.class); + ResizeVolumePayload payload = new ResizeVolumePayload( + 21474836480L, null, null, null, false, "i-2-VM", null, false); when(volumeObject.getId()).thenReturn(100L); when(volumeObject.getUuid()).thenReturn("volume-uuid"); + when(volumeObject.getpayload()).thenReturn(payload); when(volumeDao.findById(100L)).thenReturn(volumeVO); when(volumeVO.getPath()).thenReturn("volume-uuid"); when(volumeVO.getSize()).thenReturn(5368709120L); @@ -1044,7 +1048,74 @@ public void testResizeCloudStackVolume_SendsResizeCommand() { strategy.resizeCloudStackVolume(request, 21474836480L); - verify(endPoint).sendMessage(any(ResizeVolumeCommand.class)); + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(ResizeVolumeCommand.class); + verify(endPoint).sendMessage(commandCaptor.capture()); + ResizeVolumeCommand command = commandCaptor.getValue(); + assertEquals("volume-uuid", command.getPath()); + assertEquals(5368709120L, command.getCurrentSize()); + assertEquals(21474836480L, command.getNewSize()); + assertEquals("i-2-VM", command.getInstanceName()); + } + + @Test + public void testResizeCloudStackVolume_WithoutPayloadUsesDetachedInstanceName() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + EndPoint endPoint = mock(EndPoint.class); + + when(volumeObject.getId()).thenReturn(100L); + when(volumeObject.getUuid()).thenReturn("volume-uuid"); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPath()).thenReturn("volume-uuid"); + when(volumeVO.getSize()).thenReturn(5368709120L); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(storagePool); + when(epSelector.select(volumeObject)).thenReturn(endPoint); + when(endPoint.sendMessage(any(ResizeVolumeCommand.class))).thenReturn(new Answer(null, true, "Success")); + + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + strategy.resizeCloudStackVolume(request, 21474836480L); + + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(ResizeVolumeCommand.class); + verify(endPoint).sendMessage(commandCaptor.capture()); + assertEquals("none", commandCaptor.getValue().getInstanceName()); + } + + @Test + public void testResizeCloudStackVolume_AgentFailureThrowsException() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + EndPoint endPoint = mock(EndPoint.class); + + when(volumeObject.getId()).thenReturn(100L); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(storagePool); + when(epSelector.select(volumeObject)).thenReturn(endPoint); + when(endPoint.sendMessage(any(ResizeVolumeCommand.class))) + .thenReturn(new Answer(null, false, "qemu-img resize failed")); + + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> strategy.resizeCloudStackVolume(request, 21474836480L)); + assertEquals("qemu-img resize failed", exception.getMessage()); + } + + @Test + public void testResizeCloudStackVolume_InvalidSizeDoesNotSendCommand() { + VolumeObject volumeObject = mock(VolumeObject.class); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(request, 0L)); + + verify(epSelector, never()).select(any(org.apache.cloudstack.engine.subsystem.api.storage.DataObject.class)); } @Test @@ -1133,6 +1204,47 @@ public void testResizeCloudStackVolume_InvalidRequest_Throws() { assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(withVol, 0L)); } + // ========================================================================= + // getStorageVolume(String uuid) tests – exercising the new overload added + // to StorageStrategy that fetches a FlexVolume by UUID from ONTAP REST API + // ========================================================================= + + @Test + public void testGetStorageVolume_ByUuid_Success() { + org.apache.cloudstack.storage.feign.model.Volume expected = + new org.apache.cloudstack.storage.feign.model.Volume(); + expected.setUuid("flexvol-uuid-123"); + + when(volumeFeignClient.getVolumeByUUID(anyString(), eq("flexvol-uuid-123"))).thenReturn(expected); + + org.apache.cloudstack.storage.feign.model.Volume result = strategy.getStorageVolume("flexvol-uuid-123"); + + assertNotNull(result); + assertEquals("flexvol-uuid-123", result.getUuid()); + verify(volumeFeignClient).getVolumeByUUID(anyString(), eq("flexvol-uuid-123")); + } + + @Test + public void testGetStorageVolume_ByUuid_NotFound_ReturnsNull() { + FeignException notFound = mock(FeignException.class); + when(notFound.status()).thenReturn(404); + doThrow(notFound).when(volumeFeignClient).getVolumeByUUID(anyString(), eq("missing-uuid")); + + org.apache.cloudstack.storage.feign.model.Volume result = strategy.getStorageVolume("missing-uuid"); + + assertNull(result); + } + + @Test + public void testGetStorageVolume_ByUuid_ServerError_Throws() { + FeignException serverError = mock(FeignException.class); + when(serverError.status()).thenReturn(500); + when(serverError.getMessage()).thenReturn("Internal Server Error"); + doThrow(serverError).when(volumeFeignClient).getVolumeByUUID(anyString(), eq("flexvol-uuid-999")); + + assertThrows(CloudRuntimeException.class, () -> strategy.getStorageVolume("flexvol-uuid-999")); + } + @Test public void testResizeCloudStackVolume_KvmHostFails_Throws() { VolumeObject volumeObject = mock(VolumeObject.class); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index 700b63d15575..43043cdad0e1 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -18,9 +18,11 @@ */ package org.apache.cloudstack.storage.service; -import com.cloud.host.HostVO; -import com.cloud.utils.exception.CloudRuntimeException; -import feign.FeignException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo; import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; @@ -36,19 +38,6 @@ import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.MockedStatic; -import org.mockito.junit.jupiter.MockitoExtension; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -56,10 +45,16 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.eq; +import org.mockito.Mock; +import org.mockito.MockedStatic; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; @@ -68,6 +63,12 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import org.mockito.junit.jupiter.MockitoExtension; + +import com.cloud.host.HostVO; +import com.cloud.utils.exception.CloudRuntimeException; + +import feign.FeignException; @ExtendWith(MockitoExtension.class) class UnifiedSANStrategyTest { From 91847be70dc8fc37b8fe1067a7cbefab931e12d7 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Wed, 23 Sep 2026 11:11:17 +0530 Subject: [PATCH 2/7] CSTACKEX-261: RESOLVING COMMENTS --- .../driver/OntapPrimaryDatastoreDriver.java | 22 +++++++++---------- .../storage/service/StorageStrategy.java | 3 +++ .../storage/service/UnifiedSANStrategy.java | 19 +++++++++++++++- 3 files changed, 31 insertions(+), 13 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 5e4c0559ac30..8ee1f681a518 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 @@ -602,8 +602,14 @@ public boolean canCopy(DataObject srcData, DataObject destData) { public void resize(DataObject data, AsyncCompletionCallback callback) { CreateCmdResult result = null; try { + if (!(data instanceof VolumeInfo)) { + throw new CloudRuntimeException("resize: Expected VolumeInfo but received " + + (data != null ? data.getClass().getSimpleName() : "null")); + } VolumeInfo volumeInfo = (VolumeInfo) data; - ResizeVolumePayload payload = (ResizeVolumePayload) volumeInfo.getpayload(); + Object rawPayload = volumeInfo.getpayload(); + ResizeVolumePayload payload = (rawPayload instanceof ResizeVolumePayload) + ? (ResizeVolumePayload) rawPayload : null; if (payload == null || payload.newSize == null) { throw new CloudRuntimeException("Invalid resize payload for volume " + volumeInfo.getId()); } @@ -626,19 +632,11 @@ public void resize(DataObject data, AsyncCompletionCallback cal CloudStackVolume cloudStackVolume = new CloudStackVolume(); cloudStackVolume.setVolumeInfo(volumeInfo); - - if (ProtocolType.ISCSI.name().equals(details.get(OntapStorageConstants.PROTOCOL))) { - VolumeDetailVO lunUuidDetail = volumeDetailsDao.findDetail(volumeInfo.getId(), OntapStorageConstants.LUN_DOT_UUID); - if (lunUuidDetail == null || lunUuidDetail.getValue() == null) { - throw new CloudRuntimeException("LUN UUID not found in volume details for volume " + volumeInfo.getId()); - } - Lun lun = new Lun(); - lun.setUuid(lunUuidDetail.getValue()); - cloudStackVolume.setLun(lun); - } + // delegates to UnifiedSANStrategy (PATCH /api/storage/luns/{uuid}) for iSCSI - // or to UnifiedNASStrategy (ResizeVolumeCommand to KVM agent) for NFS3 + // or to UnifiedNASStrategy (ResizeVolumeCommand to KVM agent) for NFS3; + // protocol-specific setup (e.g. LUN UUID lookup) is handled inside each strategy storageStrategy.resizeCloudStackVolume(cloudStackVolume, payload.newSize); volumeVO.setSize(payload.newSize); 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 a15cf9d52135..bfa812065824 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 @@ -450,6 +450,9 @@ public Volume getStorageVolume(Volume volume) { } public Volume getStorageVolume(String uuid) { + if (uuid == null || uuid.isBlank()) { + throw new CloudRuntimeException("Cannot fetch ONTAP volume: UUID is null or empty"); + } logger.info("getStorageVolume: Fetching ONTAP volume by UUID: {}", uuid); String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); try { 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 b9e32b081e4d..2ff7a2edaa4a 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 @@ -20,6 +20,8 @@ package org.apache.cloudstack.storage.service; import com.cloud.host.HostVO; +import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.dao.VolumeDetailsDao; import com.cloud.utils.exception.CloudRuntimeException; import feign.FeignException; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; @@ -53,6 +55,8 @@ public class UnifiedSANStrategy extends SANStrategy { private static final Logger logger = LogManager.getLogger(UnifiedSANStrategy.class); @Inject private StoragePoolDetailsDao storagePoolDetailsDao; + @Inject + private VolumeDetailsDao volumeDetailsDao; public UnifiedSANStrategy(OntapStorage ontapStorage) { super(ontapStorage); @@ -280,13 +284,26 @@ private void validateCreatedLun(Lun lun, String requestName, String operation) { */ @Override public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeInBytes) { - if (cloudstackVolume == null || cloudstackVolume.getLun() == null || cloudstackVolume.getLun().getUuid() == null) { + if (cloudstackVolume == null || cloudstackVolume.getVolumeInfo() == null) { logger.error("resizeCloudStackVolume: Lun resize failed. Invalid request: {}", cloudstackVolume); throw new CloudRuntimeException("Failed to resize Lun, invalid request"); } if (sizeInBytes <= 0) { throw new CloudRuntimeException("Failed to resize Lun, invalid size " + sizeInBytes); } + + // Resolve LUN UUID from volume details when not pre-populated on the cloudstackVolume + if (cloudstackVolume.getLun() == null || cloudstackVolume.getLun().getUuid() == null) { + long volumeId = cloudstackVolume.getVolumeInfo().getId(); + VolumeDetailVO lunUuidDetail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.LUN_DOT_UUID); + if (lunUuidDetail == null || lunUuidDetail.getValue() == null) { + throw new CloudRuntimeException("LUN UUID not found in volume details for volume " + volumeId); + } + Lun resolvedLun = new Lun(); + resolvedLun.setUuid(lunUuidDetail.getValue()); + cloudstackVolume.setLun(resolvedLun); + } + String lunUuid = cloudstackVolume.getLun().getUuid(); logger.trace("resizeCloudStackVolume: Resizing Lun {} to {} bytes", lunUuid, sizeInBytes); try { From 3b2be0c24cb5d654c587b32692e726b5ae5464b5 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Wed, 23 Sep 2026 11:33:05 +0530 Subject: [PATCH 3/7] CSTACKEX-261: RESOLVING COMMENTS - PT2 --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 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 8ee1f681a518..04d375e3e20c 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 @@ -68,9 +68,11 @@ import org.jetbrains.annotations.Nullable; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.storage.ResizeVolumeCommand; import com.cloud.agent.api.to.DataObjectType; import com.cloud.agent.api.to.DataStoreTO; import com.cloud.agent.api.to.DataTO; +import com.cloud.agent.api.to.StorageFilerTO; import com.cloud.exception.InvalidParameterValueException; import com.cloud.host.Host; import com.cloud.host.HostVO; @@ -633,7 +635,6 @@ public void resize(DataObject data, AsyncCompletionCallback cal CloudStackVolume cloudStackVolume = new CloudStackVolume(); cloudStackVolume.setVolumeInfo(volumeInfo); - // delegates to UnifiedSANStrategy (PATCH /api/storage/luns/{uuid}) for iSCSI // or to UnifiedNASStrategy (ResizeVolumeCommand to KVM agent) for NFS3; // protocol-specific setup (e.g. LUN UUID lookup) is handled inside each strategy @@ -641,13 +642,16 @@ public void resize(DataObject data, AsyncCompletionCallback cal volumeVO.setSize(payload.newSize); volumeDao.update(volumeVO.getId(), volumeVO); + String instanceName = payload.instanceName != null ? payload.instanceName : "none"; - result = new CreateCmdResult(null, new Answer(null, true, null)); + ResizeVolumeCommand resizeCmd = new ResizeVolumeCommand(volumeVO.getPath(), + new StorageFilerTO(storagePool), volumeVO.getSize(), payload.newSize, + false, instanceName); + result = new CreateCmdResult(volumeVO.getPath(), new Answer(resizeCmd, true, null)); logger.info("resize: Successfully resized volume [{}] to [{}] bytes", volumeInfo.getId(), payload.newSize); } catch (Exception e) { String errMsg = e.getMessage(); logger.error("resize: Failed for volume [{}]: {}", data.getId(), errMsg, e); - result = new CreateCmdResult(null, new Answer(null, false, errMsg)); result.setResult(errMsg); } finally { callback.complete(result); From 067268d488cf80c7c70cd5d639694ca8317002f4 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Wed, 23 Sep 2026 12:16:23 +0530 Subject: [PATCH 4/7] CSTACKEX-261: resolving comments and build issues --- .../driver/OntapPrimaryDatastoreDriver.java | 24 ++--- .../OntapPrimaryDatastoreDriverTest.java | 92 ++++++------------- 2 files changed, 42 insertions(+), 74 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 04d375e3e20c..a9a0b74931c4 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 @@ -634,7 +634,7 @@ public void resize(DataObject data, AsyncCompletionCallback cal CloudStackVolume cloudStackVolume = new CloudStackVolume(); cloudStackVolume.setVolumeInfo(volumeInfo); - + // delegates to UnifiedSANStrategy (PATCH /api/storage/luns/{uuid}) for iSCSI // or to UnifiedNASStrategy (ResizeVolumeCommand to KVM agent) for NFS3; // protocol-specific setup (e.g. LUN UUID lookup) is handled inside each strategy @@ -652,6 +652,7 @@ public void resize(DataObject data, AsyncCompletionCallback cal } catch (Exception e) { String errMsg = e.getMessage(); logger.error("resize: Failed for volume [{}]: {}", data.getId(), errMsg, e); + result = new CreateCmdResult(null, new Answer(null, false, errMsg)); result.setResult(errMsg); } finally { callback.complete(result); @@ -1082,8 +1083,11 @@ private boolean isTemplateCachedOnPool(VMTemplateStoragePoolVO templatePoolRef, * Returns the bytes available on the FlexVolume backing this pool, read directly from ONTAP * ({@code space.available}). * - *

Returns {@code 0} if the FlexVolume UUID is not yet recorded in pool details, or if the - * ONTAP REST call fails for any reason (array unreachable, auth error, etc.).

+ *

Returns {@code 0} if the ONTAP REST call fails for any reason (array unreachable, auth + * error, etc.). Throws if the FlexVolume UUID is not recorded in pool details, since that + * indicates the pool was never fully provisioned.

+ * + * @throws CloudRuntimeException if the pool has no FlexVolume UUID in its details */ @Override public long getUsedBytes(StoragePool storagePool) { @@ -1091,16 +1095,14 @@ public long getUsedBytes(StoragePool storagePool) { return 0; } - try { - Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); - String flexVolUuid = poolDetails != null ? poolDetails.get(OntapStorageConstants.VOLUME_UUID) : null; + Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + String flexVolUuid = poolDetails != null ? poolDetails.get(OntapStorageConstants.VOLUME_UUID) : null; - if (StringUtils.isBlank(flexVolUuid)) { - logger.warn("getUsedBytes: No FlexVolume UUID recorded for pool [{}]; returning 0", - storagePool.getId()); - return 0; - } + if (StringUtils.isBlank(flexVolUuid)) { + throw new CloudRuntimeException("FlexVolume UUID not found in pool details for pool " + storagePool.getId()); + } + try { StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); var flexVol = strategy.getStorageVolume(flexVolUuid); 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 49026361d321..f1bb8eb7679a 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 @@ -78,9 +78,9 @@ import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor; +import com.cloud.storage.ResizeVolumePayload; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; -import com.cloud.storage.ResizeVolumePayload; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.VolumeVO; @@ -1438,6 +1438,7 @@ private void stubResizeCommon(long currentSize, long newSize) { when(volumeVO.getSize()).thenReturn(currentSize); // volumeVO.getId() is only needed in the success path (volumeDao.update call) lenient().when(volumeVO.getId()).thenReturn(100L); + lenient().when(volumeVO.getPath()).thenReturn("/vol/vol1/lun1"); } @Test @@ -1446,9 +1447,6 @@ void testResize_iSCSI_Success() { long newSize = 21474836480L; // 20 GB stubResizeCommon(currentSize, newSize); - VolumeDetailVO lunUuidDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, "lun-uuid-123", false); - when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(lunUuidDetail); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(sanStrategy); @@ -1460,10 +1458,12 @@ void testResize_iSCSI_Success() { verify(createCallback).complete(resultCaptor.capture()); assertTrue(resultCaptor.getValue().isSuccess()); - // verify LUN UUID was wired into the request and ONTAP was called + // Driver only wires VolumeInfo; LUN UUID lookup is inside UnifiedSANStrategy ArgumentCaptor volumeCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); verify(sanStrategy).resizeCloudStackVolume(volumeCaptor.capture(), eq(newSize)); - assertEquals("lun-uuid-123", volumeCaptor.getValue().getLun().getUuid()); + assertEquals(volumeInfo, volumeCaptor.getValue().getVolumeInfo()); + assertNull(volumeCaptor.getValue().getLun()); + verify(volumeDetailsDao, never()).findDetail(anyLong(), eq(OntapStorageConstants.LUN_DOT_UUID)); verify(volumeVO).setSize(newSize); verify(volumeDao).update(eq(100L), any(VolumeVO.class)); @@ -1488,8 +1488,10 @@ void testResize_NFS_Success() { verify(createCallback).complete(resultCaptor.capture()); assertTrue(resultCaptor.getValue().isSuccess()); - verify(nasStrategy).resizeCloudStackVolume(any(CloudStackVolume.class), eq(newSize)); - // NFS has no LUN – volumeDetailsDao must never be queried for a LUN UUID + ArgumentCaptor volumeCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); + verify(nasStrategy).resizeCloudStackVolume(volumeCaptor.capture(), eq(newSize)); + assertEquals(volumeInfo, volumeCaptor.getValue().getVolumeInfo()); + // NFS has no LUN – driver must not look up a LUN UUID verify(volumeDetailsDao, never()).findDetail(anyLong(), eq(OntapStorageConstants.LUN_DOT_UUID)); verify(volumeVO).setSize(newSize); verify(volumeDao).update(eq(100L), any(VolumeVO.class)); @@ -1587,61 +1589,12 @@ void testResize_ShrinkAttempt_Fails() { } } - @Test - void testResize_iSCSI_LunUuidNotFound_Fails() { - long currentSize = 10737418240L; - long newSize = 21474836480L; - stubResizeCommon(currentSize, newSize); - - // LUN UUID detail is missing - when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(null); - - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { - utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) - .thenReturn(sanStrategy); - - driver.resize(volumeInfo, createCallback); - - ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); - verify(createCallback).complete(resultCaptor.capture()); - assertFalse(resultCaptor.getValue().isSuccess()); - assertTrue(resultCaptor.getValue().getResult().contains("LUN UUID not found")); - verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); - } - } - - @Test - void testResize_iSCSI_LunUuidValueNull_Fails() { - long currentSize = 10737418240L; - long newSize = 21474836480L; - stubResizeCommon(currentSize, newSize); - - // Detail exists but value is null - VolumeDetailVO blankDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, null, false); - when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(blankDetail); - - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { - utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) - .thenReturn(sanStrategy); - - driver.resize(volumeInfo, createCallback); - - ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); - verify(createCallback).complete(resultCaptor.capture()); - assertFalse(resultCaptor.getValue().isSuccess()); - assertTrue(resultCaptor.getValue().getResult().contains("LUN UUID not found")); - } - } - @Test void testResize_StrategyThrows_Fails() { long currentSize = 10737418240L; long newSize = 21474836480L; stubResizeCommon(currentSize, newSize); - VolumeDetailVO lunUuidDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, "lun-uuid-123", false); - when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(lunUuidDetail); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(sanStrategy); @@ -1664,27 +1617,40 @@ void testResize_StrategyThrows_Fails() { // ========================================================================= @Test - void testGetUsedBytes_NullPool_ReturnsZero() { - assertEquals(0L, driver.getUsedBytes(null)); + void testGetUsedBytes_NullPool_ThrowsException() { + InvalidParameterValueException ex = assertThrows(InvalidParameterValueException.class, + () -> driver.getUsedBytes(null)); + assertTrue(ex.getMessage().contains("storagePool should not be null")); } @Test - void testGetUsedBytes_NoFlexVolUuid_ReturnsZero() { + void testGetUsedBytes_NoFlexVolUuid_ThrowsException() { // VOLUME_UUID key is absent from pool details storagePoolDetails.remove(OntapStorageConstants.VOLUME_UUID); when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - assertEquals(0L, driver.getUsedBytes(storagePool)); + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> driver.getUsedBytes(storagePool)); + assertTrue(ex.getMessage().contains("FlexVolume UUID not found in pool details for pool 1")); } @Test - void testGetUsedBytes_BlankFlexVolUuid_ReturnsZero() { + void testGetUsedBytes_BlankFlexVolUuid_ThrowsException() { storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, " "); when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - assertEquals(0L, driver.getUsedBytes(storagePool)); + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> driver.getUsedBytes(storagePool)); + assertTrue(ex.getMessage().contains("FlexVolume UUID not found in pool details for pool 1")); + } + + @Test + void testGetUsedBytes_NullPoolDetails_ThrowsException() { + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(null); + + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> driver.getUsedBytes(storagePool)); + assertTrue(ex.getMessage().contains("FlexVolume UUID not found in pool details for pool 1")); } @Test From c9f59ea85ee4fa63164984a58499d9abc6c65e82 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Wed, 23 Sep 2026 14:15:12 +0530 Subject: [PATCH 5/7] CSTACKEX-261: Adding more UT's --- .../driver/OntapPrimaryDatastoreDriver.java | 3 +- .../OntapPrimaryDatastoreDriverTest.java | 2 +- .../service/UnifiedSANStrategyTest.java | 48 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 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 a9a0b74931c4..61d2d1f7893c 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 @@ -1087,12 +1087,13 @@ private boolean isTemplateCachedOnPool(VMTemplateStoragePoolVO templatePoolRef, * error, etc.). Throws if the FlexVolume UUID is not recorded in pool details, since that * indicates the pool was never fully provisioned.

* + * @throws InvalidParameterValueException if {@code storagePool} is null * @throws CloudRuntimeException if the pool has no FlexVolume UUID in its details */ @Override public long getUsedBytes(StoragePool storagePool) { if (storagePool == null) { - return 0; + throw new InvalidParameterValueException("storagePool is null, ensure the pool exists and is fully initialised before querying used bytes"); } Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); 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 f1bb8eb7679a..e17eaa947260 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 @@ -1620,7 +1620,7 @@ void testResize_StrategyThrows_Fails() { void testGetUsedBytes_NullPool_ThrowsException() { InvalidParameterValueException ex = assertThrows(InvalidParameterValueException.class, () -> driver.getUsedBytes(null)); - assertTrue(ex.getMessage().contains("storagePool should not be null")); + assertTrue(ex.getMessage().contains("storagePool is null, ensure the pool exists and is fully initialised before querying used bytes")); } @Test diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index 43043cdad0e1..fb518d926507 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -25,6 +25,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo; import org.apache.cloudstack.engine.subsystem.api.storage.Scope; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.Igroup; @@ -66,6 +67,8 @@ import org.mockito.junit.jupiter.MockitoExtension; import com.cloud.host.HostVO; +import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.dao.VolumeDetailsDao; import com.cloud.utils.exception.CloudRuntimeException; import feign.FeignException; @@ -88,6 +91,9 @@ class UnifiedSANStrategyTest { @Mock private StoragePoolDetailsDao storagePoolDetailsDao; + @Mock + private VolumeDetailsDao volumeDetailsDao; + private UnifiedSANStrategy unifiedSANStrategy; private String authHeader; @@ -115,6 +121,11 @@ void setUp() { java.lang.reflect.Field storagePoolDetailsDaoField = UnifiedSANStrategy.class.getDeclaredField("storagePoolDetailsDao"); storagePoolDetailsDaoField.setAccessible(true); storagePoolDetailsDaoField.set(unifiedSANStrategy, storagePoolDetailsDao); + + // Inject volumeDetailsDao, used to resolve the LUN UUID during resize + java.lang.reflect.Field volumeDetailsDaoField = UnifiedSANStrategy.class.getDeclaredField("volumeDetailsDao"); + volumeDetailsDaoField.setAccessible(true); + volumeDetailsDaoField.set(unifiedSANStrategy, volumeDetailsDao); } catch (Exception e) { throw new RuntimeException(e); } @@ -1013,6 +1024,7 @@ void testResizeCloudStackVolume_ValidRequest_PatchesSize() { lun.setUuid("lun-uuid-123"); CloudStackVolume request = new CloudStackVolume(); request.setLun(lun); + request.setVolumeInfo(mock(VolumeInfo.class)); unifiedSANStrategy.resizeCloudStackVolume(request, 21474836480L); @@ -1023,19 +1035,54 @@ void testResizeCloudStackVolume_ValidRequest_PatchesSize() { @Test void testResizeCloudStackVolume_NoUuid_Throws() { + VolumeInfo volumeInfo = mock(VolumeInfo.class); + when(volumeInfo.getId()).thenReturn(100L); CloudStackVolume request = new CloudStackVolume(); request.setLun(new Lun()); + request.setVolumeInfo(volumeInfo); + + // LUN UUID is absent on the request and cannot be resolved from volume details + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(null); assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.resizeCloudStackVolume(request, 100L)); verify(sanFeignClient, never()).updateLun(any(), any(), any()); } + @Test + void testResizeCloudStackVolume_NoVolumeInfo_Throws() { + Lun lun = new Lun(); + lun.setUuid("lun-uuid-123"); + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.resizeCloudStackVolume(request, 100L)); + verify(sanFeignClient, never()).updateLun(any(), any(), any()); + } + + @Test + void testResizeCloudStackVolume_UuidResolvedFromVolumeDetails() { + VolumeInfo volumeInfo = mock(VolumeInfo.class); + when(volumeInfo.getId()).thenReturn(100L); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeInfo); + + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, "lun-uuid-123", false)); + + unifiedSANStrategy.resizeCloudStackVolume(request, 21474836480L); + + ArgumentCaptor lunCaptor = ArgumentCaptor.forClass(Lun.class); + verify(sanFeignClient).updateLun(any(), eq("lun-uuid-123"), lunCaptor.capture()); + assertEquals(21474836480L, lunCaptor.getValue().getSpace().getSize()); + } + @Test void testResizeCloudStackVolume_InvalidSize_Throws() { Lun lun = new Lun(); lun.setUuid("lun-uuid-123"); CloudStackVolume request = new CloudStackVolume(); request.setLun(lun); + request.setVolumeInfo(mock(VolumeInfo.class)); assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.resizeCloudStackVolume(request, 0L)); assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.resizeCloudStackVolume(null, 100L)); @@ -1048,6 +1095,7 @@ void testResizeCloudStackVolume_FeignException_Throws() { lun.setUuid("lun-uuid-123"); CloudStackVolume request = new CloudStackVolume(); request.setLun(lun); + request.setVolumeInfo(mock(VolumeInfo.class)); FeignException feignException = mock(FeignException.class); when(feignException.status()).thenReturn(500); From 82c7d156a61ee18c1184c915717aaf5be2d9b156 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Wed, 23 Sep 2026 15:31:31 +0530 Subject: [PATCH 6/7] CSTACKEX-261: handling casee where lun is deleted on ontap and trying to resize --- .../storage/service/UnifiedSANStrategy.java | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) 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 2ff7a2edaa4a..f4b3ce7e6a2a 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 @@ -19,22 +19,23 @@ package org.apache.cloudstack.storage.service; -import com.cloud.host.HostVO; -import com.cloud.storage.VolumeDetailVO; -import com.cloud.storage.dao.VolumeDetailsDao; -import com.cloud.utils.exception.CloudRuntimeException; -import feign.FeignException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import javax.inject.Inject; + import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Initiator; -import org.apache.cloudstack.storage.feign.model.Svm; -import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunMap; import org.apache.cloudstack.storage.feign.model.LunSpace; -import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; +import org.apache.cloudstack.storage.feign.model.OntapStorage; +import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; @@ -45,10 +46,13 @@ import org.apache.commons.collections.CollectionUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import javax.inject.Inject; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; + +import com.cloud.host.HostVO; +import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.utils.exception.CloudRuntimeException; + +import feign.FeignException; public class UnifiedSANStrategy extends SANStrategy { @@ -315,12 +319,19 @@ public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeI sanFeignClient.updateLun(authHeader, lunUuid, patch); logger.debug("resizeCloudStackVolume: Lun {} resized to {} bytes", lunUuid, sizeInBytes); } catch (FeignException e) { - logger.error("FeignException occurred while resizing LUN: {}, Status: {}, Exception: {}", + logger.error("FeignException occurred while resizing LUN [{}], Status: {}, Exception: {}", lunUuid, e.status(), e.getMessage()); - throw new CloudRuntimeException("Failed to resize Lun: " + e.getMessage()); + if (OntapStorageUtils.isOntapObjectNotFoundError(e)) { + throw new CloudRuntimeException(String.format( + "LUN [%s] no longer exists on ONTAP; it may have been deleted externally. " + + "Verify the LUN is present before retrying the resize.", lunUuid)); + } + throw new CloudRuntimeException(String.format( + "Failed to resize LUN [%s]: %s",lunUuid, e.getMessage())); } catch (Exception e) { - logger.error("Exception occurred while resizing LUN: {}, Exception: {}", lunUuid, e.getMessage()); - throw new CloudRuntimeException("Failed to resize Lun: " + e.getMessage()); + logger.error("Exception occurred while resizing LUN [{}]: {}", lunUuid, e.getMessage()); + throw new CloudRuntimeException(String.format( + "Unexpected error while resizing LUN [%s]: %s", lunUuid, e.getMessage())); } } From 57f98f2b2e52f48311a4dce4128d7d3a187ccc54 Mon Sep 17 00:00:00 2001 From: sr73318 Date: Thu, 24 Sep 2026 11:43:39 +0530 Subject: [PATCH 7/7] CSTACKEX-261: adding more UT's and any null checks --- .../driver/OntapPrimaryDatastoreDriver.java | 54 ++++--- .../storage/service/UnifiedSANStrategy.java | 3 +- .../OntapPrimaryDatastoreDriverTest.java | 136 +++++++++++++++++- .../service/UnifiedNASStrategyTest.java | 113 +++++++++++++++ .../service/UnifiedSANStrategyTest.java | 95 +++++++++++- 5 files changed, 367 insertions(+), 34 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 61d2d1f7893c..d83f91b4ca87 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 @@ -615,6 +615,9 @@ public void resize(DataObject data, AsyncCompletionCallback cal if (payload == null || payload.newSize == null) { throw new CloudRuntimeException("Invalid resize payload for volume " + volumeInfo.getId()); } + if (volumeInfo.getDataStore() == null) { + throw new CloudRuntimeException("Data store not found for volume " + volumeInfo.getId()); + } StoragePoolVO storagePool = storagePoolDao.findById(volumeInfo.getDataStore().getId()); if (storagePool == null) { @@ -622,7 +625,6 @@ public void resize(DataObject data, AsyncCompletionCallback cal } Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); - StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); VolumeVO volumeVO = volumeDao.findById(volumeInfo.getId()); if (volumeVO == null) { throw new CloudRuntimeException("Volume not found for id " + volumeInfo.getId()); @@ -632,26 +634,27 @@ public void resize(DataObject data, AsyncCompletionCallback cal "Storage pool %s does not support shrinking a volume.", storagePool.getName())); } + StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); CloudStackVolume cloudStackVolume = new CloudStackVolume(); cloudStackVolume.setVolumeInfo(volumeInfo); - - // delegates to UnifiedSANStrategy (PATCH /api/storage/luns/{uuid}) for iSCSI - // or to UnifiedNASStrategy (ResizeVolumeCommand to KVM agent) for NFS3; - // protocol-specific setup (e.g. LUN UUID lookup) is handled inside each strategy storageStrategy.resizeCloudStackVolume(cloudStackVolume, payload.newSize); + long currentSize = volumeVO.getSize(); volumeVO.setSize(payload.newSize); - volumeDao.update(volumeVO.getId(), volumeVO); + if (!volumeDao.update(volumeVO.getId(), volumeVO)) { + throw new CloudRuntimeException("Failed to update volume " + volumeVO.getId() + + " after resizing the ONTAP backing object"); + } String instanceName = payload.instanceName != null ? payload.instanceName : "none"; ResizeVolumeCommand resizeCmd = new ResizeVolumeCommand(volumeVO.getPath(), - new StorageFilerTO(storagePool), volumeVO.getSize(), payload.newSize, + new StorageFilerTO(storagePool), currentSize, payload.newSize, false, instanceName); result = new CreateCmdResult(volumeVO.getPath(), new Answer(resizeCmd, true, null)); logger.info("resize: Successfully resized volume [{}] to [{}] bytes", volumeInfo.getId(), payload.newSize); } catch (Exception e) { String errMsg = e.getMessage(); - logger.error("resize: Failed for volume [{}]: {}", data.getId(), errMsg, e); + logger.error("resize: Failed for volume [{}]: {}", data != null ? data.getId() : null, errMsg, e); result = new CreateCmdResult(null, new Answer(null, false, errMsg)); result.setResult(errMsg); } finally { @@ -1080,15 +1083,16 @@ private boolean isTemplateCachedOnPool(VMTemplateStoragePoolVO templatePoolRef, } /** - * Returns the bytes available on the FlexVolume backing this pool, read directly from ONTAP - * ({@code space.available}). + * Returns the bytes used on the FlexVolume backing this pool, read directly from ONTAP + * ({@code space.used}). * - *

Returns {@code 0} if the ONTAP REST call fails for any reason (array unreachable, auth - * error, etc.). Throws if the FlexVolume UUID is not recorded in pool details, since that - * indicates the pool was never fully provisioned.

+ *

Fails closed when ONTAP cannot provide trustworthy usage data. Returning zero for an + * unreachable or incomplete backend would make capacity checks treat an unknown pool as empty + * and could incorrectly authorize a volume grow.

* * @throws InvalidParameterValueException if {@code storagePool} is null - * @throws CloudRuntimeException if the pool has no FlexVolume UUID in its details + * @throws CloudRuntimeException if the pool has no FlexVolume UUID in its details, ONTAP + * cannot be queried, or used-space data is missing */ @Override public long getUsedBytes(StoragePool storagePool) { @@ -1107,19 +1111,27 @@ public long getUsedBytes(StoragePool storagePool) { StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); var flexVol = strategy.getStorageVolume(flexVolUuid); - if (flexVol == null || flexVol.getSpace() == null) { - logger.warn("getUsedBytes: FlexVolume [{}] not found or has no space info for pool [{}]; returning 0", - flexVolUuid, storagePool.getId()); - return 0; + if (flexVol == null) { + throw new CloudRuntimeException(String.format( + "FlexVolume [%s] backing pool [%s] was not found on ONTAP", + flexVolUuid, storagePool.getId())); + } + if (flexVol.getSpace() == null) { + throw new CloudRuntimeException(String.format( + "ONTAP returned no space information for FlexVolume [%s] backing pool [%s]", + flexVolUuid, storagePool.getId())); } logger.debug("getUsedBytes: FlexVolume [{}] backing pool [{}] reports {} bytes used", flexVolUuid, storagePool.getId(), flexVol.getSpace().getUsed()); return flexVol.getSpace().getUsed(); + } catch (CloudRuntimeException e) { + logger.error("getUsedBytes: Failed to get used bytes for pool [{}]", storagePool.getId(), e); + throw e; } catch (Exception e) { - logger.warn("getUsedBytes: Could not read used space from ONTAP for pool [{}]; returning 0", - storagePool.getId(), e); - return 0; + logger.error("getUsedBytes: Failed to get used bytes for pool [{}]", storagePool.getId(), e); + throw new CloudRuntimeException( + String.format("Could not read used space for pool [%s]: %s", storagePool.getId(), e.getMessage()), e); } } 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 f4b3ce7e6a2a..9b5eb3261601 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 @@ -44,6 +44,7 @@ import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -300,7 +301,7 @@ public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeI if (cloudstackVolume.getLun() == null || cloudstackVolume.getLun().getUuid() == null) { long volumeId = cloudstackVolume.getVolumeInfo().getId(); VolumeDetailVO lunUuidDetail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.LUN_DOT_UUID); - if (lunUuidDetail == null || lunUuidDetail.getValue() == null) { + if (lunUuidDetail == null || StringUtils.isBlank(lunUuidDetail.getValue())) { throw new CloudRuntimeException("LUN UUID not found in volume details for volume " + volumeId); } Lun resolvedLun = new Lun(); 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 e17eaa947260..5b7f83c3f67b 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 @@ -68,6 +68,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.mockito.junit.jupiter.MockitoExtension; @@ -1439,6 +1440,7 @@ private void stubResizeCommon(long currentSize, long newSize) { // volumeVO.getId() is only needed in the success path (volumeDao.update call) lenient().when(volumeVO.getId()).thenReturn(100L); lenient().when(volumeVO.getPath()).thenReturn("/vol/vol1/lun1"); + lenient().when(volumeDao.update(100L, volumeVO)).thenReturn(true); } @Test @@ -1511,6 +1513,28 @@ void testResize_NullPayload_Fails() { assertTrue(resultCaptor.getValue().getResult().contains("Invalid resize payload")); } + @Test + void testResize_NullData_FailsAndCompletesCallbackOnce() { + driver.resize(null, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback, times(1)).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Expected VolumeInfo")); + } + + @Test + void testResize_NonVolumeInfo_FailsAndCompletesCallbackOnce() { + when(templateInfo.getId()).thenReturn(50L); + + driver.resize(templateInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback, times(1)).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Expected VolumeInfo")); + } + @Test void testResize_NullNewSize_Fails() { // payload.newSize is null @@ -1543,6 +1567,21 @@ void testResize_StoragePoolNotFound_Fails() { assertTrue(resultCaptor.getValue().getResult().contains("Storage pool not found")); } + @Test + void testResize_MissingDataStore_Fails() { + ResizeVolumePayload payload = new ResizeVolumePayload(21474836480L, null, null, null, false, "none", null, false); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getpayload()).thenReturn(payload); + when(volumeInfo.getDataStore()).thenReturn(null); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Data store not found")); + } + @Test void testResize_VolumeVONotFound_Fails() { long newSize = 21474836480L; @@ -1569,6 +1608,27 @@ void testResize_VolumeVONotFound_Fails() { } } + @Test + void testResize_MissingStoragePoolDetails_Fails() { + long currentSize = 10737418240L; + long newSize = 21474836480L; + stubResizeCommon(currentSize, newSize); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(null); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(null)) + .thenThrow(new CloudRuntimeException("Storage pool details are missing")); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback, times(1)).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Storage pool details are missing")); + verify(volumeVO, never()).setSize(anyLong()); + } + } + @Test void testResize_ShrinkAttempt_Fails() { long currentSize = 21474836480L; // 20 GB @@ -1612,6 +1672,44 @@ void testResize_StrategyThrows_Fails() { } } + @Test + void testResize_EqualSize_InvokesStrategyAndSucceeds() { + long size = 10737418240L; + stubResizeCommon(size, size); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + + driver.resize(volumeInfo, createCallback); + + verify(sanStrategy).resizeCloudStackVolume(any(), eq(size)); + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + } + } + + @Test + void testResize_DatabaseUpdateFailure_ReturnsFailure() { + long currentSize = 10737418240L; + long newSize = 21474836480L; + stubResizeCommon(currentSize, newSize); + when(volumeDao.update(100L, volumeVO)).thenReturn(false); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + + driver.resize(volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback, times(1)).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Failed to update volume")); + } + } + // ========================================================================= // getUsedBytes() tests // ========================================================================= @@ -1674,7 +1772,7 @@ void testGetUsedBytes_Success_ReturnsUsedBytes() { } @Test - void testGetUsedBytes_FlexVolNotFound_ReturnsZero() { + void testGetUsedBytes_FlexVolNotFound_ThrowsException() { storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-123"); when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); @@ -1684,12 +1782,14 @@ void testGetUsedBytes_FlexVolNotFound_ReturnsZero() { .thenReturn(sanStrategy); when(sanStrategy.getStorageVolume("flexvol-uuid-123")).thenReturn(null); - assertEquals(0L, driver.getUsedBytes(storagePool)); + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, + () -> driver.getUsedBytes(storagePool)); + assertTrue(ex.getMessage().contains("was not found on ONTAP")); } } @Test - void testGetUsedBytes_FlexVolNullSpace_ReturnsZero() { + void testGetUsedBytes_FlexVolNullSpace_ThrowsException() { storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-123"); when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); @@ -1702,12 +1802,14 @@ void testGetUsedBytes_FlexVolNullSpace_ReturnsZero() { .thenReturn(sanStrategy); when(sanStrategy.getStorageVolume("flexvol-uuid-123")).thenReturn(flexVol); - assertEquals(0L, driver.getUsedBytes(storagePool)); + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, + () -> driver.getUsedBytes(storagePool)); + assertTrue(ex.getMessage().contains("no space information")); } } @Test - void testGetUsedBytes_OntapException_ReturnsZero() { + void testGetUsedBytes_OntapException_PropagatesException() { storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-123"); when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); @@ -1718,7 +1820,29 @@ void testGetUsedBytes_OntapException_ReturnsZero() { when(sanStrategy.getStorageVolume("flexvol-uuid-123")) .thenThrow(new com.cloud.utils.exception.CloudRuntimeException("ONTAP unreachable")); - assertEquals(0L, driver.getUsedBytes(storagePool)); + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, + () -> driver.getUsedBytes(storagePool)); + assertEquals("ONTAP unreachable", ex.getMessage()); + } + } + + @Test + void testGetUsedBytes_UnexpectedException_IsWrapped() { + storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-123"); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(sanStrategy); + when(sanStrategy.getStorageVolume("flexvol-uuid-123")) + .thenThrow(new IllegalStateException("invalid ONTAP response")); + + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, + () -> driver.getUsedBytes(storagePool)); + assertTrue(ex.getMessage().contains("Could not read used space")); + assertTrue(ex.getMessage().contains("invalid ONTAP response")); + assertTrue(ex.getCause() instanceof IllegalStateException); } } } 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 index c79f3640e71c..c7311d884cc0 100755 --- 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 @@ -73,6 +73,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyMap; @@ -1118,6 +1119,17 @@ public void testResizeCloudStackVolume_InvalidSizeDoesNotSendCommand() { verify(epSelector, never()).select(any(org.apache.cloudstack.engine.subsystem.api.storage.DataObject.class)); } + @Test + public void testResizeCloudStackVolume_NegativeSizeDoesNotSendCommand() { + VolumeObject volumeObject = mock(VolumeObject.class); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(request, -1L)); + + verify(epSelector, never()).select(any(org.apache.cloudstack.engine.subsystem.api.storage.DataObject.class)); + } + @Test public void testDeleteFileByPath_Treats404AsSuccess() { FeignException feignException = mock(FeignException.class); @@ -1287,4 +1299,105 @@ public void testResizeCloudStackVolume_NoEndpoint_Throws() { assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(request, 21474836480L)); } + + @Test + public void testResizeCloudStackVolume_VolumeNotFound_Throws() { + VolumeObject volumeObject = mock(VolumeObject.class); + when(volumeObject.getId()).thenReturn(100L); + when(volumeDao.findById(100L)).thenReturn(null); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> strategy.resizeCloudStackVolume(request, 21474836480L)); + + assertTrue(exception.getMessage().contains("Volume not found")); + verify(epSelector, never()).select(any(org.apache.cloudstack.engine.subsystem.api.storage.DataObject.class)); + } + + @Test + public void testResizeCloudStackVolume_StoragePoolNotFound_Throws() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + when(volumeObject.getId()).thenReturn(100L); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(null); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> strategy.resizeCloudStackVolume(request, 21474836480L)); + + assertTrue(exception.getMessage().contains("Storage Pool not found")); + verify(epSelector, never()).select(any(org.apache.cloudstack.engine.subsystem.api.storage.DataObject.class)); + } + + @Test + public void testResizeCloudStackVolume_NullAnswer_Throws() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + EndPoint endPoint = mock(EndPoint.class); + when(volumeObject.getId()).thenReturn(100L); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(storagePool); + when(epSelector.select(volumeObject)).thenReturn(endPoint); + when(endPoint.sendMessage(any(ResizeVolumeCommand.class))).thenReturn(null); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> strategy.resizeCloudStackVolume(request, 21474836480L)); + + assertTrue(exception.getMessage().contains("Failed to resize qcow2 on KVM host")); + } + + @Test + public void testResizeCloudStackVolume_AgentException_Propagates() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + EndPoint endPoint = mock(EndPoint.class); + when(volumeObject.getId()).thenReturn(100L); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(storagePool); + when(epSelector.select(volumeObject)).thenReturn(endPoint); + when(endPoint.sendMessage(any(ResizeVolumeCommand.class))) + .thenThrow(new CloudRuntimeException("KVM agent unreachable")); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> strategy.resizeCloudStackVolume(request, 21474836480L)); + + assertEquals("KVM agent unreachable", exception.getMessage()); + } + + @Test + public void testResizeCloudStackVolume_DeletedFile_ThrowsAgentError() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + EndPoint endPoint = mock(EndPoint.class); + when(volumeObject.getId()).thenReturn(100L); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPath()).thenReturn("deleted-volume.qcow2"); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(storagePool); + when(epSelector.select(volumeObject)).thenReturn(endPoint); + when(endPoint.sendMessage(any(ResizeVolumeCommand.class))) + .thenReturn(new Answer(null, false, + "qemu-img: Could not open 'deleted-volume.qcow2': No such file or directory")); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> strategy.resizeCloudStackVolume(request, 21474836480L)); + + assertTrue(exception.getMessage().contains("No such file or directory")); + assertTrue(exception.getMessage().contains("deleted-volume.qcow2")); + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index fb518d926507..14ea0f0a3dc4 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -1048,6 +1048,22 @@ void testResizeCloudStackVolume_NoUuid_Throws() { verify(sanFeignClient, never()).updateLun(any(), any(), any()); } + @Test + void testResizeCloudStackVolume_BlankUuidDetail_Throws() { + VolumeInfo volumeInfo = mock(VolumeInfo.class); + when(volumeInfo.getId()).thenReturn(100L); + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeInfo); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, " ", false)); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.resizeCloudStackVolume(request, 100L)); + + assertTrue(exception.getMessage().contains("LUN UUID not found")); + verify(sanFeignClient, never()).updateLun(any(), any(), any()); + } + @Test void testResizeCloudStackVolume_NoVolumeInfo_Throws() { Lun lun = new Lun(); @@ -1090,27 +1106,94 @@ void testResizeCloudStackVolume_InvalidSize_Throws() { } @Test - void testResizeCloudStackVolume_FeignException_Throws() { + void testResizeCloudStackVolume_FeignExceptions_Throw() { Lun lun = new Lun(); lun.setUuid("lun-uuid-123"); CloudStackVolume request = new CloudStackVolume(); request.setLun(lun); request.setVolumeInfo(mock(VolumeInfo.class)); - FeignException feignException = mock(FeignException.class); - when(feignException.status()).thenReturn(500); - when(feignException.getMessage()).thenReturn("resize failed"); + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + for (int status : new int[] {401, 403, 409, 500, -1}) { + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(status); + when(feignException.getMessage()).thenReturn( + status == -1 ? "connection timed out" : "resize failed"); + doThrow(feignException).when(sanFeignClient) + .updateLun(eq(authHeader), eq("lun-uuid-123"), any(Lun.class)); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.resizeCloudStackVolume(request, 21474836480L)); + } + } + } + + @Test + void testResizeCloudStackVolume_DeletedLun_ThrowsSpecificMessage() { + Lun lun = new Lun(); + lun.setUuid("deleted-lun-uuid"); + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + request.setVolumeInfo(mock(VolumeInfo.class)); + FeignException notFound = mock(FeignException.class); + when(notFound.status()).thenReturn(404); + when(notFound.getMessage()).thenReturn("LUN not found"); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) .thenReturn(authHeader); - doThrow(feignException).when(sanFeignClient).updateLun(eq(authHeader), eq("lun-uuid-123"), any(Lun.class)); + utilityMock.when(() -> OntapStorageUtils.isOntapObjectNotFoundError(notFound)) + .thenReturn(true); + doThrow(notFound).when(sanFeignClient) + .updateLun(eq(authHeader), eq("deleted-lun-uuid"), any(Lun.class)); - assertThrows(CloudRuntimeException.class, + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.resizeCloudStackVolume(request, 21474836480L)); + + assertTrue(exception.getMessage().contains("no longer exists on ONTAP")); + assertTrue(exception.getMessage().contains("deleted-lun-uuid")); + } + } + + @Test + void testResizeCloudStackVolume_UnexpectedException_IsWrapped() { + Lun lun = new Lun(); + lun.setUuid("lun-uuid-123"); + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + request.setVolumeInfo(mock(VolumeInfo.class)); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + doThrow(new IllegalStateException("unexpected failure")).when(sanFeignClient) + .updateLun(eq(authHeader), eq("lun-uuid-123"), any(Lun.class)); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.resizeCloudStackVolume(request, 21474836480L)); + + assertTrue(exception.getMessage().contains("Unexpected error")); + assertTrue(exception.getMessage().contains("unexpected failure")); } } + @Test + void testResizeCloudStackVolume_SameSizePatchIsIdempotent() { + Lun lun = new Lun(); + lun.setUuid("lun-uuid-123"); + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + request.setVolumeInfo(mock(VolumeInfo.class)); + + unifiedSANStrategy.resizeCloudStackVolume(request, 10737418240L); + + ArgumentCaptor lunCaptor = ArgumentCaptor.forClass(Lun.class); + verify(sanFeignClient).updateLun(any(), eq("lun-uuid-123"), lunCaptor.capture()); + assertEquals(10737418240L, lunCaptor.getValue().getSpace().getSize()); + } + @Test void testCloneCloudStackVolume_EmptyRecords_ThrowsException() { Lun.Source source = new Lun.Source();