Commit 7d6676ea authored by Joachim Bartosik's avatar Joachim Bartosik

Improve HPA sample sanitization

After my previous changes HPA wasn't behaving correctly in the following situation: - Pods use a lot of CPU during initilization, become ready right after they initialize, - Scale up triggers, - When new pods become ready HPA counts their usage (even though it's not related to any work that needs doing), - Another scale up, even though existing pods can handle work, no problem.
parent 816f2a48
...@@ -73,6 +73,7 @@ go_test( ...@@ -73,6 +73,7 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
......
...@@ -103,6 +103,7 @@ type testCase struct { ...@@ -103,6 +103,7 @@ type testCase struct {
reportedLevels []uint64 reportedLevels []uint64
reportedCPURequests []resource.Quantity reportedCPURequests []resource.Quantity
reportedPodReadiness []v1.ConditionStatus reportedPodReadiness []v1.ConditionStatus
reportedPodStartTime []metav1.Time
reportedPodPhase []v1.PodPhase reportedPodPhase []v1.PodPhase
scaleUpdated bool scaleUpdated bool
statusUpdated bool statusUpdated bool
...@@ -261,6 +262,10 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa ...@@ -261,6 +262,10 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa
if tc.reportedPodReadiness != nil { if tc.reportedPodReadiness != nil {
podReadiness = tc.reportedPodReadiness[i] podReadiness = tc.reportedPodReadiness[i]
} }
var podStartTime metav1.Time
if tc.reportedPodStartTime != nil {
podStartTime = tc.reportedPodStartTime[i]
}
podPhase := v1.PodRunning podPhase := v1.PodRunning
if tc.reportedPodPhase != nil { if tc.reportedPodPhase != nil {
...@@ -283,6 +288,7 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa ...@@ -283,6 +288,7 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa
Status: podReadiness, Status: podReadiness,
}, },
}, },
StartTime: &podStartTime,
}, },
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: podName, Name: podName,
...@@ -660,6 +666,14 @@ func (tc *testCase) setupController(t *testing.T) (*HorizontalController, inform ...@@ -660,6 +666,14 @@ func (tc *testCase) setupController(t *testing.T) (*HorizontalController, inform
return hpaController, informerFactory return hpaController, informerFactory
} }
func hotCpuCreationTime() metav1.Time {
return metav1.Time{Time: time.Now()}
}
func coolCpuCreationTime() metav1.Time {
return metav1.Time{Time: time.Now().Add(-3 * time.Minute)}
}
func (tc *testCase) runTestWithController(t *testing.T, hpaController *HorizontalController, informerFactory informers.SharedInformerFactory) { func (tc *testCase) runTestWithController(t *testing.T, hpaController *HorizontalController, informerFactory informers.SharedInformerFactory) {
stop := make(chan struct{}) stop := make(chan struct{})
defer close(stop) defer close(stop)
...@@ -716,6 +730,23 @@ func TestScaleUpUnreadyLessScale(t *testing.T) { ...@@ -716,6 +730,23 @@ func TestScaleUpUnreadyLessScale(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleUpHotCpuLessScale(t *testing.T) {
tc := testCase{
minReplicas: 2,
maxReplicas: 6,
initialReplicas: 3,
expectedDesiredReplicas: 4,
CPUTarget: 30,
CPUCurrent: 60,
verifyCPUCurrent: true,
reportedLevels: []uint64{300, 500, 700},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodStartTime: []metav1.Time{hotCpuCreationTime(), coolCpuCreationTime(), coolCpuCreationTime()},
useMetricsAPI: true,
}
tc.runTest(t)
}
func TestScaleUpUnreadyNoScale(t *testing.T) { func TestScaleUpUnreadyNoScale(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
...@@ -738,6 +769,29 @@ func TestScaleUpUnreadyNoScale(t *testing.T) { ...@@ -738,6 +769,29 @@ func TestScaleUpUnreadyNoScale(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleUpHotCpuNoScale(t *testing.T) {
tc := testCase{
minReplicas: 2,
maxReplicas: 6,
initialReplicas: 3,
expectedDesiredReplicas: 3,
CPUTarget: 30,
CPUCurrent: 40,
verifyCPUCurrent: true,
reportedLevels: []uint64{400, 500, 700},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionFalse, v1.ConditionFalse},
reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), hotCpuCreationTime(), hotCpuCreationTime()},
useMetricsAPI: true,
expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.AbleToScale,
Status: v1.ConditionTrue,
Reason: "ReadyForNewScale",
}),
}
tc.runTest(t)
}
func TestScaleUpIgnoresFailedPods(t *testing.T) { func TestScaleUpIgnoresFailedPods(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
...@@ -818,12 +872,12 @@ func TestScaleUpCM(t *testing.T) { ...@@ -818,12 +872,12 @@ func TestScaleUpCM(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleUpCMUnreadyLessScale(t *testing.T) { func TestScaleUpCMUnreadyAndHotCpuNoLessScale(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
initialReplicas: 3, initialReplicas: 3,
expectedDesiredReplicas: 4, expectedDesiredReplicas: 6,
CPUTarget: 0, CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{ metricsTarget: []autoscalingv2.MetricSpec{
{ {
...@@ -836,17 +890,18 @@ func TestScaleUpCMUnreadyLessScale(t *testing.T) { ...@@ -836,17 +890,18 @@ func TestScaleUpCMUnreadyLessScale(t *testing.T) {
}, },
reportedLevels: []uint64{50000, 10000, 30000}, reportedLevels: []uint64{50000, 10000, 30000},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse}, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse},
reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime()},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
} }
tc.runTest(t) tc.runTest(t)
} }
func TestScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) { func TestScaleUpCMUnreadyandCpuHot(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
initialReplicas: 3, initialReplicas: 3,
expectedDesiredReplicas: 3, expectedDesiredReplicas: 6,
CPUTarget: 0, CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{ metricsTarget: []autoscalingv2.MetricSpec{
{ {
...@@ -859,11 +914,48 @@ func TestScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) { ...@@ -859,11 +914,48 @@ func TestScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) {
}, },
reportedLevels: []uint64{50000, 15000, 30000}, reportedLevels: []uint64{50000, 15000, 30000},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionFalse}, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionFalse},
reportedPodStartTime: []metav1.Time{hotCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime()},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{ expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.AbleToScale, Type: autoscalingv2.AbleToScale,
Status: v1.ConditionTrue, Status: v1.ConditionTrue,
Reason: "ReadyForNewScale", Reason: "SucceededRescale",
}, autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.ScalingLimited,
Status: v1.ConditionTrue,
Reason: "TooManyReplicas",
}),
}
tc.runTest(t)
}
func TestScaleUpHotCpuNoScaleWouldScaleDown(t *testing.T) {
tc := testCase{
minReplicas: 2,
maxReplicas: 6,
initialReplicas: 3,
expectedDesiredReplicas: 6,
CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{
{
Type: autoscalingv2.PodsMetricSourceType,
Pods: &autoscalingv2.PodsMetricSource{
MetricName: "qps",
TargetAverageValue: resource.MustParse("15.0"),
},
},
},
reportedLevels: []uint64{50000, 15000, 30000},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodStartTime: []metav1.Time{hotCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime()},
expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.AbleToScale,
Status: v1.ConditionTrue,
Reason: "SucceededRescale",
}, autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.ScalingLimited,
Status: v1.ConditionTrue,
Reason: "TooManyReplicas",
}), }),
} }
tc.runTest(t) tc.runTest(t)
...@@ -1043,7 +1135,7 @@ func TestScaleDownPerPodCMExternal(t *testing.T) { ...@@ -1043,7 +1135,7 @@ func TestScaleDownPerPodCMExternal(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleDownIgnoresUnreadyPods(t *testing.T) { func TestScaleDownIncludeUnreadyPods(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
...@@ -1060,6 +1152,23 @@ func TestScaleDownIgnoresUnreadyPods(t *testing.T) { ...@@ -1060,6 +1152,23 @@ func TestScaleDownIgnoresUnreadyPods(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleDownIgnoreHotCpuPods(t *testing.T) {
tc := testCase{
minReplicas: 2,
maxReplicas: 6,
initialReplicas: 5,
expectedDesiredReplicas: 2,
CPUTarget: 50,
CPUCurrent: 30,
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,
reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), coolCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime(), hotCpuCreationTime()},
}
tc.runTest(t)
}
func TestScaleDownIgnoresFailedPods(t *testing.T) { func TestScaleDownIgnoresFailedPods(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
...@@ -1975,7 +2084,7 @@ func TestAvoidUncessaryUpdates(t *testing.T) { ...@@ -1975,7 +2084,7 @@ func TestAvoidUncessaryUpdates(t *testing.T) {
verifyCPUCurrent: true, verifyCPUCurrent: true,
reportedLevels: []uint64{400, 500, 700}, reportedLevels: []uint64{400, 500, 700},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionFalse, v1.ConditionFalse}, reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), hotCpuCreationTime(), hotCpuCreationTime()},
useMetricsAPI: true, useMetricsAPI: true,
} }
testClient, _, _, _, _ := tc.prepareTestClient(t) testClient, _, _, _, _ := tc.prepareTestClient(t)
......
...@@ -222,7 +222,8 @@ func (tc *legacyTestCase) prepareTestClient(t *testing.T) (*fake.Clientset, *sca ...@@ -222,7 +222,8 @@ func (tc *legacyTestCase) prepareTestClient(t *testing.T) (*fake.Clientset, *sca
podName := fmt.Sprintf("%s-%d", podNamePrefix, i) podName := fmt.Sprintf("%s-%d", podNamePrefix, i)
pod := v1.Pod{ pod := v1.Pod{
Status: v1.PodStatus{ Status: v1.PodStatus{
Phase: v1.PodRunning, StartTime: &metav1.Time{Time: time.Now().Add(-3 * time.Minute)},
Phase: v1.PodRunning,
Conditions: []v1.PodCondition{ Conditions: []v1.PodCondition{
{ {
Type: v1.PodReady, Type: v1.PodReady,
...@@ -545,8 +546,7 @@ func TestLegacyScaleUpUnreadyLessScale(t *testing.T) { ...@@ -545,8 +546,7 @@ func TestLegacyScaleUpUnreadyLessScale(t *testing.T) {
initialReplicas: 3, initialReplicas: 3,
desiredReplicas: 4, desiredReplicas: 4,
CPUTarget: 30, CPUTarget: 30,
CPUCurrent: 60, verifyCPUCurrent: false,
verifyCPUCurrent: true,
reportedLevels: []uint64{300, 500, 700}, reportedLevels: []uint64{300, 500, 700},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionTrue}, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionTrue},
...@@ -634,12 +634,12 @@ func TestLegacyScaleUpCM(t *testing.T) { ...@@ -634,12 +634,12 @@ func TestLegacyScaleUpCM(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestLegacyScaleUpCMUnreadyLessScale(t *testing.T) { func TestLegacyScaleUpCMUnreadyNoLessScale(t *testing.T) {
tc := legacyTestCase{ tc := legacyTestCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
initialReplicas: 3, initialReplicas: 3,
desiredReplicas: 4, desiredReplicas: 6,
CPUTarget: 0, CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{ metricsTarget: []autoscalingv2.MetricSpec{
{ {
...@@ -662,7 +662,7 @@ func TestLegacyScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) { ...@@ -662,7 +662,7 @@ func TestLegacyScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) {
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
initialReplicas: 3, initialReplicas: 3,
desiredReplicas: 3, desiredReplicas: 6,
CPUTarget: 0, CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{ metricsTarget: []autoscalingv2.MetricSpec{
{ {
......
...@@ -67,7 +67,8 @@ func (tc *legacyReplicaCalcTestCase) prepareTestClient(t *testing.T) *fake.Clien ...@@ -67,7 +67,8 @@ func (tc *legacyReplicaCalcTestCase) prepareTestClient(t *testing.T) *fake.Clien
podName := fmt.Sprintf("%s-%d", podNamePrefix, i) podName := fmt.Sprintf("%s-%d", podNamePrefix, i)
pod := v1.Pod{ pod := v1.Pod{
Status: v1.PodStatus{ Status: v1.PodStatus{
Phase: v1.PodRunning, Phase: v1.PodRunning,
StartTime: &metav1.Time{Time: time.Now().Add(-3 * time.Minute)},
Conditions: []v1.PodCondition{ Conditions: []v1.PodCondition{
{ {
Type: v1.PodReady, Type: v1.PodReady,
...@@ -310,10 +311,10 @@ func TestLegacyReplicaCalcScaleUpCM(t *testing.T) { ...@@ -310,10 +311,10 @@ func TestLegacyReplicaCalcScaleUpCM(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestLegacyReplicaCalcScaleUpCMUnreadyLessScale(t *testing.T) { func TestLegacyReplicaCalcScaleUpCMUnreadyNoLessScale(t *testing.T) {
tc := legacyReplicaCalcTestCase{ tc := legacyReplicaCalcTestCase{
currentReplicas: 3, currentReplicas: 3,
expectedReplicas: 4, expectedReplicas: 6,
podReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse}, podReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse},
metric: &metricInfo{ metric: &metricInfo{
name: "qps", name: "qps",
...@@ -325,16 +326,16 @@ func TestLegacyReplicaCalcScaleUpCMUnreadyLessScale(t *testing.T) { ...@@ -325,16 +326,16 @@ func TestLegacyReplicaCalcScaleUpCMUnreadyLessScale(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestLegacyReplicaCalcScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) { func TestLegacyReplicaCalcScaleUpCMUnreadyScale(t *testing.T) {
tc := legacyReplicaCalcTestCase{ tc := legacyReplicaCalcTestCase{
currentReplicas: 3, currentReplicas: 3,
expectedReplicas: 3, expectedReplicas: 7,
podReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionFalse}, podReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionFalse},
metric: &metricInfo{ metric: &metricInfo{
name: "qps", name: "qps",
levels: []int64{50000, 15000, 30000}, levels: []int64{50000, 15000, 30000},
targetUtilization: 15000, targetUtilization: 15000,
expectedUtilization: 15000, expectedUtilization: 31666,
}, },
} }
tc.runTest(t) tc.runTest(t)
......
...@@ -73,41 +73,11 @@ func (c *ReplicaCalculator) GetResourceReplicas(currentReplicas int32, targetUti ...@@ -73,41 +73,11 @@ func (c *ReplicaCalculator) GetResourceReplicas(currentReplicas int32, targetUti
return 0, 0, 0, time.Time{}, fmt.Errorf("no pods returned by selector while calculating replica count") return 0, 0, 0, time.Time{}, fmt.Errorf("no pods returned by selector while calculating replica count")
} }
requests := make(map[string]int64, itemsLen) readyPodCount, ignoredPods, missingPods := groupPods(podList.Items, metrics, resource)
readyPodCount := 0 removeMetricsForPods(metrics, ignoredPods)
unreadyPods := sets.NewString() requests, err := calculatePodRequests(podList.Items, resource)
missingPods := sets.NewString() if err != nil {
return 0, 0, 0, time.Time{}, err
for _, pod := range podList.Items {
podSum := int64(0)
for _, container := range pod.Spec.Containers {
if containerRequest, ok := container.Resources.Requests[resource]; ok {
podSum += containerRequest.MilliValue()
} else {
return 0, 0, 0, time.Time{}, fmt.Errorf("missing request for %s on container %s in pod %s/%s", resource, container.Name, namespace, pod.Name)
}
}
requests[pod.Name] = podSum
if pod.Status.Phase != v1.PodRunning || !podutil.IsPodReady(&pod) {
// save this pod name for later, but pretend it doesn't exist for now
if pod.Status.Phase != v1.PodFailed {
// Failed pods should not be counted as unready pods as they will
// not become running anymore.
unreadyPods.Insert(pod.Name)
}
delete(metrics, pod.Name)
continue
}
if _, found := metrics[pod.Name]; !found {
// save this pod name for later, but pretend it doesn't exist for now
missingPods.Insert(pod.Name)
continue
}
readyPodCount++
} }
if len(metrics) == 0 { if len(metrics) == 0 {
...@@ -119,8 +89,8 @@ func (c *ReplicaCalculator) GetResourceReplicas(currentReplicas int32, targetUti ...@@ -119,8 +89,8 @@ func (c *ReplicaCalculator) GetResourceReplicas(currentReplicas int32, targetUti
return 0, 0, 0, time.Time{}, err return 0, 0, 0, time.Time{}, err
} }
rebalanceUnready := len(unreadyPods) > 0 && usageRatio > 1.0 rebalanceIgnored := len(ignoredPods) > 0 && usageRatio > 1.0
if !rebalanceUnready && len(missingPods) == 0 { if !rebalanceIgnored && len(missingPods) == 0 {
if math.Abs(1.0-usageRatio) <= c.tolerance { if math.Abs(1.0-usageRatio) <= c.tolerance {
// return the current replicas if the change would be too small // return the current replicas if the change would be too small
return currentReplicas, utilization, rawUtilization, timestamp, nil return currentReplicas, utilization, rawUtilization, timestamp, nil
...@@ -144,9 +114,9 @@ func (c *ReplicaCalculator) GetResourceReplicas(currentReplicas int32, targetUti ...@@ -144,9 +114,9 @@ func (c *ReplicaCalculator) GetResourceReplicas(currentReplicas int32, targetUti
} }
} }
if rebalanceUnready { if rebalanceIgnored {
// on a scale-up, treat unready pods as using 0% of the resource request // on a scale-up, treat unready pods as using 0% of the resource request
for podName := range unreadyPods { for podName := range ignoredPods {
metrics[podName] = 0 metrics[podName] = 0
} }
} }
...@@ -176,7 +146,7 @@ func (c *ReplicaCalculator) GetRawResourceReplicas(currentReplicas int32, target ...@@ -176,7 +146,7 @@ func (c *ReplicaCalculator) GetRawResourceReplicas(currentReplicas int32, target
return 0, 0, time.Time{}, fmt.Errorf("unable to get metrics for resource %s: %v", resource, err) return 0, 0, time.Time{}, fmt.Errorf("unable to get metrics for resource %s: %v", resource, err)
} }
replicaCount, utilization, err = c.calcPlainMetricReplicas(metrics, currentReplicas, targetUtilization, namespace, selector) replicaCount, utilization, err = c.calcPlainMetricReplicas(metrics, currentReplicas, targetUtilization, namespace, selector, resource)
return replicaCount, utilization, timestamp, err return replicaCount, utilization, timestamp, err
} }
...@@ -189,12 +159,12 @@ func (c *ReplicaCalculator) GetMetricReplicas(currentReplicas int32, targetUtili ...@@ -189,12 +159,12 @@ func (c *ReplicaCalculator) GetMetricReplicas(currentReplicas int32, targetUtili
return 0, 0, time.Time{}, fmt.Errorf("unable to get metric %s: %v", metricName, err) return 0, 0, time.Time{}, fmt.Errorf("unable to get metric %s: %v", metricName, err)
} }
replicaCount, utilization, err = c.calcPlainMetricReplicas(metrics, currentReplicas, targetUtilization, namespace, selector) replicaCount, utilization, err = c.calcPlainMetricReplicas(metrics, currentReplicas, targetUtilization, namespace, selector, v1.ResourceName(""))
return replicaCount, utilization, timestamp, err return replicaCount, utilization, timestamp, err
} }
// calcPlainMetricReplicas calculates the desired replicas for plain (i.e. non-utilization percentage) metrics. // calcPlainMetricReplicas calculates the desired replicas for plain (i.e. non-utilization percentage) metrics.
func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMetricsInfo, currentReplicas int32, targetUtilization int64, namespace string, selector labels.Selector) (replicaCount int32, utilization int64, err error) { func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMetricsInfo, currentReplicas int32, targetUtilization int64, namespace string, selector labels.Selector, resource v1.ResourceName) (replicaCount int32, utilization int64, err error) {
podList, err := c.podsGetter.Pods(namespace).List(metav1.ListOptions{LabelSelector: selector.String()}) podList, err := c.podsGetter.Pods(namespace).List(metav1.ListOptions{LabelSelector: selector.String()})
if err != nil { if err != nil {
return 0, 0, fmt.Errorf("unable to get pods while calculating replica count: %v", err) return 0, 0, fmt.Errorf("unable to get pods while calculating replica count: %v", err)
...@@ -204,26 +174,8 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet ...@@ -204,26 +174,8 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet
return 0, 0, fmt.Errorf("no pods returned by selector while calculating replica count") return 0, 0, fmt.Errorf("no pods returned by selector while calculating replica count")
} }
readyPodCount := 0 readyPodCount, ignoredPods, missingPods := groupPods(podList.Items, metrics, resource)
unreadyPods := sets.NewString() removeMetricsForPods(metrics, ignoredPods)
missingPods := sets.NewString()
for _, pod := range podList.Items {
if pod.Status.Phase != v1.PodRunning || !hasPodBeenReadyBefore(&pod) {
// save this pod name for later, but pretend it doesn't exist for now
unreadyPods.Insert(pod.Name)
delete(metrics, pod.Name)
continue
}
if _, found := metrics[pod.Name]; !found {
// save this pod name for later, but pretend it doesn't exist for now
missingPods.Insert(pod.Name)
continue
}
readyPodCount++
}
if len(metrics) == 0 { if len(metrics) == 0 {
return 0, 0, fmt.Errorf("did not receive metrics for any ready pods") return 0, 0, fmt.Errorf("did not receive metrics for any ready pods")
...@@ -231,9 +183,9 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet ...@@ -231,9 +183,9 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet
usageRatio, utilization := metricsclient.GetMetricUtilizationRatio(metrics, targetUtilization) usageRatio, utilization := metricsclient.GetMetricUtilizationRatio(metrics, targetUtilization)
rebalanceUnready := len(unreadyPods) > 0 && usageRatio > 1.0 rebalanceIgnored := len(ignoredPods) > 0 && usageRatio > 1.0
if !rebalanceUnready && len(missingPods) == 0 { if !rebalanceIgnored && len(missingPods) == 0 {
if math.Abs(1.0-usageRatio) <= c.tolerance { if math.Abs(1.0-usageRatio) <= c.tolerance {
// return the current replicas if the change would be too small // return the current replicas if the change would be too small
return currentReplicas, utilization, nil return currentReplicas, utilization, nil
...@@ -257,9 +209,9 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet ...@@ -257,9 +209,9 @@ func (c *ReplicaCalculator) calcPlainMetricReplicas(metrics metricsclient.PodMet
} }
} }
if rebalanceUnready { if rebalanceIgnored {
// on a scale-up, treat unready pods as using 0% of the resource request // on a scale-up, treat unready pods as using 0% of the resource request
for podName := range unreadyPods { for podName := range ignoredPods {
metrics[podName] = 0 metrics[podName] = 0
} }
} }
...@@ -386,21 +338,58 @@ func (c *ReplicaCalculator) GetExternalPerPodMetricReplicas(currentReplicas int3 ...@@ -386,21 +338,58 @@ func (c *ReplicaCalculator) GetExternalPerPodMetricReplicas(currentReplicas int3
return replicaCount, utilization, timestamp, nil return replicaCount, utilization, timestamp, nil
} }
// hasPodBeenReadyBefore returns true if the pod is ready or if it's not ready func groupPods(pods []v1.Pod, metrics metricsclient.PodMetricsInfo, resource v1.ResourceName) (readyPodCount int, ignoredPods sets.String, missingPods sets.String) {
func hasPodBeenReadyBefore(pod *v1.Pod) bool { missingPods = sets.NewString()
_, readyCondition := podutil.GetPodCondition(&pod.Status, v1.PodReady) ignoredPods = sets.NewString()
if readyCondition == nil { for _, pod := range pods {
return false if pod.Status.Phase == v1.PodFailed {
continue
}
if _, found := metrics[pod.Name]; !found {
missingPods.Insert(pod.Name)
continue
}
if resource == v1.ResourceCPU {
var ignorePod bool
_, condition := podutil.GetPodCondition(&pod.Status, v1.PodReady)
if condition == nil || pod.Status.StartTime == nil {
ignorePod = true
} else {
if condition.Status == v1.ConditionTrue {
ignorePod = pod.Status.StartTime.Add(2 * time.Minute).After(time.Now())
} else {
ignorePod = pod.Status.StartTime.Add(maxDelayOfInitialReadinessStatus).After(condition.LastTransitionTime.Time)
}
}
if ignorePod {
ignoredPods.Insert(pod.Name)
continue
}
}
readyPodCount++
} }
if readyCondition.Status == v1.ConditionTrue { return
return true }
func calculatePodRequests(pods []v1.Pod, resource v1.ResourceName) (map[string]int64, error) {
requests := make(map[string]int64, len(pods))
for _, pod := range pods {
podSum := int64(0)
for _, container := range pod.Spec.Containers {
if containerRequest, ok := container.Resources.Requests[resource]; ok {
podSum += containerRequest.MilliValue()
} else {
return nil, fmt.Errorf("missing request for %s", resource)
}
}
requests[pod.Name] = podSum
} }
lastReady := readyCondition.LastTransitionTime.Time return requests, nil
if pod.Status.StartTime == nil { }
return false
func removeMetricsForPods(metrics metricsclient.PodMetricsInfo, pods sets.String) {
for _, pod := range pods.UnsortedList() {
delete(metrics, pod)
} }
started := pod.Status.StartTime.Time
// If last status change was longer than maxDelayOfInitialReadinessStatus after the pod was
// created assume it was ready in the past.
return lastReady.After(started.Add(maxDelayOfInitialReadinessStatus))
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment