KEP3998: Replace API name Criteria with Rules

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
kubernetes · Mar 8, 2024 · 6c8d5c5 · 6c8d5c5
1 parent d3cde18
commit 6c8d5c5
Showing 1 changed file with 83 additions and 52 deletions.
diff --git a/keps/sig-apps/3998-job-success-completion-policy/README.md b/keps/sig-apps/3998-job-success-completion-policy/README.md
@@ -54,6 +54,7 @@
 - [Drawbacks](#drawbacks)
 - [Alternatives](#alternatives)
   - [Relax a validation for the &quot;completions&quot; of the indexed job](#relax-a-validation-for-the-completions-of-the-indexed-job)
+  - [Alternative API Name, &quot;Criteria&quot;](#alternative-api-name-criteria)
   - [Hold succeededIndexes as []int typed in successPolicy](#hold-succeededindexes-as-int-typed-in-successpolicy)
   - [Acceptable percentage of total succeeded indexes in the succeededCount field](#acceptable-percentage-of-total-succeeded-indexes-in-the-succeededcount-field)
   - [Match succeededIndexes using CEL](#match-succeededindexes-using-cel)
@@ -141,7 +142,7 @@ spec:
   completions: 10
   completionMode: Indexed
   successPolicy:
-    criteria:
+    rules:
     - succeededIndexes: "0"
   template:
     spec:
@@ -172,7 +173,7 @@ spec:
   completions: 10
   completionMode: Indexed
   successPolicy:
-    criteria:
+    rules:
     - succeededIndexes: 0
     - succeededCount: 5
       succeededIndexes: "1-9"
@@ -210,7 +211,7 @@ Specifically, the CronJob with `Forbid` concurrentPolicy created Jobs based on J
 Switching the status from `SuccessCriteriaMet` to `Failed` would bring the confusions to the systems,
 which depends on the Job API.
 
-So, the status can never switch from `SucessCriterionMet` to `Failed`.
+So, the status can never switch from `SucessCriteriaMet` to `Failed`.
 Additionally, once the job has `SuccessCriteriaMet=true` condition, the job definitely ends with `Complete=true` condition
 even if the lingering pods could potentially meet the failure policies.
 
@@ -220,7 +221,7 @@ even if the lingering pods could potentially meet the failure policies.
 the job controller can't store a correct value in `.status.completedIndexes`,
 we probably can not evaluate the SuccessPolicy correctly.
 
-- If we allow to set unlimited size of the value in `.spec.successPolicy.criteria.succeededIndexes`,
+- If we allow to set unlimited size of the value in `.spec.successPolicy.rules.succeededIndexes`,
 we have a risk similar to [KEP-3850: Backoff Limits Per Index For Indexed Jobs](https://github.com/kubernetes/enhancements/tree/76dcd4f342cc0388feb085e685d4cc018ebe1dc9/keps/sig-apps/3850-backoff-limits-per-index-for-indexed-jobs#risks-and-mitigations).
 So, we limit the size of `succeededIndexes` to 64KiB.
 
@@ -248,20 +249,20 @@ type JobSpec struct {
 
 // SuccessPolicy describes when a Job can be declared as succeeded based on the success of some indexes.
 type SuccessPolicy struct {
-	// Criteria represents the list of alternative criteria for declaring the jobs 
-	// as successful before its completion. Once any of the criteria are met,
+	// Rules represents the list of alternative criteria for declaring the jobs 
+	// as successful before its completion. Once any of the rules are met,
 	// the "SuccessCriteriaMet" condition is added, and the lingering pods are removed.
 	// The terminal state for such a job has the "Complete" condition.
-	// Additionally, these criteria are evaluated in order; Once the job meets one of the criteria,
-	// other criteria are ignored.
+	// Additionally, these rules are evaluated in order; Once the job meets one of the rules,
+	// other rules are ignored.
 	//
 	// +optional
-	Criteria []SuccessPolicyCriterion
+	Rules []SuccessPolicyRule
 }
 
-// SuccessPolicyCriteria describes a criterion for declaring a Job as succeeded.
-// Each criterion must have at least one of the "succeededIndexes" or "succeededCount" specified.
-type SuccessPolicyCriterion struct {
+// SuccessPolicyRule describes a criteria for declaring a Job as succeeded.
+// Each criteria must have at least one of the "succeededIndexes" or "succeededCount" specified.
+type SuccessPolicyRule struct {
 	// SucceededIndexes specifies the set of indexes 
 	// which need to be contained in the actual set of succeeded indexes for the job.
 	// The list of indexes must be within 0 to ".spec.completions-1" and
@@ -298,15 +299,15 @@ const (
 )
 ```
 
-Moreover, we validate the following constraints for the `criteria`:
+Moreover, we validate the following constraints for the `rules`:
 - whether each criterion have at least one of the `succeededIndexes` or `succeededCount` specified.
 - whether the specified indexes in the `succeededIndexes` and
   the number of indexes in the `succeededCount` don't exceed the value of `completions`.
 - whether `Indexed` is specified in the `completionMode` field.
 - whether the size of `succeededIndexes` is under 64Ki.
 - whether the `succeededIndexes` field has a valid format.
 - whether the `succeededCount` field has an absolute number.
-- whether the criteria haven't changed.
+- whether the rules haven't changed.
 - whether the successPolicies meet the `succeededCount <= |succeededIndexes|`, 
 where `|succeededIndexes|` means the number of indexes in the `succeededIndexes`. 
 
@@ -331,7 +332,7 @@ after all pods are terminated, the terminal condition (`Failed` or `Complete`) i
 
 - `FailureTarget` is added to the Job matched with FailurePolicy with `action=FailJob` and triggers the termination of the lingering pods.
 Then, after the lingering pods are terminated, the `Failed` condition is added to the Job.
-- `SuccessCriterionMet` is added to the Job matched with SuccessPolicy and triggers the termination of lingering pods. 
+- `SuccessCriteriaMet` is added to the Job matched with SuccessPolicy and triggers the termination of lingering pods. 
 Then, after the lingering pods are terminated, the `Complete` condition is added to the Job.
 
 ### Transition of "status.conditions"
@@ -379,15 +380,15 @@ And then, if we introduce these features to this enhancement, we need to have th
 #### Possibility for the lingering pods to continue running after the job meets the successPolicy
 
 There are cases where a batch workload can be declared as succeeded, and continue the lingering pods if a number of pods succeed.
-So, it is possible to introduce a new field, `whenCriterionAchieved` to make configurable the action for the lingering pods.
+So, it is possible to introduce a new field, `whenCriteriaAchieved` to make configurable the action for the lingering pods.
 
 Note that if we introduce the `whenCriteriaChieved` field, we must have the second alpha stage.
 
 ##### Additional Story
 
 As a simulation researcher/engineer for fluid dynamics/biochemistry.
 I want to mark the job as successful and continue the lingering pods if some indexes succeed
-because I set the minimum required value for sampling in the `.successPolicy.criteria.succeededCount` and
+because I set the minimum required value for sampling in the `.successPolicy.rules.succeededCount` and
 perform the same simulation in all indexes.
 
 ```yaml
@@ -398,10 +399,10 @@ spec:
   completions: 10
   completionMode: Indexed
   successPolicy:
-    criteria:
+    rules:
     - succeededCount: 5
       succeededIndexes: "1-9"
-      whenCriterionAchieved: continue
+      whenCriteriaAchieved: continue
   template:
     spec:
       restartPolicy: Never
@@ -414,8 +415,8 @@ spec:
 ##### Job API
 
 ```golang
-// SuccessPolicyCriteria describes a Job can be succeeded based on succeeded indexes.
-type SuccessPolicyCriterion struct {
+// SuccessPolicyRules describes a Job can be succeeded based on succeeded indexes.
+type SuccessPolicyRule struct {
 	...
 	// Specifies the action to be taken on the not finished (successCriteriaMet or failed) pods 
 	// when the job achieved the requirements.
@@ -429,41 +430,41 @@ type SuccessPolicyCriterion struct {
 	// - Terminate indicates that not finished pods would be terminated.
 	//
 	// Default value is Terminate.
-	WhenCriterionAchieved WhenCriterionAchievedSuccessPolicy	
+	WhenCriteriaAchieved WhenCriteriaAchievedSuccessPolicy	
 }
 
-// WhenCriterionAchievedSuccessPolicy specifies the action to be taken on the pods
+// WhenCriteriaAchievedSuccessPolicy specifies the action to be taken on the pods
 // when the job achieved the requirements.
 // +enum
-type WhenCriterionAchievedSuccessPolicy string
+type WhenCriteriaAchievedSuccessPolicy string
 
 const (
 	// All pods wouldn't be terminated when the job reached successPolicy.
 	// When the lingering pods failed, the pods would ignore the terminating policies (backoffLimit,
 	// backoffLimitPerIndex, and podFailurePolicy, etc.) and the pods aren't re-created.
-	ContinueWhenCriterionAchievedSuccessPolicy WhenCriterionAchievedSuccessPolicy = "Continue"
+	ContinueWhenCriteriaAchievedSuccessPolicy WhenCriteriaAchievedSuccessPolicy = "Continue"
 
 	// All pods wouldn't be terminated when the job reached successPolicy.
 	// When the lingering pods failed, the pods would follow the terminating policies (backoffLimit,
 	// backoffLimitPerIndex, and podFailurePolicy, etc.) and the pods are re-created.
-	ContinueWithRecreationsWhenCriterionAchievedSuccessPolicy WhenCriterionAchievedSuccessPolicy = "ContinueWithRecreations"
+	ContinueWithRecreationsWhenCriteriaAchievedSuccessPolicy WhenCriteriaAchievedSuccessPolicy = "ContinueWithRecreations"
 
 	// Not finished pods would be terminated when the job reached successPolicy.
-	TerminateWhenCriterionAchievedSuccessPolicy WhenCriterionAchievedSuccessPolicy = "Terminate"
+	TerminateWhenCriteriaAchievedSuccessPolicy WhenCriteriaAchievedSuccessPolicy = "Terminate"
 )
 ```
 
 ##### Evaluation
 
-We need to have more discussions if we support the `continue` and `continueWithRecreatios` in the `whenCriterionAchieved`.
+We need to have more discussions if we support the `continue` and `continueWithRecreatios` in the `whenCriteriaAchieved`.
 We have main discussion points here:
 
-1. After the job meets any successPolicy with `whenCriterionAchieved=continue` and the job gets `SuccessCriteriaMet` condition,
+1. After the job meets any successPolicy with `whenCriteriaAchieved=continue` and the job gets `SuccessCriteriaMet` condition,
    what we would expect to happen, when the lingering pods are failed.
    We may be able to select one of the actions in `a: Failed pods follow terminating policies like backoffLimit and podFailurePolicy`
    or `b: Failed pods are terminated immediately, and the terminating policies are ignored`.
-   Moreover, as an alternative, we may be able to select the action `b` for the `whenCriterionAchieved=continue`,
-   and then we may be possible to introduce a new `whenCriterionAchieved`, `continueWithRecreations`, for the action `a`.
+   Moreover, as an alternative, we may be able to select the action `b` for the `whenCriteriaAchieved=continue`,
+   and then we may be possible to introduce a new `whenCriteriaAchieved`, `continueWithRecreations`, for the action `a`.
 
   - `terminate`: The current supported behavior. All pods would be terminated by the job controller.
   - `continue`: This behavior isn't supported in the alpha stage.
@@ -475,7 +476,7 @@ We have main discussion points here:
 
 ##### Transition of "status.conditions"
 
-When the job with `whenCriterionAchieved=continue` is submitted, the job `status.conditions` transits in the following:
+When the job with `whenCriteriaAchieved=continue` is submitted, the job `status.conditions` transits in the following:
 Note that the Job doesn't have an actual `Running` condition in the `status.conditions`.
 
 ```mermaid
@@ -521,7 +522,7 @@ spec:
   completions: 10
   completionMode: Indexed
   successPolicy:
-    criteria:
+    rules:
     - succeededIndexes: "0"
       setSuccessCriteriaMetReason: "LeaderSucceeded"
     - succeededIndexes: "1-9"
@@ -552,11 +553,11 @@ decreasing the machine-readability would decrease the valuable that we have this
 ###### Set the entire reason
 
 ```golang
-// SuccessPolicyCriteria describes a criterion for declaring a Job as succeeded.
-// Each criterion must have at least one of the "succeededIndexes" or "succeededCount" specified.
-type SuccessPolicyCriterion struct {
+// SuccessPolicyCriteria describes a criteria for declaring a Job as succeeded.
+// Each criteria must have at least one of the "succeededIndexes" or "succeededCount" specified.
+type SuccessPolicyCriteria struct {
 	// SetSuccessCriteriaMetReason specifies the CamelCase reason for the "SuccessCriteriaMet" condition.
-	// Once the job meets this criterion, the specified reason is set to the "status.reason".
+	// Once the job meets this criteria, the specified reason is set to the "status.reason".
 	// The default value is "JobSuccessPolicy".
 	//
 	// +optional
@@ -567,11 +568,11 @@ type SuccessPolicyCriterion struct {
 ###### Set the suffix of the reason
 
 ```golang
-// SuccessPolicyCriteria describes a criterion for declaring a Job as succeeded.
-// Each criterion must have at least one of the "succeededIndexes" or "succeededCount" specified.
-type SuccessPolicyCriterion struct {
+// SuccessPolicyRule describes a criteria for declaring a Job as succeeded.
+// Each criteria must have at least one of the "succeededIndexes" or "succeededCount" specified.
+type SuccessPolicyRule struct {
 	// SuccessCriteriaMetReasonSuffix specifies the CamelCase suffix of the reason for the "SuccessCriteriaMet" condition.
-	// Once the job meets this criterion, "JobSuccessPolicy" and the specified suffix is combined with "As".
+	// Once the job meets these criteria, "JobSuccessPolicy" and the specified suffix is combined with "As".
 	// For example, if specified suffix is "LeaderSucceeded", it is represented as "JobSuccessPolicyAsLeaderSucceeded".
 	//
 	// +optional
@@ -605,16 +606,16 @@ to implement this enhancement.
 - Test scenarios:
   - enabling, disabling and re-enabling of the `JobSuccessPolicy` feature gate
   - handling of successPolicy when all indexes succeeded
-  - handling of the `.spec.successPolicy.criteria.succeededIndexes` when some indexes remain pending
-  - handling of the `.spec.successPolicy.criteria.succeededCount` when some indexes remain pending 
+  - handling of the `.spec.successPolicy.rules.succeededIndexes` when some indexes remain pending
+  - handling of the `.spec.successPolicy.rules.succeededCount` when some indexes remain pending 
   - handling of successPolicy when some indexes of job with `backOffLimitPerIndex` fail
 
 ##### e2e tests
 
 - Test scenarios:
   - handling of successPolicy when all indexes succeeded
-  - handling of the `.spec.successPolicy.criteria.succeededIndexes` when some indexes remain pending
-  - handling of the `.spec.successPolicy.criteria.succeededCount` when some indexes remain pending
+  - handling of the `.spec.successPolicy.rules.succeededIndexes` when some indexes remain pending
+  - handling of the `.spec.successPolicy.rules.succeededCount` when some indexes remain pending
 
 ### Graduation Criteria
 
@@ -625,7 +626,7 @@ to implement this enhancement.
 
 #### Optional Second Alpha
 
-- Decided whether we introduce a `whenCriterionAchived=continue` and `whenCriterionAchived=continueWithRecreations`.
+- Decided whether we introduce a `whenCriteriaAchived=continue` and `whenCriteriaAchived=continueWithRecreations`.
 Please see the [Possibility for the lingering pods to continue running after the job meets the successPolicy](#possibility-for-the-lingering-pods-to-continue-running-after-the-job-meets-the-successpolicy)
 section for discussion points.
 - Decided whether we introduce a new CronJob concurrentPolicy, `ForbidUntilJobSuccessful`.
@@ -786,7 +787,7 @@ No.
 Yes, it will increase the size of existing API objects only when the `.spec.successPolicy` is set.
 
 - API type(s): Job
-- Estimated increase in size: `.spec.successPolicy.criteria.succeededIndexes` field are impacted.
+- Estimated increase in size: `.spec.successPolicy.rules.succeededIndexes` field are impacted.
 In the worst case, the size of `succeededIndexes` can be estimated about 64KiB (see [Risks and Mitigations](#risks-and-mitigations)).
 
 ###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?
@@ -833,6 +834,7 @@ consider tuning the parameters for [APF](https://kubernetes.io/docs/concepts/clu
 - 2023.06.09: API design is updated.
 - 2023.10.03: API design is updated.
 - 2024.02.07: API is finalized for the alpha stage.
+- 2024.03.05: "Criteria" is replaced with "Rules".
 
 ## Drawbacks
 
@@ -845,13 +847,42 @@ Currently, the indexed job is restricted `.spec.completion!=nil`.
 By relaxing the validation, the indexed job can be declared as succeeded when some indexes succeeded,
 similar to NonIndexed jobs.
 
+### Alternative API Name, "Criteria"
+
+The `criteria` would be matched to express the behavior of the successPolicy compared with `rules`.
+But, we decided to adopt the API name, `rules` to keep consistency with the existing `podFailurePolicy.rules`.
+
+```golang
+// SuccessPolicy describes when a Job can be declared as succeeded based on the success of some indexes.
+type SuccessPolicy struct {
+	// Criteria represents the list of alternative criteria for declaring the jobs 
+	// as successful before its completion. Once any of the criteria are met,
+	// the "SuccessCriteriaMet" condition is added, and the lingering pods are removed.
+	// The terminal state for such a job has the "Complete" condition.
+	// Additionally, these criteria are evaluated in order; Once the job meets one of the criteria,
+	// other criteria are ignored.
+	//
+	// +optional
+	Criteria []SuccessPolicyCriteria
+}
+
+...
+
+// These are valid conditions of a job.
+const (
+	// JobSuccessCriteriaMet means the job has been succeeded.
+	JobSucceessCriteriaMet JobConditionType = "SuccessCriteriaMet"
+	...
+)
+```
+
 ### Hold succeededIndexes as []int typed in successPolicy
 
 We can hold the `succeededIndexes` as []int typed that a job can be declared as succeeded.
 
 ```golang
 // SuccessPolicyOnSucceededPods describes a Job can be succeeded based on succeeded pods.
-type SuccessPolicyCriterion struct {
+type SuccessPolicyCriteria struct {
 	// Specifies a set of required indexes.
 	// The job is declared as succeeded if a set of indexes are succeeded. 
 	// The list of indexes must come within 0 to `.spec.completions` and
@@ -882,8 +913,8 @@ However, there is no effective use case for typical stories using elastic horovo
 as all pods must be completed.
 
 ```golang
-// SuccessPolicyCriterion describes a Job can be succeeded based on succeeded indexes.
-type SuccessPolicyCriterion struct {
+// SuccessPolicyCriteria describes a Job can be succeeded based on succeeded indexes.
+type SuccessPolicyCriteria struct {
 	...
 	// Specifies the required number of indexes when .spec.completionMode =
 	// "Indexed".
@@ -903,8 +934,8 @@ We can accept a set of required indexes represented by CEL in the `succeededInde
 However, it is difficult to represent the set of indexes without regularity. 
 
 ```golang
-// SuccessPolicyCriterion describes a Job can be succeeded based on succeeded indexes.
-type SuccessPolicyCriterion struct {
+// SuccessPolicyCriteria describes a Job can be succeeded based on succeeded indexes.
+type SuccessPolicyCriteria struct {
 	...
 	// Specifies a set of required indexes using CEL.
 	// For example, if the completed indexes are only the last index, they are