From a62eb45ae2bbbe0ce2f7cc460821d33fba48e735 Mon Sep 17 00:00:00 2001 From: Kevin Hannon Date: Thu, 14 Sep 2023 13:24:12 -0400 Subject: [PATCH] Rename job reasons to JobReasons as part of api review --- pkg/apis/batch/types.go | 17 --------- pkg/controller/job/job_controller.go | 22 +++++++----- pkg/controller/job/job_controller_test.go | 42 +++++++++++------------ staging/src/k8s.io/api/batch/v1/types.go | 16 ++++----- 4 files changed, 40 insertions(+), 57 deletions(-) diff --git a/pkg/apis/batch/types.go b/pkg/apis/batch/types.go index 7d15530425a..cb5e6eb22e7 100644 --- a/pkg/apis/batch/types.go +++ b/pkg/apis/batch/types.go @@ -533,23 +533,6 @@ const ( JobFailureTarget JobConditionType = "FailureTarget" ) -type JobReasonType string - -const ( - // PodFailurePolicy reason indicates a job failure condition is added due to - // a failed pod matching a pod failure policy rule - // https://kep.k8s.io/3329 - // This is currently a beta field. - PodFailurePolicyMatched JobReasonType = "PodFailurePolicy" - // BackOffLimitExceeded reason indicates that pods within a job have failed a number of - // times higher than backOffLimit times. - BackoffLimitExceeded JobReasonType = "BackoffLimitExceeded" - // DeadlineExceeded means job duration is past ActiveDeadline - DeadlineExceeded JobReasonType = "DeadlineExceeded" - // FailedIndexes means Job has failed indexes. - FailedIndexes JobReasonType = "FailedIndexes" -) - // JobCondition describes current state of a job. type JobCondition struct { // Type of job condition. diff --git a/pkg/controller/job/job_controller.go b/pkg/controller/job/job_controller.go index f283c708f76..2713980afe5 100644 --- a/pkg/controller/job/job_controller.go +++ b/pkg/controller/job/job_controller.go @@ -79,10 +79,14 @@ var ( MaxPodCreateDeletePerSync = 500 ) -// MaxFailedIndexesExceeded indicates that an indexed of a job failed -// https://kep.k8s.io/3850 -// In Beta, this should be moved to staging as an API field. -const maxFailedIndexesExceeded string = "MaxFailedIndexesExceeded" +const ( + // MaxFailedIndexesExceeded indicates that an indexed of a job failed + // https://kep.k8s.io/3850 + // In Beta, this should be moved to staging as an API field. + jobReasonMaxFailedIndexesExceeded string = "MaxFailedIndexesExceeded" + // FailedIndexes means Job has failed indexes. + jobReasonFailedIndexes string = "FailedIndexes" +) // Controller ensures that all Job objects have corresponding pods to // run their configured workload. @@ -815,16 +819,16 @@ func (jm *Controller) syncJob(ctx context.Context, key string) (rErr error) { jobCtx.finishedCondition = newFailedConditionForFailureTarget(failureTargetCondition, jm.clock.Now()) } else if failJobMessage := getFailJobMessage(&job, pods); failJobMessage != nil { // Prepare the interim FailureTarget condition to record the failure message before the finalizers (allowing removal of the pods) are removed. - jobCtx.finishedCondition = newCondition(batch.JobFailureTarget, v1.ConditionTrue, string(batch.PodFailurePolicyMatched), *failJobMessage, jm.clock.Now()) + jobCtx.finishedCondition = newCondition(batch.JobFailureTarget, v1.ConditionTrue, batch.JobReasonPodFailurePolicy, *failJobMessage, jm.clock.Now()) } } if jobCtx.finishedCondition == nil { if exceedsBackoffLimit || pastBackoffLimitOnFailure(&job, pods) { // check if the number of pod restart exceeds backoff (for restart OnFailure only) // OR if the number of failed jobs increased since the last syncJob - jobCtx.finishedCondition = newCondition(batch.JobFailed, v1.ConditionTrue, string(batch.BackoffLimitExceeded), "Job has reached the specified backoff limit", jm.clock.Now()) + jobCtx.finishedCondition = newCondition(batch.JobFailed, v1.ConditionTrue, batch.JobReasonBackoffLimitExceeded, "Job has reached the specified backoff limit", jm.clock.Now()) } else if jm.pastActiveDeadline(&job) { - jobCtx.finishedCondition = newCondition(batch.JobFailed, v1.ConditionTrue, string(batch.DeadlineExceeded), "Job was active longer than specified deadline", jm.clock.Now()) + jobCtx.finishedCondition = newCondition(batch.JobFailed, v1.ConditionTrue, batch.JobReasonDeadlineExceeded, "Job was active longer than specified deadline", jm.clock.Now()) } else if job.Spec.ActiveDeadlineSeconds != nil && !jobSuspended(&job) { syncDuration := time.Duration(*job.Spec.ActiveDeadlineSeconds)*time.Second - jm.clock.Since(job.Status.StartTime.Time) logger.V(2).Info("Job has activeDeadlineSeconds configuration. Will sync this job again", "key", key, "nextSyncIn", syncDuration) @@ -839,9 +843,9 @@ func (jm *Controller) syncJob(ctx context.Context, key string) (rErr error) { jobCtx.failedIndexes = calculateFailedIndexes(logger, &job, pods) if jobCtx.finishedCondition == nil { if job.Spec.MaxFailedIndexes != nil && jobCtx.failedIndexes.total() > int(*job.Spec.MaxFailedIndexes) { - jobCtx.finishedCondition = newCondition(batch.JobFailed, v1.ConditionTrue, maxFailedIndexesExceeded, "Job has exceeded the specified maximal number of failed indexes", jm.clock.Now()) + jobCtx.finishedCondition = newCondition(batch.JobFailed, v1.ConditionTrue, jobReasonMaxFailedIndexesExceeded, "Job has exceeded the specified maximal number of failed indexes", jm.clock.Now()) } else if jobCtx.failedIndexes.total() > 0 && jobCtx.failedIndexes.total()+jobCtx.succeededIndexes.total() >= int(*job.Spec.Completions) { - jobCtx.finishedCondition = newCondition(batch.JobFailed, v1.ConditionTrue, string(batch.FailedIndexes), "Job has failed indexes", jm.clock.Now()) + jobCtx.finishedCondition = newCondition(batch.JobFailed, v1.ConditionTrue, jobReasonFailedIndexes, "Job has failed indexes", jm.clock.Now()) } } jobCtx.podsWithDelayedDeletionPerIndex = getPodsWithDelayedDeletionPerIndex(logger, jobCtx) diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index c830f4267ba..2358f397eca 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -1909,7 +1909,7 @@ func TestSyncJobPastDeadline(t *testing.T) { expectedDeletions: 1, expectedFailed: 1, expectedCondition: batch.JobFailed, - expectedConditionReason: string(batch.DeadlineExceeded), + expectedConditionReason: batch.JobReasonDeadlineExceeded, }, "activeDeadlineSeconds bigger than single pod execution": { parallelism: 1, @@ -1923,7 +1923,7 @@ func TestSyncJobPastDeadline(t *testing.T) { expectedSucceeded: 1, expectedFailed: 1, expectedCondition: batch.JobFailed, - expectedConditionReason: string(batch.DeadlineExceeded), + expectedConditionReason: batch.JobReasonDeadlineExceeded, }, "activeDeadlineSeconds times-out before any pod starts": { parallelism: 1, @@ -1932,7 +1932,7 @@ func TestSyncJobPastDeadline(t *testing.T) { startTime: 10, backoffLimit: 6, expectedCondition: batch.JobFailed, - expectedConditionReason: string(batch.DeadlineExceeded), + expectedConditionReason: batch.JobReasonDeadlineExceeded, }, "activeDeadlineSeconds with backofflimit reach": { parallelism: 1, @@ -1942,7 +1942,7 @@ func TestSyncJobPastDeadline(t *testing.T) { failedPods: 1, expectedFailed: 1, expectedCondition: batch.JobFailed, - expectedConditionReason: string(batch.BackoffLimitExceeded), + expectedConditionReason: batch.JobReasonBackoffLimitExceeded, }, "activeDeadlineSeconds is not triggered when Job is suspended": { suspend: true, @@ -2098,7 +2098,7 @@ func TestPastDeadlineJobFinished(t *testing.T) { if err != nil { return false, nil } - if getCondition(j, batch.JobFailed, v1.ConditionTrue, string(batch.DeadlineExceeded)) { + if getCondition(j, batch.JobFailed, v1.ConditionTrue, batch.JobReasonDeadlineExceeded) { if manager.clock.Since(j.Status.StartTime.Time) < time.Duration(*j.Spec.ActiveDeadlineSeconds)*time.Second { return true, errors.New("Job contains DeadlineExceeded condition earlier than expected") } @@ -2397,7 +2397,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container main-container for pod default/mypod-0 failed with exit code 5 matching FailJob rule at index 1", }, }, @@ -2425,7 +2425,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailureTarget, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container main-container for pod default/mypod-0 failed with exit code 5 matching FailJob rule at index 1", }, }, @@ -2452,7 +2452,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container main-container for pod default/mypod-0 failed with exit code 5 matching FailJob rule at index 1", }, }, @@ -2480,7 +2480,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailureTarget, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container main-container for pod default/already-deleted-pod failed with exit code 5 matching FailJob rule at index 1", }, }, @@ -2507,7 +2507,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container main-container for pod default/already-deleted-pod failed with exit code 5 matching FailJob rule at index 1", }, }, @@ -2596,7 +2596,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container main-container for pod default/mypod-1 failed with exit code 5 matching FailJob rule at index 1", }, }, @@ -2642,7 +2642,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container main-container for pod default/mypod-0 failed with exit code 5 matching FailJob rule at index 1", }, }, @@ -2695,7 +2695,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container main-container for pod default/mypod-0 failed with exit code 42 matching FailJob rule at index 0", }, }, @@ -2797,7 +2797,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container init-container for pod default/mypod-0 failed with exit code 5 matching FailJob rule at index 1", }, }, @@ -2924,7 +2924,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.BackoffLimitExceeded), + Reason: batch.JobReasonBackoffLimitExceeded, Message: "Job has reached the specified backoff limit", }, }, @@ -3185,7 +3185,7 @@ func TestSyncJobWithJobPodFailurePolicy(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Pod default/mypod-0 has condition DisruptionTarget matching FailJob rule at index 0", }, }, @@ -3571,13 +3571,13 @@ func TestSyncJobWithJobBackoffLimitPerIndex(t *testing.T) { { Type: batch.JobFailureTarget, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container x for pod default/mypod-0 failed with exit code 3 matching FailJob rule at index 0", }, { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.PodFailurePolicyMatched), + Reason: batch.JobReasonPodFailurePolicy, Message: "Container x for pod default/mypod-0 failed with exit code 3 matching FailJob rule at index 0", }, }, @@ -3660,7 +3660,7 @@ func TestSyncJobWithJobBackoffLimitPerIndex(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.BackoffLimitExceeded), + Reason: batch.JobReasonBackoffLimitExceeded, Message: "Job has reached the specified backoff limit", }, }, @@ -3695,7 +3695,7 @@ func TestSyncJobWithJobBackoffLimitPerIndex(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: string(batch.FailedIndexes), + Reason: jobReasonFailedIndexes, Message: "Job has failed indexes", }, }, @@ -3733,7 +3733,7 @@ func TestSyncJobWithJobBackoffLimitPerIndex(t *testing.T) { { Type: batch.JobFailed, Status: v1.ConditionTrue, - Reason: maxFailedIndexesExceeded, + Reason: jobReasonMaxFailedIndexesExceeded, Message: "Job has exceeded the specified maximal number of failed indexes", }, }, diff --git a/staging/src/k8s.io/api/batch/v1/types.go b/staging/src/k8s.io/api/batch/v1/types.go index dc97e4aa192..00144f5eb41 100644 --- a/staging/src/k8s.io/api/batch/v1/types.go +++ b/staging/src/k8s.io/api/batch/v1/types.go @@ -535,21 +535,17 @@ const ( JobFailureTarget JobConditionType = "FailureTarget" ) -type JobReasonType string - const ( - // PodFailurePolicy reason indicates a job failure condition is added due to + // JobReasonPodFailurePolicy reason indicates a job failure condition is added due to // a failed pod matching a pod failure policy rule // https://kep.k8s.io/3329 // This is currently a beta field. - PodFailurePolicyMatched JobReasonType = "PodFailurePolicy" - // BackOffLimitExceeded reason indicates that pods within a job have failed a number of + JobReasonPodFailurePolicy string = "PodFailurePolicy" + // JobReasonBackOffLimitExceeded reason indicates that pods within a job have failed a number of // times higher than backOffLimit times. - BackoffLimitExceeded JobReasonType = "BackoffLimitExceeded" - // DeadlineExceeded means job duration is past ActiveDeadline - DeadlineExceeded JobReasonType = "DeadlineExceeded" - // FailedIndexes means Job has failed indexes. - FailedIndexes JobReasonType = "FailedIndexes" + JobReasonBackoffLimitExceeded string = "BackoffLimitExceeded" + // JobReasponDeadlineExceeded means job duration is past ActiveDeadline + JobReasonDeadlineExceeded string = "DeadlineExceeded" ) // JobCondition describes current state of a job.