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

GabrielBrascher · 2020-09-23T21:00:04Z

Description

Service Offering allows ADMINs to set multiple parameters such as IOPS, memory, and CPU; however, we still are not able to configure the Root Disk size of a VM via service Offering.

This PR adds control over the Root disk size and therefore helps providers that want to enforce the root disk size in the same way that it is done with compute offering.

How does it work?

If the Service Offering is created without setting root disk size, all operations over root volume size are done as currently.
If a Service Offering is created with a root disk size, then there are some (intentionally) limitations under the root disk size resizing, as the following ones:

1. Users cannot deploy VMs with custom root disk size when using such offerings
2. Users cannot resize the VM root disk size when using such offerings
3. the Root Volume of such VMs can only be resized when changing to another Service Offering with a Root disk size equals or larger than the current one.
4. Users can change the VM offering to a service offering with a Root size of 0GB (default), and then customize the volume size.

To easily view all possible service offering resizing, the following table shows if such offering migration is supported:

#	Service Offering Root size	new Service Offering Root	Does support offering resize?
1	0GB (default)	Any	YES
2	5GB	5GB	YES
3	5GB	10GB	YES
4	10GB	5GB	NO
5	Any	0GB	YES

How users can resize the root volume if the VM was deployed with a Root disk size configured via its Service offering?

Users/Admins can at any time change to an offering with the default root disk size of 0; on such case, then it would be possible to resize as it is done nowadays.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
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)

Screenshots (if appropriate):

Setting root disk size for service offering via UI

How Has This Been Tested?

Tests:
Created the following service offerings (via API and also via UI):

0GB - default behavior with no Root Disk Size configured
5GB - 5GB Root
10GB - 10GB Root
15GB - 15GB Root

Executed following tests

1. Test root volume resize.
Expected result: not allowed to resize
Steps:
a - deploy a VM with any of the root size configured offerings (e.g. 5GB)
b - Change root volume size
c - not allowed

2. Test resize via service offering.
Expected result: allow 5GB -> 10GB, and 10 GB -> 15, fail to "dowsize" from 10GB -> 5GB
Steps:
a - deploy VM with offering 5GB
b - Stop VM
c - Change Service offering to 10GB
d - Change offering back to 5GB, fails (as expected)
e - Change offering to 15GB

3. Test resize via offering to a Service offering with the default root size (0 GB) and then customize the volume
Expected result: allow 5GB -> 0GB, successfully change the root volume to a custom size
Steps:
a - deploy VM with offering 5GB
b - Stop VM
c - Change Service offering to 0GB
d - resize volume to custom size, e.g 50 GB

GabrielBrascher · 2020-09-23T21:03:43Z

@blueorangutan package

blueorangutan · 2020-09-23T21:04:16Z

@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-09-23T21:36:58Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2074

DaanHoogland · 2020-10-21T09:13:07Z

@blueorangutan package

blueorangutan · 2020-10-21T09:14:04Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-10-21T10:08:27Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2246

GabrielBrascher · 2020-10-21T19:25:36Z

Ping for review: @svenvogel @rhtyd @andrijapanicsb @abhinandanprateek @ravening @sureshanaparti @slavkap @kiwiflyer @weizhouapache @RodrigoDLopez @Pearl1594 @wido and others.

Pearl1594 · 2020-10-22T07:13:05Z

Code LGTM
But noticed a behavioral difference between the Legacy UI and Primate @GabrielBrascher. Seems like Primate calls changeServiceForVirtualMachine when the VM is stopped and an attempt is made to scale - as a result of which most observations expected aren't seen i.e., on scaling up to an offering with a rootdisksize > current root disk size, though the operation is successful, there is no change in the disksize parameter of listVolumesMetrics. Also, scaling it down (to a service offering with lower rootdisksize value than the current size) works via primate for the same reason.
All mentioned cases work fine via legacy UI.

ravening · 2020-10-22T08:19:40Z

...src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java

    @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, required = false, description = "the Root disk size in GB.")


Do we need since parameter here?

Sounds like a good place to add since ;)
Thanks for the heads up @ravening!

ravening · 2020-10-22T08:23:22Z

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

+            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);


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

offering.setDiskSize(rootDiskSizeInGiB * GiB_TO_BYTES)

@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.

ravening · 2020-10-22T08:59:36Z

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

    }

+    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())) {


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

@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.

GabrielBrascher · 2020-10-22T09:53:54Z

@Pearl1594 thanks for bringing that up. I have noticed a few issues when updating disk offering details via changeServiceForVirtualMachine. I created issue #4125 regarding read/write rates.

Currently CloudStack UI uses scaleVirtualMachine and therefore we are not noticing these issues.

Pearl1594 · 2020-10-22T10:01:29Z

So @GabrielBrascher would it make sense to deprecate : changeServiceForVirtualMachine - or probably always call scaleVirtualMachine API in Primate - just as we currently do in the Legacy CloudStack UI? as scaleVirtualMachine handles it both for stopped and running VMs

DaanHoogland · 2020-10-22T10:07:07Z

@blueorangutan test

blueorangutan · 2020-10-22T10:08:04Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

GabrielBrascher · 2020-10-22T10:16:53Z

@Pearl1594 that might make sense indeed. There is also this issue: #3110. On both cases scaling works but the change command does not.

blueorangutan · 2020-10-22T21:18:27Z

Trillian test result (tid-3047)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38163 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4341-t3047-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 84 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	308.89	test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	`Failure`	304.84	test_vpc_redundant.py

GabrielBrascher · 2020-10-26T17:48:12Z

A couple of conflicts emerged, I will sort them out later today.

GabrielBrascher · 2020-10-27T03:15:40Z

@blueorangutan package

GabrielBrascher · 2020-10-28T06:00:57Z

@blueorangutan package

blueorangutan · 2020-10-28T06:01:34Z

@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-10-28T06:33:44Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2287

DaanHoogland · 2020-10-28T09:07:20Z

@pearl @ravening @GabrielBrascher is this good to go (after smoke tests) or should we move it to 4.16?

Pearl1594 · 2020-10-28T12:58:43Z

@DaanHoogland functionality wise LGTM. However, it wouldn't work entirely as expected with Primate because of the invocation of the changeServiceForVirtualMachine API for a stopped VM as opposed to scaleVirtualMachine.
A PR has been raised to address that, if that goes in - this is good to go.

Pearl1594 · 2020-10-28T12:59:14Z

@blueorangutan test

Service Offering can then have a root disk size configured. On such a case the Root Volume can only be resized when changing to another Service Offering with a Root disk size equals or larger than the current one.

GabrielBrascher · 2020-10-29T13:47:50Z

@blueorangutan package

blueorangutan · 2020-10-29T13:48:41Z

@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

GabrielBrascher · 2020-10-29T14:18:44Z

Synced against master (again), travis is passing now.

blueorangutan · 2020-10-29T14:21:39Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2309

DaanHoogland · 2020-10-29T15:13:12Z

@blueorangutan test

blueorangutan · 2020-10-29T15:14:41Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2020-10-30T00:22:38Z

Trillian test result (tid-3106)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30973 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4341-t3106-kvm-centos7.zip
Smoke tests completed. 86 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2020-10-30T07:54:10Z

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

        _itMgr.checkIfCanUpgrade(vmInstance, newServiceOffering);

-        DiskOfferingVO newROOTDiskOffering = _diskOfferingDao.findById(newServiceOffering.getId());
+        // Check if the new service offering can be applied to vm instance


this comment could be a method name instead canServiceOfferingBeAppliedToInstance(...)

DaanHoogland · 2020-10-30T07:58:16Z

@PaulAngus this feature has been long waiting and meets merge requirements. leave it with the RM.

wido · 2020-10-30T08:25:38Z

@PaulAngus this feature has been long waiting and meets merge requirements. leave it with the RM.

Yes! Especially in a public cloud (Like PCextreme's) this is very useful. Why? It's not always a good thing that end-users (API users) can specify the root disk size. This way they can create a VM with 1GB of memory and a 10TB disk. This can cause problems like small DoS attacks on hypervisors or resources going out of balance.

The provider should be able to dictate the exact ServiceOfferings and DiskOfferings users can deploy so that billing can also be done properly and clearly.

Pearl1594 · 2020-10-30T15:13:08Z

@DaanHoogland as mentioned earlier that Primate needed some changes to make it work as the legacy UI - apache/cloudstack-primate#818 addresses that concern. With that having merged, this is good to go. Thanks

DaanHoogland · 2020-10-30T15:56:00Z

tnx @Pearl1594 . @PaulAngus only 300 lines so I'll merge

…of type Service). (apache#4341)

GabrielBrascher added this to the 4.15.0.0 milestone Sep 23, 2020

GabrielBrascher self-assigned this Sep 23, 2020

svenvogel added type:enhancement type:new-feature component:primary-storage labels Sep 28, 2020

GabrielBrascher mentioned this pull request Sep 29, 2020

Add rootdisk size on compute offering apache/cloudstack-primate#770

Merged

GabrielBrascher mentioned this pull request Oct 9, 2020

Configure Root disk size via Service Offering apache/cloudstack-documentation#162

Merged

apache deleted a comment from blueorangutan Oct 21, 2020

DaanHoogland requested a review from Pearl1594 October 21, 2020 10:31

ravening reviewed Oct 22, 2020

View reviewed changes

apache deleted a comment from blueorangutan Oct 22, 2020

Allow to resize root size via diskoffering of type service.

4f6f7f0

Service Offering can then have a root disk size configured. On such a case the Root Volume can only be resized when changing to another Service Offering with a Root disk size equals or larger than the current one.

GabrielBrascher force-pushed the resize-root-service-offering branch from 3324b0c to 4f6f7f0 Compare October 29, 2020 13:06

DaanHoogland reviewed Oct 30, 2020

View reviewed changes

Pearl1594 approved these changes Oct 30, 2020

View reviewed changes

DaanHoogland merged commit b3a1cb4 into apache:master Oct 30, 2020

Pearl1594 pushed a commit to shapeblue/cloudstack that referenced this pull request Nov 4, 2020

Allow to configure root disk size via Service Offering (diskoffering …

2dbbbd0

…of type Service). (apache#4341)

Pearl1594 pushed a commit to shapeblue/cloudstack that referenced this pull request Nov 4, 2020

Allow to configure root disk size via Service Offering (diskoffering …

3945a08

…of type Service). (apache#4341)

gsirett mentioned this pull request Nov 11, 2020

[BUG] - rootdisk size not on Create Compute Offering screen apache/cloudstack-primate#859

Closed

DaanHoogland mentioned this pull request Nov 18, 2020

Command changeServiceForVirtualMachine fails to update read/write rate #4125

Closed

div8cn mentioned this pull request Mar 16, 2021

vm created by ISO cannot resize the root disk size #4827

Closed

wido mentioned this pull request Apr 7, 2021

Support vm dynamic scaling with kvm #4878

Merged

7 tasks

GabrielBrascher mentioned this pull request Jun 9, 2021

Failed to scale between Service Offerings with the same root disk size #5095

Merged

12 tasks

sureshanaparti mentioned this pull request Sep 23, 2021

Root volume resize performed on scaling service offering #3110

Closed

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

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

Uh oh!

Conversation

GabrielBrascher commented Sep 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

GabrielBrascher commented Sep 23, 2020

Uh oh!

blueorangutan commented Sep 23, 2020

Uh oh!

blueorangutan commented Sep 23, 2020

Uh oh!

DaanHoogland commented Oct 21, 2020

Uh oh!

blueorangutan commented Oct 21, 2020

Uh oh!

blueorangutan commented Oct 21, 2020

Uh oh!

GabrielBrascher commented Oct 21, 2020

Uh oh!

Pearl1594 commented Oct 22, 2020

Uh oh!

ravening Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

ravening Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

ravening Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher commented Oct 22, 2020

Uh oh!

Pearl1594 commented Oct 22, 2020

Uh oh!

DaanHoogland commented Oct 22, 2020

Uh oh!

blueorangutan commented Oct 22, 2020

Uh oh!

GabrielBrascher commented Oct 22, 2020

Uh oh!

blueorangutan commented Oct 22, 2020

Uh oh!

GabrielBrascher commented Oct 26, 2020

Uh oh!

GabrielBrascher commented Oct 27, 2020

Uh oh!

GabrielBrascher commented Oct 28, 2020

Uh oh!

blueorangutan commented Oct 28, 2020

Uh oh!

blueorangutan commented Oct 28, 2020

Uh oh!

DaanHoogland commented Oct 28, 2020

Uh oh!

Pearl1594 commented Oct 28, 2020

Uh oh!

Pearl1594 commented Oct 28, 2020

Uh oh!

GabrielBrascher commented Oct 29, 2020

Uh oh!

blueorangutan commented Oct 29, 2020

Uh oh!

GabrielBrascher commented Oct 29, 2020

Uh oh!

blueorangutan commented Oct 29, 2020

GabrielBrascher commented Sep 23, 2020 •

edited

Loading