| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Sorry, something went wrong.
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2074 |
Sorry, something went wrong.
|
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Sorry, something went wrong.
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2246 |
Sorry, something went wrong.
|
Ping for review: @svenvogel @rhtyd @andrijapanicsb @abhinandanprateek @ravening @sureshanaparti @slavkap @kiwiflyer @weizhouapache @RodrigoDLopez @Pearl1594 @wido and others. |
Sorry, something went wrong.
|
Code LGTM |
Sorry, something went wrong.
| @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.") |
There was a problem hiding this comment.
Do we need since parameter here?
Sorry, something went wrong.
There was a problem hiding this comment.
Sounds like a good place to add since ;)
Thanks for the heads up @ravening!
Sorry, something went wrong.
| 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); |
There was a problem hiding this comment.
An optimization suggestion here. You can write below one liner since rootDiskSizeInBytes is not used anywhere else
offering.setDiskSize(rootDiskSizeInGiB * GiB_TO_BYTES)
Sorry, something went wrong.
There was a problem hiding this comment.
@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.
Sorry, something went wrong.
| } | ||
|
|
||
| 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())) { |
There was a problem hiding this comment.
Any specific reason why we are not allowing resizing when vm is running?
Sorry, something went wrong.
There was a problem hiding this comment.
@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.
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
|
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Sorry, something went wrong.
|
@Pearl1594 that might make sense indeed. There is also this issue: #3110. On both cases scaling works but the change command does not. |
Sorry, something went wrong.
|
Trillian test result (tid-3047)
|
Sorry, something went wrong.
|
A couple of conflicts emerged, I will sort them out later today. |
Sorry, something went wrong.
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2287 |
Sorry, something went wrong.
|
@pearl @ravening @GabrielBrascher is this good to go (after smoke tests) or should we move it to 4.16? |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
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 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Sorry, something went wrong.
|
Synced against master (again), travis is passing now. |
Sorry, something went wrong.
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2309 |
Sorry, something went wrong.
|
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Sorry, something went wrong.
|
Trillian test result (tid-3106)
|
Sorry, something went wrong.
| _itMgr.checkIfCanUpgrade(vmInstance, newServiceOffering); | ||
|
|
||
| DiskOfferingVO newROOTDiskOffering = _diskOfferingDao.findById(newServiceOffering.getId()); | ||
| // Check if the new service offering can be applied to vm instance |
There was a problem hiding this comment.
this comment could be a method name instead canServiceOfferingBeAppliedToInstance(...)
Sorry, something went wrong.
|
@PaulAngus this feature has been long waiting and meets merge requirements. leave it with the RM. |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
@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 |
Sorry, something went wrong.
|
tnx @Pearl1594 . @PaulAngus only 300 lines so I'll merge |
Sorry, something went wrong.
…of type Service). (apache#4341)
…of type Service). (apache#4341)
| Back | FazBrowse Home | New Git URL |
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?
To easily view all possible service offering resizing, the following table shows if such offering migration is supported:
How users can resize the root volume if the VM was deployed with a Root disk size configured via its Service offering?
Types of changes
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):
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