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 2020/10/30 09:34:26 UTC
[GitHub] [cloudstack] shwstppr commented on a change in pull request #4329: Adding AutoScaling for cks
shwstppr commented on a change in pull request #4329:
URL: https://github.com/apache/cloudstack/pull/4329#discussion_r514963160
##########
File path: plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java
##########
@@ -845,30 +865,87 @@ private void validateKubernetesClusterScaleParameters(ScaleKubernetesClusterCmd
final Long kubernetesClusterId = cmd.getId();
final Long serviceOfferingId = cmd.getServiceOfferingId();
final Long clusterSize = cmd.getClusterSize();
+ final List<Long> nodeIds = cmd.getNodeIds();
+ final Boolean isAutoscalingEnabled = cmd.isAutoscalingEnabled();
+ final Long minSize = cmd.getMinSize();
+ final Long maxSize = cmd.getMaxSize();
+
if (kubernetesClusterId == null || kubernetesClusterId < 1L) {
throw new InvalidParameterValueException("Invalid Kubernetes cluster ID");
}
+
KubernetesClusterVO kubernetesCluster = kubernetesClusterDao.findById(kubernetesClusterId);
if (kubernetesCluster == null || kubernetesCluster.getRemoved() != null) {
throw new InvalidParameterValueException("Invalid Kubernetes cluster ID");
}
+
final DataCenter zone = dataCenterDao.findById(kubernetesCluster.getZoneId());
if (zone == null) {
logAndThrow(Level.WARN, String.format("Unable to find zone for Kubernetes cluster : %s", kubernetesCluster.getName()));
}
+ if (serviceOfferingId == null && clusterSize == null && nodeIds == null && isAutoscalingEnabled == null) {
+ throw new InvalidParameterValueException(String.format("Kubernetes cluster %s cannot be scaled, either service offering or cluster size or nodeids to be removed or autoscaling must be passed", kubernetesCluster.getName()));
+ }
+
Account caller = CallContext.current().getCallingAccount();
accountManager.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, false, kubernetesCluster);
- if (serviceOfferingId == null && clusterSize == null) {
- throw new InvalidParameterValueException(String.format("Kubernetes cluster : %s cannot be scaled, either a new service offering or a new cluster size must be passed", kubernetesCluster.getName()));
- }
-
final KubernetesSupportedVersion clusterVersion = kubernetesSupportedVersionDao.findById(kubernetesCluster.getKubernetesVersionId());
if (clusterVersion == null) {
throw new CloudRuntimeException(String.format("Invalid Kubernetes version associated with Kubernetes cluster : %s", kubernetesCluster.getName()));
}
+ if (!(kubernetesCluster.getState().equals(KubernetesCluster.State.Created) ||
+ kubernetesCluster.getState().equals(KubernetesCluster.State.Running) ||
+ kubernetesCluster.getState().equals(KubernetesCluster.State.Stopped))) {
+ throw new PermissionDeniedException(String.format("Kubernetes cluster %s is in %s state and can not be scaled", kubernetesCluster.getName(), kubernetesCluster.getState().toString()));
+ }
+
+ if (isAutoscalingEnabled != null && isAutoscalingEnabled) {
+ if (clusterSize != null || serviceOfferingId != null || nodeIds != null) {
+ throw new InvalidParameterValueException("autoscaling can not be passed along with nodeids or clustersize or service offering");
+ }
+
+ if (!KubernetesVersionManagerImpl.versionSupportsAutoscaling(clusterVersion)) {
+ throw new InvalidParameterValueException(String.format("Autoscaling requires Kubernetes Version %s or above",
+ KubernetesVersionManagerImpl.MINIMUN_AUTOSCALER_SUPPORTED_VERSION ));
+ }
+
+ validateEndpointUrl();
+
+ if (minSize == null || maxSize == null) {
+ throw new InvalidParameterValueException("autoscaling requires minsize and maxsize to be passed");
+ }
+ if (minSize < 1) {
+ throw new InvalidParameterValueException("minsize must be at least than 1");
+ }
+ if (maxSize <= minSize) {
+ throw new InvalidParameterValueException("maxsize must be greater than or equal to minsize");
+ }
+ }
+
+ if (nodeIds != null) {
+ if (clusterSize != null || serviceOfferingId != null) {
+ throw new InvalidParameterValueException("nodeids can not be passed along with clustersize or service offering");
+ }
+ List<KubernetesClusterVmMapVO> nodes = kubernetesClusterVmMapDao.listByClusterIdAndVmIdsIn(kubernetesCluster.getId(), nodeIds);
+ // Do all the nodes exist ?
+ if (nodes == null || nodes.size() != nodeIds.size()) {
+ throw new InvalidParameterValueException("Invalid node ids");
+ }
+ // Ensure there's always a master
+ long mastersToRemove = nodes.stream().filter(x -> x.isMaster()).count();
+ if (mastersToRemove >= kubernetesCluster.getMasterNodeCount()) {
+ throw new InvalidParameterValueException("Can not remove all masters from a cluster");
+ }
Review comment:
Are we allowing scaling for master nodes?
##########
File path: plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/ScaleKubernetesClusterCmd.java
##########
@@ -58,19 +61,38 @@
//////////////// API parameters /////////////////////
/////////////////////////////////////////////////////
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, required = true,
- entityType = KubernetesClusterResponse.class,
- description = "the ID of the Kubernetes cluster")
+ entityType = KubernetesClusterResponse.class,
+ description = "the ID of the Kubernetes cluster")
private Long id;
@ACL(accessType = SecurityChecker.AccessType.UseEntry)
@Parameter(name = ApiConstants.SERVICE_OFFERING_ID, type = CommandType.UUID, entityType = ServiceOfferingResponse.class,
- description = "the ID of the service offering for the virtual machines in the cluster.")
+ description = "the ID of the service offering for the virtual machines in the cluster.")
private Long serviceOfferingId;
@Parameter(name=ApiConstants.SIZE, type = CommandType.LONG,
- description = "number of Kubernetes cluster nodes")
+ description = "number of Kubernetes cluster nodes")
private Long clusterSize;
+ @Parameter(name = ApiConstants.NODE_IDS,
+ type = CommandType.LIST,
+ collectionType = CommandType.UUID,
+ entityType = UserVmResponse.class,
+ description = "the IDs of the nodes to be removed")
+ private List<Long> nodeIds;
+
+ @Parameter(name=ApiConstants.AUTOSCALING_ENABLED, type = CommandType.BOOLEAN,
Review comment:
We don't support allowing autoscaling while deployment itself and it can be enabled only after the cluster is deployed?
##########
File path: plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java
##########
@@ -845,30 +865,87 @@ private void validateKubernetesClusterScaleParameters(ScaleKubernetesClusterCmd
final Long kubernetesClusterId = cmd.getId();
final Long serviceOfferingId = cmd.getServiceOfferingId();
final Long clusterSize = cmd.getClusterSize();
+ final List<Long> nodeIds = cmd.getNodeIds();
+ final Boolean isAutoscalingEnabled = cmd.isAutoscalingEnabled();
+ final Long minSize = cmd.getMinSize();
+ final Long maxSize = cmd.getMaxSize();
+
if (kubernetesClusterId == null || kubernetesClusterId < 1L) {
throw new InvalidParameterValueException("Invalid Kubernetes cluster ID");
}
+
KubernetesClusterVO kubernetesCluster = kubernetesClusterDao.findById(kubernetesClusterId);
if (kubernetesCluster == null || kubernetesCluster.getRemoved() != null) {
throw new InvalidParameterValueException("Invalid Kubernetes cluster ID");
}
+
final DataCenter zone = dataCenterDao.findById(kubernetesCluster.getZoneId());
if (zone == null) {
logAndThrow(Level.WARN, String.format("Unable to find zone for Kubernetes cluster : %s", kubernetesCluster.getName()));
}
+ if (serviceOfferingId == null && clusterSize == null && nodeIds == null && isAutoscalingEnabled == null) {
+ throw new InvalidParameterValueException(String.format("Kubernetes cluster %s cannot be scaled, either service offering or cluster size or nodeids to be removed or autoscaling must be passed", kubernetesCluster.getName()));
+ }
+
Account caller = CallContext.current().getCallingAccount();
accountManager.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, false, kubernetesCluster);
- if (serviceOfferingId == null && clusterSize == null) {
- throw new InvalidParameterValueException(String.format("Kubernetes cluster : %s cannot be scaled, either a new service offering or a new cluster size must be passed", kubernetesCluster.getName()));
- }
-
final KubernetesSupportedVersion clusterVersion = kubernetesSupportedVersionDao.findById(kubernetesCluster.getKubernetesVersionId());
if (clusterVersion == null) {
throw new CloudRuntimeException(String.format("Invalid Kubernetes version associated with Kubernetes cluster : %s", kubernetesCluster.getName()));
}
+ if (!(kubernetesCluster.getState().equals(KubernetesCluster.State.Created) ||
+ kubernetesCluster.getState().equals(KubernetesCluster.State.Running) ||
+ kubernetesCluster.getState().equals(KubernetesCluster.State.Stopped))) {
+ throw new PermissionDeniedException(String.format("Kubernetes cluster %s is in %s state and can not be scaled", kubernetesCluster.getName(), kubernetesCluster.getState().toString()));
+ }
+
+ if (isAutoscalingEnabled != null && isAutoscalingEnabled) {
+ if (clusterSize != null || serviceOfferingId != null || nodeIds != null) {
+ throw new InvalidParameterValueException("autoscaling can not be passed along with nodeids or clustersize or service offering");
+ }
+
+ if (!KubernetesVersionManagerImpl.versionSupportsAutoscaling(clusterVersion)) {
+ throw new InvalidParameterValueException(String.format("Autoscaling requires Kubernetes Version %s or above",
+ KubernetesVersionManagerImpl.MINIMUN_AUTOSCALER_SUPPORTED_VERSION ));
+ }
+
Review comment:
Minor nit, inconsistency in exception messages. At some places it is bit detailed while at some places it is single phrase starting with lowercase
##########
File path: plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java
##########
@@ -202,6 +204,10 @@ public static int compareSemanticVersions(String v1, String v2) throws IllegalAr
return 0;
}
+ public static boolean versionSupportsAutoscaling(KubernetesSupportedVersion clusterVersion) {
Review comment:
For UI, do we need to return this in KubernetesSupportedVersionResponse? Or can we deprecate older k8s version?
----------------------------------------------------------------
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