Added stop signal to container termination logic and container status by sreeram-venkitesh · Pull Request #11774 · containerd/containerd

sreeram-venkitesh · 2025-04-30T10:07:26Z

PR for #11617. Tests would fail since cri-api is not bumped to the latest version which has the API changes for stop signals in ContainerConfig and ContainerStatus.

Waiting for #11763 to be merged so that I can rebase the cri-api update into this PR.

k8s-ci-robot · 2025-04-30T10:07:37Z

Hi @sreeram-venkitesh. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

chrishenzie · 2025-04-30T16:50:12Z

Waiting for #11763 to be merged so that I can rebase the cri-api update into this PR.

Left a comment on that PR. Unfortunately this is blocked until we can remove the go1.24 requirement from the CRI API module.

sreeram-venkitesh · 2025-05-28T07:35:05Z

Rebased with the cri-api bump. I'll resume working on this PR again now. I need to add tests.

fuweid · 2025-10-30T02:40:43Z

@sreeram-venkitesh could you please rebase that change? thanks

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sreeram-venkitesh · 2026-04-22T09:07:26Z

@mikebrow The tests are all passing finally! Copilot has reviewed the code too, I'm taking a look..

fuweid · 2026-04-24T21:01:23Z

Any update on this change?

fuweid · 2026-04-24T21:01:42Z

I can take over this if need.

mikebrow · 2026-04-24T21:22:50Z

nod let's help push this through.. just needs a gofmt, rebase, and if you have any issues you'd like to address @fuweid

mikebrow · 2026-04-24T22:17:42Z

a lot of commits could you also squash pls.. at least the last one :) though some maintainers like to thin it down to standalone features that could be cherry picked individually.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T22:57:15Z

+	var statusStopSignal runtime.Signal
+	stopsignal := container.Config.GetStopSignal()
+
+	if stopsignal == runtime.Signal_RUNTIME_DEFAULT {
+		if container.Metadata.StopSignal != "" {
+			statusStopSignal = toCRISignal(container.Metadata.StopSignal)
+		} else {
+			statusStopSignal = runtime.Signal_SIGTERM
+		}
+	} else {
+		statusStopSignal = stopsignal
+	}


toCRIContainerStatus uses toCRISignal(container.Metadata.StopSignal) whenever Metadata.StopSignal is non-empty. If the stored stop signal is something moby/sys/signal.ParseSignal accepts (e.g., TERM or a numeric value like 15 from an image config) but toCRISignal doesn’t recognize it, the status will end up reporting RUNTIME_DEFAULT instead of a concrete signal (or the default SIGTERM). Consider normalizing/validating the string (trim optional SIG prefix, uppercase, handle numeric via signal.ParseSignal) and/or falling back to SIGTERM when conversion fails.

mikebrow

LGTM

fuweid · 2026-04-25T00:30:23Z

kindly ping @dmcgowan @samuelkarp for approval

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: sreeram-venkitesh <[email protected]> Signed-off-by: Wei Fu <[email protected]>

github-project-automation Bot added this to Pull Request Review Apr 30, 2025

github-project-automation Bot moved this to Needs Triage in Pull Request Review Apr 30, 2025

k8s-ci-robot added do-not-merge/work-in-progress needs-ok-to-test size/L labels Apr 30, 2025

dosubot Bot added area/cri Container Runtime Interface (CRI) kind/feature status/has-dependency labels Apr 30, 2025

sreeram-venkitesh marked this pull request as draft April 30, 2025 10:09

sreeram-venkitesh mentioned this pull request Apr 30, 2025

[SIG-Node]: KEP-4960 - Container Stop Signals #11617

Closed

djdongjin mentioned this pull request May 7, 2025

Bump up go version to 1.24 and cri-api to 0.33.0 #11823

Merged

sreeram-venkitesh mentioned this pull request May 19, 2025

Container Stop Signals kubernetes/enhancements#4960

Open

16 tasks

sreeram-venkitesh force-pushed the kep-4960-container-stop-signals branch from ddf890c to c9d4331 Compare May 28, 2025 07:34

sreeram-venkitesh marked this pull request as ready for review May 28, 2025 07:34

sreeram-venkitesh force-pushed the kep-4960-container-stop-signals branch 2 times, most recently from 9c69bdc to 7707926 Compare May 28, 2025 11:01

sreeram-venkitesh changed the title ~~[WIP] Added stop signal to container termination logic and container status~~ Added stop signal to container termination logic and container status Oct 1, 2025

k8s-ci-robot removed the do-not-merge/work-in-progress label Oct 1, 2025

sreeram-venkitesh force-pushed the kep-4960-container-stop-signals branch from 7707926 to a634070 Compare October 1, 2025 09:15

k8s-ci-robot added the needs-rebase label Oct 25, 2025

sreeram-venkitesh force-pushed the kep-4960-container-stop-signals branch from a634070 to ae630fc Compare January 6, 2026 11:12

k8s-ci-robot removed the needs-rebase label Jan 6, 2026

mikebrow force-pushed the kep-4960-container-stop-signals branch 2 times, most recently from 8f670c5 to a8f0823 Compare January 20, 2026 23:07

sreeram-venkitesh force-pushed the kep-4960-container-stop-signals branch 2 times, most recently from f37ab4d to e989d97 Compare February 2, 2026 06:41

Copilot started reviewing on behalf of sreeram-venkitesh April 22, 2026 07:39 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Comment thread internal/cri/server/container_status_test.go

Copilot AI review requested due to automatic review settings April 24, 2026 22:50

fuweid force-pushed the kep-4960-container-stop-signals branch from d6de978 to 625e7be Compare April 24, 2026 22:50

Copilot started reviewing on behalf of fuweid April 24, 2026 22:51 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

mikebrow approved these changes Apr 24, 2026

View reviewed changes

fuweid force-pushed the kep-4960-container-stop-signals branch from 625e7be to a89225c Compare April 25, 2026 00:00

mikebrow added this to the 2.3 milestone Apr 25, 2026

mikebrow moved this from Needs Triage to Review In Progress in Pull Request Review Apr 25, 2026

Copilot AI review requested due to automatic review settings April 26, 2026 14:18

mikebrow force-pushed the kep-4960-container-stop-signals branch from a89225c to ef09699 Compare April 26, 2026 14:18

Copilot started reviewing on behalf of mikebrow April 26, 2026 14:18 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread internal/cri/server/container_stop_signal.go

Added stop signal to container termination logic and container status

1b1aba4

Signed-off-by: sreeram-venkitesh <[email protected]> Signed-off-by: Wei Fu <[email protected]>

mikebrow force-pushed the kep-4960-container-stop-signals branch from ef09699 to 1b1aba4 Compare April 27, 2026 12:52

mxpv approved these changes Apr 27, 2026

View reviewed changes

mxpv added this pull request to the merge queue Apr 27, 2026

hsiangkao approved these changes Apr 27, 2026

View reviewed changes

github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Apr 27, 2026

mikebrow added this pull request to the merge queue Apr 27, 2026

Merged via the queue into containerd:main with commit d8e7c71 Apr 28, 2026
94 of 96 checks passed

github-project-automation Bot moved this from Review In Progress to Done in Pull Request Review Apr 28, 2026

Conversation

sreeram-venkitesh commented Apr 30, 2025

Uh oh!

k8s-ci-robot commented Apr 30, 2025

Uh oh!

chrishenzie commented Apr 30, 2025

Uh oh!

sreeram-venkitesh commented May 28, 2025

Uh oh!

fuweid commented Oct 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

sreeram-venkitesh commented Apr 22, 2026

Uh oh!

fuweid commented Apr 24, 2026

Uh oh!

fuweid commented Apr 24, 2026

Uh oh!

mikebrow commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikebrow commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

fuweid commented Apr 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

mikebrow commented Apr 24, 2026 •

edited

Loading

mikebrow commented Apr 24, 2026 •

edited

Loading