Fix empty describedObject in hpa status (kubernetes#124555)

* fix empty DescribedObject in hpa MetricStatus when object target type is AverageValue

Signed-off-by: mchtech <[email protected]>

* add test

Signed-off-by: mchtech <[email protected]>

---------

Signed-off-by: mchtech <[email protected]>

Author: mchtech

pkg/controller/podautoscaler/horizontal.go

@@ -529,44 +529,33 @@ func (a *HorizontalController) reconcileKey(ctx context.Context, key string) (de
 
 // computeStatusForObjectMetric computes the desired number of replicas for the specified metric of type ObjectMetricSourceType.
 func (a *HorizontalController) computeStatusForObjectMetric(specReplicas, statusReplicas int32, metricSpec autoscalingv2.MetricSpec, hpa *autoscalingv2.HorizontalPodAutoscaler, selector labels.Selector, status *autoscalingv2.MetricStatus, metricSelector labels.Selector) (replicas int32, timestamp time.Time, metricName string, condition autoscalingv2.HorizontalPodAutoscalerCondition, err error) {
+	metricStatus := autoscalingv2.MetricStatus{
+		Type: autoscalingv2.ObjectMetricSourceType,
+		Object: &autoscalingv2.ObjectMetricStatus{
+			DescribedObject: metricSpec.Object.DescribedObject,
+			Metric: autoscalingv2.MetricIdentifier{
+				Name:     metricSpec.Object.Metric.Name,
+				Selector: metricSpec.Object.Metric.Selector,
+			},
+		},
+	}
 	if metricSpec.Object.Target.Type == autoscalingv2.ValueMetricType && metricSpec.Object.Target.Value != nil {
 		replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetObjectMetricReplicas(specReplicas, metricSpec.Object.Target.Value.MilliValue(), metricSpec.Object.Metric.Name, hpa.Namespace, &metricSpec.Object.DescribedObject, selector, metricSelector)
 		if err != nil {
 			condition := a.getUnableComputeReplicaCountCondition(hpa, "FailedGetObjectMetric", err)
 			return 0, timestampProposal, "", condition, err
 		}
-		*status = autoscalingv2.MetricStatus{
-			Type: autoscalingv2.ObjectMetricSourceType,
-			Object: &autoscalingv2.ObjectMetricStatus{
-				DescribedObject: metricSpec.Object.DescribedObject,
-				Metric: autoscalingv2.MetricIdentifier{
-					Name:     metricSpec.Object.Metric.Name,
-					Selector: metricSpec.Object.Metric.Selector,
-				},
-				Current: autoscalingv2.MetricValueStatus{
-					Value: resource.NewMilliQuantity(usageProposal, resource.DecimalSI),
-				},
-			},
-		}
+		metricStatus.Object.Current.Value = resource.NewMilliQuantity(usageProposal, resource.DecimalSI)
+		*status = metricStatus
 		return replicaCountProposal, timestampProposal, fmt.Sprintf("%s metric %s", metricSpec.Object.DescribedObject.Kind, metricSpec.Object.Metric.Name), autoscalingv2.HorizontalPodAutoscalerCondition{}, nil
 	} else if metricSpec.Object.Target.Type == autoscalingv2.AverageValueMetricType && metricSpec.Object.Target.AverageValue != nil {
 		replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetObjectPerPodMetricReplicas(statusReplicas, metricSpec.Object.Target.AverageValue.MilliValue(), metricSpec.Object.Metric.Name, hpa.Namespace, &metricSpec.Object.DescribedObject, metricSelector)
 		if err != nil {
 			condition := a.getUnableComputeReplicaCountCondition(hpa, "FailedGetObjectMetric", err)
 			return 0, time.Time{}, "", condition, fmt.Errorf("failed to get %s object metric: %v", metricSpec.Object.Metric.Name, err)
 		}
-		*status = autoscalingv2.MetricStatus{
-			Type: autoscalingv2.ObjectMetricSourceType,
-			Object: &autoscalingv2.ObjectMetricStatus{
-				Metric: autoscalingv2.MetricIdentifier{
-					Name:     metricSpec.Object.Metric.Name,
-					Selector: metricSpec.Object.Metric.Selector,
-				},
-				Current: autoscalingv2.MetricValueStatus{
-					AverageValue: resource.NewMilliQuantity(usageProposal, resource.DecimalSI),
-				},
-			},
-		}
+		metricStatus.Object.Current.AverageValue = resource.NewMilliQuantity(usageProposal, resource.DecimalSI)
+		*status = metricStatus
 		return replicaCountProposal, timestampProposal, fmt.Sprintf("external metric %s(%+v)", metricSpec.Object.Metric.Name, metricSpec.Object.Metric.Selector), autoscalingv2.HorizontalPodAutoscalerCondition{}, nil
 	}
 	errMsg := "invalid object metric source: neither a value target nor an average value target was set"

pkg/controller/podautoscaler/horizontal_test.go

@@ -382,6 +382,13 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa
 					assert.Equal(t, tc.CPUCurrent, *utilization, "the report CPU utilization percentage should be as expected")
 				}
 			}
+
+			if len(obj.Spec.Metrics) > 0 && obj.Spec.Metrics[0].Object != nil && len(obj.Status.CurrentMetrics) > 0 && obj.Status.CurrentMetrics[0].Object != nil {
+				assert.Equal(t, obj.Spec.Metrics[0].Object.DescribedObject.APIVersion, obj.Status.CurrentMetrics[0].Object.DescribedObject.APIVersion)
+				assert.Equal(t, obj.Spec.Metrics[0].Object.DescribedObject.Kind, obj.Status.CurrentMetrics[0].Object.DescribedObject.Kind)
+				assert.Equal(t, obj.Spec.Metrics[0].Object.DescribedObject.Name, obj.Status.CurrentMetrics[0].Object.DescribedObject.Name)
+			}
+
 			actualConditions := obj.Status.Conditions
 			// TODO: it's ok not to sort these because statusOk
 			// contains all the conditions, so we'll never be appending.