Repository navigation
Conversation
Only clean up networking and healthcheck units for SIGKILL or the container's configured stop signal, defaulting to SIGTERM. Other signals may reload a service that still needs these resources and should not mark it explicitly stopped. Validate the task state and deliver the signal before tearing down the network. Tolerate metadata already removed by the auto-remove path after successful signal delivery. Add recording-CNI regression tests for signal selection and failure paths, and nginx published-port tests for SIGHUP and SIGUSR1. Addresses containerd#5327 Assisted-by: OpenAI Codex Signed-off-by: jony montana <hy352144278@gmail.com>
jonyhy96
marked this pull request as ready for review
November 6, 2026 06:17
Member
Why? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #5327.
Change
killnow validates the task and sends the signal before network teardown. Only SIGKILL or the configured stop signal (SIGTERM by default) triggers network/healthcheck cleanup and the explicitly-stopped label. Unrelated signals such as nginx's SIGHUP and SIGUSR1 preserve these resources.Cleanup tolerates container metadata disappearing after successful signal delivery, as can happen with
run --rm. Other cleanup failures are still reported.Verification
Passed locally:
Killentry point and go-cni, using in-memory containerd services and a recording CNI plugin. These cover signals, custom stop signals, task/status/signal failures, paused tasks, cleanup errors, and auto-removal. The original network-gate cases fail on unmodified main.go test -race ./pkg/cmd/container -count=1go vet ./pkg/cmd/container ./cmd/nerdctl/containermake -B binaries, Windows package cross-compilation, andgo mod tidy --diffcontainer_list.goformatting issue on main.make testwas also run.mountutil,portutil, andresolvconffail due to missing host paths, iptables, and systemd's resolv.conf; the same failures were reproduced on unmodified main.Added nginx published-port regression tests for SIGHUP and SIGUSR1. They compile, but execution is blocked by the absence of a running rootless containerd/RootlessKit environment. Real bridge/IPAM, forwarding, and systemd healthcheck behavior have not been verified locally. Runtime validation remains outstanding.
Scope
This follows the signal-based approach proposed in the issue, not an exit-status-based redesign. A process can still trap or ignore its configured stop signal; cleanup for that signal retains the existing limitation. A cleanup error can be returned after the signal has already taken effect.
stopis unchanged.AI assistance
The implementation and tests were substantially prepared with OpenAI Codex assistance. Local verification is listed above. The commit includes
Assisted-byand DCO sign-off trailers.