From c70a3ac28ecee2d3a2aa01d2938f298e683437ed Mon Sep 17 00:00:00 2001
From: nati-elmaliach <45143653+nati-elmaliach@users.noreply.github.com>
Date: Sat, 3 Aug 2024 22:43:51 +0300
Subject: [PATCH] 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
---
README.md | 2 +-
pkg/kor/jobs.go | 14 ++++++---
pkg/kor/jobs_test.go | 69 +++++++++++++++++++++++++++++++++++++++++---
3 files changed, 76 insertions(+), 9 deletions(-)
diff --git a/README.md b/README.md
index f64957b9..6fbc2c32 100644
--- 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
PDBs not used in StatefulSets | |
-| Jobs | Jobs status is completed
Jobs failed with no retries left | |
+| Jobs | Jobs status is completed
Jobs status is suspended
Jobs failed with backoff limit exceeded (including indexed jobs)
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
index d25757f3..ddae0f9c 100644
--- 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
index 557f6799..3c30c42c 100644
--- 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",
},
},
}