From 3bb36411da9b2c0fc8c35392948970d0c419308a Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Wed, 29 Aug 2018 13:42:16 -0400 Subject: [PATCH] Fix conversion for autoscaling/v1 ObjectMetricSource and add MetricIdentifier fuzzer Selectors in ObjectMetricSource's weren't being persisted through roundtrip conversions, and this wasn't caught because we had no fuzzer testing MetricIdentifier selectors --- pkg/apis/autoscaling/fuzzer/fuzzer.go | 19 ++- pkg/apis/autoscaling/v1/conversion.go | 1 + pkg/apis/autoscaling/v2beta1/conversion.go | 120 ------------------ .../v2beta1/zz_generated.conversion.go | 20 +-- 4 files changed, 27 insertions(+), 133 deletions(-) diff --git a/pkg/apis/autoscaling/fuzzer/fuzzer.go b/pkg/apis/autoscaling/fuzzer/fuzzer.go index 8322057d288..14e14a1b55e 100644 --- a/pkg/apis/autoscaling/fuzzer/fuzzer.go +++ b/pkg/apis/autoscaling/fuzzer/fuzzer.go @@ -51,21 +51,34 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { return q } + var podMetricID autoscaling.MetricIdentifier + var objMetricID autoscaling.MetricIdentifier + c.Fuzz(&podMetricID) + c.Fuzz(&objMetricID) + targetUtilization := int32(c.RandUint64()) averageValue := randomQuantity() s.Metrics = []autoscaling.MetricSpec{ { Type: autoscaling.PodsMetricSourceType, Pods: &autoscaling.PodsMetricSource{ - Metric: autoscaling.MetricIdentifier{ - Name: c.RandString(), - }, + Metric: podMetricID, Target: autoscaling.MetricTarget{ Type: autoscaling.AverageValueMetricType, AverageValue: &averageValue, }, }, }, + { + Type: autoscaling.ObjectMetricSourceType, + Object: &autoscaling.ObjectMetricSource{ + Metric: objMetricID, + Target: autoscaling.MetricTarget{ + Type: autoscaling.ValueMetricType, + Value: &averageValue, + }, + }, + }, { Type: autoscaling.ResourceMetricSourceType, Resource: &autoscaling.ResourceMetricSource{ diff --git a/pkg/apis/autoscaling/v1/conversion.go b/pkg/apis/autoscaling/v1/conversion.go index 1fb3c343cb7..3f85e75ba28 100644 --- a/pkg/apis/autoscaling/v1/conversion.go +++ b/pkg/apis/autoscaling/v1/conversion.go @@ -111,6 +111,7 @@ func Convert_autoscaling_ObjectMetricSource_To_v1_ObjectMetricSource(in *autosca APIVersion: in.DescribedObject.APIVersion, } out.MetricName = in.Metric.Name + out.Selector = in.Metric.Selector return nil } diff --git a/pkg/apis/autoscaling/v2beta1/conversion.go b/pkg/apis/autoscaling/v2beta1/conversion.go index 289aab878d8..eebc1043053 100644 --- a/pkg/apis/autoscaling/v2beta1/conversion.go +++ b/pkg/apis/autoscaling/v2beta1/conversion.go @@ -17,8 +17,6 @@ limitations under the License. package v2beta1 import ( - "encoding/json" - autoscalingv2beta1 "k8s.io/api/autoscaling/v2beta1" v1 "k8s.io/api/core/v1" @@ -290,121 +288,3 @@ func Convert_v2beta1_PodsMetricStatus_To_autoscaling_PodsMetricStatus(in *autosc } return nil } - -func Convert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in *autoscalingv2beta1.HorizontalPodAutoscaler, out *autoscaling.HorizontalPodAutoscaler, s conversion.Scope) error { - if err := autoConvert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in, out, s); err != nil { - return err - } - if selectorMetricsEnc, hasOtherMetrics := out.Annotations[autoscaling.MetricSpecsAnnotation]; hasOtherMetrics { - var selectorMetrics []autoscalingv2beta1.MetricSpec - if err := json.Unmarshal([]byte(selectorMetricsEnc), &selectorMetrics); err != nil { - return err - } - - convMetrics := make([]autoscaling.MetricSpec, len(selectorMetrics)) - for i, metric := range selectorMetrics { - if err := Convert_v2beta1_MetricSpec_To_autoscaling_MetricSpec(&metric, &convMetrics[i], s); err != nil { - return err - } - } - - outMetrics := make([]autoscaling.MetricSpec, 0, len(selectorMetrics)+len(out.Spec.Metrics)) - for _, convMetric := range convMetrics { - outMetrics = append(outMetrics, convMetric) - } - - for _, metric := range out.Spec.Metrics { - outMetrics = append(outMetrics, metric) - } - - out.Spec.Metrics = outMetrics - delete(out.Annotations, autoscaling.MetricSpecsAnnotation) - } - - if currentMetricsEnc, hasCurrentMetrics := out.Annotations[autoscaling.MetricStatusesAnnotation]; hasCurrentMetrics { - var currentMetrics []autoscalingv2beta1.MetricStatus - if err := json.Unmarshal([]byte(currentMetricsEnc), ¤tMetrics); err != nil { - return err - } - - outCurrentMetrics := make([]autoscaling.MetricStatus, 0, len(currentMetrics)+len(out.Status.CurrentMetrics)) - for i, currentMetric := range currentMetrics { - if err := Convert_v2beta1_MetricStatus_To_autoscaling_MetricStatus(¤tMetric, &out.Status.CurrentMetrics[i], s); err != nil { - return err - } - } - - for _, currentMetric := range out.Status.CurrentMetrics { - outCurrentMetrics = append(outCurrentMetrics, currentMetric) - } - - out.Status.CurrentMetrics = outCurrentMetrics - delete(out.Annotations, autoscaling.MetricStatusesAnnotation) - } - return nil -} - -func Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(in *autoscaling.HorizontalPodAutoscaler, out *autoscalingv2beta1.HorizontalPodAutoscaler, s conversion.Scope) error { - if err := autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(in, out, s); err != nil { - return err - } - - selectorMetrics := make([]autoscalingv2beta1.MetricSpec, 0, len(in.Spec.Metrics)) - for _, metric := range in.Spec.Metrics { - if (metric.Object != nil && metric.Object.Metric.Selector == nil) || - (metric.Pods != nil && metric.Pods.Metric.Selector == nil) || - (metric.External != nil && metric.External.Metric.Selector == nil) || - (metric.Resource != nil) { - continue - } - - convMetric := autoscalingv2beta1.MetricSpec{} - if err := Convert_autoscaling_MetricSpec_To_v2beta1_MetricSpec(&metric, &convMetric, s); err != nil { - return err - } - selectorMetrics = append(selectorMetrics, convMetric) - } - - currentMetrics := make([]autoscalingv2beta1.MetricStatus, 0, len(in.Status.CurrentMetrics)) - for _, currentMetric := range in.Status.CurrentMetrics { - if (currentMetric.Object != nil && currentMetric.Object.Metric.Selector == nil) || - (currentMetric.Pods != nil && currentMetric.Pods.Metric.Selector == nil) || - (currentMetric.External != nil && currentMetric.External.Metric.Selector == nil) || - (currentMetric.Resource != nil) { - continue - } - convMetric := autoscalingv2beta1.MetricStatus{} - if err := Convert_autoscaling_MetricStatus_To_v2beta1_MetricStatus(¤tMetric, &convMetric, s); err != nil { - return err - } - currentMetrics = append(currentMetrics, convMetric) - } - - if len(selectorMetrics) > 0 || len(currentMetrics) > 0 { - old := out.Annotations - out.Annotations = make(map[string]string, len(old)+2) - if old != nil { - for k, v := range old { - out.Annotations[k] = v - } - } - } - - if len(selectorMetrics) > 0 { - selectorMetricsEnc, err := json.Marshal(selectorMetrics) - if err != nil { - return err - } - out.Annotations[autoscaling.MetricSpecsAnnotation] = string(selectorMetricsEnc) - } - - if len(currentMetrics) > 0 { - currentMetricsEnc, err := json.Marshal(currentMetrics) - if err != nil { - return err - } - out.Annotations[autoscaling.MetricSpecsAnnotation] = string(currentMetricsEnc) - } - - return nil -} diff --git a/pkg/apis/autoscaling/v2beta1/zz_generated.conversion.go b/pkg/apis/autoscaling/v2beta1/zz_generated.conversion.go index 989a97741c2..8c98b810e5d 100644 --- a/pkg/apis/autoscaling/v2beta1/zz_generated.conversion.go +++ b/pkg/apis/autoscaling/v2beta1/zz_generated.conversion.go @@ -209,11 +209,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddConversionFunc((*autoscaling.HorizontalPodAutoscaler)(nil), (*v2beta1.HorizontalPodAutoscaler)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(a.(*autoscaling.HorizontalPodAutoscaler), b.(*v2beta1.HorizontalPodAutoscaler), scope) - }); err != nil { - return err - } if err := s.AddConversionFunc((*autoscaling.MetricTarget)(nil), (*v2beta1.CrossVersionObjectReference)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_autoscaling_MetricTarget_To_v2beta1_CrossVersionObjectReference(a.(*autoscaling.MetricTarget), b.(*v2beta1.CrossVersionObjectReference), scope) }); err != nil { @@ -264,11 +259,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddConversionFunc((*v2beta1.HorizontalPodAutoscaler)(nil), (*autoscaling.HorizontalPodAutoscaler)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(a.(*v2beta1.HorizontalPodAutoscaler), b.(*autoscaling.HorizontalPodAutoscaler), scope) - }); err != nil { - return err - } if err := s.AddConversionFunc((*v2beta1.ObjectMetricSource)(nil), (*autoscaling.ObjectMetricSource)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v2beta1_ObjectMetricSource_To_autoscaling_ObjectMetricSource(a.(*v2beta1.ObjectMetricSource), b.(*autoscaling.ObjectMetricSource), scope) }); err != nil { @@ -365,6 +355,11 @@ func autoConvert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAut return nil } +// Convert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler is an autogenerated conversion function. +func Convert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in *v2beta1.HorizontalPodAutoscaler, out *autoscaling.HorizontalPodAutoscaler, s conversion.Scope) error { + return autoConvert_v2beta1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(in, out, s) +} + func autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(in *autoscaling.HorizontalPodAutoscaler, out *v2beta1.HorizontalPodAutoscaler, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_autoscaling_HorizontalPodAutoscalerSpec_To_v2beta1_HorizontalPodAutoscalerSpec(&in.Spec, &out.Spec, s); err != nil { @@ -376,6 +371,11 @@ func autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAut return nil } +// Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler is an autogenerated conversion function. +func Convert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(in *autoscaling.HorizontalPodAutoscaler, out *v2beta1.HorizontalPodAutoscaler, s conversion.Scope) error { + return autoConvert_autoscaling_HorizontalPodAutoscaler_To_v2beta1_HorizontalPodAutoscaler(in, out, s) +} + func autoConvert_v2beta1_HorizontalPodAutoscalerCondition_To_autoscaling_HorizontalPodAutoscalerCondition(in *v2beta1.HorizontalPodAutoscalerCondition, out *autoscaling.HorizontalPodAutoscalerCondition, s conversion.Scope) error { out.Type = autoscaling.HorizontalPodAutoscalerConditionType(in.Type) out.Status = autoscaling.ConditionStatus(in.Status)