Feat: Enhance unused jobs discovery (#336)

* feat: add DeadlineExceeded indication & refactor * feat: Add tests for DeadlineExceeded * feat: add FailedIndexes indication * feat: add suspended jobs indication * rename * CR * better
yonahd · Aug 3, 2024 · c70a3ac · c70a3ac
1 parent 2338459
commit c70a3ac
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 9 deletions.
diff --git a/README.md b/README.md
@@ -174,7 +174,7 @@ kor [subcommand] --help
 | CRDs            | CRDs not used the cluster                                                                                                                                                                                                         |                                                                                                                                                                       |
 | Pvs             | PVs not bound to a PVC                                                                                                                                                                                                            |                                                                                                                                                                       |
 | Pdbs            | PDBs not used in Deployments<br/> PDBs not used in StatefulSets                                                                                                                                                                   |                                                                                                                                                                       |
-| Jobs            | Jobs status is completed<br/>  Jobs failed with no retries left                                                                                                                                                                                                         |                                                                                                                                                                       |
+| Jobs            | Jobs status is completed<br/>  Jobs status is suspended<br/>  Jobs failed with backoff limit exceeded (including indexed jobs) <br/> Jobs failed with dedaline exceeded                                                                                                                                              |                                                                                                                                                                       |
 | ReplicaSets     | replicaSets that specify replicas to 0 and has already completed it's work                                                                                                                                                        |
 | DaemonSets      | DaemonSets not scheduled on any nodes                                                                                                                                                                                             |
 | StorageClasses  | StorageClasses not used by any PVs/PVCs                                                                                                                                                                                           |

diff --git a/pkg/kor/jobs.go b/pkg/kor/jobs.go
@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"slices"
 
 	batchv1 "k8s.io/api/batch/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -52,11 +53,16 @@ func processNamespaceJobs(clientset kubernetes.Interface, namespace string, filt
 			unusedJobNames = append(unusedJobNames, ResourceInfo{Name: job.Name, Reason: reason})
 			continue
 		} else {
-			// Check if the job has a condition indicating it has exceeded the backoff limit
+			failureReasons := []string{"BackoffLimitExceeded", "DeadlineExceeded", "FailedIndexes"}
+
+			// Check if the job has a condition indicating it has failed
 			for _, condition := range job.Status.Conditions {
-				if condition.Type == batchv1.JobFailed && condition.Reason == "BackoffLimitExceeded" {
-					reason := "Job has exceeded backoff limit"
-					unusedJobNames = append(unusedJobNames, ResourceInfo{Name: job.Name, Reason: reason})
+				if condition.Type == batchv1.JobFailed && slices.Contains(failureReasons, condition.Reason) {
+					unusedJobNames = append(unusedJobNames, ResourceInfo{Name: job.Name, Reason: condition.Message})
+					break
+				}
+				if condition.Type == batchv1.JobSuspended {
+					unusedJobNames = append(unusedJobNames, ResourceInfo{Name: job.Name, Reason: condition.Message})
 					break
 				}
 			}

diff --git a/pkg/kor/jobs_test.go b/pkg/kor/jobs_test.go
@@ -90,6 +90,60 @@ func createTestJobs(t *testing.T) *fake.Clientset {
 		t.Fatalf("Error creating fake job: %v", err)
 	}
 
+	job6 := CreateTestJob(testNamespace, "test-job6", &batchv1.JobStatus{
+		Succeeded: 0,
+		Failed:    1,
+		Conditions: []batchv1.JobCondition{
+			{
+				Type:    batchv1.JobFailed,
+				Status:  corev1.ConditionTrue,
+				Reason:  "DeadlineExceeded",
+				Message: "Job was active longer than specified deadline",
+			},
+		},
+	}, AppLabels)
+
+	_, err = clientset.BatchV1().Jobs(testNamespace).Create(context.TODO(), job6, v1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating fake job: %v", err)
+	}
+
+	job7 := CreateTestJob(testNamespace, "test-job7", &batchv1.JobStatus{
+		Succeeded: 0,
+		Failed:    1,
+		Conditions: []batchv1.JobCondition{
+			{
+				Type:    batchv1.JobFailed,
+				Status:  corev1.ConditionTrue,
+				Reason:  "FailedIndexes",
+				Message: "Job has failed indexes",
+			},
+		},
+	}, AppLabels)
+
+	_, err = clientset.BatchV1().Jobs(testNamespace).Create(context.TODO(), job7, v1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating fake job: %v", err)
+	}
+
+	job8 := CreateTestJob(testNamespace, "test-job8", &batchv1.JobStatus{
+		Succeeded: 0,
+		Failed:    1,
+		Conditions: []batchv1.JobCondition{
+			{
+				Type:    batchv1.JobSuspended,
+				Status:  corev1.ConditionTrue,
+				Reason:  "JobSuspended",
+				Message: "Job suspended",
+			},
+		},
+	}, AppLabels)
+
+	_, err = clientset.BatchV1().Jobs(testNamespace).Create(context.TODO(), job8, v1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating fake job: %v", err)
+	}
+
 	return clientset
 }
 
@@ -101,12 +155,16 @@ func TestProcessNamespaceJobs(t *testing.T) {
 		t.Errorf("Expected no error, got %v", err)
 	}
 
-	if len(unusedJobs) != 3 {
-		t.Errorf("Expected 3 jobs unused got %d", len(unusedJobs))
+	expectedJobsNames := []string{"test-job2", "test-job4", "test-job5", "test-job6", "test-job7", "test-job8"}
+
+	if len(unusedJobs) != len(expectedJobsNames) {
+		t.Errorf("Expected %d jobs unused got %d", len(expectedJobsNames), len(unusedJobs))
 	}
 
-	if unusedJobs[0].Name != "test-job2" && unusedJobs[1].Name != "test-job4" && unusedJobs[2].Name != "test-job5" {
-		t.Errorf("job2', got %s", unusedJobs[0])
+	for i, job := range unusedJobs {
+		if job.Name != expectedJobsNames[i] {
+			t.Errorf("Expected %s, got %s", expectedJobsNames[i], job.Name)
+		}
 	}
 }
 
@@ -133,6 +191,9 @@ func TestGetUnusedJobsStructured(t *testing.T) {
 				"test-job2",
 				"test-job4",
 				"test-job5",
+				"test-job6",
+				"test-job7",
+				"test-job8",
 			},
 		},
 	}