From dc6a5b829d8d196c724af7bf565fa9336319097d Mon Sep 17 00:00:00 2001 From: Sathvika Date: Fri, 25 Sep 2026 14:56:42 +0530 Subject: [PATCH 1/3] =?UTF-8?q?CSTACKEX-321:=20when=20nothing=20is=20chang?= =?UTF-8?q?ed=20in=20volume=20resize=20wizard=20directl=E2=80=A6=20(#110)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calling resizeVolume with the existing volume size still invoked the storage driver, causing an unnecessary driver resize request for volumes. ### Description This PR... ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [X] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [X] Minor - [ ] Trivial ### Screenshots (if appropriate): Before Patch API is still called in case of no changes in the wizard: Screenshot 2026-09-24 at 4 33 33 PM actually resizes (IN UI) Screenshot 2026-09-24 at 4 36 00 PM after the fix Patch api is not called : Screenshot 2026-09-24 at 4 17 52 PM Shows success immediately: Screenshot 2026-09-24 at 4 28 37 PM ### How Has This Been Tested? #### How did you try to break this feature and the system with this change? --- .../java/com/cloud/storage/VolumeApiServiceImpl.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 1a2edb58a002..4fcde5b26948 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1456,11 +1456,12 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep if (currentSize != newSize || !compareEqualsIncludingNullOrZero(newMaxIops, volume.getMaxIops()) || !compareEqualsIncludingNullOrZero(newMinIops, volume.getMinIops())) { volumeResizeRequired = true; } - if (!volumeMigrateRequired && !volumeResizeRequired && newDiskOffering != null) { - _volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId()); + if (!volumeMigrateRequired && !volumeResizeRequired) { volume = _volsDao.findById(volume.getId()); - updateStorageWithTheNewDiskOffering(volume, newDiskOffering); - + if (newDiskOffering != null) { + _volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId()); + updateStorageWithTheNewDiskOffering(volume, newDiskOffering); + } return volume; } From 5670850f3878ca1b1834f5da54f4bc87c157d155 Mon Sep 17 00:00:00 2001 From: Sathvika Date: Tue, 13 Oct 2026 16:08:19 +0530 Subject: [PATCH 2/3] =?UTF-8?q?CSTACKEX-341:=20when=20disk=20offering=20pa?= =?UTF-8?q?rameter=20is=20same=20as=20the=20existing=20in=E2=80=A6=20(#120?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … volume resize return early ### Description This PR... ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [X] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [X] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [X] Minor - [ ] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? In case a newerdiskofferingID is mentioned in API then it updates the DB. Screenshot 2026-10-13 at 11 53 33 AM when same ID is given then it immediately returns. Screenshot 2026-10-13 at 11 53 44 AM #### How did you try to break this feature and the system with this change? --- .../main/java/com/cloud/storage/VolumeApiServiceImpl.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 4fcde5b26948..8b205b9b550d 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1457,9 +1457,10 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep volumeResizeRequired = true; } if (!volumeMigrateRequired && !volumeResizeRequired) { - volume = _volsDao.findById(volume.getId()); - if (newDiskOffering != null) { + + if (newDiskOffering != null && newDiskOffering.getId()!=diskOffering.getId()) { _volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId()); + volume = _volsDao.findById(volume.getId()); updateStorageWithTheNewDiskOffering(volume, newDiskOffering); } return volume; From d914b9920b2575b5a000365759b25ae03b73cc1a Mon Sep 17 00:00:00 2001 From: sandeeplocharla <85344604+sandeeplocharla@users.noreply.github.com> Date: Mon, 2 Nov 2026 17:00:52 +0530 Subject: [PATCH 3/3] Address review comments on upstream PR 14259 on apache/main (#135) ### Description This PR... ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [X] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [X] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [X] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? #### How did you try to break this feature and the system with this change? --- .../cloud/storage/VolumeApiServiceImpl.java | 2 +- .../storage/VolumeApiServiceImplTest.java | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 8b205b9b550d..11b24a9beef6 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1458,7 +1458,7 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep } if (!volumeMigrateRequired && !volumeResizeRequired) { - if (newDiskOffering != null && newDiskOffering.getId()!=diskOffering.getId()) { + if (newDiskOffering != null && newDiskOffering.getId() != diskOffering.getId()) { _volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId()); volume = _volsDao.findById(volume.getId()); updateStorageWithTheNewDiskOffering(volume, newDiskOffering); diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 334bb3f659fd..401b12ebbf46 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -55,6 +55,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; @@ -2058,6 +2059,75 @@ public void testResizeVolumeInAllocateState() throws ResourceAllocationException verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class)); } } + + @Test + public void testResizeVolumeNoChangeSkipsDriver() throws ResourceAllocationException, ExecutionException, InterruptedException { + ResizeVolumeCmd cmd = new ResizeVolumeCmd(); + ReflectionTestUtils.setField(cmd, "id", volumeMockId); + + testResizeVolumeSetup(); + when(storageMgr.storagePoolHasEnoughSpaceForResize(any(), nullable(Long.class), nullable(Long.class))).thenReturn(true); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + Volume result = volumeApiServiceImpl.resizeVolume(cmd); + + Assert.assertSame(volumeVoMock, result); + verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class)); + verify(volumeDaoMock, times(0)).updateDiskOffering(anyLong(), anyLong()); + } + } + + @Test + public void testResizeVolumeSameDiskOfferingSkipsDriver() throws ResourceAllocationException, ExecutionException, InterruptedException { + ResizeVolumeCmd cmd = new ResizeVolumeCmd(); + ReflectionTestUtils.setField(cmd, "id", volumeMockId); + ReflectionTestUtils.setField(cmd, "newDiskOfferingId", newDiskOfferingMockId); + + testResizeVolumeSetup(); + when(newDiskOfferingMock.getDiskSize()).thenReturn(volumeSizeMock); + when(diskOfferingMock.getId()).thenReturn(diskOfferingMockId); + when(newDiskOfferingMock.getId()).thenReturn(diskOfferingMockId); + when(storageMgr.storagePoolHasEnoughSpaceForResize(any(), nullable(Long.class), nullable(Long.class))).thenReturn(true); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + Volume result = volumeApiServiceImpl.resizeVolume(cmd); + + Assert.assertSame(volumeVoMock, result); + verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class)); + verify(volumeDaoMock, times(0)).updateDiskOffering(anyLong(), anyLong()); + } + } + + @Test + public void testResizeVolumeNewDiskOfferingSameSizeUpdatesOfferingWithoutResize() throws ResourceAllocationException, ExecutionException, InterruptedException { + ResizeVolumeCmd cmd = new ResizeVolumeCmd(); + ReflectionTestUtils.setField(cmd, "id", volumeMockId); + ReflectionTestUtils.setField(cmd, "newDiskOfferingId", newDiskOfferingMockId); + + testResizeVolumeSetup(); + when(newDiskOfferingMock.getDiskSize()).thenReturn(volumeSizeMock); + when(diskOfferingMock.getId()).thenReturn(diskOfferingMockId); + when(newDiskOfferingMock.getId()).thenReturn(newDiskOfferingMockId); + when(storageMgr.storagePoolHasEnoughSpaceForResize(any(), nullable(Long.class), nullable(Long.class))).thenReturn(true); + + DataStore offeringStore = Mockito.mock(DataStore.class); + PrimaryDataStoreDriver driver = Mockito.mock(PrimaryDataStoreDriver.class); + when(dataStoreMgr.getDataStore(anyLong(), eq(DataStoreRole.Primary))).thenReturn(offeringStore); + when(offeringStore.getDriver()).thenReturn(driver); + when(driver.informStorageForDiskOfferingChange()).thenReturn(true); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + Mockito.clearInvocations(volumeDaoMock); + Volume result = volumeApiServiceImpl.resizeVolume(cmd); + + Assert.assertSame(volumeVoMock, result); + verify(volumeDaoMock).updateDiskOffering(volumeMockId, newDiskOfferingMockId); + verify(volumeDaoMock, times(2)).findById(volumeMockId); + verify(driver).updateStorageWithTheNewDiskOffering(volumeVoMock, newDiskOfferingMock); + verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class)); + } + } + private UserVmVO getMockedVm() { UserVmVO vm = Mockito.mock(UserVmVO.class); Mockito.when(vm.getId()).thenReturn(1L);