Conversation
|
@blueorangutan package |
api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java
Outdated
Show resolved
Hide resolved
e49f963 to
efb6850
Compare
|
@blueorangutan package |
|
@blueorangutan test matrix |
|
These failed test cases are commonly seen on all PRs, not related to this PR changes. test_01_scale_vm on XenServer is failing with License error |
...src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
Show resolved
Hide resolved
|
|
||
| @Column(name = "dynamic_scaling_enabled") | ||
| private boolean dynamicScalingEnabled; | ||
|
|
There was a problem hiding this comment.
@harikrishna-patnala what's the difference between "dynamic_scaling_enabled" vs "is_dynamic" field?
There was a problem hiding this comment.
is_dynamic is related to custom compute offerings and thats a transient parameter.
| getCurrentTimeStampString(), | ||
| "autoScaleVm-" + asGroup.getId() + "-" + getCurrentTimeStampString(), null, null, null, HypervisorType.XenServer, HTTPMethod.GET, null, null, null, | ||
| null, true, null, null, null, null, null, null, null); | ||
| null, true, null, null, null, null, null, null, null, true); |
There was a problem hiding this comment.
@harikrishna-patnala is there a reason that we've set it to true as opposed to find the value from the service offering?
There was a problem hiding this comment.
this parameter value in deploy VM flow is the one that read from deploy VM API, where as in other cases there is no user/admin option to specify whether the VM can be dynamically scalable or not, so it is defaulted to true. All these method calls will reach checkIfDynamicScalingCanBeEnabled() where other dynamic scaling flags from template, offering and global setting are considered.
| owner, "autoScaleVm-" + asGroup.getId() + "-" + getCurrentTimeStampString(), | ||
| "autoScaleVm-" + asGroup.getId() + "-" + getCurrentTimeStampString(), null, null, null, HypervisorType.XenServer, HTTPMethod.GET, null, null, | ||
| null, null, true, null, null, null, null, null, null, null); | ||
| null, null, true, null, null, null, null, null, null, null, true); |
| vm = _userVmService.createAdvancedVirtualMachine(zone, serviceOffering, template, null, owner, "autoScaleVm-" + asGroup.getId() + "-" + | ||
| getCurrentTimeStampString(), "autoScaleVm-" + asGroup.getId() + "-" + getCurrentTimeStampString(), | ||
| null, null, null, HypervisorType.XenServer, HTTPMethod.GET, null, null, null, addrs, true, null, null, null, null, null, null, null); | ||
| null, null, null, HypervisorType.XenServer, HTTPMethod.GET, null, null, null, addrs, true, null, null, null, null, null, null, null, true); |
nvazquez
left a comment
There was a problem hiding this comment.
Nice feature, I left some comments
| } | ||
|
|
||
| public Boolean getDynamicScalingEnabled() { | ||
| return isDynamicScalingEnabled == null ? Boolean.TRUE : isDynamicScalingEnabled; |
There was a problem hiding this comment.
As the null check is in place, what about returning boolean instead of Boolean on this function? Also, this will mean that all the service offerings registered before 4.16 will now become dynamically scallable as well?
There was a problem hiding this comment.
fixed all Boolean to boolean, thanks for that.
Yes all old service offerings prior to upgrade will be marked as dynamically scalable. This is made not to break existing VMs because service offering was not part of dynamic scaling decision before. After upgrade we need to bypass that check for old VMs, which can be achieved by marking them true.
There was a problem hiding this comment.
@harikrishna-patnala and @nvazquez this method is Boolean was it meant to become boolean?
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/db/VMEntityVO.java
Outdated
Show resolved
Hide resolved
|
@sureshanaparti @Pearl1594 @nvazquez I've addressed your comments and updated the code as per the suggestions. Please review. |
|
@blueorangutan package |
|
@harikrishna-patnala a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
...src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
Show resolved
Hide resolved
…tiple checks Added the case for restore VM where VM can be restored to new template. In that case VM's dynamic scalability needs to be updated based on new template
…d as dynamically scalable.
…ce offering and template are not dynamically scalable
… be dynamically scalable
…s not enabled on template, service offering and global setting.
aebc6a0 to
d847caa
Compare
|
@blueorangutan package |
|
@harikrishna-patnala a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 110 |
|
@blueorangutan test matrix |
|
@blueorangutan test matrix |
|
@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
Trillian test result (tid-788)
|
|
LGTM |
|
@blueorangutan test centos7 vmware-67u3 |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests |
|
@blueorangutan test centos7 xenserver-71 |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + xenserver-71) has been kicked to run smoke tests |
|
Trillian Build Failed (tid-815) |
|
Trillian Build Failed (tid-814) |
|
Trillian test result (tid-858)
|
|
Trillian test result (tid-859)
|
Description
This PR introduces new granularity levels to configure VM dynamic scalability. Previously VM is configured to be dynamically scalable based on the template and global setting. Now we bringing this option to configure at service offering and VM level also.
VM can dynamically scale only when all flags are ON at VM level, template, service offering and global setting. If any of the flags is set to false then VM cannot be scalable. This result will be persisted in DB for each VM and will be honoured for that VM till it is updated.
We are introducing 'dynamicscalingallowed' parameter with permitted values of true or false for deployVM API and createServiceOffering API.
Following are the API parameter changes:
createServiceOffering API:
dynamicscalingenabled: an optional parameter of type Boolean with default value “true”.
deployVirtualMachine API:
dynamicscalingenabled: an optional parameter of type Boolean with default value “true”.
Following are the UI changes:
Service offering creation has ON/OFF switch for dynamic scaling enabled with default value true.
Deploy VM wizard has ON/OFF switch for dynamic scaling enabled with default value true in Advanced Mode section.
Added a documentation PR to support these changes
apache/cloudstack-documentation#186
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Following are the test cases that I've covered as part of my testing on VMware environment.
VM Creation:
- VM1 (scalable)
- VM2 (not scalable)
- VM3 (not scalable)
- VM4 (not scalable)
- VM5 (not scalable)
VM Update:
- VM is updated to Dynamic Scaling Enabled value false
- VM update operation fails because VM cannot be scaled with any of the template/service offering/global setting dynamic scaling enabled flag with false
VM dynamic Scaling:
While trying to scale VM from UI, it will only list service offering that are suitable to scale and also offering with "Dynamic Scaling Enabled" value same as old service offering.
- SO1
- SO2
- Successful
- Operation failed because, VM1 Service Offering is marked Dynamic Scaling Enabled but not on SO2