diff --git a/pkg/apis/autoscaling/BUILD b/pkg/apis/autoscaling/BUILD index 5fed7730d8a..34f9e875bad 100644 --- a/pkg/apis/autoscaling/BUILD +++ b/pkg/apis/autoscaling/BUILD @@ -10,6 +10,7 @@ go_library( srcs = [ "annotations.go", "doc.go", + "helpers.go", "register.go", "types.go", "zz_generated.deepcopy.go", diff --git a/pkg/apis/autoscaling/helpers.go b/pkg/apis/autoscaling/helpers.go new file mode 100644 index 00000000000..632c1983318 --- /dev/null +++ b/pkg/apis/autoscaling/helpers.go @@ -0,0 +1,58 @@ +/* +Copyright 2020 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 autoscaling + +// DropRoundTripHorizontalPodAutoscalerAnnotations removes any annotations used to serialize round-tripped fields from later API versions, +// and returns false if no changes were made and the original input object was returned. +// It should always be called when converting internal -> external versions, prior +// to setting any of the custom annotations: +// +// annotations, copiedAnnotations := DropRoundTripHorizontalPodAutoscalerAnnotations(externalObj.Annotations) +// externalObj.Annotations = annotations +// +// if internal.SomeField != nil { +// if !copiedAnnotations { +// externalObj.Annotations = DeepCopyStringMap(externalObj.Annotations) +// copiedAnnotations = true +// } +// externalObj.Annotations[...] = json.Marshal(...) +// } +func DropRoundTripHorizontalPodAutoscalerAnnotations(in map[string]string) (out map[string]string, copied bool) { + _, hasMetricsSpecs := in[MetricSpecsAnnotation] + _, hasBehaviorSpecs := in[BehaviorSpecsAnnotation] + _, hasMetricsStatuses := in[MetricStatusesAnnotation] + _, hasConditions := in[HorizontalPodAutoscalerConditionsAnnotation] + if hasMetricsSpecs || hasBehaviorSpecs || hasMetricsStatuses || hasConditions { + out = DeepCopyStringMap(in) + delete(out, MetricSpecsAnnotation) + delete(out, BehaviorSpecsAnnotation) + delete(out, MetricStatusesAnnotation) + delete(out, HorizontalPodAutoscalerConditionsAnnotation) + return out, true + } + return in, false +} + +// DeepCopyStringMap returns a copy of the input map. +// If input is nil, an empty map is returned. +func DeepCopyStringMap(in map[string]string) map[string]string { + out := make(map[string]string, len(in)) + for k, v := range in { + out[k] = v + } + return out +} diff --git a/pkg/apis/autoscaling/v1/BUILD b/pkg/apis/autoscaling/v1/BUILD index 0360cbfa5f5..e33396d1412 100644 --- a/pkg/apis/autoscaling/v1/BUILD +++ b/pkg/apis/autoscaling/v1/BUILD @@ -30,10 +30,13 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/api/legacyscheme:go_default_library", + "//pkg/apis/autoscaling:go_default_library", "//pkg/apis/autoscaling/install:go_default_library", "//pkg/apis/core/install:go_default_library", "//staging/src/k8s.io/api/autoscaling/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//vendor/k8s.io/utils/pointer:go_default_library", ], ) diff --git a/pkg/apis/autoscaling/v1/conversion.go b/pkg/apis/autoscaling/v1/conversion.go index 0d9f4c59382..0b07f4af2c6 100644 --- a/pkg/apis/autoscaling/v1/conversion.go +++ b/pkg/apis/autoscaling/v1/conversion.go @@ -260,6 +260,10 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v1_HorizontalPodAutoscaler(i return err } + // clear any pre-existing round-trip annotations to make sure the only ones set are ones we produced during conversion + annotations, copiedAnnotations := autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations) + out.Annotations = annotations + otherMetrics := make([]autoscalingv1.MetricSpec, 0, len(in.Spec.Metrics)) for _, metric := range in.Spec.Metrics { if metric.Type == autoscaling.ResourceMetricSourceType && metric.Resource != nil && metric.Resource.Name == core.ResourceCPU && metric.Resource.Target.AverageUtilization != nil { @@ -289,19 +293,16 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v1_HorizontalPodAutoscaler(i } } - if len(otherMetrics) > 0 || len(in.Status.CurrentMetrics) > 0 || len(currentConditions) > 0 || in.Spec.Behavior != nil { - old := out.Annotations - out.Annotations = make(map[string]string, len(old)+4) - for k, v := range old { - out.Annotations[k] = v - } - } - if len(otherMetrics) > 0 { otherMetricsEnc, err := json.Marshal(otherMetrics) if err != nil { return err } + // copy before mutating + if !copiedAnnotations { + copiedAnnotations = true + out.Annotations = autoscaling.DeepCopyStringMap(out.Annotations) + } out.Annotations[autoscaling.MetricSpecsAnnotation] = string(otherMetricsEnc) } @@ -310,14 +311,25 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v1_HorizontalPodAutoscaler(i if err != nil { return err } + // copy before mutating + if !copiedAnnotations { + copiedAnnotations = true + out.Annotations = autoscaling.DeepCopyStringMap(out.Annotations) + } out.Annotations[autoscaling.MetricStatusesAnnotation] = string(currentMetricsEnc) } if in.Spec.Behavior != nil { + // TODO: this is marshaling an internal type. Fix this without breaking backwards compatibility. behaviorEnc, err := json.Marshal(in.Spec.Behavior) if err != nil { return err } + // copy before mutating + if !copiedAnnotations { + copiedAnnotations = true + out.Annotations = autoscaling.DeepCopyStringMap(out.Annotations) + } out.Annotations[autoscaling.BehaviorSpecsAnnotation] = string(behaviorEnc) } @@ -326,6 +338,11 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v1_HorizontalPodAutoscaler(i if err != nil { return err } + // copy before mutating + if !copiedAnnotations { + copiedAnnotations = true + out.Annotations = autoscaling.DeepCopyStringMap(out.Annotations) + } out.Annotations[autoscaling.HorizontalPodAutoscalerConditionsAnnotation] = string(currentConditionsEnc) } @@ -339,47 +356,40 @@ func Convert_v1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(i if otherMetricsEnc, hasOtherMetrics := out.Annotations[autoscaling.MetricSpecsAnnotation]; hasOtherMetrics { var otherMetrics []autoscalingv1.MetricSpec - if err := json.Unmarshal([]byte(otherMetricsEnc), &otherMetrics); err != nil { - return err - } - - // the normal Spec conversion could have populated out.Spec.Metrics with a single element, so deal with that - outMetrics := make([]autoscaling.MetricSpec, len(otherMetrics)+len(out.Spec.Metrics)) - for i, metric := range otherMetrics { - if err := Convert_v1_MetricSpec_To_autoscaling_MetricSpec(&metric, &outMetrics[i], s); err != nil { - return err + if err := json.Unmarshal([]byte(otherMetricsEnc), &otherMetrics); err == nil { + // the normal Spec conversion could have populated out.Spec.Metrics with a single element, so deal with that + outMetrics := make([]autoscaling.MetricSpec, len(otherMetrics)+len(out.Spec.Metrics)) + for i, metric := range otherMetrics { + if err := Convert_v1_MetricSpec_To_autoscaling_MetricSpec(&metric, &outMetrics[i], s); err != nil { + return err + } } + if out.Spec.Metrics != nil { + outMetrics[len(otherMetrics)] = out.Spec.Metrics[0] + } + out.Spec.Metrics = outMetrics } - if out.Spec.Metrics != nil { - outMetrics[len(otherMetrics)] = out.Spec.Metrics[0] - } - out.Spec.Metrics = outMetrics - delete(out.Annotations, autoscaling.MetricSpecsAnnotation) } if behaviorEnc, hasConstraints := out.Annotations[autoscaling.BehaviorSpecsAnnotation]; hasConstraints { + // TODO: this is unmarshaling an internal type. Fix this without breaking backwards compatibility. var behavior autoscaling.HorizontalPodAutoscalerBehavior - if err := json.Unmarshal([]byte(behaviorEnc), &behavior); err != nil { - return err + if err := json.Unmarshal([]byte(behaviorEnc), &behavior); err == nil && behavior != (autoscaling.HorizontalPodAutoscalerBehavior{}) { + out.Spec.Behavior = &behavior } - out.Spec.Behavior = &behavior - delete(out.Annotations, autoscaling.BehaviorSpecsAnnotation) } if currentMetricsEnc, hasCurrentMetrics := out.Annotations[autoscaling.MetricStatusesAnnotation]; hasCurrentMetrics { // ignore any existing status values -- the ones here have more information var currentMetrics []autoscalingv1.MetricStatus - if err := json.Unmarshal([]byte(currentMetricsEnc), ¤tMetrics); err != nil { - return err - } - - out.Status.CurrentMetrics = make([]autoscaling.MetricStatus, len(currentMetrics)) - for i, currentMetric := range currentMetrics { - if err := Convert_v1_MetricStatus_To_autoscaling_MetricStatus(¤tMetric, &out.Status.CurrentMetrics[i], s); err != nil { - return err + if err := json.Unmarshal([]byte(currentMetricsEnc), ¤tMetrics); err == nil { + out.Status.CurrentMetrics = make([]autoscaling.MetricStatus, len(currentMetrics)) + for i, currentMetric := range currentMetrics { + if err := Convert_v1_MetricStatus_To_autoscaling_MetricStatus(¤tMetric, &out.Status.CurrentMetrics[i], s); err != nil { + return err + } } } - delete(out.Annotations, autoscaling.MetricStatusesAnnotation) } // autoscaling/v1 formerly had an implicit default applied in the controller. In v2beta1, we apply it explicitly. @@ -403,19 +413,19 @@ func Convert_v1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(i if currentConditionsEnc, hasCurrentConditions := out.Annotations[autoscaling.HorizontalPodAutoscalerConditionsAnnotation]; hasCurrentConditions { var currentConditions []autoscalingv1.HorizontalPodAutoscalerCondition - if err := json.Unmarshal([]byte(currentConditionsEnc), ¤tConditions); err != nil { - return err - } - - out.Status.Conditions = make([]autoscaling.HorizontalPodAutoscalerCondition, len(currentConditions)) - for i, currentCondition := range currentConditions { - if err := Convert_v1_HorizontalPodAutoscalerCondition_To_autoscaling_HorizontalPodAutoscalerCondition(¤tCondition, &out.Status.Conditions[i], s); err != nil { - return err + if err := json.Unmarshal([]byte(currentConditionsEnc), ¤tConditions); err == nil { + out.Status.Conditions = make([]autoscaling.HorizontalPodAutoscalerCondition, len(currentConditions)) + for i, currentCondition := range currentConditions { + if err := Convert_v1_HorizontalPodAutoscalerCondition_To_autoscaling_HorizontalPodAutoscalerCondition(¤tCondition, &out.Status.Conditions[i], s); err != nil { + return err + } } } - delete(out.Annotations, autoscaling.HorizontalPodAutoscalerConditionsAnnotation) } + // drop round-tripping annotations after converting to internal + out.Annotations, _ = autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations) + return nil } diff --git a/pkg/apis/autoscaling/v1/defaults_test.go b/pkg/apis/autoscaling/v1/defaults_test.go index e2b8cda4290..e2ad0b4fe2d 100644 --- a/pkg/apis/autoscaling/v1/defaults_test.go +++ b/pkg/apis/autoscaling/v1/defaults_test.go @@ -21,9 +21,11 @@ import ( "testing" autoscalingv1 "k8s.io/api/autoscaling/v1" - + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/kubernetes/pkg/api/legacyscheme" + "k8s.io/kubernetes/pkg/apis/autoscaling" _ "k8s.io/kubernetes/pkg/apis/autoscaling/install" . "k8s.io/kubernetes/pkg/apis/autoscaling/v1" _ "k8s.io/kubernetes/pkg/apis/core/install" @@ -67,6 +69,71 @@ func TestSetDefaultHPA(t *testing.T) { } } +func TestHorizontalPodAutoscalerAnnotations(t *testing.T) { + tests := []struct { + hpa autoscalingv1.HorizontalPodAutoscaler + test string + }{ + { + hpa: autoscalingv1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "", + autoscaling.MetricSpecsAnnotation: "", + autoscaling.BehaviorSpecsAnnotation: "", + autoscaling.MetricStatusesAnnotation: "", + }, + }, + }, + test: "test empty value for Annotations", + }, + { + hpa: autoscalingv1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "abc", + autoscaling.MetricSpecsAnnotation: "abc", + autoscaling.BehaviorSpecsAnnotation: "abc", + autoscaling.MetricStatusesAnnotation: "abc", + }, + }, + }, + test: "test random value for Annotations", + }, + { + hpa: autoscalingv1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "[]", + autoscaling.MetricSpecsAnnotation: "[]", + autoscaling.BehaviorSpecsAnnotation: "[]", + autoscaling.MetricStatusesAnnotation: "[]", + }, + }, + }, + test: "test empty array value for Annotations", + }, + } + + for _, test := range tests { + hpa := &test.hpa + hpaBeforeMuatate := *hpa.DeepCopy() + obj := roundTrip(t, runtime.Object(hpa)) + final_obj, ok := obj.(*autoscalingv1.HorizontalPodAutoscaler) + if !ok { + t.Fatalf("unexpected object: %v", obj) + } + if !reflect.DeepEqual(*hpa, hpaBeforeMuatate) { + t.Errorf("diff: %v", diff.ObjectDiff(*hpa, hpaBeforeMuatate)) + t.Errorf("expected: %#v\n actual: %#v", *hpa, hpaBeforeMuatate) + } + + if len(final_obj.ObjectMeta.Annotations) != 0 { + t.Fatalf("unexpected annotations: %v", final_obj.ObjectMeta.Annotations) + } + } +} + func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { data, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(SchemeGroupVersion), obj) if err != nil { diff --git a/pkg/apis/autoscaling/v2beta1/BUILD b/pkg/apis/autoscaling/v2beta1/BUILD index 5fbc372b458..491d1e9e41b 100644 --- a/pkg/apis/autoscaling/v2beta1/BUILD +++ b/pkg/apis/autoscaling/v2beta1/BUILD @@ -39,7 +39,9 @@ go_test( "//staging/src/k8s.io/api/autoscaling/v2beta1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/k8s.io/utils/pointer:go_default_library", ], diff --git a/pkg/apis/autoscaling/v2beta1/conversion.go b/pkg/apis/autoscaling/v2beta1/conversion.go index 2fb8c1856a2..56b67f7bc5b 100644 --- a/pkg/apis/autoscaling/v2beta1/conversion.go +++ b/pkg/apis/autoscaling/v2beta1/conversion.go @@ -264,18 +264,22 @@ func Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutosca if err := autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(in, out, s); err != nil { return err } - if in.Spec.Behavior != nil { - old := out.Annotations - out.Annotations = make(map[string]string, len(old)+1) - for k, v := range old { - out.Annotations[k] = v - } + // clear any pre-existing round-trip annotations to make sure the only ones set are ones we produced during conversion + annotations, copiedAnnotations := autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations) + out.Annotations = annotations + + if in.Spec.Behavior != nil { + // TODO: this is marshaling an internal type. Fix this without breaking backwards compatibility with n-1 API servers. behaviorEnc, err := json.Marshal(in.Spec.Behavior) if err != nil { return err } - // Even if the annotation for behavior exists, we will just overwrite it + // copy before mutating + if !copiedAnnotations { + copiedAnnotations = true + out.Annotations = autoscaling.DeepCopyStringMap(out.Annotations) + } out.Annotations[autoscaling.BehaviorSpecsAnnotation] = string(behaviorEnc) } @@ -288,13 +292,17 @@ func Convert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutosca } if behaviorEnc, hasBehaviors := out.Annotations[autoscaling.BehaviorSpecsAnnotation]; hasBehaviors { + // TODO: this is unmarshaling an internal type. Fix this without breaking backwards compatibility with n-1 API servers. var behavior autoscaling.HorizontalPodAutoscalerBehavior - if err := json.Unmarshal([]byte(behaviorEnc), &behavior); err != nil { - return err + if err := json.Unmarshal([]byte(behaviorEnc), &behavior); err == nil && behavior != (autoscaling.HorizontalPodAutoscalerBehavior{}) { + // only move well-formed data from annotations to fields + out.Spec.Behavior = &behavior } - out.Spec.Behavior = &behavior - delete(out.Annotations, autoscaling.BehaviorSpecsAnnotation) } + + // drop round-tripping annotations after converting to internal + out.Annotations, _ = autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations) + return nil } diff --git a/pkg/apis/autoscaling/v2beta1/defaults_test.go b/pkg/apis/autoscaling/v2beta1/defaults_test.go index a68e39f1597..71b0d349f91 100644 --- a/pkg/apis/autoscaling/v2beta1/defaults_test.go +++ b/pkg/apis/autoscaling/v2beta1/defaults_test.go @@ -20,9 +20,12 @@ import ( "reflect" "testing" + "k8s.io/apimachinery/pkg/util/diff" + autoscalingv2beta1 "k8s.io/api/autoscaling/v2beta1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/autoscaling" @@ -105,6 +108,71 @@ func TestSetDefaultHPA(t *testing.T) { } } +func TestHorizontalPodAutoscalerAnnotations(t *testing.T) { + tests := []struct { + hpa autoscalingv2beta1.HorizontalPodAutoscaler + test string + }{ + { + hpa: autoscalingv2beta1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "", + autoscaling.MetricSpecsAnnotation: "", + autoscaling.BehaviorSpecsAnnotation: "", + autoscaling.MetricStatusesAnnotation: "", + }, + }, + }, + test: "test empty value for Annotations", + }, + { + hpa: autoscalingv2beta1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "abc", + autoscaling.MetricSpecsAnnotation: "abc", + autoscaling.BehaviorSpecsAnnotation: "abc", + autoscaling.MetricStatusesAnnotation: "abc", + }, + }, + }, + test: "test random value for Annotations", + }, + { + hpa: autoscalingv2beta1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "[]", + autoscaling.MetricSpecsAnnotation: "[]", + autoscaling.BehaviorSpecsAnnotation: "[]", + autoscaling.MetricStatusesAnnotation: "[]", + }, + }, + }, + test: "test empty array value for Annotations", + }, + } + + for _, test := range tests { + hpa := &test.hpa + hpaBeforeMuatate := *hpa.DeepCopy() + obj := roundTrip(t, runtime.Object(hpa)) + final_obj, ok := obj.(*autoscalingv2beta1.HorizontalPodAutoscaler) + if !ok { + t.Fatalf("unexpected object: %v", obj) + } + if !reflect.DeepEqual(*hpa, hpaBeforeMuatate) { + t.Errorf("diff: %v", diff.ObjectDiff(*hpa, hpaBeforeMuatate)) + t.Errorf("expected: %#v\n actual: %#v", *hpa, hpaBeforeMuatate) + } + + if len(final_obj.ObjectMeta.Annotations) != 0 { + t.Fatalf("unexpected annotations: %v", final_obj.ObjectMeta.Annotations) + } + } +} + func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { data, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(SchemeGroupVersion), obj) if err != nil { diff --git a/pkg/apis/autoscaling/v2beta2/BUILD b/pkg/apis/autoscaling/v2beta2/BUILD index 07609190dbe..3c6694e297e 100644 --- a/pkg/apis/autoscaling/v2beta2/BUILD +++ b/pkg/apis/autoscaling/v2beta2/BUILD @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = [ + "conversion.go", "defaults.go", "doc.go", "register.go", @@ -43,7 +44,14 @@ go_test( srcs = ["defaults_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/api/legacyscheme:go_default_library", + "//pkg/apis/autoscaling:go_default_library", + "//pkg/apis/autoscaling/install:go_default_library", + "//pkg/apis/core/install:go_default_library", "//staging/src/k8s.io/api/autoscaling/v2beta2:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/k8s.io/utils/pointer:go_default_library", ], diff --git a/pkg/apis/autoscaling/v2beta2/conversion.go b/pkg/apis/autoscaling/v2beta2/conversion.go new file mode 100644 index 00000000000..3a555e8c872 --- /dev/null +++ b/pkg/apis/autoscaling/v2beta2/conversion.go @@ -0,0 +1,42 @@ +/* +Copyright 2020 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 v2beta2 + +import ( + autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + + "k8s.io/apimachinery/pkg/conversion" + "k8s.io/kubernetes/pkg/apis/autoscaling" +) + +func Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(in *autoscaling.HorizontalPodAutoscaler, out *autoscalingv2beta2.HorizontalPodAutoscaler, s conversion.Scope) error { + if err := autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(in, out, s); err != nil { + return err + } + // v2beta2 round-trips to internal without any serialized annotations, make sure any from other versions don't get serialized + out.Annotations, _ = autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations) + return nil +} + +func Convert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in *autoscalingv2beta2.HorizontalPodAutoscaler, out *autoscaling.HorizontalPodAutoscaler, s conversion.Scope) error { + if err := autoConvert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in, out, s); err != nil { + return err + } + // v2beta2 round-trips to internal without any serialized annotations, make sure any from other versions don't get serialized + out.Annotations, _ = autoscaling.DropRoundTripHorizontalPodAutoscalerAnnotations(out.Annotations) + return nil +} diff --git a/pkg/apis/autoscaling/v2beta2/defaults_test.go b/pkg/apis/autoscaling/v2beta2/defaults_test.go index fcdac48da01..728e2af4ba9 100644 --- a/pkg/apis/autoscaling/v2beta2/defaults_test.go +++ b/pkg/apis/autoscaling/v2beta2/defaults_test.go @@ -17,13 +17,21 @@ limitations under the License. package v2beta2_test import ( + "reflect" "testing" - autoscalingv2 "k8s.io/api/autoscaling/v2beta2" - . "k8s.io/kubernetes/pkg/apis/autoscaling/v2beta2" - utilpointer "k8s.io/utils/pointer" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/kubernetes/pkg/api/legacyscheme" "github.com/stretchr/testify/assert" + autoscalingv2 "k8s.io/api/autoscaling/v2beta2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/kubernetes/pkg/apis/autoscaling" + _ "k8s.io/kubernetes/pkg/apis/autoscaling/install" + . "k8s.io/kubernetes/pkg/apis/autoscaling/v2beta2" + _ "k8s.io/kubernetes/pkg/apis/core/install" + utilpointer "k8s.io/utils/pointer" ) func TestGenerateScaleDownRules(t *testing.T) { @@ -229,3 +237,88 @@ func TestGenerateScaleUpRules(t *testing.T) { }) } } + +func TestHorizontalPodAutoscalerAnnotations(t *testing.T) { + tests := []struct { + hpa autoscalingv2.HorizontalPodAutoscaler + test string + }{ + { + hpa: autoscalingv2.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "", + autoscaling.MetricSpecsAnnotation: "", + autoscaling.BehaviorSpecsAnnotation: "", + autoscaling.MetricStatusesAnnotation: "", + }, + }, + }, + test: "test empty value for Annotations", + }, + { + hpa: autoscalingv2.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "abc", + autoscaling.MetricSpecsAnnotation: "abc", + autoscaling.BehaviorSpecsAnnotation: "abc", + autoscaling.MetricStatusesAnnotation: "abc", + }, + }, + }, + test: "test random value for Annotations", + }, + { + hpa: autoscalingv2.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + autoscaling.HorizontalPodAutoscalerConditionsAnnotation: "[]", + autoscaling.MetricSpecsAnnotation: "[]", + autoscaling.BehaviorSpecsAnnotation: "[]", + autoscaling.MetricStatusesAnnotation: "[]", + }, + }, + }, + test: "test empty array value for Annotations", + }, + } + + for _, test := range tests { + hpa := &test.hpa + hpaBeforeMuatate := *hpa.DeepCopy() + obj := roundTrip(t, runtime.Object(hpa)) + final_obj, ok := obj.(*autoscalingv2.HorizontalPodAutoscaler) + if !ok { + t.Fatalf("unexpected object: %v", obj) + } + if !reflect.DeepEqual(*hpa, hpaBeforeMuatate) { + t.Errorf("diff: %v", diff.ObjectDiff(*hpa, hpaBeforeMuatate)) + t.Errorf("expected: %#v\n actual: %#v", *hpa, hpaBeforeMuatate) + } + + if len(final_obj.ObjectMeta.Annotations) != 0 { + t.Fatalf("unexpected annotations: %v", final_obj.ObjectMeta.Annotations) + } + } +} + +func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { + data, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(SchemeGroupVersion), obj) + if err != nil { + t.Errorf("%v\n %#v", err, obj) + return nil + } + obj2, err := runtime.Decode(legacyscheme.Codecs.UniversalDecoder(), data) + if err != nil { + t.Errorf("%v\nData: %s\nSource: %#v", err, string(data), obj) + return nil + } + obj3 := reflect.New(reflect.TypeOf(obj).Elem()).Interface().(runtime.Object) + err = legacyscheme.Scheme.Convert(obj2, obj3, nil) + if err != nil { + t.Errorf("%v\nSource: %#v", err, obj2) + return nil + } + return obj3 +} diff --git a/pkg/apis/autoscaling/v2beta2/zz_generated.conversion.go b/pkg/apis/autoscaling/v2beta2/zz_generated.conversion.go index df8e47c5e16..e579b4221ca 100644 --- a/pkg/apis/autoscaling/v2beta2/zz_generated.conversion.go +++ b/pkg/apis/autoscaling/v2beta2/zz_generated.conversion.go @@ -90,16 +90,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v2beta2.HorizontalPodAutoscaler)(nil), (*autoscaling.HorizontalPodAutoscaler)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(a.(*v2beta2.HorizontalPodAutoscaler), b.(*autoscaling.HorizontalPodAutoscaler), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*autoscaling.HorizontalPodAutoscaler)(nil), (*v2beta2.HorizontalPodAutoscaler)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(a.(*autoscaling.HorizontalPodAutoscaler), b.(*v2beta2.HorizontalPodAutoscaler), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*v2beta2.HorizontalPodAutoscalerBehavior)(nil), (*autoscaling.HorizontalPodAutoscalerBehavior)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v2beta2_HorizontalPodAutoscalerBehavior_To_autoscaling_HorizontalPodAutoscalerBehavior(a.(*v2beta2.HorizontalPodAutoscalerBehavior), b.(*autoscaling.HorizontalPodAutoscalerBehavior), scope) }); err != nil { @@ -260,6 +250,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*autoscaling.HorizontalPodAutoscaler)(nil), (*v2beta2.HorizontalPodAutoscaler)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(a.(*autoscaling.HorizontalPodAutoscaler), b.(*v2beta2.HorizontalPodAutoscaler), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*v2beta2.HorizontalPodAutoscaler)(nil), (*autoscaling.HorizontalPodAutoscaler)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(a.(*v2beta2.HorizontalPodAutoscaler), b.(*autoscaling.HorizontalPodAutoscaler), scope) + }); err != nil { + return err + } return nil } @@ -406,11 +406,6 @@ func autoConvert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAut return nil } -// Convert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler is an autogenerated conversion function. -func Convert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in *v2beta2.HorizontalPodAutoscaler, out *autoscaling.HorizontalPodAutoscaler, s conversion.Scope) error { - return autoConvert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in, out, s) -} - func autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(in *autoscaling.HorizontalPodAutoscaler, out *v2beta2.HorizontalPodAutoscaler, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_autoscaling_HorizontalPodAutoscalerSpec_To_v2beta2_HorizontalPodAutoscalerSpec(&in.Spec, &out.Spec, s); err != nil { @@ -422,11 +417,6 @@ func autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAut return nil } -// Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler is an autogenerated conversion function. -func Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(in *autoscaling.HorizontalPodAutoscaler, out *v2beta2.HorizontalPodAutoscaler, s conversion.Scope) error { - return autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(in, out, s) -} - func autoConvert_v2beta2_HorizontalPodAutoscalerBehavior_To_autoscaling_HorizontalPodAutoscalerBehavior(in *v2beta2.HorizontalPodAutoscalerBehavior, out *autoscaling.HorizontalPodAutoscalerBehavior, s conversion.Scope) error { out.ScaleUp = (*autoscaling.HPAScalingRules)(unsafe.Pointer(in.ScaleUp)) out.ScaleDown = (*autoscaling.HPAScalingRules)(unsafe.Pointer(in.ScaleDown)) @@ -479,7 +469,17 @@ func Convert_autoscaling_HorizontalPodAutoscalerCondition_To_v2beta2_HorizontalP func autoConvert_v2beta2_HorizontalPodAutoscalerList_To_autoscaling_HorizontalPodAutoscalerList(in *v2beta2.HorizontalPodAutoscalerList, out *autoscaling.HorizontalPodAutoscalerList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]autoscaling.HorizontalPodAutoscaler)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]autoscaling.HorizontalPodAutoscaler, len(*in)) + for i := range *in { + if err := Convert_v2beta2_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -490,7 +490,17 @@ func Convert_v2beta2_HorizontalPodAutoscalerList_To_autoscaling_HorizontalPodAut func autoConvert_autoscaling_HorizontalPodAutoscalerList_To_v2beta2_HorizontalPodAutoscalerList(in *autoscaling.HorizontalPodAutoscalerList, out *v2beta2.HorizontalPodAutoscalerList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v2beta2.HorizontalPodAutoscaler)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v2beta2.HorizontalPodAutoscaler, len(*in)) + for i := range *in { + if err := Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta2_HorizontalPodAutoscaler(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil }