From 715b79af3aebe5e0bd0312eaa7a2966e1f86831b Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Fri, 3 Dec 2021 21:44:51 +0100 Subject: [PATCH 1/5] Expects getMetricValue to filter on phase before counting Signed-off-by: Staffan Olsson --- .../kubernetes_workload_scaler_test.go | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/pkg/scalers/kubernetes_workload_scaler_test.go b/pkg/scalers/kubernetes_workload_scaler_test.go index e59361489f8..5ba462fd1e5 100644 --- a/pkg/scalers/kubernetes_workload_scaler_test.go +++ b/pkg/scalers/kubernetes_workload_scaler_test.go @@ -3,6 +3,7 @@ package scalers import ( "context" "fmt" + "strings" "testing" "time" @@ -138,3 +139,55 @@ func createPodlist(count int) *v1.PodList { } return list } + +func TestWorkloadPhase(t *testing.T) { + phases := map[v1.PodPhase]bool{ + v1.PodRunning: true, + v1.PodSucceeded: false, + v1.PodFailed: false, + v1.PodUnknown: false, + v1.PodPending: true, // polling means we have a delay, so count pending in order to avoid more delays + } + for phase, active := range phases { + list := &v1.PodList{} + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: strings.ToLower(fmt.Sprintf("phase-%s", phase)), + Namespace: "default", + Annotations: map[string]string{}, + Labels: map[string]string{ + "app": "testphases", + }, + }, + Status: v1.PodStatus{ + Phase: phase, + }, + } + list.Items = append(list.Items, *pod) + s, err := NewKubernetesWorkloadScaler( + fake.NewClientBuilder().WithRuntimeObjects(list).Build(), + &ScalerConfig{ + TriggerMetadata: map[string]string{ + "podSelector": "app=testphases", + "value": "1", + }, + AuthParams: map[string]string{}, + GlobalHTTPTimeout: 1000 * time.Millisecond, + Namespace: "default", + }, + ) + if err != nil { + t.Errorf("Failed to create test scaler -- %v", err) + } + isActive, err := s.IsActive(context.TODO()) + if err != nil { + t.Errorf("Failed to count active -- %v", err) + } + if active && !isActive { + t.Errorf("Expected active for phase %s but got inactive", phase) + } + if !active && isActive { + t.Errorf("Expected inactive for phase %s but got active", phase) + } + } +} From 1f5bd52b0dd7e77f897e2dd657299ea1b536cfdc Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sat, 4 Dec 2021 07:56:07 +0100 Subject: [PATCH 2/5] Ignores terminated pods in kubernetes-workload count Signed-off-by: Staffan Olsson --- pkg/scalers/kubernetes_workload_scaler.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/scalers/kubernetes_workload_scaler.go b/pkg/scalers/kubernetes_workload_scaler.go index 153683ee32e..55b0a889123 100644 --- a/pkg/scalers/kubernetes_workload_scaler.go +++ b/pkg/scalers/kubernetes_workload_scaler.go @@ -27,6 +27,12 @@ const ( valueKey = "value" ) +var countIgnoresPhases = []corev1.PodPhase{ + corev1.PodSucceeded, + corev1.PodFailed, + corev1.PodUnknown, +} + type kubernetesWorkloadMetadata struct { podSelector labels.Selector namespace string @@ -125,5 +131,19 @@ func (s *kubernetesWorkloadScaler) getMetricValue(ctx context.Context) (int, err return 0, err } - return len(podList.Items), nil + count := 0 + for _, pod := range podList.Items { + count += getCountValue(pod) + } + + return count, nil +} + +func getCountValue(pod corev1.Pod) int { + for _, ignore := range countIgnoresPhases { + if pod.Status.Phase == ignore { + return 0 + } + } + return 1 } From 3a5dcda7f46857d3d697e36496717b290868ba50 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sun, 5 Dec 2021 07:40:55 +0100 Subject: [PATCH 3/5] Unknown shouldn't count as terminated Signed-off-by: Staffan Olsson --- pkg/scalers/kubernetes_workload_scaler.go | 5 ++--- pkg/scalers/kubernetes_workload_scaler_test.go | 9 ++++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/scalers/kubernetes_workload_scaler.go b/pkg/scalers/kubernetes_workload_scaler.go index 55b0a889123..b7185b96ead 100644 --- a/pkg/scalers/kubernetes_workload_scaler.go +++ b/pkg/scalers/kubernetes_workload_scaler.go @@ -27,10 +27,9 @@ const ( valueKey = "value" ) -var countIgnoresPhases = []corev1.PodPhase{ +var phasesCountedAsTerminated = []corev1.PodPhase{ corev1.PodSucceeded, corev1.PodFailed, - corev1.PodUnknown, } type kubernetesWorkloadMetadata struct { @@ -140,7 +139,7 @@ func (s *kubernetesWorkloadScaler) getMetricValue(ctx context.Context) (int, err } func getCountValue(pod corev1.Pod) int { - for _, ignore := range countIgnoresPhases { + for _, ignore := range phasesCountedAsTerminated { if pod.Status.Phase == ignore { return 0 } diff --git a/pkg/scalers/kubernetes_workload_scaler_test.go b/pkg/scalers/kubernetes_workload_scaler_test.go index 5ba462fd1e5..c99feb21628 100644 --- a/pkg/scalers/kubernetes_workload_scaler_test.go +++ b/pkg/scalers/kubernetes_workload_scaler_test.go @@ -142,11 +142,14 @@ func createPodlist(count int) *v1.PodList { func TestWorkloadPhase(t *testing.T) { phases := map[v1.PodPhase]bool{ - v1.PodRunning: true, + v1.PodRunning: true, + // succeeded and failed clearly count as terminated v1.PodSucceeded: false, v1.PodFailed: false, - v1.PodUnknown: false, - v1.PodPending: true, // polling means we have a delay, so count pending in order to avoid more delays + // unknown could be for example a temporarily unresponsive node; count the pod + v1.PodUnknown: true, + // count pre-Running to avoid an additional delay on top of the poll interval + v1.PodPending: true, } for phase, active := range phases { list := &v1.PodList{} From 784d4c4952c69648e2f97823faadd463313c3453 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sun, 5 Dec 2021 08:14:27 +0100 Subject: [PATCH 4/5] Adds changelog entry for kubernetes-workload ignore terminated pods Signed-off-by: Staffan Olsson --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15289cd44a2..bd92c6b3cef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,8 @@ - Graphite Scaler: use the latest datapoint returned, not the earliest (https://github.com/kedacore/keda/pull/2365) +- Kubernetes Workload Scaler: ignore terminated pods ([#2384](https://github.com/kedacore/keda/pull/2384)) + - TODO ([#XXX](https://github.com/kedacore/keda/pull/XXX)) ### Breaking Changes From c02c58f61899da6ae4aa5c4d1b86f75bfa8385a2 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Mon, 6 Dec 2021 08:42:33 +0100 Subject: [PATCH 5/5] Aligns 2.6.0 changelog formatting with that of sections below Signed-off-by: Staffan Olsson --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd92c6b3cef..072f1d7b55d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,8 +27,7 @@ ### Improvements -- Graphite Scaler: use the latest datapoint returned, not the earliest (https://github.com/kedacore/keda/pull/2365) - +- Graphite Scaler: use the latest datapoint returned, not the earliest ([#2365](https://github.com/kedacore/keda/pull/2365)) - Kubernetes Workload Scaler: ignore terminated pods ([#2384](https://github.com/kedacore/keda/pull/2384)) - TODO ([#XXX](https://github.com/kedacore/keda/pull/XXX))