diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index b49466f585..6ec7057352 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -17,6 +17,7 @@ limitations under the License. package validation import ( + "encoding/json" "net" "regexp" "strconv" @@ -25,6 +26,7 @@ import ( "k8s.io/kubernetes/pkg/api" apivalidation "k8s.io/kubernetes/pkg/api/validation" "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/controller/podautoscaler" "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/util/intstr" "k8s.io/kubernetes/pkg/util/sets" @@ -83,9 +85,34 @@ func ValidateSubresourceReference(ref extensions.SubresourceReference, fldPath * return allErrs } +func validateHorizontalPodAutoscalerAnnotations(annotations map[string]string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if annotationValue, found := annotations[podautoscaler.HpaCustomMetricsTargetAnnotationName]; found { + // Try to parse the annotation + var targetList extensions.CustomMetricTargetList + if err := json.Unmarshal([]byte(annotationValue), &targetList); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("annotations"), annotations, "failed to parse custom metrics target annotation")) + } else { + if len(targetList.Items) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("annotations", "items"), "custom metrics target must not be empty")) + } + for _, target := range targetList.Items { + if target.Name == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("annotations", "items", "name"), "missing custom metric target name")) + } + if target.TargetValue.MilliValue() <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("annotations", "items", "value"), target.TargetValue, "custom metric target value must be greater than 0")) + } + } + } + } + return allErrs +} + func ValidateHorizontalPodAutoscaler(autoscaler *extensions.HorizontalPodAutoscaler) field.ErrorList { allErrs := apivalidation.ValidateObjectMeta(&autoscaler.ObjectMeta, true, ValidateHorizontalPodAutoscalerName, field.NewPath("metadata")) allErrs = append(allErrs, validateHorizontalPodAutoscalerSpec(autoscaler.Spec, field.NewPath("spec"))...) + allErrs = append(allErrs, validateHorizontalPodAutoscalerAnnotations(autoscaler.Annotations, field.NewPath("metadata"))...) return allErrs } diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index fec366b05b..cf9ed56136 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -23,6 +23,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/controller/podautoscaler" "k8s.io/kubernetes/pkg/util/intstr" ) @@ -59,6 +60,24 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) { MaxReplicas: 5, }, }, + { + ObjectMeta: api.ObjectMeta{ + Name: "myautoscaler", + Namespace: api.NamespaceDefault, + Annotations: map[string]string{ + podautoscaler.HpaCustomMetricsTargetAnnotationName: "{\"items\":[{\"name\":\"qps\",\"value\":\"20\"}]}", + }, + }, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{ + Kind: "ReplicationController", + Name: "myrc", + Subresource: "scale", + }, + MinReplicas: newInt(1), + MaxReplicas: 5, + }, + }, } for _, successCase := range successCases { if errs := ValidateHorizontalPodAutoscaler(&successCase); len(errs) != 0 { @@ -203,6 +222,90 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) { }, msg: "must be greater than 0", }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{ + Name: "myautoscaler", + Namespace: api.NamespaceDefault, + Annotations: map[string]string{ + podautoscaler.HpaCustomMetricsTargetAnnotationName: "broken", + }, + }, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{ + Kind: "ReplicationController", + Name: "myrc", + Subresource: "scale", + }, + MinReplicas: newInt(1), + MaxReplicas: 5, + }, + }, + msg: "failed to parse custom metrics target annotation", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{ + Name: "myautoscaler", + Namespace: api.NamespaceDefault, + Annotations: map[string]string{ + podautoscaler.HpaCustomMetricsTargetAnnotationName: "{}", + }, + }, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{ + Kind: "ReplicationController", + Name: "myrc", + Subresource: "scale", + }, + MinReplicas: newInt(1), + MaxReplicas: 5, + }, + }, + msg: "custom metrics target must not be empty", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{ + Name: "myautoscaler", + Namespace: api.NamespaceDefault, + Annotations: map[string]string{ + podautoscaler.HpaCustomMetricsTargetAnnotationName: "{\"items\":[{\"value\":\"20\"}]}", + }, + }, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{ + Kind: "ReplicationController", + Name: "myrc", + Subresource: "scale", + }, + MinReplicas: newInt(1), + MaxReplicas: 5, + }, + }, + msg: "missing custom metric target name", + }, + { + horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{ + ObjectMeta: api.ObjectMeta{ + Name: "myautoscaler", + Namespace: api.NamespaceDefault, + Annotations: map[string]string{ + podautoscaler.HpaCustomMetricsTargetAnnotationName: "{\"items\":[{\"name\":\"qps\",\"value\":\"0\"}]}", + }, + }, + Spec: extensions.HorizontalPodAutoscalerSpec{ + ScaleRef: extensions.SubresourceReference{ + Kind: "ReplicationController", + Name: "myrc", + Subresource: "scale", + }, + MinReplicas: newInt(1), + MaxReplicas: 5, + }, + }, + msg: "custom metric target value must be greater than 0", + }, } for _, c := range errorCases { diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go index c5f1c2cb7c..03de081d72 100644 --- a/pkg/controller/podautoscaler/horizontal.go +++ b/pkg/controller/podautoscaler/horizontal.go @@ -39,8 +39,8 @@ const ( // TODO: make it a flag or HPA spec element. tolerance = 0.1 - HpaCustomMetricsDefinitionAnnotationName = "alpha/definiton.custom-metrics.podautoscaler.kubernetes.io" - HpaCustomMetricsStatusAnnotationName = "alpha/status.custom-metrics.podautoscaler.kubernetes.io" + HpaCustomMetricsTargetAnnotationName = "alpha/target.custom-metrics.podautoscaler.kubernetes.io" + HpaCustomMetricsStatusAnnotationName = "alpha/status.custom-metrics.podautoscaler.kubernetes.io" ) type HorizontalController struct { @@ -190,7 +190,7 @@ func (a *HorizontalController) reconcileAutoscaler(hpa extensions.HorizontalPodA } } - if cmAnnotation, cmAnnotationFound := hpa.Annotations[HpaCustomMetricsDefinitionAnnotationName]; cmAnnotationFound { + if cmAnnotation, cmAnnotationFound := hpa.Annotations[HpaCustomMetricsTargetAnnotationName]; cmAnnotationFound { cmDesiredReplicas, cmStatus, cmTimestamp, err = a.computeReplicasForCustomMetrics(hpa, scale, cmAnnotation) if err != nil { a.eventRecorder.Event(&hpa, api.EventTypeWarning, "FailedComputeCMReplicas", err.Error()) diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go index 8303a0a776..de2535c81d 100644 --- a/pkg/controller/podautoscaler/horizontal_test.go +++ b/pkg/controller/podautoscaler/horizontal_test.go @@ -110,7 +110,7 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset { t.Fatalf("Failed to marshal cm: %v", err) } obj.Items[0].Annotations = make(map[string]string) - obj.Items[0].Annotations[HpaCustomMetricsDefinitionAnnotationName] = string(b) + obj.Items[0].Annotations[HpaCustomMetricsTargetAnnotationName] = string(b) } return true, obj, nil })