fix based on the suggestion

This commit is contained in:
Kensei Nakada 2023-03-05 15:01:34 +00:00
parent 33daba24fb
commit f76258f0ff
2 changed files with 13 additions and 15 deletions

View File

@ -431,10 +431,9 @@ func (a *HorizontalController) computeReplicasForMetric(ctx context.Context, hpa
case autoscalingv2.ContainerResourceMetricSourceType: case autoscalingv2.ContainerResourceMetricSourceType:
if !a.containerResourceMetricsEnabled { if !a.containerResourceMetricsEnabled {
// If the container resource metrics feature is disabled but the object has the one, // 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. // that means the user enabled the feature once,
// We cannot return errors in this case because that'll result in all HPAs with the container resource metric sources failing to scale down. // created some HPAs with the container resource metrics, and disabled it finally.
// Thus, here we silently ignore it and return current replica values so that it won't affect the autoscaling decision return 0, "", time.Time{}, condition, fmt.Errorf("the container resource metrics feature is disabled by the feature gate")
return specReplicas, "", time.Time{}, condition, nil
} }
replicaCountProposal, timestampProposal, metricNameProposal, condition, err = a.computeStatusForContainerResourceMetric(ctx, specReplicas, spec, hpa, selector, status) replicaCountProposal, timestampProposal, metricNameProposal, condition, err = a.computeStatusForContainerResourceMetric(ctx, specReplicas, spec, hpa, selector, status)
if err != nil { if err != nil {

View File

@ -837,34 +837,33 @@ func TestScaleUpContainer(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestIgnoreContainerMetric(t *testing.T) { func TestContainerMetricWithTheFeatureGateDisabled(t *testing.T) {
// when the container metric feature isn't enabled, it's ignored and HPA keeps the current replica. // In this test case, the container metrics will be ignored
// and only the CPUTarget will be taken into consideration.
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
specReplicas: 3, specReplicas: 3,
statusReplicas: 3, statusReplicas: 3,
expectedDesiredReplicas: 3, expectedDesiredReplicas: 4,
CPUTarget: 30,
verifyCPUCurrent: true,
metricsTarget: []autoscalingv2.MetricSpec{{ metricsTarget: []autoscalingv2.MetricSpec{{
Type: autoscalingv2.ContainerResourceMetricSourceType, Type: autoscalingv2.ContainerResourceMetricSourceType,
ContainerResource: &autoscalingv2.ContainerResourceMetricSource{ ContainerResource: &autoscalingv2.ContainerResourceMetricSource{
Name: v1.ResourceCPU, Name: v1.ResourceCPU,
Target: autoscalingv2.MetricTarget{ Target: autoscalingv2.MetricTarget{
Type: autoscalingv2.UtilizationMetricType, Type: autoscalingv2.UtilizationMetricType,
AverageUtilization: pointer.Int32(30), AverageUtilization: pointer.Int32(10),
}, },
Container: "container1", 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")}, 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) tc.runTest(t)
} }