Conversation
…icy containers FunctionAddresses.Stop returned on the first runtime Stop error, leaving the remaining runtimes running. It now attempts every runtime and returns all failures joined. StopFunctionRuntimes shared a single 5s deadline across all runtimes, shorter than Docker's default 10s stop grace period. A container slow to exit on SIGTERM made ContainerStop fail and the following ContainerRemove was skipped. Each runtime now gets its own 30s timeout, and the Remove cleanup policy uses a single forced ContainerRemove, which kills and removes the container in one call. Also correct the doc comments: Remove, not Stop, is the default cleanup policy. Fixes crossplane#397 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
jcogilvie
force-pushed
the
jco/render-robust-stop
branch
from
October 1, 2026 22:08
1df3916 to
d46f858
Compare
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
1 participant
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.
Description of your changes
Makes function runtime cleanup robust so one failing or slow runtime can't leave others (or their containers) behind:
FunctionAddresses.Stopnow attempts every runtime and returns all failures joined (each still wrapped with the function name and target), instead of returning on the first error.StopFunctionRuntimesgives each runtime its own 30s timeout rather than one shared 5s budget, which was shorter than Docker's default 10s stop grace period. Its exported signature is unchanged.Removecleanup policy now does a singleContainerRemovewithForce: true(kill + remove), so a container slow to exit on SIGTERM can no longer cause removal to be skipped.StopandOrphansemantics are unchanged.Stopis the default cleanup policy;Removeis (AnnotationValueRuntimeDockerCleanupDefault).Tested with a new unit test,
TestFunctionAddressesStop, which stops a map of fake runtimes (some failing) across many iterations so map ordering can't hide a regression, and asserts every runtime was stopped and every failure is reported. There's no existing seam for the Docker stop closure, so it isn't unit tested.Fixes #397
I have:
Run(nix isn't available locally; ran./nix.sh flake checkto ensure this PR is ready for review.go build ./...,go test ./cmd/crossplane/render/..., andgolangci-lint run ./cmd/crossplane/render/...instead, all clean.)Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.
🤖 Generated with Claude Code