From c830d94dc497f896108fb37cf5563be123e06edd Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Tue, 10 Jan 2017 17:26:13 -0500 Subject: [PATCH] HPA Controller: Check for 0-sum request value In certain conditions in which the set of metrics returned by Heapster is completely disjoint from the set of pods returned by the API server, we can have a request sum of zero, which can cause a panic (due to division by zero). This checks for that condition. Fixes #39680 --- .../podautoscaler/metrics/utilization.go | 10 ++++++++ .../podautoscaler/replica_calculator_test.go | 24 ++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/controller/podautoscaler/metrics/utilization.go b/pkg/controller/podautoscaler/metrics/utilization.go index f0e909207f7..ebae3ea645f 100644 --- a/pkg/controller/podautoscaler/metrics/utilization.go +++ b/pkg/controller/podautoscaler/metrics/utilization.go @@ -16,6 +16,10 @@ limitations under the License. package metrics +import ( + "fmt" +) + // GetResourceUtilizationRatio takes in a set of metrics, a set of matching requests, // and a target utilization percentage, and calcuates the the ratio of // desired to actual utilization (returning that and the actual utilization) @@ -34,6 +38,12 @@ func GetResourceUtilizationRatio(metrics PodResourceInfo, requests map[string]in requestsTotal += request } + // if the set of requests is completely disjoint from the set of metrics, + // then we could have an issue where the requests total is zero + if requestsTotal == 0 { + return 0, 0, fmt.Errorf("no metrics returned matched known pods") + } + currentUtilization := int32((metricsTotal * 100) / requestsTotal) return float64(currentUtilization) / float64(targetUtilization), currentUtilization, nil diff --git a/pkg/controller/podautoscaler/replica_calculator_test.go b/pkg/controller/podautoscaler/replica_calculator_test.go index bff62d984ba..785f1d48163 100644 --- a/pkg/controller/podautoscaler/replica_calculator_test.go +++ b/pkg/controller/podautoscaler/replica_calculator_test.go @@ -47,6 +47,8 @@ type resourceInfo struct { name v1.ResourceName requests []resource.Quantity levels []int64 + // only applies to pod names returned from "heapster" + podNames []string targetUtilization int32 expectedUtilization int32 @@ -134,9 +136,13 @@ func (tc *replicaCalcTestCase) prepareTestClient(t *testing.T) *fake.Clientset { if tc.resource != nil { metrics := metricsapi.PodMetricsList{} for i, resValue := range tc.resource.levels { + podName := fmt.Sprintf("%s-%d", podNamePrefix, i) + if len(tc.resource.podNames) > i { + podName = tc.resource.podNames[i] + } podMetric := metricsapi.PodMetrics{ ObjectMeta: v1.ObjectMeta{ - Name: fmt.Sprintf("%s-%d", podNamePrefix, i), + Name: podName, Namespace: testNamespace, }, Timestamp: unversioned.Time{Time: tc.timestamp}, @@ -250,6 +256,22 @@ func (tc *replicaCalcTestCase) runTest(t *testing.T) { } } +func TestReplicaCalcDisjointResourcesMetrics(t *testing.T) { + tc := replicaCalcTestCase{ + currentReplicas: 1, + expectedError: fmt.Errorf("no metrics returned matched known pods"), + resource: &resourceInfo{ + name: v1.ResourceCPU, + requests: []resource.Quantity{resource.MustParse("1.0")}, + levels: []int64{100}, + podNames: []string{"an-older-pod-name"}, + + targetUtilization: 100, + }, + } + tc.runTest(t) +} + func TestReplicaCalcScaleUp(t *testing.T) { tc := replicaCalcTestCase{ currentReplicas: 3,