Pearl1594 commented on a change in pull request #4643:
URL: https://github.com/apache/cloudstack/pull/4643#discussion_r570806371
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
##########
@@ -223,6 +223,11 @@
@Parameter(name = ApiConstants.STORAGE_POLICY, type = CommandType.UUID,
entityType = VsphereStoragePoliciesResponse.class,required = false, description
= "Name of the storage policy defined at vCenter, this is applicable only for
VMware", since = "4.15")
private Long storagePolicy;
+ @Parameter(name = ApiConstants.DYNAMIC_SCALING_ENABLED,
+ type = CommandType.BOOLEAN,
+ description = "true if virtual machine needs to be dynamically
scalable of cpu or memory")
+ protected Boolean isDynamicScalingEnabled;
+
Review comment:
@harikrishna-patnala we probably want to add the since attribute to any
new parameters added to the APIs?
##########
File path: server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java
##########
@@ -1325,18 +1325,18 @@ private long createNewVM(AutoScaleVmGroupVO asGroup) {
vm =
_userVmService.createBasicSecurityGroupVirtualMachine(zone, serviceOffering,
template, null, 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, true, null, null, null, null, null, null, null,
true);
Review comment:
@harikrishna-patnala is there a reason that we've set it to true as
opposed to find the value from the service offering?
##########
File path: server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java
##########
@@ -1325,18 +1325,18 @@ private long createNewVM(AutoScaleVmGroupVO asGroup) {
vm =
_userVmService.createBasicSecurityGroupVirtualMachine(zone, serviceOffering,
template, null, 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, true, null, null, null, null, null, null, null,
true);
} else {
if (zone.isSecurityGroupEnabled()) {
vm =
_userVmService.createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering,
template, null, null,
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);
Review comment:
same as above
##########
File path: engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java
##########
@@ -75,6 +75,9 @@
@Column(name = "deployment_planner")
private String deploymentPlanner = null;
+ @Column(name = "dynamic_scaling_enabled")
+ private boolean dynamicScalingEnabled;
+
Review comment:
@harikrishna-patnala what's the difference between
"dynamic_scaling_enabled" vs "is_dynamic" field?
##########
File path: server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java
##########
@@ -1325,18 +1325,18 @@ private long createNewVM(AutoScaleVmGroupVO asGroup) {
vm =
_userVmService.createBasicSecurityGroupVirtualMachine(zone, serviceOffering,
template, null, 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, true, null, null, null, null, null, null, null,
true);
} else {
if (zone.isSecurityGroupEnabled()) {
vm =
_userVmService.createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering,
template, null, null,
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);
} else {
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);
Review comment:
same as above
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -3899,6 +3914,15 @@ private UserVm createVirtualMachine(DataCenter zone,
ServiceOffering serviceOffe
return vm;
}
+ private Boolean checkIfDynamicScalingCanBeEnabled(Boolean
dynamicScalingEnabled, ServiceOfferingVO offering, VMTemplateVO template, Long
zoneId) {
Review comment:
can we not define this in UserVMManager and call this method at all
other places where the check is repeated - eg. at KubernetesClusterStartWorker
and other classes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
