From f76258f0ff972942ab6722043b947e58f29e66e2 Mon Sep 17 00:00:00 2001
From: Kensei Nakada <handbomusic@gmail.com>
Date: Sun, 5 Mar 2023 15:01:34 +0000
Subject: [PATCH] fix based on the suggestion

---
 pkg/controller/podautoscaler/horizontal.go    |  7 +++----
 .../podautoscaler/horizontal_test.go          | 21 +++++++++----------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go
index b01196c1d3a..ca15411abc7 100644
--- a/pkg/controller/podautoscaler/horizontal.go
+++ b/pkg/controller/podautoscaler/horizontal.go
@@ -431,10 +431,9 @@ func (a *HorizontalController) computeReplicasForMetric(ctx context.Context, hpa
 	case autoscalingv2.ContainerResourceMetricSourceType:
 		if !a.containerResourceMetricsEnabled {
 			// If the container resource metrics feature is disabled but the object has the one,
-			// that means the user enabled the feature once, created some HPAs with the container resource metrics, and disabled it finally.
-			// We cannot return errors in this case because that'll result in all HPAs with the container resource metric sources failing to scale down.
-			// Thus, here we silently ignore it and return current replica values so that it won't affect the autoscaling decision
-			return specReplicas, "", time.Time{}, condition, nil
+			// that means the user enabled the feature once,
+			// created some HPAs with the container resource metrics, and disabled it finally.
+			return 0, "", time.Time{}, condition, fmt.Errorf("the container resource metrics feature is disabled by the feature gate")
 		}
 		replicaCountProposal, timestampProposal, metricNameProposal, condition, err = a.computeStatusForContainerResourceMetric(ctx, specReplicas, spec, hpa, selector, status)
 		if err != nil {
diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go
index 23019952577..b079251fcea 100644
--- a/pkg/controller/podautoscaler/horizontal_test.go
+++ b/pkg/controller/podautoscaler/horizontal_test.go
@@ -837,34 +837,33 @@ func TestScaleUpContainer(t *testing.T) {
 	tc.runTest(t)
 }
 
-func TestIgnoreContainerMetric(t *testing.T) {
-	// when the container metric feature isn't enabled, it's ignored and HPA keeps the current replica.
+func TestContainerMetricWithTheFeatureGateDisabled(t *testing.T) {
+	// In this test case, the container metrics will be ignored
+	// and only the CPUTarget will be taken into consideration.
+
 	tc := testCase{
 		minReplicas:             2,
 		maxReplicas:             6,
 		specReplicas:            3,
 		statusReplicas:          3,
-		expectedDesiredReplicas: 3,
+		expectedDesiredReplicas: 4,
+		CPUTarget:               30,
+		verifyCPUCurrent:        true,
 		metricsTarget: []autoscalingv2.MetricSpec{{
 			Type: autoscalingv2.ContainerResourceMetricSourceType,
 			ContainerResource: &autoscalingv2.ContainerResourceMetricSource{
 				Name: v1.ResourceCPU,
 				Target: autoscalingv2.MetricTarget{
 					Type:               autoscalingv2.UtilizationMetricType,
-					AverageUtilization: pointer.Int32(30),
+					AverageUtilization: pointer.Int32(10),
 				},
 				Container: "container1",
 			},
 		}},
-		reportedLevels:      []uint64{300, 500, 700},
+		reportedLevels:      []uint64{300, 400, 500},
 		reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
-		useMetricsAPI:       true,
-		expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
-			Type:   autoscalingv2.AbleToScale,
-			Status: v1.ConditionTrue,
-			Reason: "ReadyForNewScale",
-		}),
 	}
+
 	tc.runTest(t)
 }