You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/05 09:05:46 UTC

[GitHub] [cloudstack] Pearl1594 commented on a change in pull request #4643: VM dynamic scaling option granularity

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