diff --git a/cmd/kube-controller-manager/app/autoscaling.go b/cmd/kube-controller-manager/app/autoscaling.go index 79b735c922468..ff0c6d60f30ea 100644 --- a/cmd/kube-controller-manager/app/autoscaling.go +++ b/cmd/kube-controller-manager/app/autoscaling.go @@ -22,14 +22,12 @@ package app import ( "context" - "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" "k8s.io/client-go/scale" "k8s.io/controller-manager/controller" "k8s.io/kubernetes/cmd/kube-controller-manager/names" "k8s.io/kubernetes/pkg/controller/podautoscaler" "k8s.io/kubernetes/pkg/controller/podautoscaler/metrics" - "k8s.io/kubernetes/pkg/features" resourceclient "k8s.io/metrics/pkg/client/clientset/versioned/typed/metrics/v1beta1" "k8s.io/metrics/pkg/client/custom_metrics" @@ -92,7 +90,6 @@ func startHPAControllerWithMetricsClient(ctx context.Context, controllerContext controllerContext.ComponentConfig.HPAController.HorizontalPodAutoscalerTolerance, controllerContext.ComponentConfig.HPAController.HorizontalPodAutoscalerCPUInitializationPeriod.Duration, controllerContext.ComponentConfig.HPAController.HorizontalPodAutoscalerInitialReadinessDelay.Duration, - feature.DefaultFeatureGate.Enabled(features.HPAContainerMetrics), ).Run(ctx, int(controllerContext.ComponentConfig.HPAController.ConcurrentHorizontalPodAutoscalerSyncs)) return nil, true, nil } diff --git a/pkg/apis/autoscaling/validation/validation.go b/pkg/apis/autoscaling/validation/validation.go index 516f795995c8f..bd986816cfd96 100644 --- a/pkg/apis/autoscaling/validation/validation.go +++ b/pkg/apis/autoscaling/validation/validation.go @@ -317,11 +317,7 @@ func validateMetricSpec(spec autoscaling.MetricSpec, fldPath *field.Path) field. expectedField = "external" case autoscaling.ContainerResourceMetricSourceType: if spec.ContainerResource == nil { - if utilfeature.DefaultFeatureGate.Enabled(features.HPAContainerMetrics) { - allErrs = append(allErrs, field.Required(fldPath.Child("containerResource"), "must populate information for the given metric source")) - } else { - allErrs = append(allErrs, field.Required(fldPath.Child("containerResource"), "must populate information for the given metric source (only allowed when HPAContainerMetrics feature is enabled)")) - } + allErrs = append(allErrs, field.Required(fldPath.Child("containerResource"), "must populate information for the given metric source")) } expectedField = "containerResource" default: diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go index 3c4c3c7bcb2e4..3b179a89077d0 100644 --- a/pkg/controller/podautoscaler/horizontal.go +++ b/pkg/controller/podautoscaler/horizontal.go @@ -119,9 +119,6 @@ type HorizontalController struct { // Storage of HPAs and their selectors. hpaSelectors *selectors.BiMultimap hpaSelectorsMux sync.Mutex - - // feature gates - containerResourceMetricsEnabled bool } // NewHorizontalController creates a new HorizontalController. @@ -139,7 +136,6 @@ func NewHorizontalController( tolerance float64, cpuInitializationPeriod, delayOfInitialReadinessStatus time.Duration, - containerResourceMetricsEnabled bool, ) *HorizontalController { broadcaster := record.NewBroadcaster(record.WithContext(ctx)) broadcaster.StartStructuredLogging(3) @@ -147,21 +143,20 @@ func NewHorizontalController( recorder := broadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "horizontal-pod-autoscaler"}) hpaController := &HorizontalController{ - eventRecorder: recorder, - scaleNamespacer: scaleNamespacer, - hpaNamespacer: hpaNamespacer, - downscaleStabilisationWindow: downscaleStabilisationWindow, - monitor: monitor.New(), - queue: workqueue.NewNamedRateLimitingQueue(NewDefaultHPARateLimiter(resyncPeriod), "horizontalpodautoscaler"), - mapper: mapper, - recommendations: map[string][]timestampedRecommendation{}, - recommendationsLock: sync.Mutex{}, - scaleUpEvents: map[string][]timestampedScaleEvent{}, - scaleUpEventsLock: sync.RWMutex{}, - scaleDownEvents: map[string][]timestampedScaleEvent{}, - scaleDownEventsLock: sync.RWMutex{}, - hpaSelectors: selectors.NewBiMultimap(), - containerResourceMetricsEnabled: containerResourceMetricsEnabled, + eventRecorder: recorder, + scaleNamespacer: scaleNamespacer, + hpaNamespacer: hpaNamespacer, + downscaleStabilisationWindow: downscaleStabilisationWindow, + monitor: monitor.New(), + queue: workqueue.NewNamedRateLimitingQueue(NewDefaultHPARateLimiter(resyncPeriod), "horizontalpodautoscaler"), + mapper: mapper, + recommendations: map[string][]timestampedRecommendation{}, + recommendationsLock: sync.Mutex{}, + scaleUpEvents: map[string][]timestampedScaleEvent{}, + scaleUpEventsLock: sync.RWMutex{}, + scaleDownEvents: map[string][]timestampedScaleEvent{}, + scaleDownEventsLock: sync.RWMutex{}, + hpaSelectors: selectors.NewBiMultimap(), } hpaInformer.Informer().AddEventHandlerWithResyncPeriod( @@ -476,12 +471,6 @@ func (a *HorizontalController) computeReplicasForMetric(ctx context.Context, hpa return 0, "", time.Time{}, condition, fmt.Errorf("failed to get %s resource metric value: %v", spec.Resource.Name, err) } 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. - return 0, "", time.Time{}, condition, fmt.Errorf("ContainerResource metric type is not supported: disabled by the feature gate") - } replicaCountProposal, timestampProposal, metricNameProposal, condition, err = a.computeStatusForContainerResourceMetric(ctx, specReplicas, spec, hpa, selector, status) if err != nil { return 0, "", time.Time{}, condition, fmt.Errorf("failed to get %s container metric value: %v", spec.ContainerResource.Container, err) diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go index 6d48d6905b82a..0f2fb2ed99d9d 100644 --- a/pkg/controller/podautoscaler/horizontal_test.go +++ b/pkg/controller/podautoscaler/horizontal_test.go @@ -157,8 +157,6 @@ type testCase struct { recommendations []timestampedRecommendation hpaSelectors *selectors.BiMultimap - - containerResourceMetricsEnabled bool } // Needs to be called under a lock. @@ -783,7 +781,6 @@ func (tc *testCase) setupController(t *testing.T) (*HorizontalController, inform defaultTestingTolerance, defaultTestingCPUInitializationPeriod, defaultTestingDelayOfInitialReadinessStatus, - tc.containerResourceMetricsEnabled, ) hpaController.hpaListerSynced = alwaysReady if tc.recommendations != nil { @@ -932,10 +929,9 @@ func TestScaleUpContainer(t *testing.T) { Container: "container1", }, }}, - reportedLevels: []uint64{300, 500, 700}, - reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, - useMetricsAPI: true, - containerResourceMetricsEnabled: true, + reportedLevels: []uint64{300, 500, 700}, + reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, + useMetricsAPI: true, expectedReportedReconciliationActionLabel: monitor.ActionLabelScaleUp, expectedReportedReconciliationErrorLabel: monitor.ErrorLabelNone, expectedReportedMetricComputationActionLabels: map[autoscalingv2.MetricSourceType]monitor.ActionLabel{ @@ -948,46 +944,6 @@ func TestScaleUpContainer(t *testing.T) { tc.runTest(t) } -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: 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(10), - }, - Container: "container1", - }, - }}, - reportedLevels: []uint64{300, 400, 500}, - reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, - expectedReportedReconciliationActionLabel: monitor.ActionLabelScaleUp, - expectedReportedReconciliationErrorLabel: monitor.ErrorLabelInternal, - expectedReportedMetricComputationActionLabels: map[autoscalingv2.MetricSourceType]monitor.ActionLabel{ - autoscalingv2.ResourceMetricSourceType: monitor.ActionLabelScaleUp, - autoscalingv2.ContainerResourceMetricSourceType: monitor.ActionLabelNone, - }, - expectedReportedMetricComputationErrorLabels: map[autoscalingv2.MetricSourceType]monitor.ErrorLabel{ - autoscalingv2.ResourceMetricSourceType: monitor.ErrorLabelNone, - autoscalingv2.ContainerResourceMetricSourceType: monitor.ErrorLabelInternal, - }, - } - - tc.runTest(t) -} - func TestScaleUpUnreadyLessScale(t *testing.T) { tc := testCase{ minReplicas: 2, @@ -1666,9 +1622,8 @@ func TestScaleDownContainerResource(t *testing.T) { }, }, }}, - useMetricsAPI: true, - containerResourceMetricsEnabled: true, - recommendations: []timestampedRecommendation{}, + useMetricsAPI: true, + recommendations: []timestampedRecommendation{}, expectedReportedReconciliationActionLabel: monitor.ActionLabelScaleDown, expectedReportedReconciliationErrorLabel: monitor.ErrorLabelNone, expectedReportedMetricComputationActionLabels: map[autoscalingv2.MetricSourceType]monitor.ActionLabel{ @@ -5310,7 +5265,6 @@ func TestMultipleHPAs(t *testing.T) { defaultTestingTolerance, defaultTestingCPUInitializationPeriod, defaultTestingDelayOfInitialReadinessStatus, - false, ) hpaController.scaleUpEvents = scaleUpEventsMap hpaController.scaleDownEvents = scaleDownEventsMap diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 49e3312ff598b..75056629902f3 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -277,6 +277,7 @@ const ( // kep: https://kep.k8s.io/1610 // alpha: v1.20 // beta: v1.27 + // GA: v1.30 // // Add support for the HPA to scale based on metrics from individual containers // in target pods @@ -1046,7 +1047,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS GracefulNodeShutdownBasedOnPodPriority: {Default: true, PreRelease: featuregate.Beta}, - HPAContainerMetrics: {Default: true, PreRelease: featuregate.Beta}, + HPAContainerMetrics: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 HonorPVReclaimPolicy: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/registry/autoscaling/horizontalpodautoscaler/strategy.go b/pkg/registry/autoscaling/horizontalpodautoscaler/strategy.go index 7e6fadf662dcf..8558440f7aaad 100644 --- a/pkg/registry/autoscaling/horizontalpodautoscaler/strategy.go +++ b/pkg/registry/autoscaling/horizontalpodautoscaler/strategy.go @@ -22,11 +22,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/autoscaling" "k8s.io/kubernetes/pkg/apis/autoscaling/validation" - "k8s.io/kubernetes/pkg/features" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -72,10 +70,6 @@ func (autoscalerStrategy) PrepareForCreate(ctx context.Context, obj runtime.Obje // create cannot set status newHPA.Status = autoscaling.HorizontalPodAutoscalerStatus{} - - if !utilfeature.DefaultFeatureGate.Enabled(features.HPAContainerMetrics) { - dropContainerMetricSources(newHPA.Spec.Metrics) - } } // Validate validates a new autoscaler. @@ -102,30 +96,10 @@ func (autoscalerStrategy) AllowCreateOnUpdate() bool { func (autoscalerStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { newHPA := obj.(*autoscaling.HorizontalPodAutoscaler) oldHPA := old.(*autoscaling.HorizontalPodAutoscaler) - if !utilfeature.DefaultFeatureGate.Enabled(features.HPAContainerMetrics) && !hasContainerMetricSources(oldHPA) { - dropContainerMetricSources(newHPA.Spec.Metrics) - } // Update is not allowed to set status newHPA.Status = oldHPA.Status } -// dropContainerMetricSources ensures all container resource metric sources are nil -func dropContainerMetricSources(metrics []autoscaling.MetricSpec) { - for i := range metrics { - metrics[i].ContainerResource = nil - } -} - -// hasContainerMetricSources returns true if the hpa has any container resource metric sources -func hasContainerMetricSources(hpa *autoscaling.HorizontalPodAutoscaler) bool { - for i := range hpa.Spec.Metrics { - if hpa.Spec.Metrics[i].ContainerResource != nil { - return true - } - } - return false -} - // ValidateUpdate is the default update validation for an end user. func (autoscalerStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { return validation.ValidateHorizontalPodAutoscalerUpdate(obj.(*autoscaling.HorizontalPodAutoscaler), old.(*autoscaling.HorizontalPodAutoscaler)) diff --git a/pkg/registry/autoscaling/horizontalpodautoscaler/strategy_test.go b/pkg/registry/autoscaling/horizontalpodautoscaler/strategy_test.go deleted file mode 100644 index 8bc41be06121d..0000000000000 --- a/pkg/registry/autoscaling/horizontalpodautoscaler/strategy_test.go +++ /dev/null @@ -1,110 +0,0 @@ -/* -Copyright 2015 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package horizontalpodautoscaler - -import ( - "context" - "testing" - - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/apis/autoscaling" - "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" - "k8s.io/utils/pointer" -) - -func makeTestContainerMetricsHPA(hasContainerMetric bool) *autoscaling.HorizontalPodAutoscaler { - testHPA := &autoscaling.HorizontalPodAutoscaler{ - Spec: autoscaling.HorizontalPodAutoscalerSpec{ - Metrics: []autoscaling.MetricSpec{}, - }, - } - if hasContainerMetric { - testHPA.Spec.Metrics = append(testHPA.Spec.Metrics, autoscaling.MetricSpec{ - Type: autoscaling.ContainerResourceMetricSourceType, - ContainerResource: &autoscaling.ContainerResourceMetricSource{ - Name: core.ResourceCPU, - Container: "test-container", - Target: autoscaling.MetricTarget{ - Type: autoscaling.UtilizationMetricType, - AverageUtilization: pointer.Int32Ptr(30), - }, - }, - }) - } - return testHPA -} - -func TestCreateWithFeatureEnabled(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HPAContainerMetrics, true)() - testHPA := makeTestContainerMetricsHPA(true) - Strategy.PrepareForCreate(context.Background(), testHPA) - if testHPA.Spec.Metrics[0].ContainerResource == nil { - t.Errorf("container metrics was set to nil") - } -} - -func TestCreateWithFeatureDisabled(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HPAContainerMetrics, false)() - testHPA := makeTestContainerMetricsHPA(true) - Strategy.PrepareForCreate(context.Background(), testHPA) - if testHPA.Spec.Metrics[0].ContainerResource != nil { - t.Errorf("container metrics is not nil") - } -} - -func TestAutoscalerStatusStrategy_PrepareForUpdate(t *testing.T) { - for _, tc := range []struct { - name string - featureEnabled bool - old bool - expectedNew bool - }{ - { - name: "feature disabled with existing container metrics", - featureEnabled: false, - old: true, - expectedNew: true, - }, - { - name: "feature enabled with no container metrics", - featureEnabled: true, - old: false, - expectedNew: true, - }, - { - name: "feature enabled with existing container metrics", - featureEnabled: true, - old: true, - expectedNew: true, - }, - } { - t.Run(tc.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HPAContainerMetrics, tc.featureEnabled)() - oldHPA := makeTestContainerMetricsHPA(tc.old) - newHPA := makeTestContainerMetricsHPA(true) - Strategy.PrepareForUpdate(context.Background(), newHPA, oldHPA) - if tc.expectedNew && newHPA.Spec.Metrics[0].ContainerResource == nil { - t.Errorf("container metric source is nil") - } - if !tc.expectedNew && newHPA.Spec.Metrics[0].ContainerResource != nil { - t.Errorf("container metric source is not nil") - } - }) - } -}