Skip to content

Allow to configure root disk size via Service Offering (diskoffering of type Service). #4341

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

Merged
DaanHoogland merged 1 commit into from
Oct 30, 2020
Merged

Allow to configure root disk size via Service Offering (diskoffering of type Service). #4341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions .../main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ public class CreateServiceOfferingCmd extends BaseCmd {
@Parameter(name = ApiConstants.SERVICE_OFFERING_DETAILS, type = CommandType.MAP, description = "details for planner, used to store specific parameters")
private Map details;

@Parameter(name = ApiConstants.ROOT_DISK_SIZE, type = CommandType.LONG, since = "4.15", description = "the Root disk size in GB.")
private Long rootDiskSize;

@Parameter(name = ApiConstants.BYTES_READ_RATE, type = CommandType.LONG, required = false, description = "bytes read rate of the disk offering")
private Long bytesReadRate;

Expand Down Expand Up @@ -324,6 +327,10 @@ public Map<String, String> getDetails() {
return detailsMap;
}

public Long getRootDiskSize() {
return rootDiskSize;
}

public Long getBytesReadRate() {
return bytesReadRate;
}
Expand Down
7 changes: 7 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ public ResizeVolumeCmd(Long id, Long minIops, Long maxIops) {
this.maxIops = maxIops;
}

public ResizeVolumeCmd(Long id, Long minIops, Long maxIops, long diskOfferingId) {
this.id = id;
this.minIops = minIops;
this.maxIops = maxIops;
this.newDiskOfferingId = diskOfferingId;
}

//TODO use the method getId() instead of this one.
public Long getEntityId() {
return id;
Expand Down
7 changes: 7 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ public class ServiceOfferingResponse extends BaseResponse {
@Param(description = "the vsphere storage policy tagged to the service offering in case of VMware", since = "4.15")
private String vsphereStoragePolicy;

@SerializedName(ApiConstants.ROOT_DISK_SIZE)
@Param(description = "Root disk size in GB", since = "4.15")
private Long rootDiskSize;

public ServiceOfferingResponse() {
}

Expand Down Expand Up @@ -468,4 +472,7 @@ public void setVsphereStoragePolicy(String vsphereStoragePolicy) {
this.vsphereStoragePolicy = vsphereStoragePolicy;
}

public void setRootDiskSize(Long rootDiskSize) {
this.rootDiskSize = rootDiskSize;
}
}
2 changes: 1 addition & 1 deletion engine/schema/src/main/resources/META-INF/db/schema-41400to41500.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ CREATE VIEW `cloud`.`service_offering_view` AS
`disk_offering`.`iops_write_rate_max` AS `iops_write_rate_max`,
`disk_offering`.`iops_write_rate_max_length` AS `iops_write_rate_max_length`,
`disk_offering`.`cache_mode` AS `cache_mode`,
`disk_offering`.`disk_size` AS `root_disk_size`,
`service_offering`.`cpu` AS `cpu`,
`service_offering`.`speed` AS `speed`,
`service_offering`.`ram_size` AS `ram_size`,
Expand Down Expand Up @@ -816,7 +817,6 @@ INSERT INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_vers
INSERT INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(),'VMware', '6.7.2', 'solaris11_64Guest', 328, now(), 0);
INSERT INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(),'VMware', '6.7.3', 'solaris11_64Guest', 328, now(), 0);


-- Add VMware Photon (64 bit) as support guest os
INSERT INTO `cloud`.`guest_os` (id, uuid, category_id, display_name, created) VALUES (329, UUID(), 7, 'VMware Photon (64 bit)', now());
-- VMware Photon (64 bit) VMWare guest os mapping
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ public ServiceOfferingResponse newServiceOfferingResponse(ServiceOfferingJoinVO
offeringResponse.setObjectName("serviceoffering");
offeringResponse.setIscutomized(offering.isDynamic());
offeringResponse.setCacheMode(offering.getCacheMode());

if (offeringDetails != null && !offeringDetails.isEmpty()) {
String vsphereStoragePolicyId = offeringDetails.get(ApiConstants.STORAGE_POLICY);
if (vsphereStoragePolicyId != null) {
Expand All @@ -121,6 +122,9 @@ public ServiceOfferingResponse newServiceOfferingResponse(ServiceOfferingJoinVO
offeringResponse.setVsphereStoragePolicy(vsphereStoragePolicyVO.getName());
}
}

offeringResponse.setRootDiskSize(offering.getRootDiskSize());

return offeringResponse;
}

Expand Down
7 changes: 6 additions & 1 deletion server/src/main/java/com/cloud/api/query/vo/ServiceOfferingJoinVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ public class ServiceOfferingJoinVO extends BaseViewVO implements InternalIdentit
@Column(name = "vsphere_storage_policy")
String vsphereStoragePolicy;

@Column(name = "root_disk_size")
private Long rootDiskSize;

public ServiceOfferingJoinVO() {
}

Expand Down Expand Up @@ -363,7 +366,6 @@ public Long getIopsWriteRate() {

public Long getIopsWriteRateMaxLength() { return iopsWriteRateMaxLength; }


public boolean isDynamic() {
return cpu == null || speed == null || ramSize == null;
}
Expand Down Expand Up @@ -392,4 +394,7 @@ public String getVsphereStoragePolicy() {
return vsphereStoragePolicy;
}

public Long getRootDiskSize() {
return rootDiskSize ;
}
}
12 changes: 10 additions & 2 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati

private static final Set<Provider> VPC_ONLY_PROVIDERS = Sets.newHashSet(Provider.VPCVirtualRouter, Provider.JuniperContrailVpcRouter, Provider.InternalLbVm);

private static final long GiB_TO_BYTES = 1024 * 1024 * 1024;

@Override
public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException {
final String maxVolumeSizeInGbString = _configDao.getValue(Config.MaxVolumeSize.key());
Expand Down Expand Up @@ -2471,7 +2473,7 @@ public ServiceOffering createServiceOffering(final CreateServiceOfferingCmd cmd)

return createServiceOffering(userId, cmd.isSystem(), vmType, cmd.getServiceOfferingName(), cpuNumber, memory, cpuSpeed, cmd.getDisplayText(),
cmd.getProvisioningType(), localStorageRequired, offerHA, limitCpuUse, volatileVm, cmd.getTags(), cmd.getDomainIds(), cmd.getZoneIds(), cmd.getHostTag(),
cmd.getNetworkRate(), cmd.getDeploymentPlanner(), details, isCustomizedIops, cmd.getMinIops(), cmd.getMaxIops(),
cmd.getNetworkRate(), cmd.getDeploymentPlanner(), details, cmd.getRootDiskSize(), isCustomizedIops, cmd.getMinIops(), cmd.getMaxIops(),
cmd.getBytesReadRate(), cmd.getBytesReadRateMax(), cmd.getBytesReadRateMaxLength(),
cmd.getBytesWriteRate(), cmd.getBytesWriteRateMax(), cmd.getBytesWriteRateMaxLength(),
cmd.getIopsReadRate(), cmd.getIopsReadRateMax(), cmd.getIopsReadRateMaxLength(),
Expand All @@ -2482,7 +2484,7 @@ public ServiceOffering createServiceOffering(final CreateServiceOfferingCmd cmd)
protected ServiceOfferingVO createServiceOffering(final long userId, final boolean isSystem, final VirtualMachine.Type vmType,
final String name, final Integer cpu, final Integer ramSize, final Integer speed, final String displayText, final String provisioningType, final boolean localStorageRequired,
final boolean offerHA, final boolean limitResourceUse, final boolean volatileVm, String tags, final List<Long> domainIds, List<Long> zoneIds, final String hostTag,
final Integer networkRate, final String deploymentPlanner, final Map<String, String> details, final Boolean isCustomizedIops, Long minIops, Long maxIops,
final Integer networkRate, final String deploymentPlanner, final Map<String, String> details, Long rootDiskSizeInGiB, final Boolean isCustomizedIops, Long minIops, Long maxIops,
Long bytesReadRate, Long bytesReadRateMax, Long bytesReadRateMaxLength,
Long bytesWriteRate, Long bytesWriteRateMax, Long bytesWriteRateMaxLength,
Long iopsReadRate, Long iopsReadRateMax, Long iopsReadRateMaxLength,
Expand Down Expand Up @@ -2543,6 +2545,12 @@ protected ServiceOfferingVO createServiceOffering(final long userId, final boole
}
}

if (rootDiskSizeInGiB != null && rootDiskSizeInGiB <= 0L) {
throw new InvalidParameterValueException(String.format("The Root disk size is of %s GB but it must be greater than 0.", rootDiskSizeInGiB));
} else if (rootDiskSizeInGiB != null) {
long rootDiskSizeInBytes = rootDiskSizeInGiB * GiB_TO_BYTES;
offering.setDiskSize(rootDiskSizeInBytes);
Copy link
Member

@ravening ravening Oct 22, 2020

Choose a reason for hiding this comment

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

An optimization suggestion here. You can write below one liner since rootDiskSizeInBytes is not used anywhere else

offering.setDiskSize(rootDiskSizeInGiB * GiB_TO_BYTES)

Copy link
Member Author

@GabrielBrascher GabrielBrascher Oct 22, 2020

Choose a reason for hiding this comment

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

@ravening that does not make any difference. The JVM feeds its bytecodes to the Just-In-Time compiler (JIT) which will already take care of doing such optimization (among many others).

I did that way because of 2 reasons, one tecnical and the other a personal option: (i) in case of remote debugging connected via JVM it helps on changing a variable value in runtime; (ii) I prefer that way for code readability.

}

offering.setCustomizedIops(isCustomizedIops);
offering.setMinIops(minIops);
Expand Down
29 changes: 20 additions & 9 deletions server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
DiskOfferingVO newDiskOffering = null;

if (cmd.getNewDiskOfferingId() != null && volume.getDiskOfferingId() != cmd.getNewDiskOfferingId()) {
if (cmd.getNewDiskOfferingId() != null) {
newDiskOffering = _diskOfferingDao.findById(cmd.getNewDiskOfferingId());
}

Expand All @@ -913,6 +913,12 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep

// if we are to use the existing disk offering
if (newDiskOffering == null) {
if (volume.getVolumeType().equals(Volume.Type.ROOT) && diskOffering.getDiskSize() > 0) {
throw new InvalidParameterValueException(
"Failed to resize Root volume. The service offering of this Volume has been configured with a root disk size; "
+ "on such case a Root Volume can only be resized when changing to another Service Offering with a Root disk size. "
+ "For more details please check out the Official Resizing Volumes documentation.");
}
newSize = cmd.getSize();
newHypervisorSnapshotReserve = volume.getHypervisorSnapshotReserve();

Expand Down Expand Up @@ -958,10 +964,6 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
throw new InvalidParameterValueException("Requested disk offering has been removed.");
}

if (!DiskOfferingVO.Type.Disk.equals(newDiskOffering.getType())) {
throw new InvalidParameterValueException("Requested disk offering type is invalid.");
}

if (diskOffering.getTags() != null) {
if (!StringUtils.areTagsEqual(diskOffering.getTags(), newDiskOffering.getTags())) {
throw new InvalidParameterValueException("The tags on the new and old disk offerings must match.");
Expand All @@ -972,7 +974,9 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep

_configMgr.checkDiskOfferingAccess(_accountMgr.getActiveAccountById(volume.getAccountId()), newDiskOffering, _dcDao.findById(volume.getDataCenterId()));

if (newDiskOffering.isCustomized()) {
if (newDiskOffering.getDiskSize() > 0 && DiskOfferingVO.Type.Service.equals(newDiskOffering.getType())) {
newSize = newDiskOffering.getDiskSize();
} else if (newDiskOffering.isCustomized()) {
newSize = cmd.getSize();

if (newSize == null) {
Expand All @@ -989,9 +993,9 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
newSize = newDiskOffering.getDiskSize();
}

if (!volume.getSize().equals(newSize) && !volume.getVolumeType().equals(Volume.Type.DATADISK)) {
throw new InvalidParameterValueException("Only data volumes can be resized via a new disk offering.");
}
Long instanceId = volume.getInstanceId();
VMInstanceVO vmInstanceVO = _vmInstanceDao.findById(instanceId);
checkIfVolumeIsRootAndVmIsRunning(newSize, volume, vmInstanceVO);

if (newDiskOffering.isCustomizedIops() != null && newDiskOffering.isCustomizedIops()) {
newMinIops = cmd.getMinIops() != null ? cmd.getMinIops() : volume.getMinIops();
Expand Down Expand Up @@ -1140,6 +1144,13 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
shrinkOk);
}

private void checkIfVolumeIsRootAndVmIsRunning(Long newSize, VolumeVO volume, VMInstanceVO vmInstanceVO) {
if (!volume.getSize().equals(newSize) && volume.getVolumeType().equals(Volume.Type.ROOT) && !State.Stopped.equals(vmInstanceVO.getState())) {
Copy link
Member

@ravening ravening Oct 22, 2020

Choose a reason for hiding this comment

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

Any specific reason why we are not allowing resizing when vm is running?

Copy link
Member Author

@GabrielBrascher GabrielBrascher Oct 23, 2020

Choose a reason for hiding this comment

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

@ravening good question.

This was done to avoid users thinking that the service offering was updated correctly. Unfortunatelly, it is necessary to Stop/Start the VM, otherwise IOps updates would not take effect, for example. Considering that this is a service offering update command, then we need to take such situations into account.

Additionally, not all operation systems support dynamic memory scaling.

throw new InvalidParameterValueException(String.format("Cannot resize ROOT volume [%s] when VM is not on Stopped State. VM %s is in state %.", volume.getName(), vmInstanceVO
.getInstanceName(), vmInstanceVO.getState()));
}
}

private void validateIops(Long minIops, Long maxIops) {
if ((minIops == null && maxIops != null) || (minIops != null && maxIops == null)) {
throw new InvalidParameterValueException("Either 'miniops' and 'maxiops' must both be provided or neither must be provided.");
Expand Down
Loading