KEP-3998: Graduate Job SuccessPolicy to Beta #4702

tenzen-y · 2024-06-06T12:55:38Z

One-line PR description: Graduate the JobSuccessPolicy to Beta.

Issue link: Job success/completion policy #3998

Other comments:
See also: Update the managedBy KEP with fix for terminating pods #4657

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

soltysh · 2024-06-07T14:14:03Z

Lastly, make sure to transition this PR from WIP to a final PR 😉

mimowo · 2024-06-07T14:23:20Z

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

 This means that the terminating policies are respected rather than the successPolicies,
 if the Job doesn't have the `FailureTarget` or `SuccessCriteriaMet` conditions yet.

-### Future Work


Since we move it out of the scope I would suggest to remove the "Optional Second Alpha" section from Graduation Criteria.

You're right.

tenzen-y · 2024-06-07T15:12:36Z

Lastly, make sure to transition this PR from WIP to a final PR 😉

Thank you for reviewing this PR! I definitely finalize this PR soon!

tenzen-y · 2024-06-11T03:03:28Z

@soltysh @mimowo @alculquicondor @atiratree @wojtek-t I finalized this JobSuccessPolicy graduation KEP.
PTAL. Thank you!

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

mimowo

LGTM, just minor nits

mimowo · 2024-06-11T07:18:57Z

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

+In the alpha stage, the upgrade->downgrade->upgrade testing was added in the integration tests
+[here](https://github.com/kubernetes/kubernetes/blob/6346b9d1327c4b8be2398d9715bdae5475e27569/test/integration/job/job_test.go#L794).
+
+In terms of a manual test for the upgrade and rollback, we can use th v1.30.


Suggested change

In terms of a manual test for the upgrade and rollback, we can use th v1.30.

In terms of a manual test for the upgrade and rollback, we can use the v1.30.

mimowo · 2024-06-11T07:19:09Z

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

+
+3. Simulate upgrade by enabling the feature for api server and control-plane.
+
+Then, very that the pod with index=2 is terminated and the Job has `SuccessCriteriaMet` and `Complete` conditions.


Suggested change

Then, very that the pod with index=2 is terminated and the Job has `SuccessCriteriaMet` and `Complete` conditions.

Then, verify that the pod with index=2 is terminated and the Job has `SuccessCriteriaMet` and `Complete` conditions.

wojtek-t

Just two minor nits from PRR perspective, other than that LGTM

wojtek-t · 2024-06-11T09:07:38Z

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


 - E2E tests passed as designed in [TestPlan](#test-plan).
- Added a new reasons to the existing `job_finished_total` metric in [Monitoring Requirements](#monitoring-requirements).
+- Introduced a new `job_succeeded_total` metric in [Monitoring Requirements](#monitoring-requirements).


Are there any tests for feature enablement/disablement?

nit: You'r mentioning you did an integration test - can you please link it?
[I see it linked below so it's not a blocking issue, but would help for the future]

Sure! Thank you for the suggestion!

wojtek-t · 2024-06-11T09:10:49Z

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

+which indicates the following situations: 
+
+- `JobSuccessPolicy` indicates a job is declared as `SuccessCriteriaMet` because the job meets `spec.succesPolicy`.
+- `Completions` indicates a job is declared as `SuccessCriteriaMet` because the job meets `spec.completions`.


Will enabling / using this feature result in any new API calls?

Please remove the "No" from there :)

Oops, I was supposed to remove the "No" in the alpha stage :)

Thank you!

soltysh

There are several nits, mostly from Michal, so I'll let him do the final pass.

In general this is good from sig-apps pov
/approve

soltysh · 2024-06-11T11:21:12Z

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

 - when the job meets the successPolicy and the number of failed pods per indexes exceeds `backoffLimitPerIndex` in all indexes.

 To avoid the above conflicts, terminating policies are evaluated the first before successPolicies.
 This means that the terminating policies are respected rather than the successPolicies,


Nit: the ## Release Signoff Checklist should be updated such that all required bits are checked as needed.

I updated the section. If I am missing something there, please let me know.
Thanks!

soltysh · 2024-06-11T11:29:25Z

/hold
for Wojtek's PRR approval

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

tenzen-y · 2024-06-11T12:27:58Z

@mimowo @wojtek-t Thank you for reviewing this KEP! I addressed all comments. PTAL Thanks!

wojtek-t · 2024-06-11T12:32:56Z

/approve PRR

k8s-ci-robot · 2024-06-11T12:33:05Z

[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

mimowo · 2024-06-11T12:47:59Z

Thank you!
/lgtm

tenzen-y · 2024-06-11T12:50:10Z

Thanks all!
/hold cancel

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 Jun 6, 2024

k8s-ci-robot requested review from kikisdeliveryservice and kow3ns June 6, 2024 12:55

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/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 6, 2024

soltysh reviewed Jun 7, 2024

View reviewed changes

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

mimowo reviewed Jun 7, 2024

View reviewed changes

tenzen-y mentioned this pull request Jun 10, 2024

Job success/completion policy #3998

Closed

29 tasks

tenzen-y force-pushed the beta-graduation-3998 branch 2 times, most recently from 43687a4 to 2d92c7f Compare June 10, 2024 23:18

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 11, 2024

tenzen-y changed the title ~~WIP: KEP3998: Graduate Job SuccessPolicy to Beta~~ KEP3998: Graduate Job SuccessPolicy to Beta Jun 11, 2024

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 Jun 11, 2024

tenzen-y changed the title ~~KEP3998: Graduate Job SuccessPolicy to Beta~~ KEP-3998: Graduate Job SuccessPolicy to Beta Jun 11, 2024

tenzen-y commented Jun 11, 2024

View reviewed changes

keps/prod-readiness/sig-apps/3998.yaml Show resolved Hide resolved

wojtek-t self-assigned this Jun 11, 2024

mimowo reviewed Jun 11, 2024

View reviewed changes

wojtek-t assigned soltysh Jun 11, 2024

wojtek-t reviewed Jun 11, 2024

View reviewed changes

soltysh approved these changes Jun 11, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 11, 2024

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2024

KEP3998: Beta Graduation of JobSuccessPolicy

6e2ddd3

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

Address comments from PRR and SIG-Apps approvers and reviewers

141ea75

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

tenzen-y force-pushed the beta-graduation-3998 branch from 7242051 to 141ea75 Compare June 11, 2024 12:24

k8s-ci-robot assigned mimowo Jun 11, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 11, 2024

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

k8s-ci-robot merged commit 85fabef into kubernetes:master Jun 11, 2024

k8s-ci-robot added this to the v1.31 milestone Jun 11, 2024

tenzen-y deleted the beta-graduation-3998 branch June 11, 2024 12:53

	In terms of a manual test for the upgrade and rollback, we can use th v1.30.
	In terms of a manual test for the upgrade and rollback, we can use the v1.30.


		3. Simulate upgrade by enabling the feature for api server and control-plane.

		Then, very that the pod with index=2 is terminated and the Job has `SuccessCriteriaMet` and `Complete` conditions.

	Then, very that the pod with index=2 is terminated and the Job has `SuccessCriteriaMet` and `Complete` conditions.
	Then, verify that the pod with index=2 is terminated and the Job has `SuccessCriteriaMet` and `Complete` conditions.

KEP-3998: Graduate Job SuccessPolicy to Beta #4702

KEP-3998: Graduate Job SuccessPolicy to Beta #4702

Conversation

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

Uh oh!

Uh oh!

Uh oh!

soltysh commented Jun 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tenzen-y commented Jun 7, 2024

Uh oh!

tenzen-y commented Jun 11, 2024

Uh oh!

Uh oh!

mimowo 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!

wojtek-t 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!

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!

soltysh commented Jun 11, 2024

Uh oh!

tenzen-y commented Jun 11, 2024

Uh oh!

wojtek-t commented Jun 11, 2024

Uh oh!

k8s-ci-robot commented Jun 11, 2024

Uh oh!

mimowo commented Jun 11, 2024

Uh oh!

tenzen-y commented Jun 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tenzen-y commented Jun 6, 2024 •

edited

Loading

soltysh commented Jun 7, 2024 •

edited

Loading