fix(render): stop already-started function runtimes when a later one fails to start

cmd/crossplane/render/render.go

@@ -119,18 +119,24 @@ func (fa *FunctionAddresses) Stop(ctx context.Context) error {
 // gRPC addresses. The caller must call Stop on the returned FunctionAddresses
 // when done.
 func StartFunctionRuntimes(ctx context.Context, log logging.Logger, fns []pkgv1.Function) (*FunctionAddresses, error) {
+	return startFunctionRuntimes(ctx, log, fns, GetRuntime)
+}
+
+// startFunctionRuntimes implements StartFunctionRuntimes, using getRuntime to
+// get each Function's runtime.
+func startFunctionRuntimes(ctx context.Context, log logging.Logger, fns []pkgv1.Function, getRuntime func(pkgv1.Function, logging.Logger) (Runtime, error)) (*FunctionAddresses, error) {
 	addrs := make(map[string]string, len(fns))
 	contexts := make(map[string]RuntimeContext, len(fns))
 
 	for _, fn := range fns {
-		rt, err := GetRuntime(fn, log)
+		rt, err := getRuntime(fn, log)
 		if err != nil {
-			return nil, errors.Wrapf(err, "cannot get runtime for Function %q", fn.GetName())
+			return nil, stopStartedRuntimes(ctx, contexts, errors.Wrapf(err, "cannot get runtime for Function %q", fn.GetName()))
 		}
 
 		rctx, err := rt.Start(ctx)
 		if err != nil {
-			return nil, errors.Wrapf(err, "cannot start Function %q", fn.GetName())
+			return nil, stopStartedRuntimes(ctx, contexts, errors.Wrapf(err, "cannot start Function %q", fn.GetName()))
 		}
 
 		addrs[fn.GetName()] = rctx.Target
@@ -140,6 +146,27 @@ func StartFunctionRuntimes(ctx context.Context, log logging.Logger, fns []pkgv1.
 	return &FunctionAddresses{addrs: addrs, contexts: contexts}, nil
 }
 
+// stopStartedRuntimes stops the supplied already-started runtimes, per their
+// own cleanup policies, after a later Function failed to start. It attempts to
+// stop all of them and returns startErr, joined with any stop errors. The stop
+// context is detached from ctx's cancellation, since ctx may be why the start
+// failed.
+func stopStartedRuntimes(ctx context.Context, contexts map[string]RuntimeContext, startErr error) error {
+	stopCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second)
+	defer cancel()
+
+	errs := []error{startErr}
+	for name, rctx := range contexts {
+		if err := rctx.Stop(stopCtx); err != nil {
+			errs = append(errs, errors.Wrapf(err, "cannot stop function %q runtime (target %q)", name, rctx.Target))
+		}
+	}
+	if len(errs) == 1 {
+		return startErr
+	}
+	return errors.Join(errs...)
+}
+
 // RewriteAddressesForDocker rewrites function addresses so they are reachable
 // from inside a Docker container. Addresses targeting localhost or 127.0.0.1
 // are rewritten to host.docker.internal.

cmd/crossplane/render/render_test.go

@@ -1,12 +1,16 @@
 package render
 
 import (
+	"context"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"github.com/crossplane/crossplane-runtime/v2/pkg/errors"
+	"github.com/crossplane/crossplane-runtime/v2/pkg/logging"
+
 	pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1"
 )
 
@@ -91,3 +95,44 @@ func TestOverrideFunctionAnnotations(t *testing.T) {
 func functionWithAnnotations(annotations map[string]string) pkgv1.Function {
 	return pkgv1.Function{ObjectMeta: metav1.ObjectMeta{Annotations: annotations}}
 }
+
+type fakeRuntime struct {
+	start func(ctx context.Context) (RuntimeContext, error)
+}
+
+func (r fakeRuntime) Start(ctx context.Context) (RuntimeContext, error) { return r.start(ctx) }
+
+func TestStartFunctionRuntimesStopsStartedOnFailure(t *testing.T) {
+	stopped := map[string]bool{}
+	errBoom := errors.New("boom")
+
+	getRuntime := func(fn pkgv1.Function, _ logging.Logger) (Runtime, error) {
+		name := fn.GetName()
+		return fakeRuntime{start: func(_ context.Context) (RuntimeContext, error) {
+			if name == "fn-c" {
+				return RuntimeContext{}, errBoom
+			}
+			return RuntimeContext{Target: name, Stop: func(_ context.Context) error {
+				stopped[name] = true
+				return nil
+			}}, nil
+		}}, nil
+	}
+
+	fns := []pkgv1.Function{
+		{ObjectMeta: metav1.ObjectMeta{Name: "fn-a"}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "fn-b"}},
+		{ObjectMeta: metav1.ObjectMeta{Name: "fn-c"}},
+	}
+
+	fa, err := startFunctionRuntimes(t.Context(), logging.NewNopLogger(), fns, getRuntime)
+	if fa != nil {
+		t.Errorf("StartFunctionRuntimes(...): want nil FunctionAddresses, got %v", fa)
+	}
+	if !errors.Is(err, errBoom) {
+		t.Errorf("StartFunctionRuntimes(...): want error wrapping %v, got %v", errBoom, err)
+	}
+	if diff := cmp.Diff(map[string]bool{"fn-a": true, "fn-b": true}, stopped); diff != "" {
+		t.Errorf("StartFunctionRuntimes(...): -want stopped, +got stopped:\n%s", diff)
+	}
+}