Indexed job implementation #98812

alculquicondor · 2021-02-05T16:18:07Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

/sig apps
/area batch
/area workload-api/job

Implements support for Indexed Job.

For an indexed Job, the job controller creates a Pod with an associated index from 0 to (.spec.completions-1). The index is added as an annotation.

Other implementation details:

When feature gate is disabled, the job controller doesn't process indexed jobs.
The job controller creates Pods for the lowest indexes that don't have active or succeeded pods.
If there are more than one pod for an index, the controller removes all but one, using the same strategy where the number of active pods exceeds parallelism.
Active pods that don't have an index are removed.
Finished pods that don't have an index don't count towards failures or successes, but they are not removed.

Which issue(s) this PR fixes:

Fixes #97169 #14188

Ref kubernetes/enhancements#2214

Special notes for your reviewer:

Builds on top of #98441
Integration test to follow

Does this PR introduce a user-facing change?:

Support for Indexed Job: a Job that is considered completed when Pods associated to indexes from 0 to (.spec.completions-1) have succeeded.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: git.k8s.io/enhancements/keps/sig-apps/2214-indexed-job
- [Usage]: https://kubernetes.io/docs/concepts/workloads/controllers/job/#indexed-job

fejta-bot · 2021-02-05T18:03:38Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

alculquicondor · 2021-02-05T18:10:56Z

cc @ahg-g @adtac @soltysh

adtac · 2021-02-08T11:46:14Z

pkg/apis/batch/types.go

+	// The Job is considered complete when there is one successfully completed Pod
+	// for each index.
+	// When value is `Indexed`, .spec.completions must be specified and
+	// `.spec.parallelism` must be less than or equal to 10^5.


It helps to explain why there's a limit

That's a design decision. I don't think it's good to make it part of user documentation (user docs are build from this file).

adtac · 2021-02-08T11:48:03Z

pkg/apis/batch/types.go

+	// Job get an associated completion index from 0 to (.spec.completions - 1),
+	// available in the annotation batch.alpha.kubernetes.io/job-completion-index.
+	// The Job is considered complete when there is one successfully completed Pod
+	// for each index.


is it possible for a pod to successfully complete more than once? if so, should job authors design their pods around this (idempotency)?

There is a separate PR for API changes #98441

Idempotency requirement is already documented for regular Jobs.

adtac · 2021-02-08T11:49:13Z

pkg/apis/batch/types.go

+	// This field is alpha-level and is only honored by servers that enable the
+	// IndexedJob feature gate. More completion modes can be added in the future.
+	// If the Job controller observes a mode that it doesn't recognize, the
+	// controller skips updates for the Job.


hmm, not sure if silently failing is the right thing to do.

edit: the validation code seems to raise an error? please update the doc to reflect that

This is about the controller, not the apiserver. The apiserver would reject the unknown value.

This is a safeguard to add more completion modes in the future. The idea is that if the controller is one version behind during an upgrade, it has some way of handling a value it doesn't know.

pkg/controller/job/indexed_job_utils.go

adtac · 2021-02-08T12:17:36Z

pkg/controller/job/indexed_job_utils_test.go

+	phase v1.PodPhase
+}
+
+func hasFailingPods(status []indexPhase) bool {


It's used in job_controller_test.go
moved

pkg/controller/job/indexed_job_utils.go

pkg/controller/job/job_controller.go

adtac · 2021-02-08T12:47:47Z

staging/src/k8s.io/kubectl/pkg/describe/describe.go

+func capIndexesList(indexes string, softLimit int) string {
+	ix := softLimit
+	for ; ix < len(indexes); ix++ {
+		if indexes[ix] == ',' {


this will panic if len(indexes) < softLimit

line 2192 doesn't let that happen

staging/src/k8s.io/kubectl/pkg/describe/describe.go

adtac · 2021-02-08T12:50:00Z

staging/src/k8s.io/kubectl/pkg/describe/describe.go

+	if ix >= len(indexes) {
+		return indexes
+	}
+	return indexes[:ix+1] + "..."


do we expect indexes to be completed in order? i.e. is index N likely to complete before index N+K for large K?

if so, since we're cutting the string off anyway, the last 50 chars might be more informational to know how much is complete (although I'll concede that it'll be a little weird to read at first)

WDYT?

if parallelism is smaller than completions, yes, smaller indexes will complete earlier.

What should happen is that the first interval should contain most of the values, like 0-123456. The rest of the values would be more sporadic. I think the first interval actually gives you the most information: all indexes up to 123456 have completed, and there are some other completions here and there.

alculquicondor · 2021-02-08T20:21:14Z

/retest

alculquicondor · 2021-02-09T18:20:55Z

/retest

alculquicondor · 2021-02-09T19:09:11Z

/retest

ahg-g · 2021-02-11T17:02:41Z

/assign

just so I can see it on my backlog

alculquicondor · 2021-02-11T20:57:17Z

/retest

alculquicondor · 2021-02-22T21:43:44Z

/retest

soltysh

/lgtm
/approve

soltysh · 2021-02-23T18:24:01Z

/triage accepted

soltysh · 2021-02-23T18:24:16Z

/priority backlog

alculquicondor · 2021-02-23T19:48:18Z

/retest

When .spec.completionMode="Indexed"

k8s-ci-robot · 2021-03-03T22:47:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/controller/job/OWNERS~~ [soltysh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ahg-g · 2021-03-03T22:49:27Z

/lgtm

k8s-ci-robot requested review from brendandburns and deads2k February 5, 2021 16:19

alculquicondor force-pushed the indexed-job branch from 83f7f75 to 4d9f565 Compare February 5, 2021 16:48

adtac reviewed Feb 8, 2021

View reviewed changes

alculquicondor force-pushed the indexed-job branch from c81a31e to 6aed72a Compare February 8, 2021 21:00

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2021

k8s-ci-robot assigned ahg-g Feb 11, 2021

alculquicondor force-pushed the indexed-job branch from 6aed72a to 690b2a5 Compare February 11, 2021 17:55

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2021

alculquicondor force-pushed the indexed-job branch 2 times, most recently from 90f2524 to e850ccb Compare February 11, 2021 20:47

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 19, 2021

soltysh approved these changes Feb 23, 2021

View reviewed changes

k8s-ci-robot assigned soltysh Feb 23, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 23, 2021

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 23, 2021

k8s-ci-robot added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 23, 2021

alculquicondor force-pushed the indexed-job branch from ed2821c to 282646c Compare February 23, 2021 18:47

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 23, 2021

alculquicondor mentioned this pull request Feb 23, 2021

Add Job.status.uncountedTerminatedPods for Job tracking #98817

Merged

alculquicondor force-pushed the indexed-job branch from 282646c to 74a235b Compare March 1, 2021 20:24

adtac mentioned this pull request Mar 2, 2021

batch API: add suspended job #98727

Merged

alculquicondor force-pushed the indexed-job branch from 74a235b to 08338cb Compare March 3, 2021 16:16

Add completion index to Job Pods

8812531

When .spec.completionMode="Indexed"

alculquicondor force-pushed the indexed-job branch from 08338cb to 8812531 Compare March 3, 2021 22:46

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 3, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 3, 2021

alculquicondor mentioned this pull request Mar 3, 2021

Integration and e2e tests for Indexed job #99737

Merged

k8s-ci-robot merged commit afb1ee3 into kubernetes:master Mar 3, 2021

k8s-ci-robot added this to the v1.21 milestone Mar 3, 2021

annajung mentioned this pull request Mar 10, 2021

Indexed Job semantics in Job API kubernetes/enhancements#2214

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Indexed job implementation #98812

Indexed job implementation #98812

alculquicondor commented Feb 5, 2021 •

edited

fejta-bot commented Feb 5, 2021

alculquicondor commented Feb 5, 2021

adtac Feb 8, 2021

alculquicondor Feb 8, 2021

adtac Feb 8, 2021

alculquicondor Feb 8, 2021

adtac Feb 8, 2021 •

edited

alculquicondor Feb 8, 2021

adtac Feb 8, 2021

alculquicondor Feb 8, 2021

adtac Feb 8, 2021

alculquicondor Feb 8, 2021

adtac Feb 8, 2021 •

edited

alculquicondor Feb 8, 2021

alculquicondor commented Feb 8, 2021

alculquicondor commented Feb 9, 2021

alculquicondor commented Feb 9, 2021

ahg-g commented Feb 11, 2021

alculquicondor commented Feb 11, 2021

alculquicondor commented Feb 22, 2021

soltysh left a comment

soltysh commented Feb 23, 2021

soltysh commented Feb 23, 2021

alculquicondor commented Feb 23, 2021

k8s-ci-robot commented Mar 3, 2021

ahg-g commented Mar 3, 2021

Indexed job implementation #98812

Indexed job implementation #98812

Conversation

alculquicondor commented Feb 5, 2021 • edited

fejta-bot commented Feb 5, 2021

alculquicondor commented Feb 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adtac Feb 8, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adtac Feb 8, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alculquicondor commented Feb 8, 2021

alculquicondor commented Feb 9, 2021

alculquicondor commented Feb 9, 2021

ahg-g commented Feb 11, 2021

alculquicondor commented Feb 11, 2021

alculquicondor commented Feb 22, 2021

soltysh left a comment

Choose a reason for hiding this comment

soltysh commented Feb 23, 2021

soltysh commented Feb 23, 2021

alculquicondor commented Feb 23, 2021

k8s-ci-robot commented Mar 3, 2021

ahg-g commented Mar 3, 2021

alculquicondor commented Feb 5, 2021 •

edited

adtac Feb 8, 2021 •

edited

adtac Feb 8, 2021 •

edited