graduate HPAContainerMetrics to stable

This commit is contained in:
Kensei Nakada 2024-02-24 08:22:28 +00:00
parent 781da75951
commit 07e0a80216
7 changed files with 24 additions and 233 deletions

View File

@ -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"
@ -91,7 +89,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
}

View File

@ -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:

View File

@ -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.
@ -138,7 +135,6 @@ func NewHorizontalController(
tolerance float64,
cpuInitializationPeriod,
delayOfInitialReadinessStatus time.Duration,
containerResourceMetricsEnabled bool,
) *HorizontalController {
broadcaster := record.NewBroadcaster()
broadcaster.StartStructuredLogging(0)
@ -146,21 +142,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(
@ -475,12 +470,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)

View File

@ -156,8 +156,6 @@ type testCase struct {
recommendations []timestampedRecommendation
hpaSelectors *selectors.BiMultimap
containerResourceMetricsEnabled bool
}
// Needs to be called under a lock.
@ -780,7 +778,6 @@ func (tc *testCase) setupController(t *testing.T) (*HorizontalController, inform
defaultTestingTolerance,
defaultTestingCPUInitializationPeriod,
defaultTestingDelayOfInitialReadinessStatus,
tc.containerResourceMetricsEnabled,
)
hpaController.hpaListerSynced = alwaysReady
if tc.recommendations != nil {
@ -929,10 +926,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{
@ -945,46 +941,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,
@ -1663,9 +1619,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{
@ -5305,7 +5260,6 @@ func TestMultipleHPAs(t *testing.T) {
defaultTestingTolerance,
defaultTestingCPUInitializationPeriod,
defaultTestingDelayOfInitialReadinessStatus,
false,
)
hpaController.scaleUpEvents = scaleUpEventsMap
hpaController.scaleDownEvents = scaleDownEventsMap

View File

@ -280,6 +280,7 @@ const (
// kep: https://kep.k8s.io/1610
// alpha: v1.20
// beta: v1.27
// beta: v1.30
//
// Add support for the HPA to scale based on metrics from individual containers
// in target pods
@ -994,7 +995,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},
HonorPVReclaimPolicy: {Default: false, PreRelease: featuregate.Alpha},

View File

@ -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"
)
@ -67,16 +65,7 @@ func (autoscalerStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.S
}
// PrepareForCreate clears fields that are not allowed to be set by end users on creation.
func (autoscalerStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
newHPA := obj.(*autoscaling.HorizontalPodAutoscaler)
// create cannot set status
newHPA.Status = autoscaling.HorizontalPodAutoscalerStatus{}
if !utilfeature.DefaultFeatureGate.Enabled(features.HPAContainerMetrics) {
dropContainerMetricSources(newHPA.Spec.Metrics)
}
}
func (autoscalerStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {}
// Validate validates a new autoscaler.
func (autoscalerStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
@ -99,32 +88,7 @@ func (autoscalerStrategy) AllowCreateOnUpdate() bool {
}
// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
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
}
func (autoscalerStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {}
// ValidateUpdate is the default update validation for an end user.
func (autoscalerStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {

View File

@ -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")
}
})
}
}