Job's backoffLimit needs to take number of failed pods into account

[janetkuo]

Job controller uses NumRequeues of the Job workqueue to calculate backoffLimit. This makes backoffLimit unpredictable especially when a job with parallelism larger than 1 and when its restartPolicy is Never.

Take this job for example:

apiVersion: batch/v1
kind: Job
metadata:
  name: error
spec:
  backoffLimit: 10
  parallelism: 10
  template:
    metadata:
      name: job
    spec:
      restartPolicy: Never
      containers:
        - name: job
          image: busybox
          args:
            - /bin/sh
            - -c
            - exit 1

The job will eventually fail after it creates 64 failed pods, instead of 10 failed pods.

  [...]
  status:
    conditions:
    - lastProbeTime: "2018-10-25T18:12:06Z"
      lastTransitionTime: "2018-10-25T18:12:06Z"
      message: Job has reached the specified backoff limit
      reason: BackoffLimitExceeded
      status: "True"
      type: Failed
    failed: 64

If restartPolicy is set to OnFailure, total number of container restarts count will be taken into account. This seems more predictable. However, if we change the restartPolicy of the same Job to OnFailure, all the failed Pods will be deleted when they are restarted:

kubernetes/pkg/controller/job/job_controller.go

Line 520 in b6fd5d9

jm.deleteJobPods(&job, activePods, errCh)

The job will fail when its creates 10 failed pods, but will end up with 0 pods.

@kubernetes/sig-apps-bugs @soltysh @kow3ns

/king bug

[soltysh]

@janetkuo wouldn't it be better to rewrite the backoff as it is today (that's why #64787 was created) instead of adding more hacks to the current implementation? How about we try to design the new implementation and fix that for 1.14?

[janetkuo]

@soltysh I'm not trying to add hack to backoff but want to remove the use of NumRequeues completely.

Based on original design doc of backoffLimit https://github.com/kubernetes/community/blob/master/contributors/design-proposals/apps/job.md#backoff-policy-and-failed-pod-limit:

For Never we count actual pod failures (reflected in .status.failed field). With OnFailure, we take an approximate value of pod restarts (as reported in .status.containerStatuses[*].restartCount).

RestartPolicy: OnFailure jobs are implemented as described above, but RestartPolicy: Never jobs don't count actual pod failures. Is there any reason it's implemented with NumRequeues instead of comparing # of failed pods with .status.failed?

I also find it vague and confusing what "number of job restarts" actually mean, especially for jobs with RestartPolicy: Never and/or run more than one pods in parallel. We need to describe the behavior more precisely.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[stojan-jovic]

I also experiencing this issue. Is #64787 in plans for any coming release?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[yasongxu]

/reopen

any plan for this issue ？ #64787，same problem at k8s v1.19 version

[k8s-ci-robot]

@yasongxu: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

any plan for this issue ？ #64787，same problem at k8s v1.19 version

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.