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:
us...@infra.apache.org


Reply via email to