Repository navigation
Conversation
jcogilvie
force-pushed
the
jco/render-start-rollback
branch
4 times, most recently
from
October 2, 2026 20:23
8a729e4 to
06e0160
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>
…fails to start StartFunctionRuntimes starts each Function's runtime in turn. If getting or starting a later runtime failed it returned the error without stopping the runtimes it had already started, so their containers were leaked and the caller had no FunctionAddresses to stop them with. On a get or start failure, stop the runtimes started so far with StopFunctionRuntimes, which detaches the stop from ctx's cancellation (ctx may be why the start failed). The start error stays primary; any stop errors are joined to it. StartFunctionRuntimes is now a thin wrapper over startFunctionRuntimes, which takes the runtime getter as a parameter so the rollback can be tested. Fixes crossplane#396 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-start-rollback
branch
from
October 12, 2026 22:15
06e0160 to
c55979e
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.
If a later Function failed to start,
StartFunctionRuntimesreturned without stopping the runtimes it had already started, leaking their containers. It now stops them withStopFunctionRuntimes(detached from ctx's cancellation) and returns the start error with any stop errors joined.The runtime getter is now injected through an unexported
startFunctionRuntimesfor testing; the exported signature is unchanged.Fixes #396
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