From 39dd65b2242339a3ecc3dc3ee26f85c617a76a5d Mon Sep 17 00:00:00 2001 From: Sanket Sudake Date: Mon, 9 May 2022 17:16:34 +0530 Subject: [PATCH] Add support for custom metrics for HPA (#2423) * Add support for custom metrics for HPA * Cleanup TargetCPUPercent references from possible places * HPA v2beta has 80% default cpu limit if not set Signed-off-by: Sanket Sudake --- crds/v1/fission.io_functions.yaml | 398 +++++++++++++++++- pkg/apis/core/v1/types.go | 23 +- pkg/apis/core/v1/validation.go | 7 +- pkg/apis/core/v1/zz_generated.deepcopy.go | 17 +- .../v1/zz_generated.swagger_doc_generated.go | 4 +- .../executortype/container/containermgr.go | 15 +- .../executortype/newdeploy/newdeploymgr.go | 15 +- pkg/executor/util/hpa/hpa.go | 9 +- pkg/fission-cli/cmd/function/create.go | 46 +- pkg/fission-cli/cmd/function/function_test.go | 27 +- pkg/fission-cli/cmd/function/list.go | 5 +- pkg/fission-cli/cmd/spec/list.go | 5 +- .../tests/test_fn_update/test_scale_change.sh | 2 +- 13 files changed, 496 insertions(+), 77 deletions(-) diff --git a/crds/v1/fission.io_functions.yaml b/crds/v1/fission.io_functions.yaml index 97a1ab56..ec540903 100644 --- a/crds/v1/fission.io_functions.yaml +++ b/crds/v1/fission.io_functions.yaml @@ -53,8 +53,404 @@ spec: description: This is the timeout setting for executor to wait for pod specialization. type: integer TargetCPUPercent: - description: This is only for newdeploy to set up target CPU utilization of HPA. + description: 'Deprecated: use hpaMetrics instead. This is only for executor type newdeploy and container to set up target CPU utilization of HPA. Applicable for executor type newdeploy and container.' type: integer + hpaBehavior: + description: hpaBehavior is the behavior of HPA when scaling in up/down direction. Applicable for executor type newdeploy and container. + properties: + scaleDown: + description: scaleDown is scaling policy for scaling Down. If not set, the default value is to allow to scale down to minReplicas pods, with a 300 second stabilization window (i.e., the highest recommendation for the last 300sec is used). + properties: + policies: + description: policies is a list of potential scaling polices which can be used during scaling. At least one policy must be specified, otherwise the HPAScalingRules will be discarded as invalid + items: + description: HPAScalingPolicy is a single policy which must hold true for a specified past interval. + properties: + periodSeconds: + description: PeriodSeconds specifies the window of time for which the policy should hold true. PeriodSeconds must be greater than zero and less than or equal to 1800 (30 min). + format: int32 + type: integer + type: + description: Type is used to specify the scaling policy. + type: string + value: + description: Value contains the amount of change which is permitted by the policy. It must be greater than zero + format: int32 + type: integer + required: + - periodSeconds + - type + - value + type: object + type: array + selectPolicy: + description: selectPolicy is used to specify which policy should be used. If not set, the default value MaxPolicySelect is used. + type: string + stabilizationWindowSeconds: + description: 'StabilizationWindowSeconds is the number of seconds for which past recommendations should be considered while scaling up or scaling down. StabilizationWindowSeconds must be greater than or equal to zero and less than or equal to 3600 (one hour). If not set, use the default values: - For scale up: 0 (i.e. no stabilization is done). - For scale down: 300 (i.e. the stabilization window is 300 seconds long).' + format: int32 + type: integer + type: object + scaleUp: + description: 'scaleUp is scaling policy for scaling Up. If not set, the default value is the higher of: * increase no more than 4 pods per 60 seconds * double the number of pods per 60 seconds No stabilization is used.' + properties: + policies: + description: policies is a list of potential scaling polices which can be used during scaling. At least one policy must be specified, otherwise the HPAScalingRules will be discarded as invalid + items: + description: HPAScalingPolicy is a single policy which must hold true for a specified past interval. + properties: + periodSeconds: + description: PeriodSeconds specifies the window of time for which the policy should hold true. PeriodSeconds must be greater than zero and less than or equal to 1800 (30 min). + format: int32 + type: integer + type: + description: Type is used to specify the scaling policy. + type: string + value: + description: Value contains the amount of change which is permitted by the policy. It must be greater than zero + format: int32 + type: integer + required: + - periodSeconds + - type + - value + type: object + type: array + selectPolicy: + description: selectPolicy is used to specify which policy should be used. If not set, the default value MaxPolicySelect is used. + type: string + stabilizationWindowSeconds: + description: 'StabilizationWindowSeconds is the number of seconds for which past recommendations should be considered while scaling up or scaling down. StabilizationWindowSeconds must be greater than or equal to zero and less than or equal to 3600 (one hour). If not set, use the default values: - For scale up: 0 (i.e. no stabilization is done). - For scale down: 300 (i.e. the stabilization window is 300 seconds long).' + format: int32 + type: integer + type: object + type: object + hpaMetrics: + description: hpaMetrics is the list of metrics used to determine the desired replica count of the Deployment created for the function. Applicable for executor type newdeploy and container. + items: + description: MetricSpec specifies how to scale based on a single metric (only `type` and one other matching field should be set at once). + properties: + containerResource: + description: container resource refers to a resource metric (such as those specified in requests and limits) known to Kubernetes describing a single container in each pod of the current scale target (e.g. CPU or memory). Such metrics are built in to Kubernetes, and have special scaling options on top of those available to normal per-pod metrics using the "pods" source. This is an alpha feature and can be enabled by the HPAContainerMetrics feature flag. + properties: + container: + description: container is the name of the container in the pods of the scaling target + type: string + name: + description: name is the name of the resource in question. + type: string + target: + description: target specifies the target value for the given metric + properties: + averageUtilization: + description: averageUtilization is the target value of the average of the resource metric across all relevant pods, represented as a percentage of the requested value of the resource for the pods. Currently only valid for Resource metric source type + format: int32 + type: integer + averageValue: + anyOf: + - type: integer + - type: string + description: averageValue is the target value of the average of the metric across all relevant pods (as a quantity) + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: + description: type represents whether the metric type is Utilization, Value, or AverageValue + type: string + value: + anyOf: + - type: integer + - type: string + description: value is the target value of the metric (as a quantity). + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + required: + - type + type: object + required: + - container + - name + - target + type: object + external: + description: external refers to a global metric that is not associated with any Kubernetes object. It allows autoscaling based on information coming from components running outside of cluster (for example length of queue in cloud messaging service, or QPS from loadbalancer running outside of cluster). + properties: + metric: + description: metric identifies the target metric by name and selector + properties: + name: + description: name is the name of the given metric + type: string + selector: + description: selector is the string-encoded form of a standard kubernetes label selector for the given metric When set, it is passed as an additional parameter to the metrics server for more specific metrics scoping. When unset, just the metricName will be used to gather metrics. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. + properties: + key: + description: key is the label key that the selector applies to. + type: string + operator: + description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + required: + - name + type: object + target: + description: target specifies the target value for the given metric + properties: + averageUtilization: + description: averageUtilization is the target value of the average of the resource metric across all relevant pods, represented as a percentage of the requested value of the resource for the pods. Currently only valid for Resource metric source type + format: int32 + type: integer + averageValue: + anyOf: + - type: integer + - type: string + description: averageValue is the target value of the average of the metric across all relevant pods (as a quantity) + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: + description: type represents whether the metric type is Utilization, Value, or AverageValue + type: string + value: + anyOf: + - type: integer + - type: string + description: value is the target value of the metric (as a quantity). + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + required: + - type + type: object + required: + - metric + - target + type: object + object: + description: object refers to a metric describing a single kubernetes object (for example, hits-per-second on an Ingress object). + properties: + describedObject: + description: CrossVersionObjectReference contains enough information to let you identify the referred resource. + properties: + apiVersion: + description: API version of the referent + type: string + kind: + description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"' + type: string + name: + description: 'Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names' + type: string + required: + - kind + - name + type: object + metric: + description: metric identifies the target metric by name and selector + properties: + name: + description: name is the name of the given metric + type: string + selector: + description: selector is the string-encoded form of a standard kubernetes label selector for the given metric When set, it is passed as an additional parameter to the metrics server for more specific metrics scoping. When unset, just the metricName will be used to gather metrics. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. + properties: + key: + description: key is the label key that the selector applies to. + type: string + operator: + description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + required: + - name + type: object + target: + description: target specifies the target value for the given metric + properties: + averageUtilization: + description: averageUtilization is the target value of the average of the resource metric across all relevant pods, represented as a percentage of the requested value of the resource for the pods. Currently only valid for Resource metric source type + format: int32 + type: integer + averageValue: + anyOf: + - type: integer + - type: string + description: averageValue is the target value of the average of the metric across all relevant pods (as a quantity) + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: + description: type represents whether the metric type is Utilization, Value, or AverageValue + type: string + value: + anyOf: + - type: integer + - type: string + description: value is the target value of the metric (as a quantity). + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + required: + - type + type: object + required: + - describedObject + - metric + - target + type: object + pods: + description: pods refers to a metric describing each pod in the current scale target (for example, transactions-processed-per-second). The values will be averaged together before being compared to the target value. + properties: + metric: + description: metric identifies the target metric by name and selector + properties: + name: + description: name is the name of the given metric + type: string + selector: + description: selector is the string-encoded form of a standard kubernetes label selector for the given metric When set, it is passed as an additional parameter to the metrics server for more specific metrics scoping. When unset, just the metricName will be used to gather metrics. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. + properties: + key: + description: key is the label key that the selector applies to. + type: string + operator: + description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + required: + - name + type: object + target: + description: target specifies the target value for the given metric + properties: + averageUtilization: + description: averageUtilization is the target value of the average of the resource metric across all relevant pods, represented as a percentage of the requested value of the resource for the pods. Currently only valid for Resource metric source type + format: int32 + type: integer + averageValue: + anyOf: + - type: integer + - type: string + description: averageValue is the target value of the average of the metric across all relevant pods (as a quantity) + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: + description: type represents whether the metric type is Utilization, Value, or AverageValue + type: string + value: + anyOf: + - type: integer + - type: string + description: value is the target value of the metric (as a quantity). + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + required: + - type + type: object + required: + - metric + - target + type: object + resource: + description: resource refers to a resource metric (such as those specified in requests and limits) known to Kubernetes describing each pod in the current scale target (e.g. CPU or memory). Such metrics are built in to Kubernetes, and have special scaling options on top of those available to normal per-pod metrics using the "pods" source. + properties: + name: + description: name is the name of the resource in question. + type: string + target: + description: target specifies the target value for the given metric + properties: + averageUtilization: + description: averageUtilization is the target value of the average of the resource metric across all relevant pods, represented as a percentage of the requested value of the resource for the pods. Currently only valid for Resource metric source type + format: int32 + type: integer + averageValue: + anyOf: + - type: integer + - type: string + description: averageValue is the target value of the average of the metric across all relevant pods (as a quantity) + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: + description: type represents whether the metric type is Utilization, Value, or AverageValue + type: string + value: + anyOf: + - type: integer + - type: string + description: value is the target value of the metric (as a quantity). + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + required: + - type + type: object + required: + - name + - target + type: object + type: + description: 'type is the type of metric source. It should be one of "ContainerResource", "External", "Object", "Pods" or "Resource", each mapping to a matching field in the object. Note: "ContainerResource" type is available on when the feature-gate HPAContainerMetrics is enabled' + type: string + required: + - type + type: object + type: array type: object StrategyType: description: StrategyType is the strategy type of function. Now it only supports 'execution'. diff --git a/pkg/apis/core/v1/types.go b/pkg/apis/core/v1/types.go index 99fdbe7e..1f01b898 100644 --- a/pkg/apis/core/v1/types.go +++ b/pkg/apis/core/v1/types.go @@ -17,6 +17,7 @@ limitations under the License. package v1 import ( + asv2beta2 "k8s.io/api/autoscaling/v2beta2" apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -459,22 +460,36 @@ type ( // +optional ExecutorType ExecutorType `json:"ExecutorType"` - // +optional // This is only for newdeploy to set up minimum replicas of deployment. + // +optional MinScale int `json:"MinScale"` - // +optional // This is only for newdeploy to set up maximum replicas of deployment. + // +optional MaxScale int `json:"MaxScale"` + // Deprecated: use hpaMetrics instead. + // This is only for executor type newdeploy and container to set up target CPU utilization of HPA. + // Applicable for executor type newdeploy and container. // +optional - // This is only for newdeploy to set up target CPU utilization of HPA. TargetCPUPercent int `json:"TargetCPUPercent"` - // +optional // This is the timeout setting for executor to wait for pod specialization. + // +optional SpecializationTimeout int `json:"SpecializationTimeout"` + + // hpaMetrics is the list of metrics used to determine the desired replica count of the Deployment + // created for the function. + // Applicable for executor type newdeploy and container. + // +optional + Metrics []asv2beta2.MetricSpec `json:"hpaMetrics,omitempty"` + + // hpaBehavior is the behavior of HPA when scaling in up/down direction. + // Applicable for executor type newdeploy and container. + // +optional + Behavior *asv2beta2.HorizontalPodAutoscalerBehavior `json:"hpaBehavior,omitempty"` } + // FunctionReferenceType refers to type of Function FunctionReferenceType string diff --git a/pkg/apis/core/v1/validation.go b/pkg/apis/core/v1/validation.go index 6de27292..6798f214 100644 --- a/pkg/apis/core/v1/validation.go +++ b/pkg/apis/core/v1/validation.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "net/http" + "reflect" "regexp" "strings" @@ -270,7 +271,7 @@ func (spec FunctionSpec) Validate() error { result = multierror.Append(result, c.Validate()) } - if spec.InvokeStrategy != (InvokeStrategy{}) { + if !reflect.DeepEqual(spec.InvokeStrategy, InvokeStrategy{}) { result = multierror.Append(result, spec.InvokeStrategy.Validate()) } @@ -309,7 +310,7 @@ func (es ExecutionStrategy) Validate() error { result = multierror.Append(result, MakeValidationErr(ErrorUnsupportedType, "ExecutionStrategy.ExecutorType", es.ExecutorType, "not a valid executor type")) } - if es.ExecutorType == ExecutorTypeNewdeploy { + if es.ExecutorType == ExecutorTypeNewdeploy || es.ExecutorType == ExecutorTypeContainer { if es.MinScale < 0 { result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "ExecutionStrategy.MinScale", es.MinScale, "minimum scale must be greater than or equal to 0")) } @@ -322,7 +323,7 @@ func (es ExecutionStrategy) Validate() error { result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "ExecutionStrategy.MaxScale", es.MaxScale, "maximum scale must be greater than or equal to minimum scale")) } - if es.TargetCPUPercent <= 0 || es.TargetCPUPercent > 100 { + if es.TargetCPUPercent < 0 || es.TargetCPUPercent > 100 { result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "ExecutionStrategy.TargetCPUPercent", es.TargetCPUPercent, "TargetCPUPercent must be a value between 1 - 100")) } diff --git a/pkg/apis/core/v1/zz_generated.deepcopy.go b/pkg/apis/core/v1/zz_generated.deepcopy.go index fe03e18e..7651a349 100644 --- a/pkg/apis/core/v1/zz_generated.deepcopy.go +++ b/pkg/apis/core/v1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ limitations under the License. package v1 import ( + v2beta2 "k8s.io/api/autoscaling/v2beta2" corev1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -313,6 +314,18 @@ func (in *EnvironmentSpec) DeepCopy() *EnvironmentSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExecutionStrategy) DeepCopyInto(out *ExecutionStrategy) { *out = *in + if in.Metrics != nil { + in, out := &in.Metrics, &out.Metrics + *out = make([]v2beta2.MetricSpec, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Behavior != nil { + in, out := &in.Behavior, &out.Behavior + *out = new(v2beta2.HorizontalPodAutoscalerBehavior) + (*in).DeepCopyInto(*out) + } return } @@ -442,7 +455,7 @@ func (in *FunctionSpec) DeepCopyInto(out *FunctionSpec) { copy(*out, *in) } in.Resources.DeepCopyInto(&out.Resources) - out.InvokeStrategy = in.InvokeStrategy + in.InvokeStrategy.DeepCopyInto(&out.InvokeStrategy) if in.IdleTimeout != nil { in, out := &in.IdleTimeout, &out.IdleTimeout *out = new(int) @@ -580,7 +593,7 @@ func (in *IngressConfig) DeepCopy() *IngressConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InvokeStrategy) DeepCopyInto(out *InvokeStrategy) { *out = *in - out.ExecutionStrategy = in.ExecutionStrategy + in.ExecutionStrategy.DeepCopyInto(&out.ExecutionStrategy) return } diff --git a/pkg/apis/core/v1/zz_generated.swagger_doc_generated.go b/pkg/apis/core/v1/zz_generated.swagger_doc_generated.go index 7fcc4ad4..cdb0fc9d 100644 --- a/pkg/apis/core/v1/zz_generated.swagger_doc_generated.go +++ b/pkg/apis/core/v1/zz_generated.swagger_doc_generated.go @@ -143,8 +143,10 @@ var map_ExecutionStrategy = map[string]string{ "ExecutorType": "ExecutorType is the executor type of function used. Defaults to \"poolmgr\".\n\nAvailable value:\n - poolmgr\n - newdeploy\n - container", "MinScale": "This is only for newdeploy to set up minimum replicas of deployment.", "MaxScale": "This is only for newdeploy to set up maximum replicas of deployment.", - "TargetCPUPercent": "This is only for newdeploy to set up target CPU utilization of HPA.", + "TargetCPUPercent": "Deprecated: use hpaMetrics instead. This is only for executor type newdeploy and container to set up target CPU utilization of HPA. Applicable for executor type newdeploy and container.", "SpecializationTimeout": "This is the timeout setting for executor to wait for pod specialization.", + "hpaMetrics": "hpaMetrics is the list of metrics used to determine the desired replica count of the Deployment created for the function. Applicable for executor type newdeploy and container.", + "hpaBehavior": "hpaBehavior is the behavior of HPA when scaling in up/down direction. Applicable for executor type newdeploy and container.", } func (ExecutionStrategy) SwaggerDoc() map[string]string { diff --git a/pkg/executor/executortype/container/containermgr.go b/pkg/executor/executortype/container/containermgr.go index 58cac49c..bd0e7d89 100644 --- a/pkg/executor/executortype/container/containermgr.go +++ b/pkg/executor/executortype/container/containermgr.go @@ -29,7 +29,6 @@ import ( multierror "github.com/hashicorp/go-multierror" "github.com/pkg/errors" "go.uber.org/zap" - asv2beta2 "k8s.io/api/autoscaling/v2beta2" apiv1 "k8s.io/api/core/v1" k8sErrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -490,8 +489,7 @@ func (caaf *Container) updateFunction(ctx context.Context, oldFn *fv1.Function, return err } - if oldFn.Spec.InvokeStrategy != newFn.Spec.InvokeStrategy { - + if !reflect.DeepEqual(oldFn.Spec.InvokeStrategy, newFn.Spec.InvokeStrategy) { // to support backward compatibility, if the function was created in default ns, we fall back to creating the // deployment of the function in fission-function ns, so cleaning up resources there ns := caaf.namespace @@ -524,10 +522,13 @@ func (caaf *Container) updateFunction(ctx context.Context, oldFn *fv1.Function, hpaChanged = true } - if newFn.Spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent != oldFn.Spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent { - targetCpupercent := int32(newFn.Spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent) - hpaMetric := hpautils.ConvertTargetCPUToCustomMetric(targetCpupercent) - hpa.Spec.Metrics = []asv2beta2.MetricSpec{hpaMetric} + if !reflect.DeepEqual(newFn.Spec.InvokeStrategy.ExecutionStrategy.Metrics, oldFn.Spec.InvokeStrategy.ExecutionStrategy.Metrics) { + hpa.Spec.Metrics = newFn.Spec.InvokeStrategy.ExecutionStrategy.Metrics + hpaChanged = true + } + + if !reflect.DeepEqual(newFn.Spec.InvokeStrategy.ExecutionStrategy.Behavior, oldFn.Spec.InvokeStrategy.ExecutionStrategy.Behavior) { + hpa.Spec.Behavior = newFn.Spec.InvokeStrategy.ExecutionStrategy.Behavior hpaChanged = true } diff --git a/pkg/executor/executortype/newdeploy/newdeploymgr.go b/pkg/executor/executortype/newdeploy/newdeploymgr.go index fe74b892..349fe1c1 100644 --- a/pkg/executor/executortype/newdeploy/newdeploymgr.go +++ b/pkg/executor/executortype/newdeploy/newdeploymgr.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "os" + "reflect" "strconv" "strings" "sync" @@ -29,7 +30,6 @@ import ( "github.com/pkg/errors" "go.uber.org/zap" autoscalingv1 "k8s.io/api/autoscaling/v1" - asv2beta2 "k8s.io/api/autoscaling/v2beta2" apiv1 "k8s.io/api/core/v1" k8sErrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -532,7 +532,7 @@ func (deploy *NewDeploy) updateFunction(ctx context.Context, oldFn *fv1.Function deployChanged := false - if oldFn.Spec.InvokeStrategy != newFn.Spec.InvokeStrategy { + if !reflect.DeepEqual(oldFn.Spec.InvokeStrategy, newFn.Spec.InvokeStrategy) { // to support backward compatibility, if the function was created in default ns, we fall back to creating the // deployment of the function in fission-function ns, so cleaning up resources there @@ -566,10 +566,13 @@ func (deploy *NewDeploy) updateFunction(ctx context.Context, oldFn *fv1.Function hpaChanged = true } - if newFn.Spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent != oldFn.Spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent { - targetCpupercent := int32(newFn.Spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent) - hpaMetric := hpautils.ConvertTargetCPUToCustomMetric(targetCpupercent) - hpa.Spec.Metrics = []asv2beta2.MetricSpec{hpaMetric} + if !reflect.DeepEqual(newFn.Spec.InvokeStrategy.ExecutionStrategy.Metrics, oldFn.Spec.InvokeStrategy.ExecutionStrategy.Metrics) { + hpa.Spec.Metrics = newFn.Spec.InvokeStrategy.ExecutionStrategy.Metrics + hpaChanged = true + } + + if !reflect.DeepEqual(newFn.Spec.InvokeStrategy.ExecutionStrategy.Behavior, oldFn.Spec.InvokeStrategy.ExecutionStrategy.Behavior) { + hpa.Spec.Behavior = newFn.Spec.InvokeStrategy.ExecutionStrategy.Behavior hpaChanged = true } diff --git a/pkg/executor/util/hpa/hpa.go b/pkg/executor/util/hpa/hpa.go index 6729f3ac..67f6e1e7 100644 --- a/pkg/executor/util/hpa/hpa.go +++ b/pkg/executor/util/hpa/hpa.go @@ -88,12 +88,16 @@ func (hpaops *HpaOperations) CreateOrGetHpa(ctx context.Context, hpaName string, if maxRepl == 0 { maxRepl = minRepl } - targetCPU := int32(execStrategy.TargetCPUPercent) + targetCPU := int32(execStrategy.TargetCPUPercent) // nolint: staticcheck var hpaMetrics []asv2beta2.MetricSpec - if targetCPU > 0 { + if targetCPU > 0 && targetCPU < 100 { hpaMetrics = append(hpaMetrics, ConvertTargetCPUToCustomMetric(targetCPU)) } + if execStrategy.Metrics != nil { + hpaMetrics = append(hpaMetrics, execStrategy.Metrics...) + } + hpa := &asv2beta2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ Name: hpaName, @@ -105,6 +109,7 @@ func (hpaops *HpaOperations) CreateOrGetHpa(ctx context.Context, hpaName string, MinReplicas: &minRepl, MaxReplicas: maxRepl, Metrics: hpaMetrics, + Behavior: execStrategy.Behavior, }, } diff --git a/pkg/fission-cli/cmd/function/create.go b/pkg/fission-cli/cmd/function/create.go index 0ba21799..430198a8 100644 --- a/pkg/fission-cli/cmd/function/create.go +++ b/pkg/fission-cli/cmd/function/create.go @@ -21,12 +21,14 @@ import ( "github.com/pkg/errors" uuid "github.com/satori/go.uuid" + asv2beta2 "k8s.io/api/autoscaling/v2beta2" apiv1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fv1 "github.com/fission/fission/pkg/apis/core/v1" ferror "github.com/fission/fission/pkg/error" + "github.com/fission/fission/pkg/executor/util/hpa" "github.com/fission/fission/pkg/fission-cli/cliwrapper/cli" "github.com/fission/fission/pkg/fission-cli/cmd" "github.com/fission/fission/pkg/fission-cli/cmd/httptrigger" @@ -38,9 +40,8 @@ import ( ) const ( - DEFAULT_MIN_SCALE = 1 - DEFAULT_TARGET_CPU_PERCENTAGE = 80 - DEFAULT_CONCURRENCY = 500 + DEFAULT_MIN_SCALE = 1 + DEFAULT_CONCURRENCY = 500 ) type CreateSubCommand struct { @@ -463,13 +464,6 @@ func getExecutionStrategy(fnExecutor fv1.ExecutorType, input cli.Input) (strateg SpecializationTimeout: specializationTimeout, } } else { - targetCPU := DEFAULT_TARGET_CPU_PERCENTAGE - if input.IsSet(flagkey.RuntimeTargetcpu) { - targetCPU, err = getTargetCPU(input) - if err != nil { - return nil, err - } - } minScale := DEFAULT_MIN_SCALE if input.IsSet(flagkey.ReplicasMinscale) { @@ -494,9 +488,16 @@ func getExecutionStrategy(fnExecutor fv1.ExecutorType, input cli.Input) (strateg ExecutorType: fnExecutor, MinScale: minScale, MaxScale: maxScale, - TargetCPUPercent: targetCPU, SpecializationTimeout: specializationTimeout, } + + if input.IsSet(flagkey.RuntimeTargetcpu) { + targetCPU, err := getTargetCPU(input) + if err != nil { + return nil, err + } + strategy.Metrics = []asv2beta2.MetricSpec{hpa.ConvertTargetCPUToCustomMetric(int32(targetCPU))} + } } return strategy, nil @@ -547,27 +548,14 @@ func updateExecutionStrategy(input cli.Input, existingExecutionStrategy *fv1.Exe SpecializationTimeout: specializationTimeout, } } else { - targetCPU := existingExecutionStrategy.TargetCPUPercent minScale := existingExecutionStrategy.MinScale maxScale := existingExecutionStrategy.MaxScale if fnExecutor != oldExecutor { // from poolmanager to newdeploy - targetCPU = DEFAULT_TARGET_CPU_PERCENTAGE minScale = DEFAULT_MIN_SCALE maxScale = minScale } - if input.IsSet(flagkey.RuntimeTargetcpu) { - targetCPU, err = getTargetCPU(input) - if err != nil { - return nil, err - } - } else { - if targetCPU <= 0 || targetCPU > 100 { - targetCPU = DEFAULT_TARGET_CPU_PERCENTAGE - } - } - if input.IsSet(flagkey.ReplicasMinscale) { minScale = input.Int(flagkey.ReplicasMinscale) } @@ -593,9 +581,17 @@ func updateExecutionStrategy(input cli.Input, existingExecutionStrategy *fv1.Exe ExecutorType: fnExecutor, MinScale: minScale, MaxScale: maxScale, - TargetCPUPercent: targetCPU, SpecializationTimeout: specializationTimeout, } + + if input.IsSet(flagkey.RuntimeTargetcpu) { + targetCPU, err := getTargetCPU(input) + if err != nil { + return nil, err + } + strategy.Metrics = []asv2beta2.MetricSpec{hpa.ConvertTargetCPUToCustomMetric(int32(targetCPU))} + } + } return strategy, nil diff --git a/pkg/fission-cli/cmd/function/function_test.go b/pkg/fission-cli/cmd/function/function_test.go index c00cef75..ee2cb2eb 100644 --- a/pkg/fission-cli/cmd/function/function_test.go +++ b/pkg/fission-cli/cmd/function/function_test.go @@ -21,8 +21,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + asv2beta2 "k8s.io/api/autoscaling/v2beta2" fv1 "github.com/fission/fission/pkg/apis/core/v1" + "github.com/fission/fission/pkg/executor/util/hpa" "github.com/fission/fission/pkg/fission-cli/cliwrapper/driver/dummy" flagkey "github.com/fission/fission/pkg/fission-cli/flag/key" ) @@ -71,7 +73,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: DEFAULT_MIN_SCALE, MaxScale: DEFAULT_MIN_SCALE, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -92,7 +93,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: DEFAULT_MIN_SCALE, MaxScale: DEFAULT_MIN_SCALE, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -107,7 +107,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: DEFAULT_MIN_SCALE, MaxScale: DEFAULT_MIN_SCALE, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -134,7 +133,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 2, MaxScale: 3, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -164,7 +162,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 5, MaxScale: 5, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -183,7 +180,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: DEFAULT_MIN_SCALE, MaxScale: 3, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -211,7 +207,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 2, MaxScale: 5, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -230,7 +225,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 2, MaxScale: 5, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -240,7 +234,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 2, MaxScale: 9, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -257,7 +250,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 2, MaxScale: 5, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -267,7 +259,6 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 2, MaxScale: 5, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -286,7 +277,7 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: DEFAULT_MIN_SCALE, MaxScale: DEFAULT_MIN_SCALE, - TargetCPUPercent: 50, + Metrics: []asv2beta2.MetricSpec{hpa.ConvertTargetCPUToCustomMetric(50)}, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -304,7 +295,7 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 2, MaxScale: 5, - TargetCPUPercent: 88, + Metrics: []asv2beta2.MetricSpec{hpa.ConvertTargetCPUToCustomMetric(88)}, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -314,7 +305,7 @@ func TestGetInvokeStrategy(t *testing.T) { ExecutorType: fv1.ExecutorTypeNewdeploy, MinScale: 2, MaxScale: 5, - TargetCPUPercent: 20, + Metrics: []asv2beta2.MetricSpec{hpa.ConvertTargetCPUToCustomMetric(20)}, SpecializationTimeout: fv1.DefaultSpecializationTimeOut, }, }, @@ -329,10 +320,9 @@ func TestGetInvokeStrategy(t *testing.T) { existingInvokeStrategy: &fv1.InvokeStrategy{ StrategyType: fv1.StrategyTypeExecution, ExecutionStrategy: fv1.ExecutionStrategy{ - ExecutorType: fv1.ExecutorTypeNewdeploy, - MinScale: 2, - MaxScale: 5, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, + ExecutorType: fv1.ExecutorTypeNewdeploy, + MinScale: 2, + MaxScale: 5, }, }, expectedResult: &fv1.InvokeStrategy{ @@ -342,7 +332,6 @@ func TestGetInvokeStrategy(t *testing.T) { MinScale: 2, MaxScale: 5, SpecializationTimeout: 200, - TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE, }, }, expectError: false, diff --git a/pkg/fission-cli/cmd/function/list.go b/pkg/fission-cli/cmd/function/list.go index b9af9483..dee6b2d6 100644 --- a/pkg/fission-cli/cmd/function/list.go +++ b/pkg/fission-cli/cmd/function/list.go @@ -47,7 +47,7 @@ func (opts *ListSubCommand) do(input cli.Input) error { w := tabwriter.NewWriter(os.Stdout, 0, 0, 1, ' ', 0) - fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n", "NAME", "ENV", "EXECUTORTYPE", "MINSCALE", "MAXSCALE", "MINCPU", "MAXCPU", "MINMEMORY", "MAXMEMORY", "TARGETCPU", "SECRETS", "CONFIGMAPS") + fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n", "NAME", "ENV", "EXECUTORTYPE", "MINSCALE", "MAXSCALE", "MINCPU", "MAXCPU", "MINMEMORY", "MAXMEMORY", "SECRETS", "CONFIGMAPS") for _, f := range fns { secrets := f.Spec.Secrets configMaps := f.Spec.ConfigMaps @@ -59,7 +59,7 @@ func (opts *ListSubCommand) do(input cli.Input) error { configMapList = append(configMapList, configMap.Name) } - fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n", + fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n", f.ObjectMeta.Name, f.Spec.Environment.Name, f.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType, f.Spec.InvokeStrategy.ExecutionStrategy.MinScale, @@ -68,7 +68,6 @@ func (opts *ListSubCommand) do(input cli.Input) error { f.Spec.Resources.Limits.Cpu().String(), f.Spec.Resources.Requests.Memory().String(), f.Spec.Resources.Limits.Memory().String(), - f.Spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent, strings.Join(secretsList, ","), strings.Join(configMapList, ",")) } diff --git a/pkg/fission-cli/cmd/spec/list.go b/pkg/fission-cli/cmd/spec/list.go index 378aac5d..14729307 100644 --- a/pkg/fission-cli/cmd/spec/list.go +++ b/pkg/fission-cli/cmd/spec/list.go @@ -218,7 +218,7 @@ func ShowFunctions(fns []fv1.Function) { if len(fns) > 0 { w := tabwriter.NewWriter(os.Stdout, 0, 0, 1, ' ', 0) fmt.Fprintf(w, "%v\n", "Functions:") - fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n", "NAME", "ENV", "EXECUTORTYPE", "MINSCALE", "MAXSCALE", "MINCPU", "MAXCPU", "MINMEMORY", "MAXMEMORY", "TARGETCPU", "SECRETS", "CONFIGMAPS") + fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n", "NAME", "ENV", "EXECUTORTYPE", "MINSCALE", "MAXSCALE", "MINCPU", "MAXCPU", "MINMEMORY", "MAXMEMORY", "SECRETS", "CONFIGMAPS") for _, f := range fns { secrets := f.Spec.Secrets @@ -231,7 +231,7 @@ func ShowFunctions(fns []fv1.Function) { configMapList = append(configMapList, configMap.Name) } - fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n", + fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n", f.ObjectMeta.Name, f.Spec.Environment.Name, f.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType, f.Spec.InvokeStrategy.ExecutionStrategy.MinScale, @@ -240,7 +240,6 @@ func ShowFunctions(fns []fv1.Function) { f.Spec.Resources.Limits.Cpu().String(), f.Spec.Resources.Requests.Memory().String(), f.Spec.Resources.Limits.Memory().String(), - f.Spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent, strings.Join(secretsList, ","), strings.Join(configMapList, ",")) } diff --git a/test/tests/test_fn_update/test_scale_change.sh b/test/tests/test_fn_update/test_scale_change.sh index 61a6f15a..85a8ec15 100755 --- a/test/tests/test_fn_update/test_scale_change.sh +++ b/test/tests/test_fn_update/test_scale_change.sh @@ -49,7 +49,7 @@ sleep 5 func=function actualMinScale=$(kubectl -n default get $func $fn -ojsonpath='{.spec.InvokeStrategy.ExecutionStrategy.MinScale}') actualMaxScale=$(kubectl -n default get $func $fn -ojsonpath='{.spec.InvokeStrategy.ExecutionStrategy.MaxScale}') -actualTargetCPU=$(kubectl -n default get $func $fn -ojsonpath='{.spec.InvokeStrategy.ExecutionStrategy.TargetCPUPercent}') +actualTargetCPU=$(kubectl -n default get $func $fn -ojsonpath='{.spec.InvokeStrategy.ExecutionStrategy.hpaMetrics[0].target.averageUtilization}') if [ "$actualMinScale" -ne "$targetMinScale" ] then