Update the managedBy KEP with fix for terminating pods (kubernetes#4657)

* Update the managedBy KEP

* Update keps/sig-apps/4368-support-managed-by-for-batch-jobs/README.md

Co-authored-by: Filip Křepinský <[email protected]>

* Update implementation history

* Update keps/sig-apps/4368-support-managed-by-for-batch-jobs/README.md

Co-authored-by: Maciej Szulik <[email protected]>

* Mention FailureTarget and SuccessCriteriaMet conditions

* Full links

* Full links2

* Add the ForbidSoft to non-goals

* go through second alpha

* add validation for proceeding addition of Failed or Complete conditions

* remarks about related issues

* Add test plan

* Links for the integration tests

* Move validation to the second alpha

* cleanup irrelevant paragraph now

* New risks section

Signed-off-by: Michal Wozniak <[email protected]>

* add reviewers for the kep to reflect the status quo

* typo fix

* revert

* new diagram

* Add a note about the test for re-enabling

---------

Signed-off-by: Michal Wozniak <[email protected]>
Co-authored-by: Filip Křepinský <[email protected]>
Co-authored-by: Maciej Szulik <[email protected]>

Author: 3 people

keps/sig-apps/3329-retriable-and-non-retriable-failures/README.md

@@ -90,6 +90,7 @@ tags, and then generate with `hack/update-toc.sh`.
   - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional)
     - [Job-level vs. pod-level spec](#job-level-vs-pod-level-spec)
     - [Relationship with Pod.spec.restartPolicy](#relationship-with-podspecrestartpolicy)
+    - [The scope of the FailureTarget condition](#the-scope-of-the-failuretarget-condition)
     - [Current state review](#current-state-review)
       - [Preemption](#preemption)
       - [Taint-based eviction](#taint-based-eviction)
@@ -521,6 +522,20 @@ Job controller. For example, Kubelet could restart a failed container before the
 Job controller decides to terminate the corresponding job due to a rule using
 `onExitCodes`.
 
+#### The scope of the FailureTarget condition
+
+As part of this KEP we introduced the
+[FailureTarget condition](#interim-failuretarget-job-condition) scoped to the
+failures due to pod failure policy.
+
+However, we are going to extend the scope of the condition to all Job failure
+scenarios (covering also backoffLimit exceeded and ActiveDeadlineSeconds
+exceeded), as part of fixing
+(issue #123775)[https://github.com/kubernetes/kubernetes/issues/123775].
+
+See more details in the
+[Job API managed-by mechanism](https://github.com/kubernetes/enhancements/blob/master/keps/sig-apps/4368-support-managed-by-for-batch-jobs/README.md).
+
 #### Current state review
 
 Here we review the current state of kubernetes (version 1.24) regarding its

keps/sig-apps/3939-allow-replacement-when-fully-terminated/README.md

@@ -96,6 +96,7 @@ tags, and then generate with `hack/update-toc.sh`.
 - [Design Details](#design-details)
   - [Job API Definition](#job-api-definition)
     - [Defaulting and validation](#defaulting-and-validation)
+    - [Tracking the terminating pods](#tracking-the-terminating-pods)
   - [Implementation](#implementation)
   - [Test Plan](#test-plan)
     - [Prerequisite testing updates](#prerequisite-testing-updates)
@@ -406,6 +407,22 @@ is in use:
 When `podFailurePolicy` is in use, the only allowed value for `podFailurePolicy`
 is `Failed`.
 
+#### Tracking the terminating pods
+
+In order to allow the quota management for Job-level controllers [story 3](#story-3)
+we introduced the `.status.terminating` field which tracks the number of
+terminating pods. However, in the initial Beta implementation the field stops
+tracking the number of terminating pods as soon as the Job is marked as Failed
+with the `Failed` condition (see (issue #123775)[https://github.com/kubernetes/kubernetes/issues/123775]).
+The remaining pods may be occupying resources for an arbitrary amount of time.
+
+In 1.31 we are going to fix this issue by delaying the
+addition of the `Failed` or `Complete` conditions until all pods are fully
+terminated. To indicate that a Job is doomed to fail or succeed, as soon as
+possible, we extend the scope of pre-existing conditions: `FailureTarget`, and
+`SuccessCriteriaMet`, respectively, See more details in
+[Job API managed-by mechanism](https://github.com/kubernetes/enhancements/blob/master/keps/sig-apps/4368-support-managed-by-for-batch-jobs/README.md).
+
 ### Implementation
 
 As part of this KEP, we need to track pods that are terminating (`deletionTimestamp != nil` and `phase` is `Pending` or `Running`).
@@ -466,6 +483,11 @@ implementing this enhancement to ensure the enhancements have also solid foundat
 - `gc_controller.go`: `April 3rd 2023` - `82.4`
    a. Set `PodPhase` to `failed` when `JobPodReplacementPolicy` true but `PodDisruptionConditions` is false
 
+The following scenarios related to [tracking the terminating pods](#tracking-the-terminating-pods) are covered:
+- `Failed` or `Complete` conditions are not added while there are still terminating pods
+- `FailureTarget` is added when backoffLimitCount is exceeded, or activeDeadlineSeconds timeout is exceeded
+- `SuccessCriteriaMet` is added when the `completions` are satisfied
+
 ##### Integration tests
 
 <!--
@@ -515,8 +537,13 @@ Case for disable and reenable `JobPodReplacementPolicy`
   11. Verify that pod creation only occurs once pod is fully terminated.
   12. Verify that pod creation only occurs once deletion happens.
 
-To cover cases with `PodDisruptionCondition` we really only need to worry about tracking terminating fields.  
-Tests will verify counting of terminating fields regardless of `PodDisruptionCondition` being on or off.  
+To cover cases with `PodDisruptionCondition` we really only need to worry about tracking terminating fields.
+Tests will verify counting of terminating fields regardless of `PodDisruptionCondition` being on or off.
+
+The following scenarios related to [tracking the terminating pods](#tracking-the-terminating-pods) are covered:
+- `Failed` or `Complete` conditions are not added while there are still terminating pods
+- `FailureTarget` is added when backoffLimitCount is exceeded, or activeDeadlineSeconds timeout is exceeded
+- `SuccessCriteriaMet` is added when the `completions` are satisfied
 
 ##### e2e tests
 
@@ -571,6 +598,9 @@ We expect no non-infra related flakes in the last month as a GA graduation crite
 #### GA
 
 - Address reviews and bug reports from Beta users
+- Allow Job API clients tracking the number of the terminating pods until all
+  the resources are released (see [tracking the terminating pods](#tracking-the-terminating-pods)).
+  Also, link provide links for the relevant integration tests in the KEP.
 - Lock the `JobPodReplacementPolicy` feature-gate to true
 
 #### Deprecation

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

@@ -15,6 +15,7 @@
     - [Difference between "Complete" and "SuccessCriteriaMet"](#difference-between-complete-and-successcriteriamet)
     - [The CronJob concurrentPolicy is not affected by JobSuccessPolicy](#the-cronjob-concurrentpolicy-is-not-affected-by-jobsuccesspolicy)
     - [Status never switches from "SuccessCriteriaMet" to "Failed"](#status-never-switches-from-successcriteriamet-to-failed)
+    - [The scope of the SuccessCriteriaMet condition](#the-scope-of-the-successcriteriamet-condition)
   - [Risks and Mitigations](#risks-and-mitigations)
 - [Design Details](#design-details)
   - [Job API](#job-api)
@@ -215,6 +216,18 @@ 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.
 
+#### The scope of the SuccessCriteriaMet condition
+
+As part of this KEP we introduced the `SuccessCriteriaMet` condition scoped to
+the success policy.
+
+However, we are going to extend the scope of the condition to the scenario when
+the Job completes by reaching the `.spec.completions`, as part of fixing
+(issue #123775)[https://github.com/kubernetes/kubernetes/issues/123775].
+
+See more details in the
+[Job API managed-by mechanism](https://github.com/kubernetes/enhancements/blob/master/keps/sig-apps/4368-support-managed-by-for-batch-jobs/README.md).
+
 ### Risks and Mitigations
 
 - If the job object's size reaches to limit of the etcd and
@@ -346,17 +359,20 @@ Then, after the lingering pods are terminated, the `Complete` condition is added
 
 ### Transition of "status.conditions"
 
-When the job with successPolicies 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`.
+After extending the scope of the `SuccessCriteriaMet` and `FailureTarget` conditions
+as proposed in [The scope of the SuccessCriteriaMet condition](#the-scope-of-the-successcriteriamet-condition)
+the diagram of transitions looks like below:
 
 ```mermaid
 stateDiagram-v2
     [*] --> Running
-    Running --> Failed: Exceeded backoffLimit
+    Running --> FailureTarget: Exceeded backoffLimit
+    Running --> FailureTarget: Exceeded activeDeadlineSeconds
     Running --> FailureTarget: Matched FailurePolicy with action=FailJob
     FailureTarget --> Failed: All pods are terminated
     Failed --> [*]
     Running --> SuccessCriteriaMet: Matched SuccessPolicy
+    Running --> SuccessCriteriaMet: Achieved the expected completions
     SuccessCriteriaMet --> Complete: All pods are terminated
     Complete --> [*]
 ```
@@ -365,6 +381,7 @@ It means that the job's `.status.conditions` follows the following rules:
 
 - The job could have both `SuccessCriteriaMet=true` and `Complete=true` conditions.
 - The job can't have both `Failed=true` and `SuccessCriteriaMet=true` conditions.
+- The job can't have both `FailureTarget=true` and `SuccessCriteriaMet=true` conditions.
 - The job can't have both `Failed=true` and `Complete=true` conditions.
 
 #### The situations where successPolicy conflicts other terminating policies