Skip to content

Resize Volume form does not show current size and Custom IOPS #14262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
suryag1201 wants to merge 2 commits into
base: main
Choose a base branch
from
+202 −17
Open

Resize Volume form does not show current size and Custom IOPS #14262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
962393c
[CCS-5] Volume Resize Fix for IOPS and Size (#106)
suryag1201 Sep 28, 2026
c2c30e8
Resize Upstream PR Review Comment (#124)
suryag1201 Oct 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co
@Param(description = "Max IOPS of the disk volume")
private Long maxIops;

@SerializedName("iscustomizediops")
@Param(description = "True if the volume's disk offering uses custom IOPS, false otherwise")
private Boolean customizedIops;

@SerializedName(ApiConstants.CREATED)
@Param(description = "The date the disk volume was created")
private Date created;
Expand Down Expand Up @@ -410,6 +414,10 @@ public void setMaxIops(Long maxIops) {
this.maxIops = maxIops;
}

public void setCustomizedIops(Boolean customizedIops) {
this.customizedIops = customizedIops;
}

public void setCreated(Date created) {
this.created = created;
}
Expand Down Expand Up @@ -712,6 +720,14 @@ public Long getMaxIops() {
return maxIops;
}

public Boolean isCustomizedIops() {
return customizedIops;
}

public Boolean getCustomizedIops() {
return customizedIops;
}

public Date getCreated() {
return created;
}
Expand Down
6 changes: 6 additions & 0 deletions server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,12 @@ public VolumeResponse newVolumeResponse(ResponseView view, VolumeJoinVO volume)
volResponse.setDiskOfferingName(volume.getDiskOfferingName());
volResponse.setDiskOfferingDisplayText(volume.getDiskOfferingDisplayText());
}
DiskOffering diskOffering = computeOnlyDiskOffering != null
? computeOnlyDiskOffering
: ApiDBUtils.findDiskOfferingById(volume.getDiskOfferingId());
if (diskOffering != null) {
volResponse.setCustomizedIops(diskOffering.isCustomizedIops());
}

if (view == ResponseView.Full) {
volResponse.setStorageType(volume.isUseLocalStorage() ? ServiceOffering.StorageType.local.toString() : ServiceOffering.StorageType.shared.toString());
Expand Down
173 changes: 171 additions & 2 deletions server/src/test/java/com/cloud/api/query/dao/VolumeJoinDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,32 +16,70 @@
// under the License.
package com.cloud.api.query.dao;

import com.cloud.api.query.vo.VolumeJoinVO;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.when;

import java.util.function.Consumer;

import org.apache.cloudstack.annotation.dao.AnnotationDao;
import org.apache.cloudstack.api.ResponseObject.ResponseView;
import org.apache.cloudstack.api.response.VolumeResponse;
import org.junit.Assert;
import org.apache.cloudstack.context.CallContext;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mockito;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.junit.MockitoJUnitRunner;

import com.cloud.api.ApiDBUtils;
import com.cloud.api.ApiResponseHelper;
import com.cloud.api.query.vo.VolumeJoinVO;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.Volume;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;

@RunWith(MockitoJUnitRunner.class)
public class VolumeJoinDaoImplTest extends GenericDaoBaseWithTagInformationBaseTest<VolumeJoinVO, VolumeResponse> {

private static final long DISK_OFFERING_ID = 10L;

@InjectMocks
private VolumeJoinDaoImpl _volumeJoinDaoImpl;

@Mock
private AnnotationDao annotationDao;

@Mock
private AccountManager _accountMgr;

private VolumeJoinVO volume = new VolumeJoinVO();
private VolumeResponse volumeResponse = new VolumeResponse();

@Before
public void setup() {
prepareSetup();
lenient().when(annotationDao.hasAnnotations(anyString(), anyString(), anyBoolean())).thenReturn(false);
lenient().when(_accountMgr.isRootAdmin(eq(1L))).thenReturn(true);
}

@Test
public void testUpdateVolumeTagInfo(){
prepareSetup();
testUpdateTagInformation(_volumeJoinDaoImpl, volume, volumeResponse);
}

Expand All @@ -62,4 +100,135 @@ public void testSetThrottleRatesMapsReadAndWriteDistinctly() {
Assert.assertEquals(Long.valueOf(400L), response.getIopsWriteRate());
}

@Test
public void testNewVolumeResponseSetsCustomizedIopsForComputeOfferingRoot() {
VolumeJoinVO volumeJoin = mockVolumeWithDiskOffering();
DiskOfferingVO computeOnlyOffering = mock(DiskOfferingVO.class);
when(computeOnlyOffering.isCustomizedIops()).thenReturn(true);
ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
when(serviceOffering.getId()).thenReturn(5L);
when(serviceOffering.getName()).thenReturn("FullCustom_Compute");
when(serviceOffering.getDisplayText()).thenReturn("FullCustom_Compute");

VolumeResponse response = newVolumeResponse(volumeJoin, apiDBUtils -> {
apiDBUtils.when(() -> ApiDBUtils.findComputeOnlyDiskOfferingById(DISK_OFFERING_ID))
.thenReturn(computeOnlyOffering);
apiDBUtils.when(() -> ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(DISK_OFFERING_ID, false))
.thenReturn(serviceOffering);
});

assertTrue(response.isCustomizedIops());
assertEquals("5", response.getServiceOfferingId());
assertEquals("FullCustom_Compute", response.getServiceOfferingName());
assertNull(response.getDiskOfferingId());
}

@Test
public void testNewVolumeResponseSetsCustomizedIopsForDiskOffering() {
VolumeJoinVO volumeJoin = mockVolumeWithDiskOffering();
when(volumeJoin.getDiskOfferingUuid()).thenReturn("disk-offering-uuid");
when(volumeJoin.getDiskOfferingName()).thenReturn("NFS_Disk_Custom75");
when(volumeJoin.getDiskOfferingDisplayText()).thenReturn("NFS_Disk_Custom75");
DiskOfferingVO diskOffering = mock(DiskOfferingVO.class);
when(diskOffering.isCustomizedIops()).thenReturn(true);

VolumeResponse response = newVolumeResponse(volumeJoin, apiDBUtils -> {
apiDBUtils.when(() -> ApiDBUtils.findComputeOnlyDiskOfferingById(DISK_OFFERING_ID)).thenReturn(null);
apiDBUtils.when(() -> ApiDBUtils.findDiskOfferingById(DISK_OFFERING_ID)).thenReturn(diskOffering);
});

assertTrue(response.isCustomizedIops());
assertEquals("disk-offering-uuid", response.getDiskOfferingId());
assertNull(response.getServiceOfferingId());
}

@Test
public void testNewVolumeResponseSetsCustomizedIopsFalseWhenOfferingIsFixed() {
VolumeJoinVO volumeJoin = mockVolumeWithDiskOffering();
DiskOfferingVO diskOffering = mock(DiskOfferingVO.class);
when(diskOffering.isCustomizedIops()).thenReturn(false);

VolumeResponse response = newVolumeResponse(volumeJoin, apiDBUtils -> {
apiDBUtils.when(() -> ApiDBUtils.findComputeOnlyDiskOfferingById(DISK_OFFERING_ID)).thenReturn(null);
apiDBUtils.when(() -> ApiDBUtils.findDiskOfferingById(DISK_OFFERING_ID)).thenReturn(diskOffering);
});

assertFalse(response.isCustomizedIops());
}

@Test
public void testNewVolumeResponseSetsCustomizedIopsFalseForFixedComputeOffering() {
VolumeJoinVO volumeJoin = mockVolumeWithDiskOffering();
DiskOfferingVO computeOnlyOffering = mock(DiskOfferingVO.class);
when(computeOnlyOffering.isCustomizedIops()).thenReturn(false);
ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
when(serviceOffering.getId()).thenReturn(5L);
when(serviceOffering.getName()).thenReturn("Fixed_Compute");
when(serviceOffering.getDisplayText()).thenReturn("Fixed_Compute");

VolumeResponse response = newVolumeResponse(volumeJoin, apiDBUtils -> {
apiDBUtils.when(() -> ApiDBUtils.findComputeOnlyDiskOfferingById(DISK_OFFERING_ID))
.thenReturn(computeOnlyOffering);
apiDBUtils.when(() -> ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(DISK_OFFERING_ID, false))
.thenReturn(serviceOffering);
});

assertFalse(response.isCustomizedIops());
assertEquals("5", response.getServiceOfferingId());
assertEquals("Fixed_Compute", response.getServiceOfferingName());
assertNull(response.getDiskOfferingId());
}

@Test
public void testNewVolumeResponseLeavesCustomizedIopsNullWhenOfferingValueIsNull() {
VolumeJoinVO volumeJoin = mockVolumeWithDiskOffering();
when(volumeJoin.getDiskOfferingUuid()).thenReturn("disk-offering-uuid");
DiskOfferingVO diskOffering = mock(DiskOfferingVO.class);
when(diskOffering.isCustomizedIops()).thenReturn(null);

VolumeResponse response = newVolumeResponse(volumeJoin, apiDBUtils -> {
apiDBUtils.when(() -> ApiDBUtils.findComputeOnlyDiskOfferingById(DISK_OFFERING_ID)).thenReturn(null);
apiDBUtils.when(() -> ApiDBUtils.findDiskOfferingById(DISK_OFFERING_ID)).thenReturn(diskOffering);
});

assertNull(response.isCustomizedIops());
assertNull(response.getCustomizedIops());
}

private VolumeJoinVO mockVolumeWithDiskOffering() {
VolumeJoinVO volumeJoin = mock(VolumeJoinVO.class);
when(volumeJoin.getUuid()).thenReturn("volume-uuid");
when(volumeJoin.getName()).thenReturn("ROOT-67");
when(volumeJoin.getDiskOfferingId()).thenReturn(DISK_OFFERING_ID);
when(volumeJoin.getVmId()).thenReturn(0L);
when(volumeJoin.getState()).thenReturn(Volume.State.Ready);
when(volumeJoin.getVolumeType()).thenReturn(Volume.Type.ROOT);
when(volumeJoin.getTemplateId()).thenReturn(0L);
when(volumeJoin.getTagId()).thenReturn(0L);
when(volumeJoin.getMinIops()).thenReturn(0L);
when(volumeJoin.getMaxIops()).thenReturn(1000L);
when(volumeJoin.getDeleteProtection()).thenReturn(false);
when(volumeJoin.isDisplayVolume()).thenReturn(true);
when(volumeJoin.getKmsWrappedKeyId()).thenReturn(null);
when(volumeJoin.getJobId()).thenReturn(null);
return volumeJoin;
}

private VolumeResponse newVolumeResponse(VolumeJoinVO volumeJoin, Consumer<MockedStatic<ApiDBUtils>> apiStubs) {
Account callingAccount = mock(Account.class);
when(callingAccount.getId()).thenReturn(1L);
CallContext callContext = mock(CallContext.class);
when(callContext.getCallingAccount()).thenReturn(callingAccount);

try (MockedStatic<ApiDBUtils> apiDBUtils = mockStatic(ApiDBUtils.class);
MockedStatic<CallContext> callContextMocked = mockStatic(CallContext.class);
MockedStatic<ApiResponseHelper> apiResponseHelperMocked = mockStatic(ApiResponseHelper.class)) {
apiStubs.accept(apiDBUtils);
callContextMocked.when(CallContext::current).thenReturn(callContext);
apiResponseHelperMocked.when(() -> ApiResponseHelper.populateOwner(any(), any()))
.thenAnswer(invocation -> null);
return _volumeJoinDaoImpl.newVolumeResponse(ResponseView.Restricted, volumeJoin);
}
}

}
24 changes: 9 additions & 15 deletions ui/src/views/storage/ResizeVolume.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
</template>
<script>
import { ref, reactive, toRaw } from 'vue'
import { getAPI, postAPI } from '@/api'
import { postAPI } from '@/api'
import { mixinForm } from '@/utils/mixin'
import TooltipLabel from '@/components/widgets/TooltipLabel'

Expand All @@ -83,8 +83,6 @@ export default {
},
data () {
return {
offerings: [],
customDiskOffering: false,
loading: false,
customDiskOfferingIops: false
}
Expand All @@ -104,18 +102,14 @@ export default {
})
},
fetchData () {
this.loading = true
getAPI('listDiskOfferings', {
zoneid: this.resource.zoneid,
listall: true
}).then(json => {
this.offerings = json.listdiskofferingsresponse.diskoffering || []
this.form.diskofferingid = this.offerings[0].id || ''
this.customDiskOffering = this.offerings[0].iscustomized || false
this.customDiskOfferingIops = this.offerings[0].iscustomizediops || false
}).finally(() => {
this.loading = false
})
if (this.resource.size != null) {
this.form.size = this.resource.size / (1024 * 1024 * 1024)

@Damans227 Damans227 Oct 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when the volume isnt a whole number of gb, like a root disk from a 2.2 gb template? resize only takes whole numbers so hitting ok without touching the size looks like it fails

@suryag1201 suryag1201 Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Damans227 for reviewing this PR
Root and data disk sizes are entered in GB, and both parameters are LONG, so a fractional value is rejected before the VM or volume is created. For example:
deployVirtualMachine with rootdisksize=3.5 returns: Unable to execute API command deployvirtualmachine due to invalid value 3.5 for parameter rootdisksize
createVolume with size=5.6 returns: Unable to execute API command createvolume due to invalid value 5.6 for parameter size
resizeVolume.size is also a LONG in GB. A volume created through these calls therefore has a whole-GB size, and that is the value prefilled on Resize.

@Damans227 Damans227 Oct 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about a root disk deployed without a size? it takes the template's size, so a 2.2 gb template still gives a 2.2 gb disk and ok fails on it

@rajiv-jain-netapp rajiv-jain-netapp Oct 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Damans227 This is a very good observation for this particular use case. Had we represented the size fields consistently in bytes instead of displaying GB values in some places, it would have made it much easier to identify and address the issue effectively.

The current fix helps streamline the behaviour for the remaining scenarios where the size values are whole numbers rather than fractional values. However, the behaviour is still not fully correct even today. For example, if a user navigates to this screen and attempts to modify only the IOPS without providing a size value, the operation can still fail or behave unexpectedly.

What would you recommend as a more holistic solution to address this behaviour and ensure consistent handling across all size-related scenarios?

@Damans227 Damans227 Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we leave size out of the call when it wasnt changed? resize keeps the current size when none is sent, so a 2.2 gb disk and an iops only change would both work

@suryag1201 suryag1201 Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Damans227, So you are suggesting,
Leave 2.2 visible in the Size field. If the user does not change it, do not include that value in the submit. The current disk size is then left as it is, so a 2.2 GB disk and an IOPS-only change both succeed. If the user enters a new whole-GB size, send that value.

@Damans227 Damans227 Oct 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes thats it. the new commit doesnt do this yet, the form still sends the size on ok, so a 2.2 gb disk still fails. a root disk on an offering with a fixed root size fails too, even if only iops changed

}
this.customDiskOfferingIops = this.resource.iscustomizediops
if (this.customDiskOfferingIops) {
this.form.miniops = this.resource.miniops
this.form.maxiops = this.resource.maxiops
}
},
handleSubmit (e) {
if (this.loading) return
Expand Down