WIP: Add RestartContainerDuringTermination feature #125710

gjkim42 · 2024-06-25T21:03:50Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #121398

TODOs

fix e2e tests of https://github.com/kubernetes/kubernetes/pull/124086/files
fix Should transition to Failed phase a pod which is deleted while pending
Liveness, readiness, and startup probing
container lifecycle hooks running
secret and configmap volume updates

This manages container termination in a non-blocking way.
xref:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

The new feature gate "RestartContainerDuringTermination" is now available. The feature allows containers to restart during another container's termination or pod termination.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/4438-container-restart-termination

/uncc all

k8s-ci-robot · 2024-06-25T21:03:58Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

gjkim42 · 2024-07-02T13:30:13Z

/test pull-kubernetes-node-kubelet-serial-containerd-sidecar-containers
/test pull-kubernetes-node-e2e-containerd-features
/test pull-kubernetes-cos-cgroupv2-containerd-node-e2e-features
/test pull-kubernetes-cos-cgroupv1-containerd-node-e2e-features

gjkim42 · 2024-07-02T14:21:33Z

/test pull-kubernetes-node-kubelet-serial-containerd-sidecar-containers
/test pull-kubernetes-node-e2e-containerd-features
/test pull-kubernetes-cos-cgroupv2-containerd-node-e2e-features
/test pull-kubernetes-cos-cgroupv1-containerd-node-e2e-features

gjkim42 · 2024-07-07T07:07:54Z

pkg/kubelet/kubelet.go

+	var podStopped bool
+	var err error
+	if !utilfeature.DefaultFeatureGate.Enabled(features.RestartContainerDuringTermination) {
+		p := kubecontainer.ConvertPodStatusToRunningPod(kl.getRuntime().Type(), podStatus)
+		err = kl.killPod(ctx, pod, p, gracePeriod)
+	} else {
+		podStopped, err = kl.syncTerminatingPod(ctx, pod, podStatus, gracePeriod, kl.getPullSecretsForPod(pod), kl.backOff)
+	}
+	if err != nil {
 		kl.recorder.Eventf(pod, v1.EventTypeWarning, events.FailedToKillPod, "error killing pod: %v", err)
 		// there was an error killing the pod, so we return that error directly
 		utilruntime.HandleError(err)
 		return err
 	}

+	if !podStopped {
+		// If the pod is not yet stopped, we should return an error to signal
+		// that the sync loop should back off.
+		return fmt.Errorf("pod is not yet stopped")
+	}
+


/cc @smarterclayton

https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/4438-container-restart-termination
This PR is trying to change the (*kubelet).killPod in a non-blocking way so that other parts of the syncLoop can work during the container termination.

What do you think?

LaurentGoderre · 2024-07-17T14:33:35Z

pkg/features/kube_features.go

@@ -1158,6 +1166,9 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS

 	ServiceTrafficDistribution: {Default: false, PreRelease: featuregate.Alpha},

+	// FIXME: Disable by default
+	RestartContainerDuringTermination: {Default: true, PreRelease: featuregate.Alpha},


Change this before merge?

ffromani · 2025-03-18T10:35:12Z

hi @gjkim42 froim sig-node ! the 1.33 code freeze deadline is looming and this PR is still marked as WIP. Are still up for the 1.33 milestone? do you need reviews?

gjkim42 · 2025-03-18T10:39:10Z

Unfortunately, I don't have the bandwidth to work on this PR.

matthyx · 2025-03-18T10:40:59Z

Unfortunately, I don't have the bandwidth to work on this PR.

I can check if you want... did you have some pending changes to push?

gjkim42 · 2025-03-18T11:03:53Z

No, I didn't. I still couldn't check the test case you mentioned before.

ffromani · 2025-03-18T11:07:46Z

Unfortunately, I don't have the bandwidth to work on this PR.

ack, no worries, just checking to plan the rest of the time till the code freeze

k8s-ci-robot · 2025-04-25T01:51:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gjkim42
Once this PR has been reviewed and has the lgtm label, please assign mrunalp for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

pkg/features/OWNERS
pkg/kubelet/OWNERS
staging/src/k8s.io/cri-api/OWNERS
test/compatibility_lifecycle/reference/OWNERS
test/e2e/feature/OWNERS
test/e2e_node/OWNERS

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

Signed-off-by: Matthias Bertschy <[email protected]>

k8s-ci-robot · 2025-05-08T13:13:24Z

@gjkim42: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-node-e2e-containerd-features	`12b5db2`	link	false	`/test pull-kubernetes-node-e2e-containerd-features`
pull-kubernetes-linter-hints	`f1b7bd7`	link	false	`/test pull-kubernetes-linter-hints`
pull-kubernetes-unit-windows-master	`f1b7bd7`	link	false	`/test pull-kubernetes-unit-windows-master`
pull-kubernetes-e2e-gce-cos-alpha-features	`f1b7bd7`	link	false	`/test pull-kubernetes-e2e-gce-cos-alpha-features`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

k8s-ci-robot · 2025-06-27T21:27:12Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

gjkim42 changed the title ~~WIP: Manage container termination in a non-blocking manner~~ WIP: Add RestartContainerDuringTermination feature Jun 25, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2024

gjkim42 force-pushed the non-blocking-container-termination branch from dc8af25 to a2fdfd5 Compare July 2, 2024 13:27

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2024

gjkim42 force-pushed the non-blocking-container-termination branch from a2fdfd5 to cb0164e Compare July 2, 2024 13:29

gjkim42 force-pushed the non-blocking-container-termination branch from cb0164e to 12b5db2 Compare July 2, 2024 14:03

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 2, 2024

gjkim42 commented Jul 7, 2024

View reviewed changes

k8s-ci-robot requested a review from smarterclayton July 7, 2024 07:07

sreeram-venkitesh mentioned this pull request Jul 7, 2024

Restarting sidecar containers during Pod termination kubernetes/enhancements#4438

Open

4 tasks

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2024

LaurentGoderre reviewed Jul 17, 2024

View reviewed changes

gjkim42 force-pushed the non-blocking-container-termination branch from 12b5db2 to a54bf2e Compare July 23, 2024 11:35

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2025

toVersus mentioned this pull request Mar 5, 2025

fix container status remains ready state #129585

Open

gjkim42 force-pushed the non-blocking-container-termination branch from 9c789d2 to 024bc2f Compare March 18, 2025 13:00

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 18, 2025

gjkim42 force-pushed the non-blocking-container-termination branch from e29a2f7 to af633df Compare April 10, 2025 09:02

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2025

gjkim42 force-pushed the non-blocking-container-termination branch from 07c592e to f9d74f3 Compare April 15, 2025 04:45

gjkim42 force-pushed the non-blocking-container-termination branch 3 times, most recently from 21a8232 to bf98fec Compare April 25, 2025 01:51

gjkim42 force-pushed the non-blocking-container-termination branch from bf98fec to e77f9fd Compare April 25, 2025 02:51

gjkim42 and others added 7 commits May 8, 2025 21:32

Add e2e tests for RestartContainerDuringTermination

7aa779c

Manage container termination in a non-blocking manner

4e101d3

Stop pod if there is no container to kill

f6f0b8e

Restart container if liveness or startup probe fails

b11edd7

better separate feature gate logic

94fbeac

Signed-off-by: Matthias Bertschy <[email protected]>

call probeManager.RemovePod after all containers have stopped

a1c44cb

Signed-off-by: Matthias Bertschy <[email protected]>

chore: Fix syntax errors

f1b7bd7

gjkim42 force-pushed the non-blocking-container-termination branch from e77f9fd to f1b7bd7 Compare May 8, 2025 12:33

sreeram-venkitesh mentioned this pull request May 14, 2025

A container cannot restart when there is any terminating container in the same pod #121398

Open

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

WIP: Add RestartContainerDuringTermination feature #125710

WIP: Add RestartContainerDuringTermination feature #125710

Uh oh!

gjkim42 commented Jun 25, 2024 •

edited

Loading

Uh oh!

k8s-ci-robot commented Jun 25, 2024

Uh oh!

gjkim42 commented Jul 2, 2024

Uh oh!

gjkim42 commented Jul 2, 2024

Uh oh!

gjkim42 Jul 7, 2024

Uh oh!

LaurentGoderre Jul 17, 2024

Uh oh!

ffromani commented Mar 18, 2025

Uh oh!

gjkim42 commented Mar 18, 2025

Uh oh!

matthyx commented Mar 18, 2025

Uh oh!

gjkim42 commented Mar 18, 2025

Uh oh!

ffromani commented Mar 18, 2025

Uh oh!

k8s-ci-robot commented Apr 25, 2025

Uh oh!

k8s-ci-robot commented May 8, 2025

Uh oh!

k8s-ci-robot commented Jun 27, 2025

Uh oh!

Uh oh!

WIP: Add RestartContainerDuringTermination feature #125710

Are you sure you want to change the base?

WIP: Add RestartContainerDuringTermination feature #125710

Uh oh!

Conversation

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

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-ci-robot commented Jun 25, 2024

Uh oh!

gjkim42 commented Jul 2, 2024

Uh oh!

gjkim42 commented Jul 2, 2024

Uh oh!

gjkim42 Jul 7, 2024

Choose a reason for hiding this comment

Uh oh!

LaurentGoderre Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

ffromani commented Mar 18, 2025

Uh oh!

gjkim42 commented Mar 18, 2025

Uh oh!

matthyx commented Mar 18, 2025

Uh oh!

gjkim42 commented Mar 18, 2025

Uh oh!

ffromani commented Mar 18, 2025

Uh oh!

k8s-ci-robot commented Apr 25, 2025

Uh oh!

k8s-ci-robot commented May 8, 2025

Uh oh!

k8s-ci-robot commented Jun 27, 2025

Uh oh!

Uh oh!

gjkim42 commented Jun 25, 2024 •

edited

Loading