From 5e1b5ec398c7b236425a9a93806b8b4fdb168928 Mon Sep 17 00:00:00 2001
From: Aldo Culquicondor <acondor@google.com>
Date: Mon, 12 Jul 2021 15:45:25 +0000
Subject: [PATCH] Revert counting deleted pods as failures for Job

When JobTrackingWithFinalizers is disabled. To preserve existing behavior.

Change-Id: Id1752f96feed322911712fe9e918e91e42eca809
---
 pkg/controller/job/job_controller.go      | 10 +++++++---
 pkg/controller/job/job_controller_test.go | 10 ++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/pkg/controller/job/job_controller.go b/pkg/controller/job/job_controller.go
index 551c5e58e2c..1a00f6b7e21 100644
--- a/pkg/controller/job/job_controller.go
+++ b/pkg/controller/job/job_controller.go
@@ -1143,9 +1143,13 @@ func getStatus(job *batch.Job, pods []*v1.Pod, uncounted *uncountedTerminatedPod
 		return p.Status.Phase == v1.PodSucceeded
 	}))
 	failed += int32(countValidPodsWithFilter(job, pods, uncounted.Failed(), func(p *v1.Pod) bool {
-		// Counting deleted Pods as failures to account for orphan Pods that never
-		// have a chance to reach the Failed phase.
-		return p.Status.Phase == v1.PodFailed || (p.DeletionTimestamp != nil && p.Status.Phase != v1.PodSucceeded)
+		if p.Status.Phase == v1.PodFailed {
+			return true
+		}
+		// When tracking with finalizers: counting deleted Pods as failures to
+		// account for orphan Pods that never have a chance to reach the Failed
+		// phase.
+		return uncounted != nil && p.DeletionTimestamp != nil && p.Status.Phase != v1.PodSucceeded
 	}))
 	return succeeded, failed
 }
diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go
index e19c855ada3..527d8e42dc8 100644
--- a/pkg/controller/job/job_controller_test.go
+++ b/pkg/controller/job/job_controller_test.go
@@ -1066,6 +1066,16 @@ func TestGetStatus(t *testing.T) {
 			wantFailed:    4,
 		},
 		"deleted pods": {
+			pods: []*v1.Pod{
+				buildPod().uid("a").phase(v1.PodSucceeded).deletionTimestamp().Pod,
+				buildPod().uid("b").phase(v1.PodFailed).deletionTimestamp().Pod,
+				buildPod().uid("c").phase(v1.PodRunning).deletionTimestamp().Pod,
+				buildPod().uid("d").phase(v1.PodPending).deletionTimestamp().Pod,
+			},
+			wantSucceeded: 1,
+			wantFailed:    1,
+		},
+		"deleted pods, tracking with finalizers": {
 			job: batch.Job{
 				Status: batch.JobStatus{
 					Succeeded:               1,