KEP-3998: Graduate JobSuccessPolicy to GA #5148

tenzen-y · 2025-02-06T21:34:20Z

One-line PR description: Graduate JobSuccessPolicy to GA.

Issue link: Job success/completion policy #3998

Other comments:

k8s-ci-robot · 2025-02-06T21:34:24Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tenzen-y · 2025-02-06T21:35:33Z

keps/prod-readiness/sig-apps/3998.yaml

+stable:
+  approver: "@wojtek-t"


Hi, @wojtek-t!
Would you mind taking this graduation as PRR approver?
Thanks.

Sorry - I was out for last 3 weeks. Taking a look now.

No problem. Thanks for the review!

Signed-off-by: Yuki Iwai <[email protected]>

tenzen-y · 2025-02-06T22:00:36Z

@mimowo @soltysh @kannon92 @atiratree This is JobSuccessPolicy GA graduation KEP. Please take a look, thanks!

soltysh

A few minor comments.

soltysh · 2025-02-07T11:35:39Z

keps/sig-apps/3998-job-success-completion-policy/kep.yaml

 participating-sigs:
  - sig-apps
-status: implementable
+status: implemented


Don't change this, yet. If this slips 1.33 this will be misleading. We usually update this field after the release, while closing the issue tracking a particular feature.

Oh, I did not know that. I will revert this change. Thanks.

Addressed in 9585ac6

soltysh · 2025-02-07T11:46:04Z

keps/sig-apps/3998-job-success-completion-policy/kep.yaml

 kep-number: 3998
 authors:
  - "@tenzen-y"
 owning-sig: sig-apps


I'm missing the following updates in the README.md part of this enhancement:

Check appropriate boxes in the Release Signoff Checklist

You'll want to update integration and e2e tests sections with links to triage board, see template.

One of the GA criteria mentions mentions verifying Complete condition, what are the plans? I'd expect at this point in time this wording to be change do we will or we won't.

Update Implementation history with this edit information.

Let me check those, then share them with you here.

@soltysh I addressed them in 9b465bb.

Regarding 4, it would be better to "will do" in this GA graduation since we introduced the dedicated Completions reason, CompletionsReached for exiting Completions behavior and SuccessPolicy for new Completions with SuccessPolicy behavior.

Signed-off-by: Yuki Iwai <[email protected]>

soltysh

A few more comments.

soltysh · 2025-02-07T13:24:23Z

keps/sig-apps/3998-job-success-completion-policy/README.md

  - [ ] (R) [all GA Endpoints](https://github.com/kubernetes/community/pull/1806) must be hit by [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) 
- [ ] (R) Production readiness review completed
- [ ] (R) Production readiness review approved
+- [x] (R) Production readiness review completed


Nit: above (R) Ensure GA e2e tests meet requirements for [Conformance Tests should also be checked

I confirmed that, thanks.

soltysh · 2025-02-07T13:27:59Z

keps/sig-apps/3998-job-success-completion-policy/README.md

-  - [handling of the `.spec.successPolicy.rules.succeededIndexes` when some indexes remain pending](https://github.com/kubernetes/kubernetes/blob/6346b9d1327c4b8be2398d9715bdae5475e27569/test/integration/job/job_test.go#L608)
-  - [handling of the `.spec.successPolicy.rules.succeededCount` when some indexes remain pending](https://github.com/kubernetes/kubernetes/blob/6346b9d1327c4b8be2398d9715bdae5475e27569/test/integration/job/job_test.go#L653) 
-  - [handling of successPolicy when some indexes of job with `backOffLimitPerIndex` fail](https://github.com/kubernetes/kubernetes/blob/6346b9d1327c4b8be2398d9715bdae5475e27569/test/integration/job/job_test.go#L698)
+  - [enabling, disabling and re-enabling of the `JobSuccessPolicy` feature gate](https://github.com/kubernetes/kubernetes/blob/20b12ad5c389ff74792988bf1e0c10fe2820d9a1/test/integration/job/job_test.go#L872)


You're missing a link like this one https://storage.googleapis.com/k8s-triage/index.html?job=ci-kubernetes-integration&test=test%2Fintegration%2Fjob&xtest=TestSuccessPolicy_ReEnabling

Make sure to provide a link for each of the tests pointing to appropriate test.

I attached the dashboard link as a result.

5714f1c

soltysh · 2025-02-07T13:29:45Z

keps/sig-apps/3998-job-success-completion-policy/README.md

-  - [handling of successPolicy when all indexes succeeded](https://github.com/kubernetes/kubernetes/blob/3a8a60eba29940e26ac8db52329a91ba87305114/test/e2e/apps/job.go#L524-L530)
-  - [handling of the `.spec.successPolicy.rules.succeededIndexes` when some indexes remain pending](https://github.com/kubernetes/kubernetes/blob/3a8a60eba29940e26ac8db52329a91ba87305114/test/e2e/apps/job.go#L563-L569)
-  - [handling of the `.spec.successPolicy.rules.succeededCount` when some indexes remain pending](https://github.com/kubernetes/kubernetes/blob/3a8a60eba29940e26ac8db52329a91ba87305114/test/e2e/apps/job.go#L602-L608)
+  - [handling of successPolicy when all indexes succeeded](https://github.com/kubernetes/kubernetes/blob/20b12ad5c389ff74792988bf1e0c10fe2820d9a1/test/e2e/apps/job.go#L478-L484)


Similarly here: https://storage.googleapis.com/k8s-triage/index.html?sig=apps&test=with%20successPolicy%20should%20succeeded%20when%20all%20indexes%20succeeded

I attached the dashboard link as a result.

5714f1c

Signed-off-by: Yuki Iwai <[email protected]>

soltysh

/lgtm
/approve
from sig-apps pov
/hold
for @wojtek-t 's PRR review
/label tide/merge-method-squash

mimowo

LGTM

wojtek-t · 2025-02-10T09:28:20Z

/approve PRR

/hold cancel

k8s-ci-robot · 2025-02-10T09:28:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: soltysh, tenzen-y, wojtek-t

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:

~~keps/prod-readiness/OWNERS~~ [soltysh,wojtek-t]
~~keps/sig-apps/OWNERS~~ [soltysh]

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

* Graduate JobSuccessPolicy to GA Signed-off-by: Yuki Iwai <[email protected]> * Address review commentes Signed-off-by: Yuki Iwai <[email protected]> * Address review comments Signed-off-by: Yuki Iwai <[email protected]> --------- Signed-off-by: Yuki Iwai <[email protected]>

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 6, 2025

k8s-ci-robot requested review from kow3ns and palnabarun February 6, 2025 21:34

k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/apps Categorizes an issue or PR as relevant to SIG Apps. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 6, 2025

tenzen-y commented Feb 6, 2025

View reviewed changes

tenzen-y mentioned this pull request Feb 6, 2025

Job success/completion policy #3998

Closed

29 tasks

tenzen-y marked this pull request as ready for review February 6, 2025 21:46

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 6, 2025

k8s-ci-robot requested a review from jeremyrickard February 6, 2025 21:46

Graduate JobSuccessPolicy to GA

44a832b

Signed-off-by: Yuki Iwai <[email protected]>

tenzen-y force-pushed the graduate-3998-stable branch from 270a64e to 44a832b Compare February 6, 2025 21:56

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 6, 2025

soltysh reviewed Feb 7, 2025

View reviewed changes

tenzen-y force-pushed the graduate-3998-stable branch from 9585ac6 to e192264 Compare February 7, 2025 12:36

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 7, 2025

Address review commentes

9b465bb

Signed-off-by: Yuki Iwai <[email protected]>

tenzen-y force-pushed the graduate-3998-stable branch from e192264 to 9b465bb Compare February 7, 2025 12:40

soltysh reviewed Feb 7, 2025

View reviewed changes

Address review comments

5714f1c

Signed-off-by: Yuki Iwai <[email protected]>

soltysh approved these changes Feb 7, 2025

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. labels Feb 7, 2025

k8s-ci-robot assigned soltysh Feb 7, 2025

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 7, 2025

wojtek-t self-assigned this Feb 10, 2025

mimowo reviewed Feb 10, 2025

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 10, 2025

k8s-ci-robot merged commit d9d6411 into kubernetes:master Feb 10, 2025
4 checks passed

k8s-ci-robot added this to the v1.33 milestone Feb 10, 2025

tenzen-y deleted the graduate-3998-stable branch February 10, 2025 09:41

KEP-3998: Graduate JobSuccessPolicy to GA #5148

KEP-3998: Graduate JobSuccessPolicy to GA #5148

Conversation

tenzen-y commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Feb 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tenzen-y commented Feb 6, 2025

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

mimowo left a comment

Choose a reason for hiding this comment

Uh oh!

wojtek-t commented Feb 10, 2025

Uh oh!

k8s-ci-robot commented Feb 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tenzen-y commented Feb 6, 2025 •

edited

Loading