From caa78e0b3e3a68b84456120e04d4f219e25ce20d Mon Sep 17 00:00:00 2001
From: Solly Ross <sross@redhat.com>
Date: Mon, 20 Feb 2017 01:45:49 -0500
Subject: [PATCH] Fix HPA v1 Conversion Bug

There was a bug in the HPA v1 conversion logic that would occur when
a custom metric and a metric that was encoded in v1 as
targetCPUUtilizationPercentage were used at the same time.  In this
case, the custom metric could overwrite the CPU metric, or vice versa.

This fixes that bug, and ensures that the fuzzer tests round-tripping
with multiple metrics.
---
 pkg/api/testing/fuzzer.go             | 35 ++++++++++++++++++++++-----
 pkg/apis/autoscaling/v1/conversion.go |  9 +++++--
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go
index b6894944286..a5063c4e808 100644
--- a/pkg/api/testing/fuzzer.go
+++ b/pkg/api/testing/fuzzer.go
@@ -547,11 +547,23 @@ func autoscalingFuncs(t apitesting.TestingCommon) []interface{} {
 			minReplicas := int32(c.Rand.Int31())
 			s.MinReplicas = &minReplicas
 
-			// NB: since this is used for round-tripping, we can only fuzz
-			// fields that round-trip successfully, so only the resource source
-			// type is usable here
+			randomQuantity := func() resource.Quantity {
+				var q resource.Quantity
+				c.Fuzz(&q)
+				// precalc the string for benchmarking purposes
+				_ = q.String()
+				return q
+			}
+
 			targetUtilization := int32(c.RandUint64())
 			s.Metrics = []autoscaling.MetricSpec{
+				{
+					Type: autoscaling.PodsMetricSourceType,
+					Pods: &autoscaling.PodsMetricSource{
+						MetricName:         c.RandString(),
+						TargetAverageValue: randomQuantity(),
+					},
+				},
 				{
 					Type: autoscaling.ResourceMetricSourceType,
 					Resource: &autoscaling.ResourceMetricSource{
@@ -563,11 +575,22 @@ func autoscalingFuncs(t apitesting.TestingCommon) []interface{} {
 		},
 		func(s *autoscaling.HorizontalPodAutoscalerStatus, c fuzz.Continue) {
 			c.FuzzNoCustom(s) // fuzz self without calling this function again
-			// NB: since this is used for round-tripping, we can only fuzz
-			// fields that round-trip successfully, so only the resource status
-			// type is usable here
+			randomQuantity := func() resource.Quantity {
+				var q resource.Quantity
+				c.Fuzz(&q)
+				// precalc the string for benchmarking purposes
+				_ = q.String()
+				return q
+			}
 			currentUtilization := int32(c.RandUint64())
 			s.CurrentMetrics = []autoscaling.MetricStatus{
+				{
+					Type: autoscaling.PodsMetricSourceType,
+					Pods: &autoscaling.PodsMetricStatus{
+						MetricName:          c.RandString(),
+						CurrentAverageValue: randomQuantity(),
+					},
+				},
 				{
 					Type: autoscaling.ResourceMetricSourceType,
 					Resource: &autoscaling.ResourceMetricStatus{
diff --git a/pkg/apis/autoscaling/v1/conversion.go b/pkg/apis/autoscaling/v1/conversion.go
index d1723aa1a48..b8096af807d 100644
--- a/pkg/apis/autoscaling/v1/conversion.go
+++ b/pkg/apis/autoscaling/v1/conversion.go
@@ -108,12 +108,17 @@ func Convert_v1_HorizontalPodAutoscaler_To_autoscaling_HorizontalPodAutoscaler(i
 			return err
 		}
 
-		out.Spec.Metrics = make([]autoscaling.MetricSpec, len(otherMetrics))
+		// 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, &out.Spec.Metrics[i], s); err != nil {
+			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
 		delete(out.Annotations, autoscaling.MetricSpecsAnnotation)
 	}