From 7d7c48a647a787ffafcdafd8c56bb38664011bc6 Mon Sep 17 00:00:00 2001 From: Joachim Bartosik Date: Wed, 19 Sep 2018 13:11:59 +0200 Subject: [PATCH] HPA stabilizes initial recommendation HPA will treat initial size of autoscalee to avoid hastily overriding recomendations made by HPA (if HPA set size and then was restarted) or by user (initial size should be treated as human-generated recommendation). --- pkg/controller/podautoscaler/horizontal.go | 7 +++ .../podautoscaler/horizontal_test.go | 46 +++++++++++++++++-- .../podautoscaler/legacy_horizontal_test.go | 10 +++- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go index b899f267863..2e1c763084a 100644 --- a/pkg/controller/podautoscaler/horizontal.go +++ b/pkg/controller/podautoscaler/horizontal.go @@ -464,6 +464,12 @@ func (a *HorizontalController) computeStatusForExternalMetric(currentReplicas in return 0, time.Time{}, "", fmt.Errorf(errMsg) } +func (a *HorizontalController) recordInitialRecommendation(currentReplicas int32, key string) { + if a.recommendations[key] == nil { + a.recommendations[key] = []timestampedRecommendation{{currentReplicas, time.Now()}} + } +} + func (a *HorizontalController) reconcileAutoscaler(hpav1Shared *autoscalingv1.HorizontalPodAutoscaler, key string) error { // make a copy so that we never mutate the shared informer cache (conversion can mutate the object) hpav1 := hpav1Shared.DeepCopy() @@ -508,6 +514,7 @@ func (a *HorizontalController) reconcileAutoscaler(hpav1Shared *autoscalingv1.Ho } setCondition(hpa, autoscalingv2.AbleToScale, v1.ConditionTrue, "SucceededGetScale", "the HPA controller was able to get the target's current scale") currentReplicas := scale.Status.Replicas + a.recordInitialRecommendation(currentReplicas, key) var metricStatuses []autoscalingv2.MetricStatus metricDesiredReplicas := int32(0) diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go index 939fe8b7b58..0cc8922835f 100644 --- a/pkg/controller/podautoscaler/horizontal_test.go +++ b/pkg/controller/podautoscaler/horizontal_test.go @@ -1112,6 +1112,32 @@ func TestScaleDown(t *testing.T) { reportedLevels: []uint64{100, 300, 500, 250, 250}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, useMetricsAPI: true, + recommendations: []timestampedRecommendation{}, + } + tc.runTest(t) +} + +func TestScaleDownStabilizeInitialSize(t *testing.T) { + tc := testCase{ + minReplicas: 2, + maxReplicas: 6, + initialReplicas: 5, + expectedDesiredReplicas: 5, + CPUTarget: 50, + verifyCPUCurrent: true, + reportedLevels: []uint64{100, 300, 500, 250, 250}, + reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, + useMetricsAPI: true, + recommendations: nil, + expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{ + Type: autoscalingv2.AbleToScale, + Status: v1.ConditionTrue, + Reason: "ReadyForNewScale", + }, autoscalingv2.HorizontalPodAutoscalerCondition{ + Type: autoscalingv2.AbleToScale, + Status: v1.ConditionTrue, + Reason: "ScaleDownStabilized", + }), } tc.runTest(t) } @@ -1139,6 +1165,7 @@ func TestScaleDownCM(t *testing.T) { }, reportedLevels: []uint64{12000, 12000, 12000, 12000, 12000}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1171,6 +1198,7 @@ func TestScaleDownCMObject(t *testing.T) { }, reportedLevels: []uint64{12000}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1195,7 +1223,8 @@ func TestScaleDownCMExternal(t *testing.T) { }, }, }, - reportedLevels: []uint64{8600}, + reportedLevels: []uint64{8600}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1220,7 +1249,8 @@ func TestScaleDownPerPodCMExternal(t *testing.T) { }, }, }, - reportedLevels: []uint64{8600}, + reportedLevels: []uint64{8600}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1238,6 +1268,7 @@ func TestScaleDownIncludeUnreadyPods(t *testing.T) { reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, useMetricsAPI: true, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse, v1.ConditionFalse}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1255,6 +1286,7 @@ func TestScaleDownIgnoreHotCpuPods(t *testing.T) { reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, useMetricsAPI: true, reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), coolCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime(), hotCpuCreationTime()}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1273,6 +1305,7 @@ func TestScaleDownIgnoresFailedPods(t *testing.T) { useMetricsAPI: true, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse, v1.ConditionFalse}, reportedPodPhase: []v1.PodPhase{v1.PodRunning, v1.PodRunning, v1.PodRunning, v1.PodRunning, v1.PodRunning, v1.PodFailed, v1.PodFailed}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1292,6 +1325,7 @@ func TestScaleDownIgnoresDeletionPods(t *testing.T) { reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse, v1.ConditionFalse}, reportedPodPhase: []v1.PodPhase{v1.PodRunning, v1.PodRunning, v1.PodRunning, v1.PodRunning, v1.PodRunning, v1.PodRunning, v1.PodRunning}, reportedPodDeletionTimestamp: []bool{false, false, false, false, false, true, true}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1457,6 +1491,7 @@ func TestMinReplicas(t *testing.T) { Status: v1.ConditionTrue, Reason: "TooFewReplicas", }), + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1476,6 +1511,7 @@ func TestMinReplicasDesiredZero(t *testing.T) { Status: v1.ConditionTrue, Reason: "TooFewReplicas", }), + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1580,6 +1616,7 @@ func TestMissingMetrics(t *testing.T) { reportedLevels: []uint64{400, 95}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, useMetricsAPI: true, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -1665,6 +1702,7 @@ func TestMissingReports(t *testing.T) { reportedLevels: []uint64{200}, reportedCPURequests: []resource.Quantity{resource.MustParse("0.2")}, useMetricsAPI: true, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -2168,7 +2206,8 @@ func TestComputedToleranceAlgImplementation(t *testing.T) { resource.MustParse(fmt.Sprint(perPodRequested) + "m"), resource.MustParse(fmt.Sprint(perPodRequested) + "m"), }, - useMetricsAPI: true, + useMetricsAPI: true, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) @@ -2241,6 +2280,7 @@ func TestAvoidUncessaryUpdates(t *testing.T) { reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), hotCpuCreationTime(), hotCpuCreationTime()}, useMetricsAPI: true, lastScaleTime: &now, + recommendations: []timestampedRecommendation{}, } testClient, _, _, _, _ := tc.prepareTestClient(t) tc.testClient = testClient diff --git a/pkg/controller/podautoscaler/legacy_horizontal_test.go b/pkg/controller/podautoscaler/legacy_horizontal_test.go index f29e19012ca..52919a7362a 100644 --- a/pkg/controller/podautoscaler/legacy_horizontal_test.go +++ b/pkg/controller/podautoscaler/legacy_horizontal_test.go @@ -98,7 +98,8 @@ type legacyTestCase struct { resource *fakeResource // Last scale time - lastScaleTime *metav1.Time + lastScaleTime *metav1.Time + recommendations []timestampedRecommendation } // Needs to be called under a lock. @@ -504,6 +505,10 @@ func (tc *legacyTestCase) runTest(t *testing.T) { ) hpaController.hpaListerSynced = alwaysReady + if tc.recommendations != nil { + hpaController.recommendations["test-namespace/test-hpa"] = tc.recommendations + } + stop := make(chan struct{}) defer close(stop) informerFactory.Start(stop) @@ -689,6 +694,7 @@ func TestLegacyScaleDown(t *testing.T) { reportedLevels: []uint64{100, 300, 500, 250, 250}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, useMetricsAPI: true, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -711,6 +717,7 @@ func TestLegacyScaleDownCM(t *testing.T) { }, reportedLevels: []uint64{12, 12, 12, 12, 12}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) } @@ -728,6 +735,7 @@ func TestLegacyScaleDownIgnoresUnreadyPods(t *testing.T) { reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, useMetricsAPI: true, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse, v1.ConditionFalse}, + recommendations: []timestampedRecommendation{}, } tc.runTest(t) }