Repository navigation
Conversation
jcogilvie
force-pushed
the
jco/render-report-cleanup-failures
branch
from
October 2, 2026 17:49
c1c06a1 to
93af7ce
Compare
2 of 5 tasks
jcogilvie
force-pushed
the
jco/render-report-cleanup-failures
branch
from
October 2, 2026 20:23
93af7ce to
030c086
Compare
… Remove-policy 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, and ContainerStop used the daemon's default 10s grace period. A container slow to exit on SIGTERM made ContainerStop fail, and the ContainerRemove that should follow was skipped. The Stop and Remove cleanup policies now stop with an explicit 3s grace period. Remove then force removes the container whether or not the stop succeeded, so a slow SIGTERM can no longer skip removal. Removal success means success; if removal fails, the removal and stop errors are joined. The Docker runtime bounds its own cleanup: ContainerStop and ContainerRemove each get their own timeout, so a stop that hangs can't leave removal without time to run. StopFunctionRuntimes only detaches the caller's cancellation, so cleanup still runs after the render context is cancelled; it no longer needs to know the Docker runtime's timings. This changes the exported signature of StopFunctionRuntimes from (logging.Logger, *FunctionAddresses) to (context.Context, *FunctionAddresses) error, matching other cleanup functions in the repo. The xr and op render commands log the returned error as before. 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>
StopFunctionRuntimes returns its error, but `crossplane render xr` and `crossplane render op` only reported it via log.Info, and the CLI's logger is a no-op unless --verbose is set. As a result both commands silently left Function containers behind when stopping them failed. Add WarnCleanupFailure, which writes a short warning (including the error) to a writer and does nothing for a nil error, and call it with kong's stderr from both commands' deferred cleanup alongside the existing log line. The exit code is unchanged. Fixes crossplane#399 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-report-cleanup-failures
branch
from
October 12, 2026 22:15
030c086 to
6a10910
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
Stacked on #404; only the top commit is new.
render xr/opreported cleanup failures only vialog.Info, which is a no-op without--verbose, so leaked containers went unnoticed. They now also print to stderr:via a small exported
render.WarnCleanupFailure(w, err), used by both command packages. Exit code is unchanged.Fixes #399
I have:
./nix.sh flake checkto ensure this PR is ready for review.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