From 364e2d799daeeeac740aa00b8eb3343a706af896 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 7 Jun 2018 14:12:29 -0400 Subject: [PATCH] Revert "Add validation code for the Vertical Pod Autoscaler API." This reverts commit 390cfec6172fb3efdb5e13b96dcf942bc7bf2124. --- pkg/apis/autoscaling/validation/BUILD | 13 +- pkg/apis/autoscaling/validation/vpa.go | 184 --------------- pkg/apis/autoscaling/validation/vpa_test.go | 235 -------------------- 3 files changed, 2 insertions(+), 430 deletions(-) delete mode 100644 pkg/apis/autoscaling/validation/vpa.go delete mode 100644 pkg/apis/autoscaling/validation/vpa_test.go diff --git a/pkg/apis/autoscaling/validation/BUILD b/pkg/apis/autoscaling/validation/BUILD index c3352581571..308861fca80 100644 --- a/pkg/apis/autoscaling/validation/BUILD +++ b/pkg/apis/autoscaling/validation/BUILD @@ -8,17 +8,12 @@ load( go_library( name = "go_default_library", - srcs = [ - "validation.go", - "vpa.go", - ], + srcs = ["validation.go"], importpath = "k8s.io/kubernetes/pkg/apis/autoscaling/validation", deps = [ "//pkg/apis/autoscaling:go_default_library", - "//pkg/apis/core:go_default_library", "//pkg/apis/core/validation:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/validation/path:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/validation:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", ], @@ -26,10 +21,7 @@ go_library( go_test( name = "go_default_test", - srcs = [ - "validation_test.go", - "vpa_test.go", - ], + srcs = ["validation_test.go"], embed = [":go_default_library"], deps = [ "//pkg/apis/autoscaling:go_default_library", @@ -37,7 +29,6 @@ go_test( "//pkg/util/pointer:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", ], ) diff --git a/pkg/apis/autoscaling/validation/vpa.go b/pkg/apis/autoscaling/validation/vpa.go deleted file mode 100644 index e98bbd868ef..00000000000 --- a/pkg/apis/autoscaling/validation/vpa.go +++ /dev/null @@ -1,184 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package validation - -import ( - metavalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/kubernetes/pkg/apis/autoscaling" - core "k8s.io/kubernetes/pkg/apis/core" - corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" -) - -var supportedUpdateModes = sets.NewString( - string(autoscaling.UpdateModeOff), - string(autoscaling.UpdateModeInitial), - string(autoscaling.UpdateModeAuto), -) - -var supportedContainerScalingModes = sets.NewString( - string(autoscaling.ContainerScalingModeAuto), - string(autoscaling.ContainerScalingModeOff), -) - -var supportedResources = sets.NewString( - string(core.ResourceCPU), - string(core.ResourceMemory), -) - -func validateUpdateMode(updateMode *autoscaling.UpdateMode, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if updateMode != nil && !supportedUpdateModes.Has(string(*updateMode)) { - allErrs = append(allErrs, field.NotSupported(fldPath, updateMode, supportedUpdateModes.List())) - } - return allErrs -} - -func validateContainerScalingMode(containerScalingMode *autoscaling.ContainerScalingMode, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if containerScalingMode != nil && !supportedContainerScalingModes.Has(string(*containerScalingMode)) { - allErrs = append(allErrs, field.NotSupported(fldPath, containerScalingMode, supportedContainerScalingModes.List())) - } - return allErrs -} - -func validateResourceName(resourceName *core.ResourceName, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if resourceName != nil && !supportedResources.Has(string(*resourceName)) { - allErrs = append(allErrs, field.NotSupported(fldPath, resourceName, supportedResources.List())) - } - return allErrs -} - -func validatePodUpdatePolicy(podUpdatePolicy *autoscaling.PodUpdatePolicy, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if podUpdatePolicy != nil { - allErrs = append(allErrs, validateUpdateMode(podUpdatePolicy.UpdateMode, fldPath.Child("updateMode"))...) - } - return allErrs -} - -// Verifies that the core.ResourceList contains valid and supported resources (see supportedResources). -// Additionally checks that the quantity of resources in resourceList does not exceed the corresponding -// quantity in upperBound, if present. -func validateResourceList(resourceList core.ResourceList, upperBound core.ResourceList, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - for resourceName, quantity := range resourceList { - resPath := fldPath.Key(string(resourceName)) - // Validate resource name. - allErrs = append(allErrs, validateResourceName(&resourceName, resPath)...) - // Validate resource quantity. - allErrs = append(allErrs, corevalidation.ValidateResourceQuantityValue(string(resourceName), quantity, resPath)...) - if upperBound != nil { - // Check that request <= limit. - upperBoundQuantity, exists := upperBound[resourceName] - if exists && quantity.Cmp(upperBoundQuantity) > 0 { - allErrs = append(allErrs, field.Invalid(fldPath, quantity.String(), - "must be less than or equal to the upper bound")) - } - } - } - return allErrs -} - -func validateContainerResourcePolicy(containerResourcePolicy *autoscaling.ContainerResourcePolicy, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if containerResourcePolicy != nil { - allErrs = append(allErrs, validateContainerScalingMode(containerResourcePolicy.Mode, fldPath.Child("mode"))...) - allErrs = append(allErrs, validateResourceList(containerResourcePolicy.MinAllowed, containerResourcePolicy.MaxAllowed, fldPath.Child("minAllowed"))...) - allErrs = append(allErrs, validateResourceList(containerResourcePolicy.MaxAllowed, core.ResourceList{}, fldPath.Child("maxAllowed"))...) - } - return allErrs -} - -func validatePodResourcePolicy(podResourcePolicy *autoscaling.PodResourcePolicy, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if podResourcePolicy != nil { - for i, containerPolicy := range podResourcePolicy.ContainerPolicies { - allErrs = append(allErrs, validateContainerResourcePolicy(&containerPolicy, fldPath.Child("containerPolicies").Index(i))...) - } - } - return allErrs -} - -func validateVerticalPodAutoscalerSpec(spec *autoscaling.VerticalPodAutoscalerSpec, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if spec.Selector == nil { - allErrs = append(allErrs, field.Required(fldPath.Child("selector"), "")) - } else { - allErrs = append(allErrs, metavalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...) - } - allErrs = append(allErrs, validatePodUpdatePolicy(spec.UpdatePolicy, fldPath.Child("updatePolicy"))...) - allErrs = append(allErrs, validatePodResourcePolicy(spec.ResourcePolicy, fldPath.Child("resourcePolicy"))...) - return allErrs -} - -func validateRecommendedContainerResources(recommendedContainerResources *autoscaling.RecommendedContainerResources, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if recommendedContainerResources != nil { - allErrs = append(allErrs, validateResourceList(recommendedContainerResources.LowerBound, recommendedContainerResources.Target, fldPath.Child("minRecommended"))...) - allErrs = append(allErrs, validateResourceList(recommendedContainerResources.Target, recommendedContainerResources.UpperBound, fldPath.Child("target"))...) - allErrs = append(allErrs, validateResourceList(recommendedContainerResources.UpperBound, core.ResourceList{}, fldPath.Child("maxRecommended"))...) - } - return allErrs -} - -func validateRecommendedPodResources(recommendedPodResources *autoscaling.RecommendedPodResources, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if recommendedPodResources != nil { - for i, containerRecommendation := range recommendedPodResources.ContainerRecommendations { - allErrs = append(allErrs, validateRecommendedContainerResources(&containerRecommendation, fldPath.Child("containerRecommendations").Index(i))...) - } - } - return allErrs -} - -func validateVerticalPodAutoscalerStatus(status *autoscaling.VerticalPodAutoscalerStatus, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if status != nil { - allErrs = append(allErrs, validateRecommendedPodResources(status.Recommendation, fldPath.Child("recommendation"))...) - } - return allErrs -} - -// ValidateVerticalPodAutoscalerName verifies that the vertical pod autoscaler name is valid. -var ValidateVerticalPodAutoscalerName = corevalidation.ValidateReplicationControllerName - -// ValidateVerticalPodAutoscaler that VerticalPodAutoscaler is valid. -func ValidateVerticalPodAutoscaler(autoscaler *autoscaling.VerticalPodAutoscaler) field.ErrorList { - allErrs := corevalidation.ValidateObjectMeta(&autoscaler.ObjectMeta, true, ValidateVerticalPodAutoscalerName, field.NewPath("metadata")) - if autoscaler != nil { - allErrs = append(allErrs, validateVerticalPodAutoscalerSpec(&autoscaler.Spec, field.NewPath("spec"))...) - allErrs = append(allErrs, validateVerticalPodAutoscalerStatus(&autoscaler.Status, field.NewPath("status"))...) - } - return allErrs -} - -// ValidateVerticalPodAutoscalerUpdate that VerticalPodAutoscaler update is valid. -func ValidateVerticalPodAutoscalerUpdate(newAutoscaler, oldAutoscaler *autoscaling.VerticalPodAutoscaler) field.ErrorList { - allErrs := corevalidation.ValidateObjectMetaUpdate(&newAutoscaler.ObjectMeta, &oldAutoscaler.ObjectMeta, field.NewPath("metadata")) - allErrs = append(allErrs, validateVerticalPodAutoscalerSpec(&newAutoscaler.Spec, field.NewPath("spec"))...) - return allErrs -} - -// ValidateVerticalPodAutoscalerStatusUpdate that VerticalPodAutoscaler status update is valid. -func ValidateVerticalPodAutoscalerStatusUpdate(newAutoscaler, oldAutoscaler *autoscaling.VerticalPodAutoscaler) field.ErrorList { - allErrs := corevalidation.ValidateObjectMetaUpdate(&newAutoscaler.ObjectMeta, &oldAutoscaler.ObjectMeta, field.NewPath("metadata")) - allErrs = append(allErrs, validateVerticalPodAutoscalerStatus(&newAutoscaler.Status, field.NewPath("status"))...) - return allErrs -} diff --git a/pkg/apis/autoscaling/validation/vpa_test.go b/pkg/apis/autoscaling/validation/vpa_test.go deleted file mode 100644 index 48715807995..00000000000 --- a/pkg/apis/autoscaling/validation/vpa_test.go +++ /dev/null @@ -1,235 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package validation - -import ( - "strings" - "testing" - "time" - - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/kubernetes/pkg/apis/autoscaling" - core "k8s.io/kubernetes/pkg/apis/core" -) - -func expectErrorWithMessage(t *testing.T, errs field.ErrorList, expectedMsg string) { - if len(errs) == 0 { - t.Errorf("expected failure with message '%s'", expectedMsg) - } else if !strings.Contains(errs[0].Error(), expectedMsg) { - t.Errorf("unexpected error: '%v', expected: '%s'", errs[0], expectedMsg) - } -} - -func makeValidAutoscaler() *autoscaling.VerticalPodAutoscaler { - return &autoscaling.VerticalPodAutoscaler{ - ObjectMeta: metav1.ObjectMeta{Name: "my-vpa", Namespace: metav1.NamespaceDefault}, - Spec: autoscaling.VerticalPodAutoscalerSpec{ - Selector: &metav1.LabelSelector{}, - }, - } -} - -func TestValidateUpdateModeSuccess(t *testing.T) { - autoscaler := makeValidAutoscaler() - validUpdateMode := autoscaling.UpdateMode("Initial") - autoscaler.Spec.UpdatePolicy = &autoscaling.PodUpdatePolicy{UpdateMode: &validUpdateMode} - if errs := ValidateVerticalPodAutoscaler(autoscaler); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } -} - -func TestValidateUpdateModeFailure(t *testing.T) { - autoscaler := makeValidAutoscaler() - invalidUpdateMode := autoscaling.UpdateMode("SomethingElse") - autoscaler.Spec.UpdatePolicy = &autoscaling.PodUpdatePolicy{UpdateMode: &invalidUpdateMode} - expectErrorWithMessage(t, ValidateVerticalPodAutoscaler(autoscaler), "Unsupported value: \"SomethingElse\"") -} - -func TestValidateContainerScalingModeSuccess(t *testing.T) { - autoscaler := makeValidAutoscaler() - validContainerScalingMode := autoscaling.ContainerScalingMode("Off") - autoscaler.Spec.ResourcePolicy = &autoscaling.PodResourcePolicy{ - ContainerPolicies: []autoscaling.ContainerResourcePolicy{{ - ContainerName: "container1", - Mode: &validContainerScalingMode, - }}, - } - if errs := ValidateVerticalPodAutoscaler(autoscaler); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } -} - -func TestValidateContainerScalingModeFailure(t *testing.T) { - autoscaler := makeValidAutoscaler() - invalidContainerScalingMode := autoscaling.ContainerScalingMode("SomethingElse") - autoscaler.Spec.ResourcePolicy = &autoscaling.PodResourcePolicy{ - ContainerPolicies: []autoscaling.ContainerResourcePolicy{{ - ContainerName: "container1", - Mode: &invalidContainerScalingMode, - }}, - } - expectErrorWithMessage(t, ValidateVerticalPodAutoscaler(autoscaler), "Unsupported value: \"SomethingElse\"") -} - -func TestValidateResourceListSuccess(t *testing.T) { - cases := []struct { - resources core.ResourceList - upperBound core.ResourceList - }{ - // Specified CPU and memory. Upper bound not specified for any resource. - { - core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("250m"), - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - }, - core.ResourceList{}, - }, - // Specified memory only. Upper bound for memory not specified. - { - core.ResourceList{ - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - }, - core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("250m"), - }, - }, - // Specified CPU and memory. Upper bound for CPU and memory equal or greater. - { - core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("250m"), - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - }, - core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("300m"), - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - }, - }, - } - for _, c := range cases { - if errs := validateResourceList(c.resources, c.upperBound, field.NewPath("resources")); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } - } -} - -func TestValidateResourceListFailure(t *testing.T) { - cases := []struct { - resources core.ResourceList - upperBound core.ResourceList - expectedMsg string - }{ - // Invalid resource type. - { - core.ResourceList{core.ResourceName(core.ResourceStorage): resource.MustParse("10G")}, - core.ResourceList{}, - "Unsupported value: storage", - }, - // Invalid resource quantity. - { - core.ResourceList{core.ResourceName(core.ResourceCPU): resource.MustParse("-250m")}, - core.ResourceList{}, - "Invalid value: \"-250m\"", - }, - // Lower bound exceeds upper bound. - { - core.ResourceList{core.ResourceName(core.ResourceCPU): resource.MustParse("250m")}, - core.ResourceList{core.ResourceName(core.ResourceCPU): resource.MustParse("200m")}, - "must be less than or equal to the upper bound", - }, - } - for _, c := range cases { - expectErrorWithMessage(t, validateResourceList(c.resources, c.upperBound, field.NewPath("resources")), - c.expectedMsg) - } -} - -func TestMissingRequiredSelector(t *testing.T) { - autoscaler := makeValidAutoscaler() - autoscaler.Spec.Selector = nil - expectedMsg := "spec.selector: Required value" - if errs := ValidateVerticalPodAutoscaler(autoscaler); len(errs) == 0 { - t.Errorf("expected failure with message '%s'", expectedMsg) - } else if !strings.Contains(errs[0].Error(), expectedMsg) { - t.Errorf("unexpected error: '%v', expected: '%s'", errs[0], expectedMsg) - } -} - -func TestInvalidAutoscalerName(t *testing.T) { - autoscaler := makeValidAutoscaler() - autoscaler.ObjectMeta = metav1.ObjectMeta{Name: "@@@", Namespace: metav1.NamespaceDefault} - expectedMsg := "metadata.name: Invalid value: \"@@@\"" - if errs := ValidateVerticalPodAutoscaler(autoscaler); len(errs) == 0 { - t.Errorf("expected failure with message '%s'", expectedMsg) - } else if !strings.Contains(errs[0].Error(), expectedMsg) { - t.Errorf("unexpected error: '%v', expected: '%s'", errs[0], expectedMsg) - } -} - -func TestMinimalValidAutoscaler(t *testing.T) { - autoscaler := makeValidAutoscaler() - if errs := ValidateVerticalPodAutoscaler(autoscaler); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } -} - -func TestCompleteValidAutoscaler(t *testing.T) { - sampleResourceList := core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("250m"), - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - } - validUpdateMode := autoscaling.UpdateMode("Initial") - validContainerScalingMode := autoscaling.ContainerScalingMode("Auto") - autoscaler := &autoscaling.VerticalPodAutoscaler{ - ObjectMeta: metav1.ObjectMeta{Name: "my-vpa", Namespace: metav1.NamespaceDefault}, - Spec: autoscaling.VerticalPodAutoscalerSpec{ - Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, - UpdatePolicy: &autoscaling.PodUpdatePolicy{ - UpdateMode: &validUpdateMode, - }, - ResourcePolicy: &autoscaling.PodResourcePolicy{ - ContainerPolicies: []autoscaling.ContainerResourcePolicy{{ - ContainerName: "container1", - Mode: &validContainerScalingMode, - MinAllowed: sampleResourceList, - MaxAllowed: sampleResourceList, - }}, - }, - }, - Status: autoscaling.VerticalPodAutoscalerStatus{ - Recommendation: &autoscaling.RecommendedPodResources{ - ContainerRecommendations: []autoscaling.RecommendedContainerResources{{ - ContainerName: "container1", - Target: sampleResourceList, - LowerBound: sampleResourceList, - UpperBound: sampleResourceList, - }}, - }, - Conditions: []autoscaling.VerticalPodAutoscalerCondition{{ - Type: autoscaling.RecommendationProvided, - Status: core.ConditionStatus("True"), - LastTransitionTime: metav1.NewTime(time.Date(2018, time.January, 15, 0, 0, 0, 0, time.UTC)), - Reason: "Some reason", - Message: "Some message", - }}, - }, - } - if errs := ValidateVerticalPodAutoscaler(autoscaler); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } -}