From 381ccf0f4c723f7164d4363d2bcd76f4e34e5441 Mon Sep 17 00:00:00 2001 From: mchtech Date: Wed, 19 Mar 2025 00:33:56 +0800 Subject: [PATCH] Fix empty describedObject in hpa status (#124555) * fix empty DescribedObject in hpa MetricStatus when object target type is AverageValue Signed-off-by: mchtech * add test Signed-off-by: mchtech --------- Signed-off-by: mchtech --- pkg/controller/podautoscaler/horizontal.go | 39 +++++++------------ .../podautoscaler/horizontal_test.go | 7 ++++ 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go index b7c8485fa03..57bea5e1372 100644 --- a/pkg/controller/podautoscaler/horizontal.go +++ b/pkg/controller/podautoscaler/horizontal.go @@ -529,25 +529,24 @@ 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) @@ -555,18 +554,8 @@ func (a *HorizontalController) computeStatusForObjectMetric(specReplicas, status 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" diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go index 0a959bfecba..250e45d7682 100644 --- a/pkg/controller/podautoscaler/horizontal_test.go +++ b/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.