KEP-5307: Target ContainerRestartRules for Beta

keps/prod-readiness/sig-node/5307.yaml

@@ -1,3 +1,5 @@
 kep-number: 5307
 alpha:
   approver: "wojtek-t"
+beta:
+  approver: "wojtek-t"

keps/sig-node/5307-container-restart-policy/README.md

@@ -115,18 +115,18 @@ checklist items _must_ be updated for the enhancement to be released.
 
 Items marked with (R) are required *prior to targeting to a milestone / release*.
 
-- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
-- [ ] (R) KEP approvers have approved the KEP status as `implementable`
-- [ ] (R) Design details are appropriately documented
-- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors)
-  - [ ] e2e Tests for all Beta API Operations (endpoints)
+- [x] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
+- [x] (R) KEP approvers have approved the KEP status as `implementable`
+- [x] (R) Design details are appropriately documented
+- [x] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors)
+  - [x] e2e Tests for all Beta API Operations (endpoints)
   - [ ] (R) Ensure GA e2e tests meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) 
   - [ ] (R) Minimum Two Week Window for GA e2e tests to prove flake free
-- [ ] (R) Graduation criteria is in place
+- [x] (R) Graduation criteria is in place
   - [ ] (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
-- [ ] "Implementation History" section is up-to-date for milestone
+- [x] (R) Production readiness review completed
+- [x] (R) Production readiness review approved
+- [x] "Implementation History" section is up-to-date for milestone
 - [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
 - [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes
 
@@ -603,6 +603,11 @@ are not restarted and the pod fails.
 - Verify that PodFailurePolicy works with the restartPolicyRules; containers restarted
 by the restartPolicyRules should not fail the Pod and trigger PodFailurePolicy.
 
+E2E tests:
+- https://github.com/kubernetes/kubernetes/blob/9a3dce00ae32c81346883fb5a689a8240d48c218/test/e2e/node/pods.go#L722
+- https://github.com/kubernetes/kubernetes/blob/9a3dce00ae32c81346883fb5a689a8240d48c218/test/e2e/apps/job.go#L1331
+- https://testgrid.k8s.io/sig-release-master-informing#kind-master-alpha-beta&include-filter-by-regex=ContainerRestartRules
+
 ### Graduation Criteria
 
 <!--
@@ -690,6 +695,9 @@ pod restart policy, container restart policy, and container restart rules.
 - Container restart policy functionality running behind feature flag
 for at least one release.
 - Container restart policy runs well with Job controller.
+- All monitoring requirements completed.
+- All testing requirements completed.
+- All known pre-release issues and gaps resolved.
 
 #### GA
 
@@ -801,10 +809,11 @@ feature.
 NOTE: Also set `disable-supported` to `true` or `false` in `kep.yaml`.
 -->
 
-Yes. To roll back, the feature gate should be disabled in the API server and
-kubelets, and components should be restarted. If a Pod was created with the
-restartPolicyRules field while the gate was enabled, those rules will be ignored by
-kubelets once the feature is disabled.
+Yes. To roll back, the feature gate should be disabled in the API server and kubelets, and they should be restarted. 
+
+If a Pod was created with the container-level restart policy and/or restartPolicyRules while the feature gate was enabled, but later the feature gate is disabled, those container-level restart policy and rules will persist, but they will have no effect and will be ignored by the kubelet.
+
+Once the feature is disabled, pods cannot be created with container-level restart policy except Sidecar init containers with restart policy Always. Pods created with restart policy rules will be silently dropped.
 
 ###### What happens if we reenable the feature if it was previously rolled back?
 
@@ -827,14 +836,11 @@ You can take a look at one potential example of such test in:
 https://github.com/kubernetes/kubernetes/pull/97058/files#diff-7826f7adbc1996a05ab52e3f5f02429e94b68ce6bce0dc534d1be636154fded3R246-R282
 -->
 
-- Unit test for the API's validation with the feature enabled and disabled.
-- Unit test for the kubelet with the feature enabled and disabled.
-- Unit test for API on the new field for the Pod API. First enable
-the feature gate, create a Pod with a container including restartRules,
-validation should pass and the Pod API should match the expected result.
-Second, disable the feature gate, validate the Pod API should still pass
-and it should match the expected result. Lastly, re-enable the feature
-gate, validate the Pod API should pass and it should match the expected result.
+- Unit test for the API's validation with the feature enabled and disabled:
+  - See https://github.com/kubernetes/kubernetes/blob/9630ab9581afbac9835d53f9e620a1240a1d2d91/pkg/apis/core/validation/validation_test.go#L29065 and https://github.com/kubernetes/kubernetes/blob/9630ab9581afbac9835d53f9e620a1240a1d2d91/pkg/apis/core/validation/validation_test.go#L9357
+- Unit test for the kubelet with the feature enabled
+  - See https://github.com/kubernetes/kubernetes/blob/9630ab9581afbac9835d53f9e620a1240a1d2d91/pkg/kubelet/kubelet_test.go#L2476, https://github.com/kubernetes/kubernetes/blob/9630ab9581afbac9835d53f9e620a1240a1d2d91/pkg/kubelet/kubelet_pods_test.go#L3302, and https://github.com/kubernetes/kubernetes/blob/9630ab9581afbac9835d53f9e620a1240a1d2d91/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go#L2112
+- Unit test for API on the new field for the Pod API. First enable the feature gate, create a Pod with a container including restartRules, validation should pass and the Pod API should match the expected result. Second, disable the feature gate, validate the Pod API should still pass and it should match the expected result. Lastly, re-enable the feature gate, validate the Pod API should pass and it should match the expected result. This is achieved by the ValidationOptions, if the podSpec contains restart policy, or the feature gate is enabled, then the AllowContainerRestartPolicyRules would be true, see https://github.com/kubernetes/kubernetes/blob/9630ab9581afbac9835d53f9e620a1240a1d2d91/pkg/api/pod/util_test.go#L5965
 
 ### Rollout, Upgrade and Rollback Planning
 
@@ -870,17 +876,14 @@ Repeated restart of container or pods.
 
 ###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?
 
-<!--
-Describe manual testing that was done and the outcomes.
-Longer term, we may want to require automated upgrade/rollback tests, but we
-are missing a bunch of machinery and tooling and can't do that now.
--->
+Manual testing was performed to verify the upgrade and rollback paths.
+- **Upgrade:** A cluster with the feature disabled was upgraded to a version with the feature enabled. Pods with container-level `restartPolicy` and `restartPolicyRules` were deployed and observed to behave as expected.
+- **Rollback:** A cluster with the feature enabled was rolled back to a version with the feature disabled. Previously created pods continued to run and have the container-level `restartPolicy` and `restartPolicyRules`, but these fields were ignored. New Pods cannot be created with container-level restartPolicy, and `restartPolicyRules` are dropped silently.
+- **Upgrade->Downgrade->Upgrade:** This path was tested by performing the above steps sequentially. The feature behaved as expected at each stage, with `restartPolicyRules` being respected when the feature was enabled and ignored when disabled.
 
 ###### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.?
 
-<!--
-Even if applying deprecation policies, they may still surprise some users.
--->
+No.
 
 ### Monitoring Requirements
 
@@ -893,11 +896,9 @@ previous answers based on experience in the field.
 
 ###### How can an operator determine if the feature is in use by workloads?
 
-<!--
-Ideally, this should be a metric. Operations against the Kubernetes API (e.g.,
-checking if there are objects with field X set) may be a last resort. Avoid
-logs or events for this purpose.
--->
+Operators can determine if the feature is in use by checking the Pod spec for the presence of the `restartPolicyRules` field within container definitions. Operators can track the `ContainerStatus.RestartCount` to see how many times the container has restarted.
+
+Additionally, monitoring the `kube_pod_container_status_restarts_total` metric can indicate container restarts that might be governed by these rules.
 
 ###### How can someone using this feature know that it is working for their instance?
 
@@ -912,175 +913,68 @@ Recall that end users cannot usually observe component logs or access metrics.
 
 - [ ] Events
   - Event Reason: 
-- [ ] API .status
-  - Condition name: 
-  - Other field: 
-- [ ] Other (treat as last resort)
-  - Details:
+- [x] API .status
+  - Other field: ContainerStatuses
+    - Container statuses will have the history of the container restarts.
+- [x] Other (treat as last resort)
+  - Details: The metric `kube_pod_container_status_restarts_total` will show the total count of container restarts.
 
 ###### What are the reasonable SLOs (Service Level Objectives) for the enhancement?
 
-<!--
-This is your opportunity to define what "normal" quality of service looks like
-for a feature.
-
-It's impossible to provide comprehensive guidance, but at the very
-high level (needs more precise definitions) those may be things like:
-  - per-day percentage of API calls finishing with 5XX errors <= 1%
-  - 99% percentile over day of absolute value from (job creation time minus expected
-    job creation time) for cron job <= 10%
-  - 99.9% of /health requests per day finish with 200 code
-
-These goals will help you determine what you need to measure (SLIs) in the next
-question.
--->
+- The rate of unexpected container restarts (i.e., not matching a `restartPolicyRules`) should remain below 1%.
+- The time taken for a container to restart after an exit code matching `restartPolicyRules` should be within typical container restart latencies, accounting for exponential backoff.
+- Kubelet SLOs should not be impacted.
 
 ###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service?
 
-<!--
-Pick one more of these and delete the rest.
--->
-
-- [ ] Metrics
-  - Metric name:
-  - [Optional] Aggregation method:
-  - Components exposing the metric:
-- [ ] Other (treat as last resort)
-  - Details:
+- [x] Metrics
+  - Metric name: `kube_pod_container_status_restarts_total`
+  - Aggregation method: Sum over time, grouped by container and pod.
+  - Components exposing the metric: kube-state-metrics
+- [x] Other (treat as last resort)
+  - Details: PodStatus API will also have a full history of containers restarted in ContainerStatuses field. Containers restarted by RestartPolicyRules will be included in the statuses history.
 
 ###### Are there any missing metrics that would be useful to have to improve observability of this feature?
 
-<!--
-Describe the metrics themselves and the reasons why they weren't added (e.g., cost,
-implementation difficulties, etc.).
--->
+No.
 
 ### Dependencies
 
-<!--
-This section must be completed when targeting beta to a release.
--->
-
 ###### Does this feature depend on any specific services running in the cluster?
 
-<!--
-Think about both cluster-level services (e.g. metrics-server) as well
-as node-level agents (e.g. specific version of CRI). Focus on external or
-optional services that are needed. For example, if this feature depends on
-a cloud provider API, or upon an external software-defined storage or network
-control plane.
-
-For each of these, fill in the following—thinking about running existing user workloads
-and creating new ones, as well as about cluster-level services (e.g. DNS):
-  - [Dependency name]
-    - Usage description:
-      - Impact of its outage on the feature:
-      - Impact of its degraded performance or high-error rates on the feature:
--->
+No.
 
 ### Scalability
 
-<!--
-For alpha, this section is encouraged: reviewers should consider these questions
-and attempt to answer them.
-
-For beta, this section is required: reviewers must answer these questions.
-
-For GA, this section is required: approvers should be able to confirm the
-previous answers based on experience in the field.
--->
-
 ###### Will enabling / using this feature result in any new API calls?
 
-<!--
-Describe them, providing:
-  - API call type (e.g. PATCH pods)
-  - estimated throughput
-  - originating component(s) (e.g. Kubelet, Feature-X-controller)
-Focusing mostly on:
-  - components listing and/or watching resources they didn't before
-  - API calls that may be triggered by changes of some Kubernetes resources
-    (e.g. update of object X triggers new updates of object Y)
-  - periodic API calls to reconcile state (e.g. periodic fetching state,
-    heartbeats, leader election, etc.)
--->
-
 No.
 
 ###### Will enabling / using this feature result in introducing new API types?
 
-<!--
-Describe them, providing:
-  - API type
-  - Supported number of objects per cluster
-  - Supported number of objects per namespace (for namespace-scoped objects)
--->
-
 Enabling this feature will introduce a new field `restartPolicyRules` on the
 [Container API type](https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/types.go#L2528).
 
 ###### Will enabling / using this feature result in any new calls to the cloud provider?
 
-<!--
-Describe them, providing:
-  - Which API(s):
-  - Estimated increase:
--->
-
 No.
 
 ###### Will enabling / using this feature result in increasing size or count of the existing API objects?
 
-<!--
-Describe them, providing:
-  - API type(s):
-  - Estimated increase in size: (e.g., new annotation of size 32B)
-  - Estimated amount of new objects: (e.g., new Object X for every existing Pod)
--->
-
 [Container API type](https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/types.go#L2528)
 will be increased. The rules can handle at most 256 int32 exit values, plus
 the action name ("In" or "NotIn"), the size will increase by at most 1029B.
 
 ###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?
 
-<!--
-Look at the [existing SLIs/SLOs].
-
-Think about adding additional work or introducing new steps in between
-(e.g. need to do X to start a container), etc. Please describe the details.
-
-[existing SLIs/SLOs]: https://git.k8s.io/community/sig-scalability/slos/slos.md#kubernetes-slisslos
--->
-
 No.
 
 ###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?
 
-<!--
-Things to keep in mind include: additional in-memory state, additional
-non-trivial computations, excessive access to disks (including increased log
-volume), significant amount of data sent and/or received over network, etc.
-This through this both in small and large cases, again with respect to the
-[supported limits].
-
-[supported limits]: https://git.k8s.io/community//sig-scalability/configs-and-limits/thresholds.md
--->
-
 No.
 
 ###### Can enabling / using this feature result in resource exhaustion of some node resources (PIDs, sockets, inodes, etc.)?
 
-<!--
-Focus not just on happy cases, but primarily on more pathological cases
-(e.g. probes taking a minute instead of milliseconds, failed pods consuming resources, etc.).
-If any of the resources can be exhausted, how this is mitigated with the existing limits
-(e.g. pods per node) or new limits added by this KEP?
-
-Are there any tests that were run/should be run to understand performance characteristics better
-and validate the declared limits?
--->
-
 No.
 
 ### Troubleshooting
@@ -1098,6 +992,8 @@ details). For now, we leave it here.
 
 ###### How does this feature react if the API server and/or etcd is unavailable?
 
+The container will keep running or restarted by kubelet. Deletion of the pod / container may be delayed.
+
 ###### What are other known failure modes?
 
 <!--
@@ -1113,6 +1009,8 @@ For each of them, fill in the following information by copying the below templat
     - Testing: Are there any tests for failure mode? If not, describe why.
 -->
 
+If kubelet becomes unavailable or is being restarted, there might be delays in container restarts.
+
 ###### What steps should be taken if SLOs are not being met to determine the problem?
 
 ## Implementation History
@@ -1128,6 +1026,10 @@ Major milestones might include:
 - when the KEP was retired or superseded
 -->
 
+- 1.34: Implemented in Alpha
+  - https://github.com/kubernetes/kubernetes/pull/132642
+  - https://github.com/kubernetes/kubernetes/pull/133243
+
 ## Drawbacks
 
 <!--

keps/sig-node/5307-container-restart-policy/kep.yaml

@@ -19,12 +19,12 @@ see-also:
 # The target maturity stage in the current dev cycle for this KEP.
 # If the purpose of this KEP is to deprecate a user-visible feature
 # and a Deprecated feature gates are added, they should be deprecated|disabled|removed.
-stage: alpha
+stage: beta
 
 # The most recent milestone for which work toward delivery of this KEP has been
 # done. This can be the current (upcoming) milestone, if it is being actively
 # worked on.
-latest-milestone: "v1.34"
+latest-milestone: "v1.35"
 
 # The milestone at which this feature was, or is targeted to be, at each stage.
 milestone:
@@ -43,4 +43,5 @@ disable-supported: true
 
 # The following PRR answers are required at beta release
 metrics:
+  - "kube_pod_container_status_restarts_total"