Skip to content

Commit 2dd9eda

Browse files
jm-francjm-franc
committed
Add configurable tolerance logic.
1 parent 11b6e2a commit 2dd9eda

File tree

4 files changed

+377
-44
lines changed

4 files changed

+377
-44
lines changed

‎pkg/controller/podautoscaler/horizontal.go

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"k8s.io/apimachinery/pkg/runtime/schema"
3838
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
3939
"k8s.io/apimachinery/pkg/util/wait"
40+
utilfeature "k8s.io/apiserver/pkg/util/feature"
4041
autoscalinginformers "k8s.io/client-go/informers/autoscaling/v2"
4142
coreinformers "k8s.io/client-go/informers/core/v1"
4243
"k8s.io/client-go/kubernetes/scheme"
@@ -53,6 +54,7 @@ import (
5354
metricsclient "k8s.io/kubernetes/pkg/controller/podautoscaler/metrics"
5455
"k8s.io/kubernetes/pkg/controller/podautoscaler/monitor"
5556
"k8s.io/kubernetes/pkg/controller/util/selectors"
57+
"k8s.io/kubernetes/pkg/features"
5658
)
5759

5860
var (
@@ -86,6 +88,7 @@ type HorizontalController struct {
8688
hpaNamespacer autoscalingclient.HorizontalPodAutoscalersGetter
8789
mapper apimeta.RESTMapper
8890

91+
tolerance float64
8992
replicaCalc *ReplicaCalculator
9093
eventRecorder record.EventRecorder
9194

@@ -146,6 +149,7 @@ func NewHorizontalController(
146149
eventRecorder: recorder,
147150
scaleNamespacer: scaleNamespacer,
148151
hpaNamespacer: hpaNamespacer,
152+
tolerance: tolerance,
149153
downscaleStabilisationWindow: downscaleStabilisationWindow,
150154
monitor: monitor.New(),
151155
queue: workqueue.NewTypedRateLimitingQueueWithConfig(
@@ -181,7 +185,6 @@ func NewHorizontalController(
181185
replicaCalc := NewReplicaCalculator(
182186
metricsClient,
183187
hpaController.podLister,
184-
tolerance,
185188
cpuInitializationPeriod,
186189
delayOfInitialReadinessStatus,
187190
)
@@ -539,8 +542,9 @@ func (a *HorizontalController) computeStatusForObjectMetric(specReplicas, status
539542
},
540543
},
541544
}
545+
tolerances := a.tolerancesForHpa(hpa)
542546
if metricSpec.Object.Target.Type == autoscalingv2.ValueMetricType && metricSpec.Object.Target.Value != nil {
543-
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetObjectMetricReplicas(specReplicas, metricSpec.Object.Target.Value.MilliValue(), metricSpec.Object.Metric.Name, hpa.Namespace, &metricSpec.Object.DescribedObject, selector, metricSelector)
547+
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetObjectMetricReplicas(specReplicas, metricSpec.Object.Target.Value.MilliValue(), metricSpec.Object.Metric.Name, tolerances, hpa.Namespace, &metricSpec.Object.DescribedObject, selector, metricSelector)
544548
if err != nil {
545549
condition := a.getUnableComputeReplicaCountCondition(hpa, "FailedGetObjectMetric", err)
546550
return 0, timestampProposal, "", condition, err
@@ -549,7 +553,7 @@ func (a *HorizontalController) computeStatusForObjectMetric(specReplicas, status
549553
*status = metricStatus
550554
return replicaCountProposal, timestampProposal, fmt.Sprintf("%s metric %s", metricSpec.Object.DescribedObject.Kind, metricSpec.Object.Metric.Name), autoscalingv2.HorizontalPodAutoscalerCondition{}, nil
551555
} else if metricSpec.Object.Target.Type == autoscalingv2.AverageValueMetricType && metricSpec.Object.Target.AverageValue != nil {
552-
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetObjectPerPodMetricReplicas(statusReplicas, metricSpec.Object.Target.AverageValue.MilliValue(), metricSpec.Object.Metric.Name, hpa.Namespace, &metricSpec.Object.DescribedObject, metricSelector)
556+
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetObjectPerPodMetricReplicas(statusReplicas, metricSpec.Object.Target.AverageValue.MilliValue(), metricSpec.Object.Metric.Name, tolerances, hpa.Namespace, &metricSpec.Object.DescribedObject, metricSelector)
553557
if err != nil {
554558
condition := a.getUnableComputeReplicaCountCondition(hpa, "FailedGetObjectMetric", err)
555559
return 0, time.Time{}, "", condition, fmt.Errorf("failed to get %s object metric: %v", metricSpec.Object.Metric.Name, err)
@@ -566,7 +570,8 @@ func (a *HorizontalController) computeStatusForObjectMetric(specReplicas, status
566570

567571
// computeStatusForPodsMetric computes the desired number of replicas for the specified metric of type PodsMetricSourceType.
568572
func (a *HorizontalController) computeStatusForPodsMetric(currentReplicas int32, metricSpec autoscalingv2.MetricSpec, hpa *autoscalingv2.HorizontalPodAutoscaler, selector labels.Selector, status *autoscalingv2.MetricStatus, metricSelector labels.Selector) (replicaCountProposal int32, timestampProposal time.Time, metricNameProposal string, condition autoscalingv2.HorizontalPodAutoscalerCondition, err error) {
569-
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetMetricReplicas(currentReplicas, metricSpec.Pods.Target.AverageValue.MilliValue(), metricSpec.Pods.Metric.Name, hpa.Namespace, selector, metricSelector)
573+
tolerances := a.tolerancesForHpa(hpa)
574+
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetMetricReplicas(currentReplicas, metricSpec.Pods.Target.AverageValue.MilliValue(), metricSpec.Pods.Metric.Name, tolerances, hpa.Namespace, selector, metricSelector)
570575
if err != nil {
571576
condition = a.getUnableComputeReplicaCountCondition(hpa, "FailedGetPodsMetric", err)
572577
return 0, timestampProposal, "", condition, err
@@ -588,12 +593,14 @@ func (a *HorizontalController) computeStatusForPodsMetric(currentReplicas int32,
588593
}
589594

590595
func (a *HorizontalController) computeStatusForResourceMetricGeneric(ctx context.Context, currentReplicas int32, target autoscalingv2.MetricTarget,
591-
resourceName v1.ResourceName, namespace string, container string, selector labels.Selector, sourceType autoscalingv2.MetricSourceType) (replicaCountProposal int32,
596+
resourceName v1.ResourceName, hpa *autoscalingv2.HorizontalPodAutoscaler, container string, selector labels.Selector, sourceType autoscalingv2.MetricSourceType) (replicaCountProposal int32,
592597
metricStatus *autoscalingv2.MetricValueStatus, timestampProposal time.Time, metricNameProposal string,
593598
condition autoscalingv2.HorizontalPodAutoscalerCondition, err error) {
599+
namespace := hpa.Namespace
600+
tolerances := a.tolerancesForHpa(hpa)
594601
if target.AverageValue != nil {
595602
var rawProposal int64
596-
replicaCountProposal, rawProposal, timestampProposal, err := a.replicaCalc.GetRawResourceReplicas(ctx, currentReplicas, target.AverageValue.MilliValue(), resourceName, namespace, selector, container)
603+
replicaCountProposal, rawProposal, timestampProposal, err := a.replicaCalc.GetRawResourceReplicas(ctx, currentReplicas, target.AverageValue.MilliValue(), resourceName, tolerances, namespace, selector, container)
597604
if err != nil {
598605
return 0, nil, time.Time{}, "", condition, fmt.Errorf("failed to get %s usage: %v", resourceName, err)
599606
}
@@ -610,7 +617,7 @@ func (a *HorizontalController) computeStatusForResourceMetricGeneric(ctx context
610617
}
611618

612619
targetUtilization := *target.AverageUtilization
613-
replicaCountProposal, percentageProposal, rawProposal, timestampProposal, err := a.replicaCalc.GetResourceReplicas(ctx, currentReplicas, targetUtilization, resourceName, namespace, selector, container)
620+
replicaCountProposal, percentageProposal, rawProposal, timestampProposal, err := a.replicaCalc.GetResourceReplicas(ctx, currentReplicas, targetUtilization, resourceName, tolerances, namespace, selector, container)
614621
if err != nil {
615622
return 0, nil, time.Time{}, "", condition, fmt.Errorf("failed to get %s utilization: %v", resourceName, err)
616623
}
@@ -630,7 +637,7 @@ func (a *HorizontalController) computeStatusForResourceMetricGeneric(ctx context
630637
func (a *HorizontalController) computeStatusForResourceMetric(ctx context.Context, currentReplicas int32, metricSpec autoscalingv2.MetricSpec, hpa *autoscalingv2.HorizontalPodAutoscaler,
631638
selector labels.Selector, status *autoscalingv2.MetricStatus) (replicaCountProposal int32, timestampProposal time.Time,
632639
metricNameProposal string, condition autoscalingv2.HorizontalPodAutoscalerCondition, err error) {
633-
replicaCountProposal, metricValueStatus, timestampProposal, metricNameProposal, condition, err := a.computeStatusForResourceMetricGeneric(ctx, currentReplicas, metricSpec.Resource.Target, metricSpec.Resource.Name, hpa.Namespace, "", selector, autoscalingv2.ResourceMetricSourceType)
640+
replicaCountProposal, metricValueStatus, timestampProposal, metricNameProposal, condition, err := a.computeStatusForResourceMetricGeneric(ctx, currentReplicas, metricSpec.Resource.Target, metricSpec.Resource.Name, hpa, "", selector, autoscalingv2.ResourceMetricSourceType)
634641
if err != nil {
635642
condition = a.getUnableComputeReplicaCountCondition(hpa, "FailedGetResourceMetric", err)
636643
return replicaCountProposal, timestampProposal, metricNameProposal, condition, err
@@ -649,7 +656,7 @@ func (a *HorizontalController) computeStatusForResourceMetric(ctx context.Contex
649656
func (a *HorizontalController) computeStatusForContainerResourceMetric(ctx context.Context, currentReplicas int32, metricSpec autoscalingv2.MetricSpec, hpa *autoscalingv2.HorizontalPodAutoscaler,
650657
selector labels.Selector, status *autoscalingv2.MetricStatus) (replicaCountProposal int32, timestampProposal time.Time,
651658
metricNameProposal string, condition autoscalingv2.HorizontalPodAutoscalerCondition, err error) {
652-
replicaCountProposal, metricValueStatus, timestampProposal, metricNameProposal, condition, err := a.computeStatusForResourceMetricGeneric(ctx, currentReplicas, metricSpec.ContainerResource.Target, metricSpec.ContainerResource.Name, hpa.Namespace, metricSpec.ContainerResource.Container, selector, autoscalingv2.ContainerResourceMetricSourceType)
659+
replicaCountProposal, metricValueStatus, timestampProposal, metricNameProposal, condition, err := a.computeStatusForResourceMetricGeneric(ctx, currentReplicas, metricSpec.ContainerResource.Target, metricSpec.ContainerResource.Name, hpa, metricSpec.ContainerResource.Container, selector, autoscalingv2.ContainerResourceMetricSourceType)
653660
if err != nil {
654661
condition = a.getUnableComputeReplicaCountCondition(hpa, "FailedGetContainerResourceMetric", err)
655662
return replicaCountProposal, timestampProposal, metricNameProposal, condition, err
@@ -667,8 +674,9 @@ func (a *HorizontalController) computeStatusForContainerResourceMetric(ctx conte
667674

668675
// computeStatusForExternalMetric computes the desired number of replicas for the specified metric of type ExternalMetricSourceType.
669676
func (a *HorizontalController) computeStatusForExternalMetric(specReplicas, statusReplicas int32, metricSpec autoscalingv2.MetricSpec, hpa *autoscalingv2.HorizontalPodAutoscaler, selector labels.Selector, status *autoscalingv2.MetricStatus) (replicaCountProposal int32, timestampProposal time.Time, metricNameProposal string, condition autoscalingv2.HorizontalPodAutoscalerCondition, err error) {
677+
tolerances := a.tolerancesForHpa(hpa)
670678
if metricSpec.External.Target.AverageValue != nil {
671-
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetExternalPerPodMetricReplicas(statusReplicas, metricSpec.External.Target.AverageValue.MilliValue(), metricSpec.External.Metric.Name, hpa.Namespace, metricSpec.External.Metric.Selector)
679+
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetExternalPerPodMetricReplicas(statusReplicas, metricSpec.External.Target.AverageValue.MilliValue(), metricSpec.External.Metric.Name, tolerances, hpa.Namespace, metricSpec.External.Metric.Selector)
672680
if err != nil {
673681
condition = a.getUnableComputeReplicaCountCondition(hpa, "FailedGetExternalMetric", err)
674682
return 0, time.Time{}, "", condition, fmt.Errorf("failed to get %s external metric: %v", metricSpec.External.Metric.Name, err)
@@ -688,7 +696,7 @@ func (a *HorizontalController) computeStatusForExternalMetric(specReplicas, stat
688696
return replicaCountProposal, timestampProposal, fmt.Sprintf("external metric %s(%+v)", metricSpec.External.Metric.Name, metricSpec.External.Metric.Selector), autoscalingv2.HorizontalPodAutoscalerCondition{}, nil
689697
}
690698
if metricSpec.External.Target.Value != nil {
691-
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetExternalMetricReplicas(specReplicas, metricSpec.External.Target.Value.MilliValue(), metricSpec.External.Metric.Name, hpa.Namespace, metricSpec.External.Metric.Selector, selector)
699+
replicaCountProposal, usageProposal, timestampProposal, err := a.replicaCalc.GetExternalMetricReplicas(specReplicas, metricSpec.External.Target.Value.MilliValue(), metricSpec.External.Metric.Name, tolerances, hpa.Namespace, metricSpec.External.Metric.Selector, selector)
692700
if err != nil {
693701
condition = a.getUnableComputeReplicaCountCondition(hpa, "FailedGetExternalMetric", err)
694702
return 0, time.Time{}, "", condition, fmt.Errorf("failed to get external metric %s: %v", metricSpec.External.Metric.Name, err)
@@ -835,6 +843,7 @@ func (a *HorizontalController) reconcileAutoscaler(ctx context.Context, hpaShare
835843
logger.V(4).Info("Proposing desired replicas",
836844
"desiredReplicas", metricDesiredReplicas,
837845
"metric", metricName,
846+
"tolerances", a.tolerancesForHpa(hpa),
838847
"timestamp", metricTimestamp,
839848
"scaleTarget", reference)
840849

@@ -1384,6 +1393,25 @@ func (a *HorizontalController) updateStatus(ctx context.Context, hpa *autoscalin
13841393
return nil
13851394
}
13861395

1396+
// tolerancesForHpa returns the metrics usage ratio tolerances for a given HPA.
1397+
// It ignores configurable tolerances set in the HPA spec.behavior field if the
1398+
// HPAConfigurableTolerance feature gate is disabled.
1399+
func (a *HorizontalController) tolerancesForHpa(hpa *autoscalingv2.HorizontalPodAutoscaler) Tolerances {
1400+
t := Tolerances{a.tolerance, a.tolerance}
1401+
behavior := hpa.Spec.Behavior
1402+
allowConfigurableTolerances := utilfeature.DefaultFeatureGate.Enabled(features.HPAConfigurableTolerance)
1403+
if behavior == nil || !allowConfigurableTolerances {
1404+
return t
1405+
}
1406+
if behavior.ScaleDown != nil && behavior.ScaleDown.Tolerance != nil {
1407+
t.scaleDown = behavior.ScaleDown.Tolerance.AsApproximateFloat64()
1408+
}
1409+
if behavior.ScaleUp != nil && behavior.ScaleUp.Tolerance != nil {
1410+
t.scaleUp = behavior.ScaleUp.Tolerance.AsApproximateFloat64()
1411+
}
1412+
return t
1413+
}
1414+
13871415
// setCondition sets the specific condition type on the given HPA to the specified value with the given reason
13881416
// and message. The message and args are treated like a format string. The condition will be added if it is
13891417
// not present.

‎pkg/controller/podautoscaler/horizontal_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,19 @@ import (
3737
"k8s.io/apimachinery/pkg/runtime/schema"
3838
"k8s.io/apimachinery/pkg/util/wait"
3939
"k8s.io/apimachinery/pkg/watch"
40+
utilfeature "k8s.io/apiserver/pkg/util/feature"
4041
"k8s.io/client-go/informers"
4142
"k8s.io/client-go/kubernetes/fake"
4243
scalefake "k8s.io/client-go/scale/fake"
4344
core "k8s.io/client-go/testing"
45+
featuregatetesting "k8s.io/component-base/featuregate/testing"
4446
"k8s.io/kubernetes/pkg/api/legacyscheme"
4547
autoscalingapiv2 "k8s.io/kubernetes/pkg/apis/autoscaling/v2"
4648
"k8s.io/kubernetes/pkg/controller"
4749
"k8s.io/kubernetes/pkg/controller/podautoscaler/metrics"
4850
"k8s.io/kubernetes/pkg/controller/podautoscaler/monitor"
4951
"k8s.io/kubernetes/pkg/controller/util/selectors"
52+
"k8s.io/kubernetes/pkg/features"
5053
"k8s.io/kubernetes/test/utils/ktesting"
5154
cmapi "k8s.io/metrics/pkg/apis/custom_metrics/v1beta2"
5255
emapi "k8s.io/metrics/pkg/apis/external_metrics/v1beta1"
@@ -2216,6 +2219,107 @@ func TestTolerance(t *testing.T) {
22162219
tc.runTest(t)
22172220
}
22182221

2222+
func TestConfigurableTolerance(t *testing.T) {
2223+
onePercentQuantity := resource.MustParse("0.01")
2224+
ninetyPercentQuantity := resource.MustParse("0.9")
2225+
2226+
testCases := []struct {
2227+
name string
2228+
configurableToleranceGate bool
2229+
replicas int32
2230+
scaleUpRules *autoscalingv2.HPAScalingRules
2231+
scaleDownRules *autoscalingv2.HPAScalingRules
2232+
reportedLevels []uint64
2233+
reportedCPURequests []resource.Quantity
2234+
expectedDesiredReplicas int32
2235+
expectedConditionReason string
2236+
expectedActionLabel monitor.ActionLabel
2237+
}{
2238+
{
2239+
name: "Scaling up because of a 1% configurable tolerance",
2240+
configurableToleranceGate: true,
2241+
replicas: 3,
2242+
scaleUpRules: &autoscalingv2.HPAScalingRules{
2243+
Tolerance: &onePercentQuantity,
2244+
},
2245+
reportedLevels: []uint64{1010, 1030, 1020},
2246+
reportedCPURequests: []resource.Quantity{resource.MustParse("0.9"), resource.MustParse("1.0"), resource.MustParse("1.1")},
2247+
expectedDesiredReplicas: 4,
2248+
expectedConditionReason: "SucceededRescale",
2249+
expectedActionLabel: monitor.ActionLabelScaleUp,
2250+
},
2251+
{
2252+
name: "No scale-down because of a 90% configurable tolerance",
2253+
configurableToleranceGate: true,
2254+
replicas: 3,
2255+
scaleDownRules: &autoscalingv2.HPAScalingRules{
2256+
Tolerance: &ninetyPercentQuantity,
2257+
},
2258+
reportedLevels: []uint64{300, 300, 300},
2259+
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
2260+
expectedDesiredReplicas: 3,
2261+
expectedConditionReason: "ReadyForNewScale",
2262+
expectedActionLabel: monitor.ActionLabelNone,
2263+
},
2264+
{
2265+
name: "No scaling because of the large default tolerance",
2266+
configurableToleranceGate: true,
2267+
replicas: 3,
2268+
reportedLevels: []uint64{1010, 1030, 1020},
2269+
reportedCPURequests: []resource.Quantity{resource.MustParse("0.9"), resource.MustParse("1.0"), resource.MustParse("1.1")},
2270+
expectedDesiredReplicas: 3,
2271+
expectedConditionReason: "ReadyForNewScale",
2272+
expectedActionLabel: monitor.ActionLabelNone,
2273+
},
2274+
{
2275+
name: "No scaling because the configurable tolerance is ignored as the feature gate is disabled",
2276+
configurableToleranceGate: false,
2277+
replicas: 3,
2278+
scaleUpRules: &autoscalingv2.HPAScalingRules{
2279+
Tolerance: &onePercentQuantity,
2280+
},
2281+
reportedLevels: []uint64{1010, 1030, 1020},
2282+
reportedCPURequests: []resource.Quantity{resource.MustParse("0.9"), resource.MustParse("1.0"), resource.MustParse("1.1")},
2283+
expectedDesiredReplicas: 3,
2284+
expectedConditionReason: "ReadyForNewScale",
2285+
expectedActionLabel: monitor.ActionLabelNone,
2286+
},
2287+
}
2288+
2289+
for _, tc := range testCases {
2290+
t.Run(tc.name, func(t *testing.T) {
2291+
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HPAConfigurableTolerance, tc.configurableToleranceGate)
2292+
tc := testCase{
2293+
minReplicas: 1,
2294+
maxReplicas: 5,
2295+
specReplicas: tc.replicas,
2296+
statusReplicas: tc.replicas,
2297+
scaleDownRules: tc.scaleDownRules,
2298+
scaleUpRules: tc.scaleUpRules,
2299+
expectedDesiredReplicas: tc.expectedDesiredReplicas,
2300+
CPUTarget: 100,
2301+
reportedLevels: tc.reportedLevels,
2302+
reportedCPURequests: tc.reportedCPURequests,
2303+
useMetricsAPI: true,
2304+
expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
2305+
Type: autoscalingv2.AbleToScale,
2306+
Status: v1.ConditionTrue,
2307+
Reason: tc.expectedConditionReason,
2308+
}),
2309+
expectedReportedReconciliationActionLabel: tc.expectedActionLabel,
2310+
expectedReportedReconciliationErrorLabel: monitor.ErrorLabelNone,
2311+
expectedReportedMetricComputationActionLabels: map[autoscalingv2.MetricSourceType]monitor.ActionLabel{
2312+
autoscalingv2.ResourceMetricSourceType: tc.expectedActionLabel,
2313+
},
2314+
expectedReportedMetricComputationErrorLabels: map[autoscalingv2.MetricSourceType]monitor.ErrorLabel{
2315+
autoscalingv2.ResourceMetricSourceType: monitor.ErrorLabelNone,
2316+
},
2317+
}
2318+
tc.runTest(t)
2319+
})
2320+
}
2321+
}
2322+
22192323
func TestToleranceCM(t *testing.T) {
22202324
averageValue := resource.MustParse("20.0")
22212325
tc := testCase{

0 commit comments

Comments
 (0)