Skip to content

Commit dafb4a0

Browse files
sathvikaragisandeeplocharla
sathvikaragi
and
sandeeplocharla
authored
Driver is called when nothing is changed in volume resize wizard (#14259)
* CSTACKEX-321: when nothing is changed in volume resize wizard directl… (#110) Calling resizeVolume with the existing volume size still invoked the storage driver, causing an unnecessary driver resize request for volumes. ### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### 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: <img width="1423" height="1034" alt="Screenshot 2026-09-24 at 4 33 33 PM" src="https://github.com/user-attachments/assets/5480986e-bc79-4bc7-b05c-62077c2cc93e" /> actually resizes (IN UI) <img width="1441" height="638" alt="Screenshot 2026-09-24 at 4 36 00 PM" src="https://github.com/user-attachments/assets/8430eaa3-947e-41d4-8573-518f7831d7f0" /> after the fix Patch api is not called : <img width="1393" height="1035" alt="Screenshot 2026-09-24 at 4 17 52 PM" src="https://github.com/user-attachments/assets/b53e1146-560a-437a-9b0c-501be9ec4d76" /> Shows success immediately: <img width="1471" height="728" alt="Screenshot 2026-09-24 at 4 28 37 PM" src="https://github.com/user-attachments/assets/9cdd57b0-2b1f-4550-9830-46f87c439061" /> ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> * CSTACKEX-341: when disk offering parameter is same as the existing in… (#120) … volume resize return early ### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### 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. <img width="1394" height="814" alt="Screenshot 2026-10-13 at 11 53 33 AM" src="https://github.com/user-attachments/assets/9b4e8249-c639-4d29-8ca4-9431b4f1b6cd" /> when same ID is given then it immediately returns. <img width="1404" height="522" alt="Screenshot 2026-10-13 at 11 53 44 AM" src="https://github.com/user-attachments/assets/d6d0350c-464b-4644-9470-b8879ebdb48c" /> <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> * Address review comments on upstream PR 14259 on apache/main (#135) ### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### 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? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: sandeeplocharla <85344604+sandeeplocharla@users.noreply.github.com>
1 parent 3370d9f commit dafb4a0

2 files changed

Lines changed: 76 additions & 4 deletions

File tree

‎server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,11 +1456,13 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
14561456
if (currentSize != newSize || !compareEqualsIncludingNullOrZero(newMaxIops, volume.getMaxIops()) || !compareEqualsIncludingNullOrZero(newMinIops, volume.getMinIops())) {
14571457
volumeResizeRequired = true;
14581458
}
1459-
if (!volumeMigrateRequired && !volumeResizeRequired && newDiskOffering != null) {
1460-
_volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId());
1461-
volume = _volsDao.findById(volume.getId());
1462-
updateStorageWithTheNewDiskOffering(volume, newDiskOffering);
1459+
if (!volumeMigrateRequired && !volumeResizeRequired) {
14631460

1461+
if (newDiskOffering != null && newDiskOffering.getId() != diskOffering.getId()) {
1462+
_volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId());
1463+
volume = _volsDao.findById(volume.getId());
1464+
updateStorageWithTheNewDiskOffering(volume, newDiskOffering);
1465+
}
14641466
return volume;
14651467
}
14661468

‎server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
5656
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
5757
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
58+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver;
5859
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
5960
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
6061
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
@@ -2058,6 +2059,75 @@ public void testResizeVolumeInAllocateState() throws ResourceAllocationException
20582059
verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class));
20592060
}
20602061
}
2062+
2063+
@Test
2064+
public void testResizeVolumeNoChangeSkipsDriver() throws ResourceAllocationException, ExecutionException, InterruptedException {
2065+
ResizeVolumeCmd cmd = new ResizeVolumeCmd();
2066+
ReflectionTestUtils.setField(cmd, "id", volumeMockId);
2067+
2068+
testResizeVolumeSetup();
2069+
when(storageMgr.storagePoolHasEnoughSpaceForResize(any(), nullable(Long.class), nullable(Long.class))).thenReturn(true);
2070+
2071+
try (MockedStatic<UsageEventUtils> ignored = Mockito.mockStatic(UsageEventUtils.class)) {
2072+
Volume result = volumeApiServiceImpl.resizeVolume(cmd);
2073+
2074+
Assert.assertSame(volumeVoMock, result);
2075+
verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class));
2076+
verify(volumeDaoMock, times(0)).updateDiskOffering(anyLong(), anyLong());
2077+
}
2078+
}
2079+
2080+
@Test
2081+
public void testResizeVolumeSameDiskOfferingSkipsDriver() throws ResourceAllocationException, ExecutionException, InterruptedException {
2082+
ResizeVolumeCmd cmd = new ResizeVolumeCmd();
2083+
ReflectionTestUtils.setField(cmd, "id", volumeMockId);
2084+
ReflectionTestUtils.setField(cmd, "newDiskOfferingId", newDiskOfferingMockId);
2085+
2086+
testResizeVolumeSetup();
2087+
when(newDiskOfferingMock.getDiskSize()).thenReturn(volumeSizeMock);
2088+
when(diskOfferingMock.getId()).thenReturn(diskOfferingMockId);
2089+
when(newDiskOfferingMock.getId()).thenReturn(diskOfferingMockId);
2090+
when(storageMgr.storagePoolHasEnoughSpaceForResize(any(), nullable(Long.class), nullable(Long.class))).thenReturn(true);
2091+
2092+
try (MockedStatic<UsageEventUtils> ignored = Mockito.mockStatic(UsageEventUtils.class)) {
2093+
Volume result = volumeApiServiceImpl.resizeVolume(cmd);
2094+
2095+
Assert.assertSame(volumeVoMock, result);
2096+
verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class));
2097+
verify(volumeDaoMock, times(0)).updateDiskOffering(anyLong(), anyLong());
2098+
}
2099+
}
2100+
2101+
@Test
2102+
public void testResizeVolumeNewDiskOfferingSameSizeUpdatesOfferingWithoutResize() throws ResourceAllocationException, ExecutionException, InterruptedException {
2103+
ResizeVolumeCmd cmd = new ResizeVolumeCmd();
2104+
ReflectionTestUtils.setField(cmd, "id", volumeMockId);
2105+
ReflectionTestUtils.setField(cmd, "newDiskOfferingId", newDiskOfferingMockId);
2106+
2107+
testResizeVolumeSetup();
2108+
when(newDiskOfferingMock.getDiskSize()).thenReturn(volumeSizeMock);
2109+
when(diskOfferingMock.getId()).thenReturn(diskOfferingMockId);
2110+
when(newDiskOfferingMock.getId()).thenReturn(newDiskOfferingMockId);
2111+
when(storageMgr.storagePoolHasEnoughSpaceForResize(any(), nullable(Long.class), nullable(Long.class))).thenReturn(true);
2112+
2113+
DataStore offeringStore = Mockito.mock(DataStore.class);
2114+
PrimaryDataStoreDriver driver = Mockito.mock(PrimaryDataStoreDriver.class);
2115+
when(dataStoreMgr.getDataStore(anyLong(), eq(DataStoreRole.Primary))).thenReturn(offeringStore);
2116+
when(offeringStore.getDriver()).thenReturn(driver);
2117+
when(driver.informStorageForDiskOfferingChange()).thenReturn(true);
2118+
2119+
try (MockedStatic<UsageEventUtils> ignored = Mockito.mockStatic(UsageEventUtils.class)) {
2120+
Mockito.clearInvocations(volumeDaoMock);
2121+
Volume result = volumeApiServiceImpl.resizeVolume(cmd);
2122+
2123+
Assert.assertSame(volumeVoMock, result);
2124+
verify(volumeDaoMock).updateDiskOffering(volumeMockId, newDiskOfferingMockId);
2125+
verify(volumeDaoMock, times(2)).findById(volumeMockId);
2126+
verify(driver).updateStorageWithTheNewDiskOffering(volumeVoMock, newDiskOfferingMock);
2127+
verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class));
2128+
}
2129+
}
2130+
20612131
private UserVmVO getMockedVm() {
20622132
UserVmVO vm = Mockito.mock(UserVmVO.class);
20632133
Mockito.when(vm.getId()).thenReturn(1L);

0 commit comments

Comments
 (0)