Conversation
jcogilvie
force-pushed
the
jco/render-start-rollback
branch
from
October 1, 2026 21:39
96fede9 to
e931f12
Compare
…fails to start StartFunctionRuntimes starts Functions in order. When Function N failed to get or start a runtime it returned nil and an error, dropping the RuntimeContexts of Functions 1..N-1. Callers only defer StopFunctionRuntimes after a successful start, so those runtimes (e.g. Docker containers) were leaked. On failure, stop every runtime already started via its own Stop (honouring its cleanup policy), using a bounded context detached from the caller's cancellation. All are attempted; stop errors are joined after the original start error so it stays primary. 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 1, 2026 22:07
e931f12 to
44b6388
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
StartFunctionRuntimesstarts Functions in order; if Function N fails to get or start its runtime, it returnednil, errand the runtimes for Functions 1..N-1 were leaked, since callers onlydefer StopFunctionRuntimesafter a successful start.Now, on failure, every runtime already started is stopped via its own
RuntimeContext.Stop(so each runtime's cleanup policy is honoured). The stop uses a 5s timeout on a context detached from the caller's cancellation (context.WithoutCancel), mirroringStopFunctionRuntimes. All runtimes are attempted, and stop errors are joined after the original start error so it stays primary. No exported signatures change.StartFunctionRuntimesnow delegates to an unexportedstartFunctionRuntimesthat takes the runtime getter as a parameter, so the test injects a fake without package-level state.Tested with a new unit test (
TestStartFunctionRuntimesStopsStartedOnFailure) that fails the third of three Functions and asserts the first two were stopped../nix.sh flake checkcould not be run (nix isn't available in my environment); instead I rango build ./...,go test ./cmd/crossplane/render/..., andgolangci-lint run ./cmd/crossplane/render/..., all clean.Fixes #396
I have:
Run./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