diff --git a/pkg/controllers/depends_on.go b/pkg/controllers/depends_on.go index 387cf64a..3020dec8 100644 --- a/pkg/controllers/depends_on.go +++ b/pkg/controllers/depends_on.go @@ -4,22 +4,21 @@ import ( jobset "sigs.k8s.io/jobset/api/jobset/v1alpha2" ) -// isJobReachedStatus checks if ReplicatedJob reaches Ready or Complete status. -func isJobReachedStatus(dependsOnJobName string, dependsOnJobStatus jobset.DependsOnStatus, rJobsReplicas map[string]int32, rJobsStatuses []jobset.ReplicatedJobStatus) bool { - +// isDependsOnJobReachedStatus checks if depends on ReplicatedJob reaches Ready or Complete status. +func isDependsOnJobReachedStatus(dependsOnJob jobset.DependsOn, dependsOnJobReplicas int32, rJobsStatuses []jobset.ReplicatedJobStatus) bool { // If the actual status is empty, return false. - actualStatus := findReplicatedJobStatus(rJobsStatuses, dependsOnJobName) + actualStatus := findReplicatedJobStatus(rJobsStatuses, dependsOnJob.Name) if actualStatus == nil { return false } // For Complete status, number of replicas must be equal to number of succeeded Jobs. - if dependsOnJobStatus == jobset.CompleteStatus && rJobsReplicas[dependsOnJobName] == actualStatus.Succeeded { + if dependsOnJob.Status == jobset.CompleteStatus && dependsOnJobReplicas == actualStatus.Succeeded { return true } // For Ready status, number of replicas must be equal to sum of ready, failed, and succeeded Jobs. - if dependsOnJobStatus == jobset.ReadyStatus && rJobsReplicas[dependsOnJobName] == actualStatus.Failed+actualStatus.Ready+actualStatus.Succeeded { + if dependsOnJob.Status == jobset.ReadyStatus && dependsOnJobReplicas == actualStatus.Failed+actualStatus.Ready+actualStatus.Succeeded { return true } diff --git a/pkg/controllers/depends_on_test.go b/pkg/controllers/depends_on_test.go new file mode 100644 index 00000000..9f7a2304 --- /dev/null +++ b/pkg/controllers/depends_on_test.go @@ -0,0 +1,117 @@ +package controllers + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + jobset "sigs.k8s.io/jobset/api/jobset/v1alpha2" +) + +func TestIsDependsOnJobReachedStatus(t *testing.T) { + tests := []struct { + name string + dependsOnJob jobset.DependsOn + dependsOnJobReplicas int32 + rJobsStatuses []jobset.ReplicatedJobStatus + expected bool + }{ + { + name: "status for ReplicatedJob is nil", + dependsOnJob: jobset.DependsOn{ + Name: "initializer", Status: jobset.CompleteStatus, + }, + dependsOnJobReplicas: 1, + rJobsStatuses: []jobset.ReplicatedJobStatus{ + { + Name: "invalid", + Ready: 0, + Succeeded: 1, + Failed: 0, + Suspended: 0, + Active: 0, + }, + }, + expected: false, + }, + { + name: "depends on ReplicatedJob reaches complete status", + dependsOnJob: jobset.DependsOn{ + Name: "initializer", Status: jobset.CompleteStatus, + }, + dependsOnJobReplicas: 2, + rJobsStatuses: []jobset.ReplicatedJobStatus{ + { + Name: "initializer", + Ready: 0, + Succeeded: 2, + Failed: 0, + Suspended: 0, + Active: 0, + }, + }, + expected: true, + }, + { + name: "depends on ReplicatedJob doesn't reach complete status", + dependsOnJob: jobset.DependsOn{ + Name: "initializer", Status: jobset.CompleteStatus, + }, + dependsOnJobReplicas: 2, + rJobsStatuses: []jobset.ReplicatedJobStatus{ + { + Name: "initializer", + Ready: 1, + Succeeded: 1, + Failed: 0, + Suspended: 0, + Active: 0, + }, + }, + expected: false, + }, + { + name: "depends on ReplicatedJob reaches ready status", + dependsOnJob: jobset.DependsOn{ + Name: "initializer", Status: jobset.ReadyStatus, + }, + dependsOnJobReplicas: 3, + rJobsStatuses: []jobset.ReplicatedJobStatus{ + { + Name: "initializer", + Ready: 1, + Succeeded: 1, + Failed: 1, + Suspended: 0, + Active: 0, + }, + }, + expected: true, + }, + { + name: "depends on ReplicatedJob doesn't reach ready status", + dependsOnJob: jobset.DependsOn{ + Name: "initializer", Status: jobset.ReadyStatus, + }, + dependsOnJobReplicas: 3, + rJobsStatuses: []jobset.ReplicatedJobStatus{ + { + Name: "initializer", + Ready: 2, + Succeeded: 0, + Failed: 0, + Suspended: 0, + Active: 1, + }, + }, + expected: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + actual := isDependsOnJobReachedStatus(tc.dependsOnJob, tc.dependsOnJobReplicas, tc.rJobsStatuses) + if diff := cmp.Diff(tc.expected, actual); diff != "" { + t.Errorf("unexpected finished value (+got/-want): %s", diff) + } + }) + } +} diff --git a/pkg/controllers/jobset_controller.go b/pkg/controllers/jobset_controller.go index ee7d0bf0..869cc4be 100644 --- a/pkg/controllers/jobset_controller.go +++ b/pkg/controllers/jobset_controller.go @@ -424,7 +424,7 @@ func (r *JobSetReconciler) resumeJobsIfNecessary(ctx context.Context, js *jobset rJobsReplicas[replicatedJob.Name] = replicatedJob.Replicas // For depends on, the Job is created only after the previous replicatedJob reached the status. - if replicatedJob.DependsOn != nil && !isJobReachedStatus(replicatedJob.DependsOn[0].Name, replicatedJob.DependsOn[0].Status, rJobsReplicas, replicatedJobStatuses) { + if replicatedJob.DependsOn != nil && !isDependsOnJobReachedStatus(replicatedJob.DependsOn[0], rJobsReplicas[replicatedJob.DependsOn[0].Name], replicatedJobStatuses) { continue } @@ -518,7 +518,7 @@ func (r *JobSetReconciler) reconcileReplicatedJobs(ctx context.Context, js *jobs rJobsReplicas[replicatedJob.Name] = replicatedJob.Replicas // For depends on, the Job is created only after the previous replicatedJob reached the status. - if replicatedJob.DependsOn != nil && !isJobReachedStatus(replicatedJob.DependsOn[0].Name, replicatedJob.DependsOn[0].Status, rJobsReplicas, replicatedJobStatuses) { + if replicatedJob.DependsOn != nil && !isDependsOnJobReachedStatus(replicatedJob.DependsOn[0], rJobsReplicas[replicatedJob.DependsOn[0].Name], replicatedJobStatuses) { continue }