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/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/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/v2beta2/BUILD b/pkg/apis/autoscaling/v2beta2/BUILD index 07609190dbe..0dd2134e644 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", 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/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 }