From 342eee680c1e51f3f81fb91b6940279d0de0b14c Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Sun, 13 Dec 2015 09:54:43 +0100 Subject: [PATCH] Revert "[hpa] Parameterize tolerance, downscale, and upscale into HPAController, and add corresponding unit test for backsolved tolerance." --- .../app/controllermanager.go | 6 +- pkg/controller/podautoscaler/horizontal.go | 48 +++++-------- .../podautoscaler/horizontal_test.go | 72 +------------------ 3 files changed, 23 insertions(+), 103 deletions(-) diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index fade90f98a..eec5d2c8b1 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -365,11 +365,7 @@ func (s *CMServer) Run(_ []string) error { metrics.DefaultHeapsterService, metrics.DefaultHeapsterPort, ) - // TODO parameterize tolerance/downscale/upscale options. - tolerance := 1.0 - downScale := time.Duration(5) * time.Second - upScale := time.Duration(3) * time.Second - podautoscaler.NewHorizontalController(kubeClient, metricsClient, tolerance, downScale, upScale). + podautoscaler.NewHorizontalController(hpaClient, metricsClient). Run(s.HorizontalPodAutoscalerSyncPeriod) } diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go index 29f34b9d1c..63219c1c32 100644 --- a/pkg/controller/podautoscaler/horizontal.go +++ b/pkg/controller/podautoscaler/horizontal.go @@ -31,40 +31,30 @@ import ( "k8s.io/kubernetes/pkg/util" ) +const ( + // Usage shoud exceed the tolerance before we start downscale or upscale the pods. + // TODO: make it a flag or HPA spec element. + tolerance = 0.1 +) + type HorizontalController struct { - client client.Interface - metricsClient metrics.MetricsClient - eventRecorder record.EventRecorder - tolerance float64 - downscaleForbiddenWindow time.Duration - upscaleForbiddenWindow time.Duration + client client.Interface + metricsClient metrics.MetricsClient + eventRecorder record.EventRecorder } -func NewHorizontalController(client client.Interface, metricsClient metrics.MetricsClient, tol float64, dScale, uScale time.Duration) *HorizontalController { +var downscaleForbiddenWindow = 5 * time.Minute +var upscaleForbiddenWindow = 3 * time.Minute + +func NewHorizontalController(client client.Interface, metricsClient metrics.MetricsClient) *HorizontalController { broadcaster := record.NewBroadcaster() broadcaster.StartRecordingToSink(client.Events("")) recorder := broadcaster.NewRecorder(api.EventSource{Component: "horizontal-pod-autoscaler"}) - if tol < 0 || tol > 1 { - glog.Warningf("Invalid tolerance provided %v using default.", tol) - tol = .1 - } - if uScale == 0*time.Second { - glog.Warningf("Invalid upscale value provided, %v using default.", uScale) - uScale = 3 * time.Minute - } - if dScale == 0*time.Second { - glog.Warningf("Invalid downscale value provided, %v using default.", dScale) - dScale = 5 * time.Minute - } - glog.V(2).Infof("Created Horizontal Controller with downscale %v, upscale %v, and tolerance %v", tol, uScale, dScale) return &HorizontalController{ - client: client, - metricsClient: metricsClient, - eventRecorder: recorder, - tolerance: tol, - downscaleForbiddenWindow: dScale, - upscaleForbiddenWindow: uScale, + client: client, + metricsClient: metricsClient, + eventRecorder: recorder, } } @@ -93,7 +83,7 @@ func (a *HorizontalController) computeReplicasForCPUUtilization(hpa extensions.H } usageRatio := float64(*currentUtilization) / float64(hpa.Spec.CPUUtilization.TargetPercentage) - if math.Abs(1.0-usageRatio) > a.tolerance { + if math.Abs(1.0-usageRatio) > tolerance { return int(math.Ceil(usageRatio * float64(currentReplicas))), currentUtilization, timestamp, nil } else { return currentReplicas, currentUtilization, timestamp, nil @@ -135,7 +125,7 @@ func (a *HorizontalController) reconcileAutoscaler(hpa extensions.HorizontalPodA // and there was no rescaling in the last downscaleForbiddenWindow. if desiredReplicas < currentReplicas && (hpa.Status.LastScaleTime == nil || - hpa.Status.LastScaleTime.Add(a.downscaleForbiddenWindow).Before(timestamp)) { + hpa.Status.LastScaleTime.Add(downscaleForbiddenWindow).Before(timestamp)) { rescale = true } @@ -143,7 +133,7 @@ func (a *HorizontalController) reconcileAutoscaler(hpa extensions.HorizontalPodA // and there was no rescaling in the last upscaleForbiddenWindow. if desiredReplicas > currentReplicas && (hpa.Status.LastScaleTime == nil || - hpa.Status.LastScaleTime.Add(a.upscaleForbiddenWindow).Before(timestamp)) { + hpa.Status.LastScaleTime.Add(upscaleForbiddenWindow).Before(timestamp)) { rescale = true } } diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go index 680cea232f..03334e7f50 100644 --- a/pkg/controller/podautoscaler/horizontal_test.go +++ b/pkg/controller/podautoscaler/horizontal_test.go @@ -20,7 +20,6 @@ import ( "encoding/json" "fmt" "io" - "math" "testing" "time" @@ -33,14 +32,9 @@ import ( "k8s.io/kubernetes/pkg/controller/podautoscaler/metrics" "k8s.io/kubernetes/pkg/runtime" - glog "github.com/golang/glog" - "github.com/stretchr/testify/assert" heapster "k8s.io/heapster/api/v1/types" -) -// unit tests need tolerance awareness to calibrate. -const ( - tolerance = .1 + "github.com/stretchr/testify/assert" ) func (w fakeResponseWrapper) DoRaw() ([]byte, error) { @@ -212,7 +206,7 @@ func (tc *testCase) verifyResults(t *testing.T) { func (tc *testCase) runTest(t *testing.T) { testClient := tc.prepareTestClient(t) metricsClient := metrics.NewHeapsterMetricsClient(testClient, metrics.DefaultHeapsterNamespace, metrics.DefaultHeapsterScheme, metrics.DefaultHeapsterService, metrics.DefaultHeapsterPort) - hpaController := NewHorizontalController(testClient, metricsClient, tolerance, time.Second, time.Second) + hpaController := NewHorizontalController(testClient, metricsClient) err := hpaController.reconcileAutoscalers() assert.Equal(t, nil, err) if tc.verifyEvents { @@ -366,64 +360,4 @@ func TestEventNotCreated(t *testing.T) { tc.runTest(t) } -// TestComputedToleranceAlgImplementation is a regression test which -// back-calculates a minimal percentage for downscaling based on a small percentage -// increase in pod utilization which is calibrated against the tolerance value. -func TestComputedToleranceAlgImplementation(t *testing.T) { - - startPods := 10 - // 150 mCPU per pod. - totalUsedCPUOfAllPods := uint64(startPods * 150) - // Each pod starts out asking for 2X what is really needed. - // This means we will have a 50% ratio of used/requested - totalRequestedCPUOfAllPods := 2 * totalUsedCPUOfAllPods - requestedToUsed := float64(totalRequestedCPUOfAllPods / totalUsedCPUOfAllPods) - // Spread the amount we ask over 10 pods. We can add some jitter later in reportedLevels. - perPodRequested := int(totalRequestedCPUOfAllPods) / startPods - - // Force a minimal scaling event by satisfying (tolerance < 1 - resourcesUsedRatio). - target := math.Abs(1/(requestedToUsed*(1-tolerance))) + .01 - finalCpuPercentTarget := int(target * 100) - resourcesUsedRatio := float64(totalUsedCPUOfAllPods) / float64(float64(totalRequestedCPUOfAllPods)*target) - // the autoscaler will compare this vs. tolearnce. Lets calculate the usageRatio, which will be - // compared w tolerance. - usageRatioToleranceValue := float64(1 - resourcesUsedRatio) - // i.e. .60 * 20 -> scaled down expectation. - finalPods := math.Ceil(resourcesUsedRatio * float64(startPods)) - - glog.Infof("To breach tolerance %f we will create a utilization ratio difference of %f", tolerance, usageRatioToleranceValue) - tc := testCase{ - minReplicas: 0, - maxReplicas: 1000, - initialReplicas: startPods, - desiredReplicas: int(finalPods), - CPUTarget: finalCpuPercentTarget, - reportedLevels: []uint64{ - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - totalUsedCPUOfAllPods / 10, - }, - reportedCPURequests: []resource.Quantity{ - resource.MustParse(fmt.Sprint(perPodRequested+100) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested-100) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested+10) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested-10) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested+2) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested-2) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested+1) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested-1) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested) + "m"), - resource.MustParse(fmt.Sprint(perPodRequested) + "m"), - }, - } - tc.runTest(t) -} - -// TODO: add more tests, e.g., enforcement of upscal/downscale window. +// TODO: add more tests