diff --git a/cmd/crossplane/render/op/cmd.go b/cmd/crossplane/render/op/cmd.go index 0cee112c..86d2c5a0 100644 --- a/cmd/crossplane/render/op/cmd.go +++ b/cmd/crossplane/render/op/cmd.go @@ -41,6 +41,7 @@ import ( "github.com/crossplane/cli/v2/cmd/crossplane/render/contextfn" "github.com/crossplane/cli/v2/internal/async" "github.com/crossplane/cli/v2/internal/config" + "github.com/crossplane/cli/v2/internal/containerruntime" "github.com/crossplane/cli/v2/internal/dependency" "github.com/crossplane/cli/v2/internal/project" "github.com/crossplane/cli/v2/internal/project/functions" @@ -172,14 +173,22 @@ func (c *Cmd) Run(baseCtx context.Context, k *kong.Context, log logging.Logger, c.SetDefaultCrossplaneDockerNetwork(fns) - engine := c.newEngine(&c.EngineFlags, log) + sess := render.NewSession(c.EngineFlags, log, + render.WithEngineFactory(c.newEngine), + render.WithLeaseDir(containerruntime.DefaultLeaseDir()), + // Leasing this run and reclaiming dead runs is best effort, and + // must not change this command's output. + render.WithWarn(func(err error) { + log.Debug("Cannot lease this run or reclaim dead runs' Docker resources", "error", err) + }), + ) seedCtx := len(c.ContextValues) > 0 || len(c.ContextFiles) > 0 captureCtx := c.IncludeContext var ctxHandle *contextfn.Handle if seedCtx || captureCtx { - if err := engine.CheckContextSupport(); err != nil { + if err := sess.CheckContextSupport(); err != nil { return err } @@ -208,28 +217,23 @@ func (c *Cmd) Run(baseCtx context.Context, k *kong.Context, log logging.Logger, } } - cleanup, err := engine.Setup(ctx, fns) - if err != nil { - return err - } - defer cleanup() - - // Start function runtimes to get their addresses. - fnAddrs, err := render.StartFunctionRuntimes(ctx, log, fns) - if err != nil { - return errors.Wrap(err, "cannot start function runtimes") - } + // Close stops the function runtimes, then removes what Setup created. defer func() { - if err := render.StopFunctionRuntimes(ctx, fnAddrs); err != nil { - log.Info("Error stopping function runtimes", "error", err) + if err := sess.Close(ctx); err != nil { + log.Info("Error cleaning up render session", "error", err) render.WarnCleanupFailure(k.Stderr, err) } }() + fnAddrs, err := sess.Start(ctx, fns) + if err != nil { + return err + } + // Build and execute the render request. in := render.OperationInputs{ Operation: op, - FunctionAddrs: fnAddrs.Addresses(), + FunctionAddrs: fnAddrs, RequiredResources: rrs, RequiredSchemas: rsc, FunctionCredentials: fcreds, @@ -239,7 +243,7 @@ func (c *Cmd) Run(baseCtx context.Context, k *kong.Context, log logging.Logger, return errors.Wrap(err, "cannot build render request") } - rsp, err := engine.Render(ctx, req) + rsp, err := sess.Render(ctx, req) if err != nil { return errors.Wrap(err, "cannot render operation") } diff --git a/cmd/crossplane/render/op/cmd_test.go b/cmd/crossplane/render/op/cmd_test.go index ed68908e..52493ae4 100644 --- a/cmd/crossplane/render/op/cmd_test.go +++ b/cmd/crossplane/render/op/cmd_test.go @@ -98,6 +98,11 @@ func mustNewStruct(t *testing.T, data map[string]any) *structpb.Struct { } func TestCmdRun(t *testing.T) { + // Run leases this run in the user's cache directory, and reaps the dead + // runs it finds there; keep both off the developer's real one. + t.Setenv("HOME", t.TempDir()) + t.Setenv("XDG_CACHE_HOME", t.TempDir()) + type args struct { cmd Cmd } diff --git a/cmd/crossplane/render/op/help/render.md b/cmd/crossplane/render/op/help/render.md index 39ab9e50..21ceb9e4 100644 --- a/cmd/crossplane/render/op/help/render.md +++ b/cmd/crossplane/render/op/help/render.md @@ -20,7 +20,7 @@ they're run: | `render.crossplane.io/runtime: "Development"` | Connect to a Function running locally, instead of using Docker, for example when developing or debugging a new Function. The Function must be listening at `localhost:9443` and running with the `--insecure` flag. | | `render.crossplane.io/runtime-development-target: "dns:///example.org:7443"` | Connect to a Function running somewhere other than `localhost:9443`. The target uses gRPC target syntax (for example, `dns:///example.org:7443` or `example.org:7443`). | | `render.crossplane.io/runtime-docker-cleanup: "Orphan"` | Don't stop the Function's Docker container after rendering. | -| `render.crossplane.io/runtime-docker-name: ""` | Create or reuse a container with the given name. Restart the container if needed. | +| `render.crossplane.io/runtime-docker-name: ""` | Create or reuse a container with the given name. Restart the container if needed. See below for when a running container is reused. | | `render.crossplane.io/runtime-docker-pull-policy: "Always"` | Always pull the Function's package, even if it already exists locally. Other supported values are `Never` or `IfNotPresent`. | | `render.crossplane.io/runtime-docker-publish-address: "0.0.0.0"` | Host address that Docker should publish the Function's container port to. Defaults to `127.0.0.1` (localhost only). Use `0.0.0.0` to publish to all host network interfaces, enabling access from remote machines. | | `render.crossplane.io/runtime-docker-target: "docker-host"` | Address that the render CLI should use to connect to the Function's Docker container. If not specified, uses the publish address. | @@ -31,6 +31,23 @@ to the Docker daemon. See the [Docker environment variables](https://docs.docker.com/engine/reference/commandline/cli/#environment-variables) reference. +## Cleaning up after a crashed run + +If a run is killed before it cleans up (for example by `kill -9`), the next +`composition render` or `operation render` run cleans up what it left behind, +following each container's `runtime-docker-cleanup` policy. This doesn't apply +to runs started with `--session-id`, to runs on Windows, or to a different +`DOCKER_HOST`. + +If a container named by `runtime-docker-name` already exists: + +| Container | What this run does | +|---|---| +| Stopped | Restarts it. On exit it keeps it as long as either run's policy asks (`Orphan` > `Stop` > `Remove`). | +| Running, `Orphan` | Shares it and leaves it running. | +| Running, started by this run | Shares it. | +| Running, `Stop` or `Remove`, started by another run | Fails, because that run will stop or remove it. Use `Orphan` to share a container between runs. | + ## Project support When running `render` in a Crossplane Project (any directory containing a diff --git a/cmd/crossplane/render/xr/cmd.go b/cmd/crossplane/render/xr/cmd.go index 98cabd08..cc9dbfe7 100644 --- a/cmd/crossplane/render/xr/cmd.go +++ b/cmd/crossplane/render/xr/cmd.go @@ -48,6 +48,7 @@ import ( "github.com/crossplane/cli/v2/cmd/crossplane/render/contextfn" "github.com/crossplane/cli/v2/internal/async" "github.com/crossplane/cli/v2/internal/config" + "github.com/crossplane/cli/v2/internal/containerruntime" "github.com/crossplane/cli/v2/internal/dependency" "github.com/crossplane/cli/v2/internal/project" "github.com/crossplane/cli/v2/internal/project/functions" @@ -232,14 +233,22 @@ func (c *Cmd) Run(baseCtx context.Context, k *kong.Context, log logging.Logger, c.SetDefaultCrossplaneDockerNetwork(fns) - engine := c.newEngine(&c.EngineFlags, log) + sess := render.NewSession(c.EngineFlags, log, + render.WithEngineFactory(c.newEngine), + render.WithLeaseDir(containerruntime.DefaultLeaseDir()), + // Leasing this run and reclaiming dead runs is best effort, and + // must not change this command's output. + render.WithWarn(func(err error) { + log.Debug("Cannot lease this run or reclaim dead runs' Docker resources", "error", err) + }), + ) seedCtx := len(c.ContextValues) > 0 || len(c.ContextFiles) > 0 captureCtx := c.IncludeContext var ctxHandle *contextfn.Handle if seedCtx || captureCtx { - if err := engine.CheckContextSupport(); err != nil { + if err := sess.CheckContextSupport(); err != nil { return err } @@ -268,29 +277,24 @@ func (c *Cmd) Run(baseCtx context.Context, k *kong.Context, log logging.Logger, } } - cleanup, err := engine.Setup(ctx, fns) - if err != nil { - return err - } - defer cleanup() - - // Start function runtimes to get their addresses. - fnAddrs, err := render.StartFunctionRuntimes(ctx, log, fns) - if err != nil { - return errors.Wrap(err, "cannot start function runtimes") - } + // Close stops the function runtimes, then removes what Setup created. defer func() { - if err := render.StopFunctionRuntimes(ctx, fnAddrs); err != nil { - log.Info("Error stopping function runtimes", "error", err) + if err := sess.Close(ctx); err != nil { + log.Info("Error cleaning up render session", "error", err) render.WarnCleanupFailure(k.Stderr, err) } }() + fnAddrs, err := sess.Start(ctx, fns) + if err != nil { + return err + } + // Build and execute the render request. in := render.CompositionInputs{ CompositeResource: xr, Composition: comp, - FunctionAddrs: fnAddrs.Addresses(), + FunctionAddrs: fnAddrs, ObservedResources: ors, RequiredResources: rrs, RequiredSchemas: rsc, @@ -302,7 +306,7 @@ func (c *Cmd) Run(baseCtx context.Context, k *kong.Context, log logging.Logger, return errors.Wrap(err, "cannot build render request") } - rsp, err := engine.Render(ctx, req) + rsp, err := sess.Render(ctx, req) if err != nil { return errors.Wrap(err, "cannot render composite resource") } diff --git a/cmd/crossplane/render/xr/cmd_test.go b/cmd/crossplane/render/xr/cmd_test.go index dbd0d3db..d2ed5ae3 100644 --- a/cmd/crossplane/render/xr/cmd_test.go +++ b/cmd/crossplane/render/xr/cmd_test.go @@ -129,6 +129,11 @@ func fillResourceRefs(t *testing.T, xr *structpb.Struct) *structpb.Struct { } func TestCmdRun(t *testing.T) { + // Run leases this run in the user's cache directory, and reaps the dead + // runs it finds there; keep both off the developer's real one. + t.Setenv("HOME", t.TempDir()) + t.Setenv("XDG_CACHE_HOME", t.TempDir()) + type args struct { cmd Cmd } diff --git a/cmd/crossplane/render/xr/help/render.md b/cmd/crossplane/render/xr/help/render.md index 7bcce965..322802ee 100644 --- a/cmd/crossplane/render/xr/help/render.md +++ b/cmd/crossplane/render/xr/help/render.md @@ -23,7 +23,7 @@ they're run: | `render.crossplane.io/runtime: "Development"` | Connect to a Function running locally, instead of using Docker, for example when developing or debugging a new Function. The Function must be listening at `localhost:9443` and running with the `--insecure` flag. | | `render.crossplane.io/runtime-development-target: "dns:///example.org:7443"` | Connect to a Function running somewhere other than `localhost:9443`. The target uses gRPC target syntax (for example, `dns:///example.org:7443` or `example.org:7443`). | | `render.crossplane.io/runtime-docker-cleanup: "Orphan"` | Don't stop the Function's Docker container after rendering. | -| `render.crossplane.io/runtime-docker-name: ""` | Create or reuse a container with the given name. Restart the container if needed. | +| `render.crossplane.io/runtime-docker-name: ""` | Create or reuse a container with the given name. Restart the container if needed. See below for when a running container is reused. | | `render.crossplane.io/runtime-docker-pull-policy: "Always"` | Always pull the Function's package, even if it already exists locally. Other supported values are `Never` or `IfNotPresent`. | | `render.crossplane.io/runtime-docker-publish-address: "0.0.0.0"` | Host address that Docker should publish the Function's container port to. Defaults to `127.0.0.1` (localhost only). Use `0.0.0.0` to publish to all host network interfaces, enabling access from remote machines. | | `render.crossplane.io/runtime-docker-target: "docker-host"` | Address that the render CLI should use to connect to the Function's Docker container. If not specified, uses the publish address. | @@ -34,6 +34,23 @@ to the Docker daemon. See the [Docker environment variables](https://docs.docker.com/engine/reference/commandline/cli/#environment-variables) reference. +## Cleaning up after a crashed run + +If a run is killed before it cleans up (for example by `kill -9`), the next +`composition render` or `operation render` run cleans up what it left behind, +following each container's `runtime-docker-cleanup` policy. This doesn't apply +to runs started with `--session-id`, to runs on Windows, or to a different +`DOCKER_HOST`. + +If a container named by `runtime-docker-name` already exists: + +| Container | What this run does | +|---|---| +| Stopped | Restarts it. On exit it keeps it as long as either run's policy asks (`Orphan` > `Stop` > `Remove`). | +| Running, `Orphan` | Shares it and leaves it running. | +| Running, started by this run | Shares it. | +| Running, `Stop` or `Remove`, started by another run | Fails, because that run will stop or remove it. Use `Orphan` to share a container between runs. | + ## Project support When running `render` in a Crossplane Project (any directory containing a diff --git a/go.mod b/go.mod index 2ee84228..a4cd7dd5 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,7 @@ require ( github.com/willabides/kongplete v0.4.0 golang.org/x/mod v0.42.0 golang.org/x/sync v0.24.0 + golang.org/x/sys v0.49.0 golang.org/x/text v0.43.0 golang.org/x/tools v0.52.0 google.golang.org/genproto/googleapis/rpc v0.0.0-20260831171406-18b4a7587f8a @@ -319,7 +320,6 @@ require ( golang.org/x/exp v0.0.0-20260824195058-e88cd73687aa // indirect golang.org/x/net v0.62.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect - golang.org/x/sys v0.49.0 // indirect golang.org/x/term v0.47.0 // indirect golang.org/x/time v0.15.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect diff --git a/internal/containerruntime/container.go b/internal/containerruntime/container.go index 20e8cace..c123dd65 100644 --- a/internal/containerruntime/container.go +++ b/internal/containerruntime/container.go @@ -46,10 +46,19 @@ type Container struct { Image string // Name of the container. When set, an existing container with this name - // is reused (and restarted if needed) instead of creating a new one. - // Empty means Docker assigns a random name and nothing is reused. + // is reused (and restarted if needed) instead of creating a new one, + // subject to Domain. Empty means Docker assigns a random name and nothing + // is reused. Name string + // Domain is the annotation domain of Labels, e.g. DomainRender. When set + // with Name, Start reuses an existing container only when its labels say + // that's safe, and refuses one that another run will stop or remove; a + // run never stops or removes a container an earlier run labelled to keep + // longer. find has the full rules. Empty means any container with Name + // is reused. + Domain string + // Cmd is the command the container runs, e.g. a server subcommand and // its flags. Cmd []string @@ -113,8 +122,8 @@ type Client interface { ContainerStart(ctx context.Context, containerID string, options client.ContainerStartOptions) (client.ContainerStartResult, error) } -// CleanupClient is the subset of the Docker client Running.Stop uses to -// clean up a container. +// CleanupClient is the subset of the Docker client a Run uses to clean up a +// container. type CleanupClient interface { ContainerStop(ctx context.Context, containerID string, options client.ContainerStopOptions) (client.ContainerStopResult, error) ContainerRemove(ctx context.Context, containerID string, options client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) @@ -139,7 +148,18 @@ type Running struct { // rather than creating a new one. Reused bool - cli CleanupClient + // keep is the cleanup policy of the container Start reused, when Stop + // must keep it at least that long: Orphan for a running container another + // run keeps, or the label of a stopped container Start took over. Empty + // means Stop applies the policy it's given. + keep Cleanup + + // startedAt is when the container last started, as Start found it. A + // later start means another run took the container over. + startedAt string + + cli Client + log logging.Logger } // Start ensures the container is running and returns a handle carrying the @@ -164,7 +184,7 @@ func (c *Container) Start(ctx context.Context) (*Running, error) { } // Try to find an existing container with the supplied container name. - containerID, err := c.find(ctx, cli) + containerID, keep, err := c.find(ctx, cli, false) if err != nil { return nil, err } @@ -173,31 +193,70 @@ func (c *Container) Start(ctx context.Context) (*Running, error) { // Create new container if not found. if containerID == "" { containerID, err = c.create(ctx, cli) + if err != nil && c.Domain != "" && c.Name != "" && errdefs.IsConflict(err) { + // Another run created a container with this name since find + // looked. Judge it by the same rules, so a near-simultaneous + // run gets the reason it can't share it, not Docker's conflict. + // It may not have started yet, so judge it as running. + var cerr error + containerID, keep, cerr = c.find(ctx, cli, true) + switch { + case cerr != nil: + return nil, cerr + case containerID != "": + reused, err = true, nil + } + } if err != nil { return nil, err } } - addr, err := c.start(ctx, cli, containerID) + addr, startedAt, err := c.start(ctx, cli, containerID) if err != nil { return nil, err } - return &Running{Addr: addr, ID: containerID, Reused: reused, cli: cli}, nil + return &Running{Addr: addr, ID: containerID, Reused: reused, keep: keep, startedAt: startedAt, cli: cli, log: c.Log}, nil } // Stop finishes the container according to the cleanup policy: Orphan leaves // it running, Stop stops it with the grace period, and Remove stops it and -// then force removes it even if the graceful stop fails. +// then force removes it even if the graceful stop fails. When Start reused a +// container whose cleanup policy keeps it longer than cleanup, that policy +// applies instead. A container that has started again since Start, because +// another run found it stopped and took it over, is left to that run. func (r *Running) Stop(ctx context.Context, cleanup Cleanup) error { grace := int(StopGracePeriod / time.Second) stopOpts := client.ContainerStopOptions{Timeout: &grace} + if r.keep != "" && retention(r.keep) > retention(cleanup) { + r.log.Debug("Applying the reused Docker container's cleanup policy, because it keeps the container longer than this run's", "container", r.ID, "cleanup", r.keep, "run-cleanup", cleanup) + cleanup = r.keep + } + + if cleanup == CleanupStop || cleanup == CleanupRemove { + res, err := inspectContainer(ctx, r.cli, r.ID) + switch { + case errdefs.IsNotFound(err): + return nil + case err != nil: + return errors.Wrap(err, "cannot inspect Docker container") + case startedAt(res.Container.State) != r.startedAt: + r.log.Debug("Leaving Docker container another run started again after this one", "container", r.ID, "cleanup", cleanup) + return nil + } + } + + // Two handles can share one container, e.g. two Functions with one name, + // so the other may have removed it already, or still be removing it: a + // forced removal conflicts only with one in progress. That handle reports + // its own failure. switch cleanup { case CleanupOrphan: return nil case CleanupStop: - if err := stopContainer(ctx, r.cli, r.ID, stopOpts); err != nil { + if err := stopContainer(ctx, r.cli, r.ID, stopOpts); err != nil && !errdefs.IsNotFound(err) { return errors.Wrap(err, "cannot stop Docker container") } case CleanupRemove: @@ -209,7 +268,7 @@ func (r *Running) Stop(ctx context.Context, cleanup Cleanup) error { if err := stopContainer(ctx, r.cli, r.ID, stopOpts); err != nil { stopErr = errors.Wrap(err, "cannot stop Docker container") } - if err := removeContainer(ctx, r.cli, r.ID); err != nil { + if err := removeContainer(ctx, r.cli, r.ID); err != nil && !errdefs.IsNotFound(err) && !errdefs.IsConflict(err) { return errors.Join(errors.Wrap(err, "cannot remove Docker container"), stopErr) } } @@ -217,20 +276,104 @@ func (r *Running) Stop(ctx context.Context, cleanup Cleanup) error { return nil } -func (c *Container) find(ctx context.Context, cli Client) (string, error) { +// retention ranks how long a cleanup policy keeps a container: Remove +// shortest, then Stop, then any other, which includes the kept ones (see +// keeps). +func retention(cl Cleanup) int { + if cl == CleanupRemove { + return 0 + } + + if cl == CleanupStop { + return 1 + } + + return 2 +} + +// find returns the ID of the container named Name that Start may reuse, or +// "" if there is none, and the policy Stop must keep it at least as long as +// (see Running.keep). raced judges a container that isn't running as if it +// were, because another run created it a moment ago. +// +// With a Domain, a container is reused only when it is stopped, it has no +// /cleanup label, it's running with a kept policy (Orphan, or one +// this version doesn't know, and Running.Stop then always leaves it +// running), or it's running with this container's /session. A +// container another session runs under the Stop or Remove policy is +// refused, since that session stops or removes it when it exits. A stopped +// container is restarted and taken over, and Running.Stop then applies +// whichever of its /cleanup label and the policy Stop is given +// keeps it longer (Orphan, then Stop, then Remove), so a run never stops or +// removes a container an earlier run labelled to keep longer. A name +// conflict at create applies the same rules to the container that won the +// name, judging it as running even if it hasn't started yet. +// +// Docker labels can't change, so a taken-over container keeps its creator's +// labels: a later run that's refused it names the creator's session, not +// the run that now owns it, and a container labelled Stop or Remove that a +// taken-over Orphan run left running is refused to other sessions until +// it's removed by hand. A stopped container is taken over even if the run +// that created it is still running, but that run's Running.Stop then finds +// it started again and leaves it. If the run that took it over is killed, +// later runs don't reclaim it, since it carries the creator's session. Two runs that +// take over the same stopped container at once both treat it as theirs. A +// running container with no /cleanup label (an older CLI's) is +// reused, and Running.Stop treats it per the policy Stop is given. +func (c *Container) find(ctx context.Context, cli Client, raced bool) (string, Cleanup, error) { if c.Name == "" { - return "", nil + return "", "", nil } inspect, err := cli.ContainerInspect(ctx, c.Name, client.ContainerInspectOptions{}) if err != nil { if errdefs.IsNotFound(err) { - return "", nil // Container doesn't exist, but that's not an error + return "", "", nil // Container doesn't exist, but that's not an error } - return "", errors.Wrapf(err, "cannot inspect Docker container %q", c.Name) + return "", "", errors.Wrapf(err, "cannot inspect Docker container %q", c.Name) + } + found := inspect.Container + if c.Domain == "" { + return found.ID, "", nil } - return inspect.Container.ID, nil + var labels map[string]string + if found.Config != nil { + labels = found.Config.Labels + } + if !raced && (found.State == nil || !found.State.Running) { + // Stopped: restart it and take it over, keeping it at least as long + // as its label asks. Docker can't relabel it. + return found.ID, Cleanup(labels[c.Domain+labelSuffixCleanup]), nil + } + session := labels[c.Domain+labelSuffixSession] + switch cl := Cleanup(labels[c.Domain+labelSuffixCleanup]); { + case cl == "": + // Created before render recorded cleanup policies: reuse it, as + // before. + return found.ID, "", nil + case keeps(cl): + // Kept by the run that created it, or by a policy this version + // doesn't know: share it, never stop or remove it. + return found.ID, CleanupOrphan, nil + default: + // Stop or Remove. + if session == c.Labels[c.Domain+labelSuffixSession] { + // This run's own, e.g. two Functions with one name. + return found.ID, "", nil + } + // Only render's Functions choose their container's name. Simulate + // names only Orphan diff servers, so it meets this only for a + // container an older CLI or a crashed run left. + hint := "" + if c.Domain == DomainRender { + hint = fmt.Sprintf("To share it between runs, set the %s/runtime-docker-cleanup annotation to %q on the Functions that name it, or drop the %s/runtime-docker-name annotation. ", + c.Domain, CleanupOrphan, c.Domain) + } + return "", "", errors.Errorf("Docker container %q is in use by another run (session %q), which will %s it when it exits (cleanup policy %q). "+ + "%sIf it is left over from a crashed run or an older CLI, remove it with: docker rm -f %s", + c.Name, session, strings.ToLower(string(cl)), cl, hint, c.Name) + } } func (c *Container) create(ctx context.Context, cli Client) (string, error) { @@ -357,11 +500,12 @@ func (c *Container) pullImage(ctx context.Context, cli Client, options client.Im return nil } -// start ensures the container is running and returns its address. -func (c *Container) start(ctx context.Context, cli Client, containerID string) (string, error) { +// start ensures the container is running and returns its address and when +// it last started. +func (c *Container) start(ctx context.Context, cli Client, containerID string) (string, string, error) { // Start the container (idempotent - safe to call on running containers) if _, err := cli.ContainerStart(ctx, containerID, client.ContainerStartOptions{}); err != nil { - return "", errors.Wrap(err, "cannot start Docker container") + return "", "", errors.Wrap(err, "cannot start Docker container") } // When using a Docker network, containers are resolvable by name within @@ -371,30 +515,30 @@ func (c *Container) start(ctx context.Context, cli Client, containerID string) ( if c.Network != "" { inspect, err := cli.ContainerInspect(ctx, containerID, client.ContainerInspectOptions{}) if err != nil { - return "", errors.Wrap(err, "cannot inspect Docker container") + return "", "", errors.Wrap(err, "cannot inspect Docker container") } if _, ok := inspect.Container.NetworkSettings.Networks[c.Network]; !ok { - return "", errors.Errorf("container %q is not connected to Docker network %q; verify the network exists and the container is attached to it", strings.TrimPrefix(inspect.Container.Name, "/"), c.Network) + return "", "", errors.Errorf("container %q is not connected to Docker network %q; verify the network exists and the container is attached to it", strings.TrimPrefix(inspect.Container.Name, "/"), c.Network) } hostname := strings.TrimPrefix(inspect.Container.Name, "/") if hostname == "" { - return "", errors.Errorf("cannot determine hostname for container %q on network %q", containerID, c.Network) + return "", "", errors.Errorf("cannot determine hostname for container %q on network %q", containerID, c.Network) } addr := net.JoinHostPort(hostname, fmt.Sprintf("%d", c.Port)) c.Log.Debug("Container reachable on network", "network", c.Network, "address", addr) - return addr, nil + return addr, startedAt(inspect.Container.State), nil } // Inspect the container to get the actual allocated host port. inspect, err := cli.ContainerInspect(ctx, containerID, client.ContainerInspectOptions{}) if err != nil { - return "", errors.Wrap(err, "cannot inspect Docker container") + return "", "", errors.Wrap(err, "cannot inspect Docker container") } // Look up the specific container port instead of taking the first one p := network.MustParsePort(fmt.Sprintf("%d/tcp", c.Port)) if len(inspect.Container.NetworkSettings.Ports[p]) == 0 { - return "", errors.Errorf("container %q has no published binding for port %s", c.Name, p.Port()) + return "", "", errors.Errorf("container %q has no published binding for port %s", c.Name, p.Port()) } binding := inspect.Container.NetworkSettings.Ports[p][0] @@ -403,10 +547,10 @@ func (c *Container) start(ctx context.Context, cli Client, containerID string) ( host = binding.HostIP.String() } if host == "" { - return "", errors.Errorf("container %q has port binding for %s but no host address", c.Name, p.Port()) + return "", "", errors.Errorf("container %q has port binding for %s but no host address", c.Name, p.Port()) } - return net.JoinHostPort(host, binding.HostPort), nil + return net.JoinHostPort(host, binding.HostPort), startedAt(inspect.Container.State), nil } func (c *Container) getPullOptions() (client.ImagePullOptions, error) { @@ -436,6 +580,24 @@ func (c *Container) getPullOptions() (client.ImagePullOptions, error) { }, nil } +// startedAt is when a container last started, or "" if Docker didn't say. +func startedAt(s *container.State) string { + if s == nil { + return "" + } + + return s.StartedAt +} + +// inspectContainer inspects a container, bounding the request by +// RemoveTimeout. +func inspectContainer(ctx context.Context, cli Client, containerID string) (client.ContainerInspectResult, error) { + ctx, cancel := context.WithTimeout(ctx, RemoveTimeout) + defer cancel() + + return cli.ContainerInspect(ctx, containerID, client.ContainerInspectOptions{}) +} + // stopContainer stops a container, bounding the request by StopTimeout. func stopContainer(ctx context.Context, cli CleanupClient, containerID string, opts client.ContainerStopOptions) error { ctx, cancel := context.WithTimeout(ctx, StopTimeout) diff --git a/internal/containerruntime/container_test.go b/internal/containerruntime/container_test.go new file mode 100644 index 00000000..d0414d39 --- /dev/null +++ b/internal/containerruntime/container_test.go @@ -0,0 +1,324 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerruntime_test + +import ( + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/moby/moby/api/types/container" + + cr "github.com/crossplane/cli/v2/internal/containerruntime" +) + +const ( + ctrName = "fn" + ctrNet = "render-net" + thisRun = "this-session" + otherRun = "other-session" +) + +// existing is a render container named ctrName on ctrNet. An empty cleanup +// means it has no cleanup label, as from an older CLI. +type existing struct { + id string + running bool + session string + cleanup cr.Cleanup +} + +func (e existing) summary(domain string) container.Summary { + state, labels := exited, cr.ManagedLabels(domain, e.session) + if e.running { + state = running + } + if e.cleanup != "" { + labels = cr.ContainerLabels(domain, e.session, e.cleanup) + } + + c := ctr(e.id, state, labels, ctrNet) + c.Names = []string{"/" + ctrName} + + return c +} + +// TestContainerNamedReuse covers which container found by name Start reuses, +// and what Stop then leaves of it. A container Start creates has ID new-fn. +func TestContainerNamedReuse(t *testing.T) { + type args struct { + noDomain bool + // domain is the Container's Domain. Empty means DomainRender. + domain string + cleanup cr.Cleanup // this run's policy + existing *existing + // raced is created under the name by another run between Start's + // lookup and its create. + raced *existing + } + type want struct { + // errContains are substrings of Start's error. Empty means no error. + errContains []string + // errExcludes are substrings Start's error must not contain. + errExcludes []string + containers map[string]container.ContainerState + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NotFoundIsCreated": { + reason: "With no container by the name, Start creates one, and Stop applies this run's policy.", + args: args{cleanup: cr.CleanupStop}, + want: want{containers: map[string]container.ContainerState{"new-fn": exited}}, + }, + "RunningOrphanIsSharedAndLeftRunning": { + reason: "A running Orphan container is shared, and this run never stops or removes it, though its own policy is Remove.", + args: args{cleanup: cr.CleanupRemove, existing: &existing{id: "old", running: true, session: otherRun, cleanup: cr.CleanupOrphan}}, + want: want{containers: map[string]container.ContainerState{"old": running}}, + }, + "RunningUnknownPolicyIsSharedAndLeftRunning": { + reason: "A running container with a policy this version doesn't know counts as Orphan, so it's shared and left running.", + args: args{cleanup: cr.CleanupRemove, existing: &existing{id: "old", running: true, session: otherRun, cleanup: cr.Cleanup("Keep")}}, + want: want{containers: map[string]container.ContainerState{"old": running}}, + }, + "RunningSameSessionIsShared": { + reason: "A running Remove container of this run's session is this run's own, so it's shared and finished under this run's policy.", + args: args{cleanup: cr.CleanupStop, existing: &existing{id: "old", running: true, session: thisRun, cleanup: cr.CleanupRemove}}, + want: want{containers: map[string]container.ContainerState{"old": exited}}, + }, + "RunningOfOtherSessionIsRefused": { + reason: "A running Remove container of another session would be removed under this run, so Start refuses it, saying why and what to do, and leaves it alone.", + args: args{cleanup: cr.CleanupRemove, existing: &existing{id: "old", running: true, session: otherRun, cleanup: cr.CleanupRemove}}, + want: want{ + errContains: []string{`"fn"`, otherRun, "another run", "remove it when it exits", `"Orphan"`, "runtime-docker-name", "docker rm -f fn"}, + containers: map[string]container.ContainerState{"old": running}, + }, + }, + "RefusalOutsideRenderGivesNoFunctionHint": { + reason: "Simulate names only Orphan diff servers, so a refusal there points at removing the leftover, not at render's Function annotations.", + args: args{domain: cr.DomainSimulate, cleanup: cr.CleanupOrphan, existing: &existing{id: "old", running: true, session: otherRun, cleanup: cr.CleanupRemove}}, + want: want{ + errContains: []string{`"fn"`, otherRun, "another run", "older CLI", "docker rm -f fn"}, + errExcludes: []string{"Functions", "runtime-docker-name"}, + containers: map[string]container.ContainerState{"old": running}, + }, + }, + "RunningWithoutCleanupLabelIsReused": { + reason: "A running container from before render recorded cleanup policies is reused under this run's policy, as before.", + args: args{cleanup: cr.CleanupStop, existing: &existing{id: "old", running: true, session: otherRun}}, + want: want{containers: map[string]container.ContainerState{"old": exited}}, + }, + "StoppedIsRestartedAndKeptPerItsLabel": { + reason: "A stopped container is restarted and taken over, and its Orphan label keeps it running past this Remove run.", + args: args{cleanup: cr.CleanupRemove, existing: &existing{id: "old", session: otherRun, cleanup: cr.CleanupOrphan}}, + want: want{containers: map[string]container.ContainerState{"old": running}}, + }, + "StoppedIsRestartedAndKeptPerThisRun": { + reason: "A stopped container is restarted and taken over, and this Stop run keeps it longer than its Remove label asks.", + args: args{cleanup: cr.CleanupStop, existing: &existing{id: "old", session: otherRun, cleanup: cr.CleanupRemove}}, + want: want{containers: map[string]container.ContainerState{"old": exited}}, + }, + "NameConflictIsRefusedByTheRules": { + reason: "When another run creates the name between lookup and create, Start judges that run's container by the same rules, refusing a Remove one rather than returning Docker's conflict.", + args: args{cleanup: cr.CleanupRemove, raced: &existing{id: "raced", running: true, session: otherRun, cleanup: cr.CleanupRemove}}, + want: want{ + errContains: []string{otherRun, "another run", "docker rm -f fn"}, + containers: map[string]container.ContainerState{"raced": running}, + }, + }, + "NoDomainReusesAnything": { + reason: "Without a Domain (the diff server manager), any container with the name is reused under this run's policy, as before.", + args: args{noDomain: true, cleanup: cr.CleanupStop, existing: &existing{id: "old", running: true, session: otherRun, cleanup: cr.CleanupRemove}}, + want: want{containers: map[string]container.ContainerState{"old": exited}}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + domain := tc.args.domain + if domain == "" { + domain = cr.DomainRender + } + + h := &host{} + if tc.args.existing != nil { + h.containers = append(h.containers, tc.args.existing.summary(domain)) + } + if tc.args.raced != nil { + c := tc.args.raced.summary(domain) + h.raced = &c + } + + c := &cr.Container{ + Image: "xpkg.crossplane.io/example/fn:v1", + Name: ctrName, + Domain: domain, + Port: 9443, + Labels: cr.ContainerLabels(domain, thisRun, tc.args.cleanup), + Network: ctrNet, + PullPolicy: cr.PullNever, + Client: h.client(), + } + if tc.args.noDomain { + c.Domain = "" + } + + r, err := c.Start(t.Context()) + if err == nil { + err = r.Stop(t.Context(), tc.args.cleanup) + } + + if len(tc.want.errContains) == 0 && err != nil { + t.Errorf("\n%s\nStart or Stop: unexpected error: %v", tc.reason, err) + } + for _, s := range tc.want.errContains { + if err == nil || !strings.Contains(err.Error(), s) { + t.Errorf("\n%s\nStart(...): want an error containing %q, got %v", tc.reason, s, err) + } + } + for _, s := range tc.want.errExcludes { + if err != nil && strings.Contains(err.Error(), s) { + t.Errorf("\n%s\nStart(...): want an error without %q, got %v", tc.reason, s, err) + } + } + + got, _ := h.left() + if diff := cmp.Diff(tc.want.containers, got, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("\n%s\ncontainers left: -want, +got:\n%s", tc.reason, diff) + } + }) + } +} + +// TestRunningStopSharedContainer covers two handles to one container, as when +// two Functions share a name: the second to stop it finds it removed, or its +// removal still in progress, which is not a failure. +func TestRunningStopSharedContainer(t *testing.T) { + cases := map[string]struct { + reason string + cleanup cr.Cleanup + // removing makes the second handle's removal conflict, as Docker + // refuses one while another is in progress. + removing bool + }{ + "RemoveFindsItRemoved": { + reason: "The second Remove handle finds the container gone.", + cleanup: cr.CleanupRemove, + }, + "RemoveFindsItBeingRemoved": { + reason: "The second Remove handle finds the first still removing the container.", + cleanup: cr.CleanupRemove, + removing: true, + }, + "StopFindsItRemoved": { + reason: "A Stop handle finds the container removed by a Remove handle.", + cleanup: cr.CleanupStop, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + h := &host{} + c := &cr.Container{ + Image: "xpkg.crossplane.io/example/fn:v1", + Name: ctrName, + Domain: cr.DomainRender, + Port: 9443, + Labels: cr.ContainerLabels(cr.DomainRender, thisRun, cr.CleanupRemove), + Network: ctrNet, + PullPolicy: cr.PullNever, + Client: h.client(), + } + + first, err := c.Start(t.Context()) + if err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + second, err := c.Start(t.Context()) + if err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + + if tc.removing { + h.removing = first.ID + } else if err := first.Stop(t.Context(), cr.CleanupRemove); err != nil { + t.Fatalf("Stop(...): unexpected error: %v", err) + } + + if err := second.Stop(t.Context(), tc.cleanup); err != nil { + t.Errorf("\n%s\nStop(...): want no error, got %v", tc.reason, err) + } + }) + } +} + +// TestRunningStopTakenOver covers a container that stopped while the run that +// started it was still running, e.g. because its process exited, and that +// another run then took over: the first run's Stop leaves it to that run. +func TestRunningStopTakenOver(t *testing.T) { + for _, cleanup := range []cr.Cleanup{cr.CleanupStop, cr.CleanupRemove} { + t.Run(string(cleanup), func(t *testing.T) { + h := &host{} + newContainer := func(session string) *cr.Container { + return &cr.Container{ + Image: "xpkg.crossplane.io/example/fn:v1", + Name: ctrName, + Domain: cr.DomainRender, + Port: 9443, + Labels: cr.ContainerLabels(cr.DomainRender, session, cleanup), + Network: ctrNet, + PullPolicy: cr.PullNever, + Client: h.client(), + } + } + + creator, err := newContainer(thisRun).Start(t.Context()) + if err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + h.containers[0].State = exited + + taker, err := newContainer(otherRun).Start(t.Context()) + if err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + + if err := creator.Stop(t.Context(), cleanup); err != nil { + t.Fatalf("Stop(...): unexpected error: %v", err) + } + if got, _ := h.left(); got["new-"+ctrName] != running { + t.Errorf("creator's Stop(...): want the taken-over container left running, got %v", got) + } + + if err := taker.Stop(t.Context(), cleanup); err != nil { + t.Fatalf("Stop(...): unexpected error: %v", err) + } + want := map[string]container.ContainerState{"new-" + ctrName: exited} + if cleanup == cr.CleanupRemove { + want = nil + } + got, _ := h.left() + if diff := cmp.Diff(want, got, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("taker's Stop(...): containers left, -want, +got:\n%s", diff) + } + }) + } +} diff --git a/internal/containerruntime/containerruntime.go b/internal/containerruntime/containerruntime.go index 3fbe8e69..3c78cc04 100644 --- a/internal/containerruntime/containerruntime.go +++ b/internal/containerruntime/containerruntime.go @@ -23,10 +23,18 @@ limitations under the License. package containerruntime import ( + "crypto/rand" + "encoding/hex" "sync" "time" +) + +const ( + // DomainRender is the label and annotation domain render uses. + DomainRender = "render.crossplane.io" - "k8s.io/apimachinery/pkg/util/rand" + // DomainSimulate is the label and annotation domain simulate uses. + DomainSimulate = "simulate.crossplane.io" ) // Cleanup specifies what to do with a package's container once its run is @@ -96,11 +104,20 @@ const ( LabelValueManagedBy = "crossplane-cli" ) +// NewSessionID returns a new random session ID: 16 lowercase hex characters +// (64 random bits). +func NewSessionID() string { + b := make([]byte, 8) + _, _ = rand.Read(b) // Never returns an error. + + return hex.EncodeToString(b) +} + // SessionID returns an identifier unique to this process. It is the session // ID used for Docker resources when the caller doesn't supply one. // //nolint:gochecknoglobals // Must be stable for the life of the process. -var SessionID = sync.OnceValue(func() string { return rand.String(12) }) +var SessionID = sync.OnceValue(NewSessionID) // ManagedLabels returns the labels marking a Docker resource as managed by // the crossplane CLI under the given annotation domain: diff --git a/internal/containerruntime/export_test.go b/internal/containerruntime/export_test.go new file mode 100644 index 00000000..2b93becc --- /dev/null +++ b/internal/containerruntime/export_test.go @@ -0,0 +1,31 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerruntime + +// TryLease reports whether the lease of run id in dir could be taken, i.e. +// whether no process holds it, and gives it straight back. +func TryLease(dir, id string) bool { + f, ok := tryLease(leasePath(dir, id)) + if ok { + _ = f.Close() + } + + return ok +} + +// LeasePath is the path of run id's lease file in dir. +var LeasePath = leasePath diff --git a/internal/containerruntime/fake/fake.go b/internal/containerruntime/fake/fake.go index f72687b4..b64a8793 100644 --- a/internal/containerruntime/fake/fake.go +++ b/internal/containerruntime/fake/fake.go @@ -36,9 +36,15 @@ type Client struct { MockContainerStart func(ctx context.Context, containerID string, options client.ContainerStartOptions) (client.ContainerStartResult, error) MockContainerStop func(ctx context.Context, containerID string, options client.ContainerStopOptions) (client.ContainerStopResult, error) MockContainerRemove func(ctx context.Context, containerID string, options client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) + MockContainerList func(ctx context.Context, options client.ContainerListOptions) (client.ContainerListResult, error) + MockNetworkList func(ctx context.Context, options client.NetworkListOptions) (client.NetworkListResult, error) + MockNetworkRemove func(ctx context.Context, networkID string, options client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) } -var _ containerruntime.Client = &Client{} +var ( + _ containerruntime.Client = &Client{} + _ containerruntime.RunClient = &Client{} +) // ImagePull calls MockImagePull. func (c *Client) ImagePull(ctx context.Context, ref string, options client.ImagePullOptions) (client.ImagePullResponse, error) { @@ -70,6 +76,21 @@ func (c *Client) ContainerRemove(ctx context.Context, containerID string, option return c.MockContainerRemove(ctx, containerID, options) } +// ContainerList calls MockContainerList. +func (c *Client) ContainerList(ctx context.Context, options client.ContainerListOptions) (client.ContainerListResult, error) { + return c.MockContainerList(ctx, options) +} + +// NetworkList calls MockNetworkList. +func (c *Client) NetworkList(ctx context.Context, options client.NetworkListOptions) (client.NetworkListResult, error) { + return c.MockNetworkList(ctx, options) +} + +// NetworkRemove calls MockNetworkRemove. +func (c *Client) NetworkRemove(ctx context.Context, networkID string, options client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) { + return c.MockNetworkRemove(ctx, networkID, options) +} + // ImagePullDone is a MockImagePull that reports an image pull that has // already completed. func ImagePullDone(context.Context, string, client.ImagePullOptions) (client.ImagePullResponse, error) { diff --git a/internal/containerruntime/lease_unix_test.go b/internal/containerruntime/lease_unix_test.go new file mode 100644 index 00000000..28fe1302 --- /dev/null +++ b/internal/containerruntime/lease_unix_test.go @@ -0,0 +1,154 @@ +//go:build unix + +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerruntime_test + +import ( + "bufio" + "context" + "fmt" + "io" + "os" + "os/exec" + "testing" + "time" + + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" + + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + + cr "github.com/crossplane/cli/v2/internal/containerruntime" +) + +const ( + envLeaseHelperDir = "CONTAINERRUNTIME_LEASE_HELPER_DIR" + envLeaseHelperID = "CONTAINERRUNTIME_LEASE_HELPER_ID" + leaseHelperReady = "leased" +) + +// TestLeaseHelper is not a test: it runs as startLeaseHelper's child process, +// where it takes a lease, says so on stdout, and holds the lease until its +// stdin closes. The parent holds the other end of stdin, and the kernel +// closes it however the parent dies, so the child never outlives it. +func TestLeaseHelper(t *testing.T) { + dir, id := os.Getenv(envLeaseHelperDir), os.Getenv(envLeaseHelperID) + if dir == "" { + t.Skip("only runs as startLeaseHelper's child process") + } + + h := &host{} + r := cr.NewRun(dir, id, true, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + if err := r.Acquire(t.Context()); err != nil { + t.Fatalf("Acquire(...): unexpected error: %v", err) + } + + _, _ = fmt.Fprintln(os.Stdout, leaseHelperReady) + + _, _ = io.Copy(io.Discard, os.Stdin) +} + +// TestLeaseAcrossProcesses pins that a lease protects a run from reapers in +// other processes while it lives, and that the kernel gives the lease up when +// the process dies, however it dies. A fake host can't span processes, so this +// process's host is seeded with resources labelled with the child's ID. +func TestLeaseAcrossProcesses(t *testing.T) { + dir, id := t.TempDir(), cr.NewSessionID() + + cmd := startLeaseHelper(t, dir, id) + + if cr.TryLease(dir, id) { + t.Fatal("TryLease(...): took the lease of a run that is alive in another process") + } + + if err := cmd.Process.Kill(); err != nil { + t.Fatalf("Kill(): %v", err) + } + + _ = cmd.Wait() + + h := &host{ + containers: []container.Summary{ctr("fn", running, cr.ContainerLabels(cr.DomainRender, id, cr.CleanupRemove), "net")}, + networks: []network.Summary{net("net", cr.ManagedLabels(cr.DomainRender, id))}, + } + + r := cr.NewRun(dir, cr.NewSessionID(), true, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + if err := r.Acquire(t.Context()); err != nil { + t.Fatalf("Acquire(...) after the child died: unexpected error: %v", err) + } + + t.Cleanup(func() { _ = r.Release(context.Background(), false, "") }) + + cs, ns := h.left() + if len(cs) != 0 || len(ns) != 0 { + t.Errorf("Acquire(...) after the child died: left containers %v and networks %v, want none", cs, ns) + } + + if _, err := os.Stat(cr.LeasePath(dir, id)); !os.IsNotExist(err) { + t.Errorf("lease file of the dead child: Stat(...) = %v, want it removed", err) + } +} + +// startLeaseHelper runs TestLeaseHelper in a child process that leases run id +// in dir, and returns once the child holds the lease. The child is killed +// when the test ends. +func startLeaseHelper(t *testing.T, dir, id string) *exec.Cmd { + t.Helper() + + cmd := exec.Command(os.Args[0], "-test.run=^TestLeaseHelper$") + cmd.Env = append(os.Environ(), envLeaseHelperDir+"="+dir, envLeaseHelperID+"="+id) + cmd.Stderr = os.Stderr + + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatalf("StdinPipe(): %v", err) + } + + out, err := cmd.StdoutPipe() + if err != nil { + t.Fatalf("StdoutPipe(): %v", err) + } + + if err := cmd.Start(); err != nil { + t.Fatalf("Start(): %v", err) + } + + t.Cleanup(func() { + _ = stdin.Close() + _ = cmd.Process.Kill() + _ = cmd.Wait() + }) + + ready := make(chan string, 1) + go func() { + s := bufio.NewScanner(out) + s.Scan() + ready <- s.Text() + }() + + select { + case got := <-ready: + if got != leaseHelperReady { + t.Fatalf("child process: got %q before stdout ended, want %q", got, leaseHelperReady) + } + case <-time.After(30 * time.Second): + t.Fatalf("child process: did not print %q within 30s", leaseHelperReady) + } + + return cmd +} diff --git a/internal/containerruntime/lock_unix.go b/internal/containerruntime/lock_unix.go new file mode 100644 index 00000000..86794c26 --- /dev/null +++ b/internal/containerruntime/lock_unix.go @@ -0,0 +1,35 @@ +//go:build unix + +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerruntime + +import ( + "os" + + "golang.org/x/sys/unix" +) + +// leasesSupported is whether runs on this platform take leases. +const leasesSupported = true + +// lock takes an exclusive flock on f without blocking. A flock belongs to the open file, not the +// process, so two opens of one lease conflict even within a process, and the kernel drops the lock +// when the process exits, however it exits. +func lock(f *os.File) error { + return unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB) +} diff --git a/internal/containerruntime/lock_windows.go b/internal/containerruntime/lock_windows.go new file mode 100644 index 00000000..666173cd --- /dev/null +++ b/internal/containerruntime/lock_windows.go @@ -0,0 +1,34 @@ +//go:build windows + +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerruntime + +import ( + "os" +) + +// leasesSupported is whether runs on this platform take leases. acquireLease +// checks it before touching the lease directory. +const leasesSupported = false + +// lock is unsupported on Windows, where a file that is open cannot be renamed or removed, which the +// lease protocol relies on. Without a lease a run's resources are never reclaimed by another run; the +// run still removes its own when it exits. +func lock(_ *os.File) error { + return errLeaseUnsupported +} diff --git a/internal/containerruntime/run.go b/internal/containerruntime/run.go new file mode 100644 index 00000000..4660c871 --- /dev/null +++ b/internal/containerruntime/run.go @@ -0,0 +1,599 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerruntime + +import ( + "context" + "encoding/hex" + "io" + "os" + "path/filepath" + "strings" + "time" + + "github.com/containerd/errdefs" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/client" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" +) + +// A Run ties the Docker resources one process creates (package containers, +// the render container and the render network) to that process, and reclaims +// the ones left behind by runs that died without cleaning up (SIGKILL, OOM, a +// second Ctrl+C, a CI runner tearing down the job). +// +// Ownership is carried by labels. A run's ID is the session ID its resources +// are labelled with: /session= alongside +// /managed-by=crossplane-cli, under either DomainRender or +// DomainSimulate. A run's resources are exactly the ones carrying both labels +// of one domain with its ID, so a container or network that lacks either is +// never touched. +// +// Liveness is a lease: a run holds an exclusive lock on ".lock" in the +// lease directory for as long as it has resources. The operating system drops +// the lock when the process exits however it exits, so a lease file whose lock +// can be taken belongs to a dead run. That needs no process table, cannot be +// fooled by PID reuse, and keeps concurrent runs from touching each other's +// resources. A lease that is not in this directory (another user, another +// container sharing the Docker socket) is never seen, so its resources are +// never touched. +// +// Reclaiming a dead run finishes what it would have done, following each +// container's recorded /cleanup label: Orphan, or a value this +// version does not know, is left alone, Stop is stopped, and Remove, or no +// cleanup label, is force removed. Its networks are removed after its +// containers. Reclaiming talks to the daemon this run connects to, so if +// DOCKER_HOST changed since a dead run, its lease is retired but its +// resources on the other daemon are left behind. +// +// A Run is not safe for concurrent use; its owner serializes access. +type Run struct { + id string + dir string + leased bool + log logging.Logger + newClient func() (RunClient, error) + + // acquired is set once Acquire has run. + acquired bool + // lease is the open, locked lease file while the run holds it; nil when + // the run is unleased, leasing failed, or the lease was released. + lease *os.File +} + +const ( + // runIDLength is the length of a run ID NewSessionID generates: 16 + // lowercase hex characters (64 random bits), so a session label carrying + // one cannot belong to anything but that run. + runIDLength = 16 + + leaseExt = ".lock" + + // tempLeaseExt ends the name of a lease file before acquireLease renames + // it into place. + tempLeaseExt = ".tmp" + + // staleTempLeaseAge is how old a temporary lease file must be before a + // reaper removes it: far longer than acquireLease holds one. + staleTempLeaseAge = time.Minute + + // reapTimeout bounds reclaiming dead runs' resources, and sweeping a run's + // own networks, so a slow Docker daemon cannot hold up a run. It covers all + // the dead runs one Acquire reclaims together, and each Stop container can + // take StopGracePeriod, so a few dead runs can run out of it. A run that is + // not fully reclaimed in time keeps its lease, and the next run finishes it. + reapTimeout = 10 * time.Second +) + +// errLeaseUnsupported is lock's error on a platform without run leases. +var errLeaseUnsupported = errors.New("run leases are not supported on this platform") + +// RunClient is the subset of the Docker client a Run uses to reclaim +// containers and networks. +type RunClient interface { + CleanupClient + ContainerList(ctx context.Context, options client.ContainerListOptions) (client.ContainerListResult, error) + NetworkList(ctx context.Context, options client.NetworkListOptions) (client.NetworkListResult, error) + NetworkRemove(ctx context.Context, networkID string, options client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) +} + +var _ RunClient = (*client.Client)(nil) + +// A RunOption configures a Run. +type RunOption func(*Run) + +// WithRunClient makes the Run talk to Docker through a client from newClient, +// instead of one configured from the environment. The Run closes each client +// newClient returns, if it is an io.Closer, once done with it. +func WithRunClient(newClient func() (RunClient, error)) RunOption { + return func(r *Run) { + r.newClient = newClient + } +} + +// DefaultLeaseDir is where runs keep their leases: a per-user directory, so +// one user's runs never judge another's. It returns "" when the user has no +// cache directory, which disables leasing and reaping. +func DefaultLeaseDir() string { + dir, err := os.UserCacheDir() + if err != nil { + return "" + } + + return filepath.Join(dir, "crossplane", "runs") +} + +// NewRun returns a Run with the given ID that keeps its lease in dir. An +// empty dir disables both leasing and reaping. leased says whether id is one +// NewSessionID generated for this run alone; only such a run takes a lease, +// because two processes sharing a supplied ID would rename over each other's +// lease. NewRun does no I/O. +func NewRun(dir, id string, leased bool, log logging.Logger, opts ...RunOption) *Run { + if log == nil { + log = logging.NewNopLogger() + } + + r := &Run{id: id, dir: dir, leased: leased, log: log, newClient: newRunClient} + for _, o := range opts { + o(r) + } + + return r +} + +// ID is the run's ID: the session label value that makes a container or +// network this run's. +func (r *Run) ID() string { + return r.id +} + +// Acquire takes the run's lease, when it is leased, then reclaims what every +// dead run in the lease directory left behind, bounded by reapTimeout. Call +// it before creating any Docker resource. Acquire is idempotent: only the +// first call does anything. +// +// The returned error is a lease or reap failure, for the caller to warn +// about. It never means the run cannot go on: an unleased run's resources +// only risk leaking if it dies, and a dead run that could not be reclaimed is +// retried by the next run. +func (r *Run) Acquire(ctx context.Context) error { + if r.acquired { + return nil + } + + r.acquired = true + + if r.dir == "" { + return nil + } + + var errs []error + + if r.leased { + f, err := r.acquireLease() + switch { + case errors.Is(err, errLeaseUnsupported): + // Not a failure to warn about on every run: the platform has no + // leases, so no run is ever reclaimed, as before leases existed. + r.log.Debug("Run leases are not supported on this platform; this run's Docker resources will not be reclaimed if it dies") + case err != nil: + errs = append(errs, errors.Wrap(err, "cannot lease this run's Docker resources; they will not be reclaimed if it dies")) + } + + r.lease = f + } + + if err := r.reapDead(ctx); err != nil { + errs = append(errs, errors.Wrap(err, "cannot clean up Docker resources left behind by an earlier run; the next run will retry")) + } + + return errors.Join(errs...) +} + +// Release removes the run's leftover session networks under sweepDomain, +// unless it is empty, then gives up the run's lease. The lease file is kept, +// so the next run reclaims what is left, when clean is false or a network +// could not be removed. clean reports whether the caller removed everything +// else the run created. Release is a no-op on the lease of an unleased run, +// and safe to call more than once. +func (r *Run) Release(ctx context.Context, clean bool, sweepDomain string) error { + var err error + if sweepDomain != "" { + err = r.sweepNetworks(ctx, sweepDomain) + } + + releaseLease(r.lease, leasePath(r.dir, r.id), clean && err == nil) + r.lease = nil + + return err +} + +// acquireLease takes the run's lease, refusing an ID that is not one +// NewSessionID generates: reapers skip such lease files, so it would protect +// nothing and never be reclaimed. +func (r *Run) acquireLease() (*os.File, error) { + if !validRunID(r.id) { + return nil, errors.Errorf("run ID %q is not %d lowercase hex characters", r.id, runIDLength) + } + + return acquireLease(r.dir, r.id) +} + +// sweepNetworks removes every network labelled with the run's session under +// domain. As in reclaim, a network that one of the run's kept containers +// (Orphan, or a cleanup policy this version does not know) is still attached +// to is left, and is not an error. +func (r *Run) sweepNetworks(ctx context.Context, domain string) error { + cli, err := r.newClient() + if err != nil { + return errors.Wrap(err, "cannot remove this run's Docker networks") + } + defer closeClient(cli, r.log) + + ctx, cancel := context.WithTimeout(ctx, reapTimeout) + defer cancel() + + kept := map[string]bool{} + + var errs []error + + res, err := cli.ContainerList(ctx, client.ContainerListOptions{All: true, Filters: sessionFilter(domain, r.id)}) + if err != nil { + // Still try the networks. The list failure is reported either way, + // so it keeps the lease and the next run retries. + errs = append(errs, errors.Wrap(err, "cannot list Docker containers")) + } + + for _, c := range res.Items { + if keeps(Cleanup(c.Labels[domain+labelSuffixCleanup])) { + keep(kept, c) + } + } + + errs = append(errs, r.removeNetworks(ctx, cli, r.id, kept, domain)) + + return errors.Wrap(errors.Join(errs...), "cannot remove this run's Docker networks") +} + +// deadLease is a lease this run took from a dead run. +type deadLease struct { + id string + path string + f *os.File +} + +// reapDead reclaims the resources of every run whose lease is in r.dir but +// held by no process. With no dead lease it makes no Docker call. +func (r *Run) reapDead(ctx context.Context) error { + // Not filepath.Glob: it would read glob syntax in r.dir as a pattern. + entries, err := os.ReadDir(r.dir) + if err != nil { + return nil // Best effort: a missing or unreadable dir has no leases to reap. + } + + var dead []deadLease + + for _, e := range entries { + if !e.Type().IsRegular() { + continue + } + + if isTempLease(e.Name()) { + removeStaleTempLease(filepath.Join(r.dir, e.Name())) + continue + } + + id, ok := strings.CutSuffix(e.Name(), leaseExt) + if !ok || id == r.id || !validRunID(id) { + continue + } + + p := filepath.Join(r.dir, e.Name()) + + if f, ok := tryLease(p); ok { + dead = append(dead, deadLease{id: id, path: p, f: f}) + } + } + + if len(dead) == 0 { + return nil + } + + cli, err := r.newClient() + if err != nil { + // NOTE(jcogilvie): a Docker-less machine that once left a lease retries this + // on every run. Drop such leases after N failures if that shows up. + for _, l := range dead { + releaseLease(l.f, l.path, false) + } + + return err + } + defer closeClient(cli, r.log) + + ctx, cancel := context.WithTimeout(ctx, reapTimeout) + defer cancel() + + var errs []error + + for _, l := range dead { + err := r.reclaim(ctx, cli, l.id) + if err != nil { + errs = append(errs, errors.Wrapf(err, "run %s", l.id)) + } else { + r.log.Debug("Reclaimed Docker resources left by a dead run", "run", l.id) + } + + releaseLease(l.f, l.path, err == nil) + } + + return errors.Join(errs...) +} + +// reclaim finishes what dead run id would have done to its containers, per +// each one's cleanup label, then removes its networks. It reports whether the +// run is fully reclaimed: a network that a kept container is attached to does +// not count against it. +func (r *Run) reclaim(ctx context.Context, cli RunClient, id string) error { + grace := int(StopGracePeriod / time.Second) + + // kept holds the names of the networks kept containers are attached to. + kept := map[string]bool{} + + var errs []error + + for _, d := range []string{DomainRender, DomainSimulate} { + res, err := cli.ContainerList(ctx, client.ContainerListOptions{All: true, Filters: sessionFilter(d, id)}) + if err != nil { + errs = append(errs, errors.Wrap(err, "cannot list Docker containers")) + continue + } + + for _, c := range res.Items { + switch cl := Cleanup(c.Labels[d+labelSuffixCleanup]); { + case keeps(cl): + if cl != CleanupOrphan { + r.log.Debug("Leaving Docker container with an unknown cleanup policy", "container", c.ID, "cleanup", cl, "run", id) + } + keep(kept, c) + case cl == CleanupStop: + if err := stopContainer(ctx, cli, c.ID, client.ContainerStopOptions{Timeout: &grace}); err != nil && !errdefs.IsNotFound(err) { + errs = append(errs, errors.Wrapf(err, "cannot stop Docker container %s", c.ID)) + } + default: + // Remove, or no cleanup label: the render container, which + // the dead run would have removed. Force stops it if running. + if err := removeContainer(ctx, cli, c.ID); err != nil && !errdefs.IsNotFound(err) { + errs = append(errs, errors.Wrapf(err, "cannot remove Docker container %s", c.ID)) + } + } + } + } + + if len(errs) > 0 { + // A container still attached would make removing its network fail, + // so networks wait for every container to be finished. + return errors.Join(errs...) + } + + return r.removeNetworks(ctx, cli, id, kept, DomainRender, DomainSimulate) +} + +// keeps reports whether cleanup policy cl leaves a container running: Orphan, +// or a policy this version does not know, perhaps from a newer one, which may +// have asked to keep it. No policy (no cleanup label) is not kept. +func keeps(cl Cleanup) bool { + return cl != CleanupStop && cl != CleanupRemove && cl != "" +} + +// keep records the networks kept container c is attached to. +func keep(kept map[string]bool, c container.Summary) { + if c.NetworkSettings == nil { + return + } + + for n := range c.NetworkSettings.Networks { + kept[n] = true + } +} + +// removeNetworks removes every network of run id under domains. A run that +// used no Docker render engine, or a network the user supplied, has none. A +// network named in kept is left with the kept container attached to it, even +// if that container is stopped: Docker would remove it then, and the +// container could not start again without it. +func (r *Run) removeNetworks(ctx context.Context, cli RunClient, id string, kept map[string]bool, domains ...string) error { + var errs []error + + for _, d := range domains { + res, err := cli.NetworkList(ctx, client.NetworkListOptions{Filters: sessionFilter(d, id)}) + if err != nil { + errs = append(errs, errors.Wrap(err, "cannot list Docker networks")) + continue + } + + for _, n := range res.Items { + if kept[n.Name] { + r.log.Debug("Leaving Docker network attached to a kept container", "network", n.Name, "run", id) + continue + } + + _, err := cli.NetworkRemove(ctx, n.ID, client.NetworkRemoveOptions{}) + + switch { + case err == nil, errdefs.IsNotFound(err): + default: + errs = append(errs, errors.Wrapf(err, "cannot remove Docker network %s", n.Name)) + } + } + } + + return errors.Join(errs...) +} + +// newRunClient returns a Docker client configured from the environment. +func newRunClient() (RunClient, error) { + cli, err := client.New(client.FromEnv) + if err != nil { + return nil, errors.Wrap(err, "cannot create Docker client using environment variables") + } + + return cli, nil +} + +// closeClient closes cli if it holds a connection. +func closeClient(cli RunClient, log logging.Logger) { + c, ok := cli.(io.Closer) + if !ok { + return + } + + if err := c.Close(); err != nil { + log.Debug("Error closing Docker client", "error", err) + } +} + +// sessionFilter matches what run id created under domain. Docker requires +// every label filter to match, so a resource is matched only if it carries +// both labels. +func sessionFilter(domain, id string) client.Filters { + return make(client.Filters).Add("label", + domain+labelSuffixSession+"="+id, + domain+labelSuffixManagedBy+"="+LabelValueManagedBy) +} + +func validRunID(id string) bool { + if len(id) != runIDLength { + return false + } + + _, err := hex.DecodeString(id) + + return err == nil && strings.ToLower(id) == id +} + +func leasePath(dir, id string) string { + return filepath.Join(dir, id+leaseExt) +} + +// acquireLease creates and locks run id's lease file. The file is locked +// under a temporary name and renamed into place, so a reaper never sees a +// lease file that is not yet locked. +func acquireLease(dir, id string) (*os.File, error) { + if !leasesSupported { + return nil, errLeaseUnsupported + } + + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, errors.Wrap(err, "cannot create lease directory") + } + + f, err := os.CreateTemp(dir, id+"-*"+tempLeaseExt) + if err != nil { + return nil, errors.Wrap(err, "cannot create lease file") + } + + if err := lock(f); err != nil { + _ = f.Close() + _ = os.Remove(f.Name()) + + return nil, errors.Wrap(err, "cannot lock lease file") + } + + if err := os.Rename(f.Name(), leasePath(dir, id)); err != nil { + _ = f.Close() + _ = os.Remove(f.Name()) + + return nil, errors.Wrap(err, "cannot publish lease file") + } + + return f, nil +} + +// isTempLease reports whether name is a lease file acquireLease has not yet +// renamed into place: "-.tmp". +func isTempLease(name string) bool { + rest, ok := strings.CutSuffix(name, tempLeaseExt) + if !ok { + return false + } + + id, _, ok := strings.Cut(rest, "-") + + return ok && validRunID(id) +} + +// removeStaleTempLease removes a temporary lease file that a run which died +// in acquireLease, between creating and renaming it, left behind. A file +// younger than staleTempLeaseAge may be another run's, about to be locked and +// renamed, so it is left; so is one whose lock is held. +func removeStaleTempLease(path string) { + fi, err := os.Stat(path) + if err != nil || time.Since(fi.ModTime()) < staleTempLeaseAge { + return + } + + if f, ok := tryLease(path); ok { + releaseLease(f, path, true) + } +} + +// tryLease takes the lease at path if no process holds it, i.e. if its run is +// dead. A lease that another reaper reclaimed and removed between our open +// and our lock is no longer at path, so it is not taken: each dead run is +// reclaimed by one reaper. +func tryLease(path string) (*os.File, bool) { + f, err := os.OpenFile(path, os.O_RDWR, 0) //nolint:gosec // The path is a lease file in our own lease directory. + if err != nil { + return nil, false + } + + if err := lock(f); err != nil { + _ = f.Close() + return nil, false + } + + held, err := f.Stat() + if err != nil { + _ = f.Close() + return nil, false + } + + if now, err := os.Stat(path); err != nil || !os.SameFile(held, now) { + _ = f.Close() + return nil, false + } + + return f, true +} + +// releaseLease gives up a lease, removing its file first when the run's +// resources are all gone, so no one can take a lease that no longer protects +// anything. A nil lease (unleased, or leasing failed) is a no-op. +func releaseLease(f *os.File, path string, remove bool) { + if f == nil { + return + } + + if remove { + _ = os.Remove(path) + } + + _ = f.Close() +} diff --git a/internal/containerruntime/run_integration_test.go b/internal/containerruntime/run_integration_test.go new file mode 100644 index 00000000..8a4459ad --- /dev/null +++ b/internal/containerruntime/run_integration_test.go @@ -0,0 +1,311 @@ +//go:build integration && unix + +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerruntime_test + +import ( + "context" + "maps" + "os" + "testing" + "time" + + "github.com/containerd/errdefs" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" + "github.com/moby/moby/client" + + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + + cr "github.com/crossplane/cli/v2/internal/containerruntime" +) + +const ( + // busybox is pinned, on the GCR mirror, which avoids Docker Hub's + // unauthenticated pull limits. + busybox = "mirror.gcr.io/library/busybox:1.37" + + // labelTest marks every resource this test creates, with a value unique + // to the test, so its cleanup removes those and nothing else. If the test + // dies without running its cleanups (a -timeout panic, a SIGKILL), remove + // what it leaked with + // + // docker rm -f $(docker ps -aq --filter label=containerruntime.crossplane.io/integration-test) + // docker network rm $(docker network ls -q --filter label=containerruntime.crossplane.io/integration-test) + labelTest = "containerruntime.crossplane.io/integration-test" +) + +// TestRunReclaimsADeadRunOnDocker pins, against a real Docker daemon, what +// Acquire does to the resources of a run that was SIGKILLed: the containers it +// would have removed are removed, a Stop container is stopped and kept, an +// Orphan container is kept running, a network holding only a stopped +// container is removed, the network of the kept Orphan container is kept, and +// the dead run's lease is dropped. While the run lives, its lease keeps all of +// it. The resources carry the child's session ID but are created here: the +// reaper finds them by label, whichever process created them. +// +// It fails, rather than skips, without a Docker daemon: the integration job +// must have one, and a skip would hide a broken runner. So it fails on a dev +// machine without Docker too. +func TestRunReclaimsADeadRunOnDocker(t *testing.T) { + ctx := t.Context() + + cli, err := client.New(client.FromEnv) + if err != nil { + t.Fatalf("client.New(...): %v", err) + } + t.Cleanup(func() { _ = cli.Close() }) + + if _, err := cli.Ping(ctx, client.PingOptions{}); err != nil { + t.Fatalf("this test needs a Docker daemon: Ping(...): %v", err) + } + + pullIfMissing(t, cli, busybox) + + testID := cr.NewSessionID() + t.Cleanup(func() { removeTestResources(t, cli, testID) }) + + dir, id := t.TempDir(), cr.NewSessionID() + child := startLeaseHelper(t, dir, id) + + render := func(c cr.Cleanup) map[string]string { return cr.ContainerLabels(cr.DomainRender, id, c) } + + net := createNetwork(t, cli, testID, cr.ManagedLabels(cr.DomainRender, id)) + orphanNet := createNetwork(t, cli, testID, cr.ManagedLabels(cr.DomainRender, id)) + + remove := runContainer(t, cli, testID, busybox, render(cr.CleanupRemove), net) + renderer := runContainer(t, cli, testID, busybox, cr.ManagedLabels(cr.DomainRender, id), net) + stop := runContainer(t, cli, testID, busybox, render(cr.CleanupStop), net) + simulate := runContainer(t, cli, testID, busybox, cr.ContainerLabels(cr.DomainSimulate, id, cr.CleanupRemove), "") + orphan := runContainer(t, cli, testID, busybox, render(cr.CleanupOrphan), orphanNet) + + all := []string{remove, renderer, stop, simulate, orphan} + + // While the child lives, its lease keeps everything. + acquire(t, dir) + + for _, c := range all { + if !containerRunning(t, cli, c) { + t.Errorf("container %s of a live run: not running after another run's Acquire, want it untouched", c) + } + } + + for _, n := range []string{net, orphanNet} { + if !networkExists(t, cli, n) { + t.Errorf("network %s of a live run: removed by another run's Acquire, want it untouched", n) + } + } + + if err := child.Process.Kill(); err != nil { + t.Fatalf("Kill(): %v", err) + } + _ = child.Wait() + + acquire(t, dir) + + for _, c := range []string{remove, renderer, simulate} { + if containerExists(t, cli, c) { + t.Errorf("container %s of the dead run: still exists after Acquire, want it removed", c) + } + } + + switch { + case !containerExists(t, cli, stop): + t.Errorf("Stop container %s of the dead run: removed by Acquire, want it stopped and kept", stop) + case containerRunning(t, cli, stop): + t.Errorf("Stop container %s of the dead run: still running after Acquire, want it stopped", stop) + } + + if !containerRunning(t, cli, orphan) { + t.Errorf("Orphan container %s of the dead run: not running after Acquire, want it untouched", orphan) + } + + if networkExists(t, cli, net) { + t.Errorf("network %s of the dead run, holding only a stopped container: still exists after Acquire, want it removed", net) + } + + if !networkExists(t, cli, orphanNet) { + t.Errorf("network %s of the dead run's Orphan container: removed by Acquire, want it kept", orphanNet) + } + + if _, err := os.Stat(cr.LeasePath(dir, id)); !os.IsNotExist(err) { + t.Errorf("lease file of the dead run: Stat(...) = %v, want it removed", err) + } +} + +// acquire runs a new run's Acquire on the lease directory, then releases it. +func acquire(t *testing.T, dir string) { + t.Helper() + + r := cr.NewRun(dir, cr.NewSessionID(), true, logging.NewNopLogger()) + if err := r.Acquire(t.Context()); err != nil { + t.Errorf("Acquire(...): unexpected error: %v", err) + } + + if err := r.Release(context.Background(), true, ""); err != nil { + t.Errorf("Release(...): unexpected error: %v", err) + } +} + +// pullIfMissing pulls image ref unless it is on the daemon, so local runs +// stay offline. +func pullIfMissing(t *testing.T, cli *client.Client, ref string) { + t.Helper() + + _, err := cli.ImageInspect(t.Context(), ref) + if !errdefs.IsNotFound(err) { + if err != nil { + t.Fatalf("ImageInspect(%q): %v", ref, err) + } + return + } + + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Minute) + defer cancel() + + if err := cr.PullImage(ctx, cli, ref, client.ImagePullOptions{}); err != nil { + t.Fatalf("PullImage(%q): %v", ref, err) + } +} + +// createNetwork creates a bridge network with labels, marked as the test's. +func createNetwork(t *testing.T, cli *client.Client, testID string, labels map[string]string) string { + t.Helper() + + name := "crossplane-cli-test-" + cr.NewSessionID() + + l := maps.Clone(labels) + l[labelTest] = testID + + if _, err := cli.NetworkCreate(t.Context(), name, client.NetworkCreateOptions{Driver: "bridge", Labels: l}); err != nil { + t.Fatalf("NetworkCreate(%q): %v", name, err) + } + + return name +} + +// runContainer starts a container of image with labels, marked as the test's, +// on network net when it is not empty, and returns its ID. +func runContainer(t *testing.T, cli *client.Client, testID, image string, labels map[string]string, net string) string { + t.Helper() + + l := maps.Clone(labels) + l[labelTest] = testID + + init := true + opts := client.ContainerCreateOptions{ + Config: &container.Config{Image: image, Cmd: []string{"sleep", "300"}, Labels: l}, + HostConfig: &container.HostConfig{Init: &init}, + } + if net != "" { + opts.NetworkingConfig = &network.NetworkingConfig{EndpointsConfig: map[string]*network.EndpointSettings{net: {}}} + } + + rsp, err := cli.ContainerCreate(t.Context(), opts) + if err != nil { + t.Fatalf("ContainerCreate(...): %v", err) + } + + if _, err := cli.ContainerStart(t.Context(), rsp.ID, client.ContainerStartOptions{}); err != nil { + t.Fatalf("ContainerStart(%s): %v", rsp.ID, err) + } + + return rsp.ID +} + +// containerExists reports whether container id exists, in any state. +func containerExists(t *testing.T, cli *client.Client, id string) bool { + t.Helper() + + _, err := cli.ContainerInspect(t.Context(), id, client.ContainerInspectOptions{}) + switch { + case err == nil: + return true + case errdefs.IsNotFound(err): + return false + } + + t.Fatalf("ContainerInspect(%s): %v", id, err) + + return false +} + +// containerRunning reports whether container id exists and is running. +func containerRunning(t *testing.T, cli *client.Client, id string) bool { + t.Helper() + + res, err := cli.ContainerInspect(t.Context(), id, client.ContainerInspectOptions{}) + switch { + case err == nil: + return res.Container.State != nil && res.Container.State.Running + case errdefs.IsNotFound(err): + return false + } + + t.Fatalf("ContainerInspect(%s): %v", id, err) + + return false +} + +// networkExists reports whether network name exists. +func networkExists(t *testing.T, cli *client.Client, name string) bool { + t.Helper() + + _, err := cli.NetworkInspect(t.Context(), name, client.NetworkInspectOptions{}) + switch { + case err == nil: + return true + case errdefs.IsNotFound(err): + return false + } + + t.Fatalf("NetworkInspect(%q): %v", name, err) + + return false +} + +// removeTestResources force removes every container, then every network, +// marked as test testID's. +func removeTestResources(t *testing.T, cli *client.Client, testID string) { + t.Helper() + + ctx := context.Background() + f := make(client.Filters).Add("label", labelTest+"="+testID) + + cs, err := cli.ContainerList(ctx, client.ContainerListOptions{All: true, Filters: f}) + if err != nil { + t.Errorf("cleanup: ContainerList(...): %v", err) + } + + for _, c := range cs.Items { + if _, err := cli.ContainerRemove(ctx, c.ID, client.ContainerRemoveOptions{Force: true}); err != nil && !errdefs.IsNotFound(err) { + t.Errorf("cleanup: ContainerRemove(%s): %v", c.ID, err) + } + } + + ns, err := cli.NetworkList(ctx, client.NetworkListOptions{Filters: f}) + if err != nil { + t.Errorf("cleanup: NetworkList(...): %v", err) + } + + for _, n := range ns.Items { + if _, err := cli.NetworkRemove(ctx, n.ID, client.NetworkRemoveOptions{}); err != nil && !errdefs.IsNotFound(err) { + t.Errorf("cleanup: NetworkRemove(%s): %v", n.Name, err) + } + } +} diff --git a/internal/containerruntime/run_test.go b/internal/containerruntime/run_test.go new file mode 100644 index 00000000..3df7f374 --- /dev/null +++ b/internal/containerruntime/run_test.go @@ -0,0 +1,976 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package containerruntime_test + +import ( + "context" + "maps" + "os" + "path/filepath" + "regexp" + "runtime" + "slices" + "strconv" + "strings" + "sync" + "testing" + "time" + + "github.com/containerd/errdefs" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" + "github.com/moby/moby/client" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + + cr "github.com/crossplane/cli/v2/internal/containerruntime" + "github.com/crossplane/cli/v2/internal/containerruntime/fake" +) + +const ( + selfID = "00000000000000ff" + deadID = "dead00000000000a" + dead2ID = "dead00000000000b" + orphanID = "dead00000000000c" + futureID = "dead00000000000d" + liveID = "1111111111111111" +) + +const ( + running = container.StateRunning + exited = container.StateExited +) + +// host is an in-memory Docker daemon behind a fake.Client, holding containers, +// found by ID or name, and networks. It behaves as the daemon +// does wherever the difference would change what is left: create refuses a +// name in use; listing applies label filters (a resource matches only if it +// carries every one) and, without All, lists only running containers; a +// running container is only removed with Force; stopping a container leaves +// it exited; and a network is not removed while a running container is +// attached to it. +type host struct { + containers []container.Summary + networks []network.Summary + + // raced, when set, is created under its name by another run just before + // this run's first create, so that create finds the name taken. + raced *container.Summary + // cannotRemove names the container the daemon fails to remove. + cannotRemove string + // removing names the container another client is removing, so a removal + // conflicts. + removing string + // starts counts each container's starts, which inspect reports as when + // it last started. + starts map[string]int + // vanished holds IDs that are listed but gone by the time they are + // stopped or removed, as when a concurrent reaper got there first. + vanished map[string]bool +} + +// client is a Docker client of the host. +func (h *host) client() *fake.Client { + return &fake.Client{ + MockContainerInspect: h.containerInspect, + MockContainerCreate: h.containerCreate, + MockContainerStart: h.containerStart, + MockContainerList: h.containerList, + MockContainerStop: h.containerStop, + MockContainerRemove: h.containerRemove, + MockNetworkList: h.networkList, + MockNetworkRemove: h.networkRemove, + } +} + +// newClient is a RunClient factory for the host. +func (h *host) newClient() (cr.RunClient, error) { + return h.client(), nil +} + +// left is what is left on the host: each container's state, and the +// networks. +func (h *host) left() (map[string]container.ContainerState, []string) { + cs := make(map[string]container.ContainerState, len(h.containers)) + for _, c := range h.containers { + cs[c.ID] = c.State + } + + ns := make([]string, 0, len(h.networks)) + for _, n := range h.networks { + ns = append(ns, n.Name) + } + + return cs, ns +} + +// named reports whether container c is named name. +func named(c container.Summary, name string) bool { + return slices.Contains(c.Names, "/"+name) +} + +// container returns the index of the container with ID or name id. +func (h *host) container(id string) (int, error) { + i := slices.IndexFunc(h.containers, func(c container.Summary) bool { return c.ID == id || named(c, id) }) + + switch { + case i < 0: + return i, errors.Wrapf(errdefs.ErrNotFound, "no such container: %s", id) + case h.vanished[id]: + h.containers = slices.Delete(h.containers, i, i+1) + return -1, errors.Wrapf(errdefs.ErrNotFound, "no such container: %s", id) + } + + return i, nil +} + +func (h *host) containerInspect(_ context.Context, id string, _ client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + i, err := h.container(id) + if err != nil { + return client.ContainerInspectResult{}, err + } + + c := h.containers[i] + + return client.ContainerInspectResult{Container: container.InspectResponse{ + ID: c.ID, + Name: c.Names[0], + State: &container.State{Running: c.State == running, StartedAt: strconv.Itoa(h.starts[c.ID])}, + Config: &container.Config{Labels: c.Labels}, + NetworkSettings: &container.NetworkSettings{Networks: c.NetworkSettings.Networks}, + }}, nil +} + +func (h *host) containerCreate(_ context.Context, o client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + if h.raced != nil { + h.containers = append(h.containers, *h.raced) + h.raced = nil + } + + if slices.ContainsFunc(h.containers, func(c container.Summary) bool { return named(c, o.Name) }) { + return client.ContainerCreateResult{}, errors.Wrapf(errdefs.ErrConflict, "container name %q is already in use", o.Name) + } + + var nets []string + if o.NetworkingConfig != nil { + nets = slices.Collect(maps.Keys(o.NetworkingConfig.EndpointsConfig)) + } + + c := ctr("new-"+o.Name, container.StateCreated, o.Config.Labels, nets...) + c.Names = []string{"/" + o.Name} + h.containers = append(h.containers, c) + + return client.ContainerCreateResult{ID: c.ID}, nil +} + +func (h *host) containerStart(_ context.Context, id string, _ client.ContainerStartOptions) (client.ContainerStartResult, error) { + i, err := h.container(id) + if err != nil { + return client.ContainerStartResult{}, err + } + + if h.containers[i].State != running { + if h.starts == nil { + h.starts = map[string]int{} + } + h.starts[h.containers[i].ID]++ + } + h.containers[i].State = running + + return client.ContainerStartResult{}, nil +} + +func (h *host) containerList(_ context.Context, opts client.ContainerListOptions) (client.ContainerListResult, error) { + var res client.ContainerListResult + + for _, c := range h.containers { + ok, err := matches(opts.Filters, c.Labels) + if err != nil { + return client.ContainerListResult{}, err + } + + if ok && (opts.All || c.State == running) { + res.Items = append(res.Items, c) + } + } + + return res, nil +} + +func (h *host) containerStop(_ context.Context, id string, _ client.ContainerStopOptions) (client.ContainerStopResult, error) { + i, err := h.container(id) + if err != nil { + return client.ContainerStopResult{}, err + } + + h.containers[i].State = exited + + return client.ContainerStopResult{}, nil +} + +func (h *host) containerRemove(_ context.Context, id string, opts client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + if id == h.cannotRemove { + return client.ContainerRemoveResult{}, errors.New("daemon says no") + } + if id == h.removing { + return client.ContainerRemoveResult{}, errors.Wrapf(errdefs.ErrConflict, "removal of container %s is already in progress", id) + } + + i, err := h.container(id) + if err != nil { + return client.ContainerRemoveResult{}, err + } + + if h.containers[i].State == running && !opts.Force { + return client.ContainerRemoveResult{}, errors.Wrapf(errdefs.ErrConflict, "container %s is running: stop it or force the removal", id) + } + + h.containers = slices.Delete(h.containers, i, i+1) + + return client.ContainerRemoveResult{}, nil +} + +func (h *host) networkList(_ context.Context, opts client.NetworkListOptions) (client.NetworkListResult, error) { + var res client.NetworkListResult + + for _, n := range h.networks { + ok, err := matches(opts.Filters, n.Labels) + if err != nil { + return client.NetworkListResult{}, err + } + + if ok { + res.Items = append(res.Items, n) + } + } + + return res, nil +} + +func (h *host) networkRemove(_ context.Context, id string, _ client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) { + i := slices.IndexFunc(h.networks, func(n network.Summary) bool { return n.ID == id || n.Name == id }) + if i < 0 { + return client.NetworkRemoveResult{}, errors.Wrapf(errdefs.ErrNotFound, "network %s not found", id) + } + + n := h.networks[i] + + switch { + case h.vanished[n.ID]: + h.networks = slices.Delete(h.networks, i, i+1) + return client.NetworkRemoveResult{}, errors.Wrapf(errdefs.ErrNotFound, "network %s not found", id) + case slices.ContainsFunc(h.containers, func(c container.Summary) bool { + _, attached := c.NetworkSettings.Networks[n.Name] + return attached && c.State == running + }): + return client.NetworkRemoveResult{}, errors.Wrapf(errdefs.ErrConflict, "error while removing network: network %s has active endpoints", n.Name) + } + + h.networks = slices.Delete(h.networks, i, i+1) + + return client.NetworkRemoveResult{}, nil +} + +// matches reports whether labels satisfy filters as the daemon's label filter +// does. It fails a list that uses any other filter term rather than ignore it. +func matches(filters client.Filters, labels map[string]string) (bool, error) { + for term, values := range filters { + if term != "label" { + return false, errors.Errorf("fake Docker daemon does not support the %q list filter", term) + } + + for v := range values { + k, want, hasValue := strings.Cut(v, "=") + if got, ok := labels[k]; !ok || (hasValue && got != want) { + return false, nil + } + } + } + + return true, nil +} + +// ctr is a container named id. +func ctr(id string, state container.ContainerState, labels map[string]string, networks ...string) container.Summary { + c := container.Summary{ID: id, Names: []string{"/" + id}, State: state, Labels: labels, NetworkSettings: &container.NetworkSettingsSummary{ + Networks: map[string]*network.EndpointSettings{}, + }} + for _, n := range networks { + c.NetworkSettings.Networks[n] = &network.EndpointSettings{} + } + + return c +} + +func net(name string, labels map[string]string) network.Summary { + return network.Summary{Network: network.Network{ID: name, Name: name, Labels: labels}} +} + +// hostContainers is a Docker host shared by three dead runs, a live run, this +// run, and things no run owns. Each run's render-domain containers are +// attached to its network. +func hostContainers() []container.Summary { + return []container.Summary{ + // The dead run: a stopped Remove function, the render container that + // was running when it died (no cleanup label), a running Stop + // function, and a running Remove diff server (simulate domain). + ctr("dead-fn", exited, cr.ContainerLabels(cr.DomainRender, deadID, cr.CleanupRemove), "dead-net"), + ctr("dead-render", running, cr.ManagedLabels(cr.DomainRender, deadID), "dead-net"), + ctr("dead-stop", running, cr.ContainerLabels(cr.DomainRender, deadID, cr.CleanupStop), "dead-net"), + ctr("dead-server", running, cr.ContainerLabels(cr.DomainSimulate, deadID, cr.CleanupRemove)), + + // A second dead run. + ctr("dead2-fn", exited, cr.ContainerLabels(cr.DomainRender, dead2ID, cr.CleanupRemove), "dead2-net"), + + // A dead run whose user asked to keep a function and a diff server. + ctr("orphan-fn", running, cr.ContainerLabels(cr.DomainRender, orphanID, cr.CleanupOrphan), "orphan-net"), + ctr("orphan-server", running, cr.ContainerLabels(cr.DomainSimulate, orphanID, cr.CleanupOrphan)), + + // A live run, and this run. + ctr("live-fn", running, cr.ContainerLabels(cr.DomainRender, liveID, cr.CleanupRemove), "live-net"), + ctr("self-fn", running, cr.ContainerLabels(cr.DomainRender, selfID, cr.CleanupRemove), "self-net"), + + // Not ours to judge: no labels, the dead run's session without + // crossplane's management, its labels split across our two domains, + // and its labels under a domain that isn't ours. + ctr("unlabelled", running, nil), + ctr("session-only", running, map[string]string{cr.DomainRender + "/session": deadID}), + ctr("mixed-domains", running, map[string]string{ + cr.DomainRender + "/managed-by": cr.LabelValueManagedBy, + cr.DomainSimulate + "/session": deadID, + }), + ctr("other-domain", running, cr.ManagedLabels("example.org", deadID)), + } +} + +func hostNetworks() []network.Summary { + return []network.Summary{ + net("dead-net", cr.ManagedLabels(cr.DomainRender, deadID)), + net("dead2-net", cr.ManagedLabels(cr.DomainRender, dead2ID)), + net("orphan-net", cr.ManagedLabels(cr.DomainRender, orphanID)), + net("live-net", cr.ManagedLabels(cr.DomainRender, liveID)), + net("self-net", cr.ManagedLabels(cr.DomainRender, selfID)), + + // Not ours to judge: one the user supplied with + // CROSSPLANE_DOCKER_NETWORK (never labelled), and one with the dead + // run's session but not crossplane's management. + net("user-net", nil), + net("session-only-net", map[string]string{cr.DomainRender + "/session": deadID}), + } +} + +// untouched is what reaping dead runs never changes. +var ( + untouchedContainers = map[string]container.ContainerState{ + "orphan-fn": running, "orphan-server": running, + "live-fn": running, "self-fn": running, + "unlabelled": running, "session-only": running, "mixed-domains": running, "other-domain": running, + } + untouchedNetworks = []string{"live-net", "self-net", "user-net", "session-only-net"} + + // deadLeft and dead2Left are what each dead run leaves while unreaped. + deadLeft = map[string]container.ContainerState{ + "dead-fn": exited, "dead-render": running, "dead-stop": running, "dead-server": running, + } + dead2Left = map[string]container.ContainerState{"dead2-fn": exited} + + // deadReaped is what the dead run leaves once reaped: its Stop function, + // stopped. + deadReaped = map[string]container.ContainerState{"dead-stop": exited} +) + +func union[K comparable, V any](ms ...map[K]V) map[K]V { + out := map[K]V{} + for _, m := range ms { + maps.Copy(out, m) + } + + return out +} + +// startRun acquires a run the way a live process does, holding its lease +// until the test ends. +func startRun(t *testing.T, dir, id string, h *host) *cr.Run { + t.Helper() + + r := cr.NewRun(dir, id, true, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + _ = r.Acquire(t.Context()) + + t.Cleanup(func() { _ = r.Release(context.Background(), false, "") }) + + return r +} + +// leaveLease leaves the lease file of a run that died: it exists, but no +// process holds its lock. +func leaveLease(t *testing.T, dir, id string) { + t.Helper() + + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(dir, id+".lock"), nil, 0o600); err != nil { + t.Fatal(err) + } +} + +func dirEntries(t *testing.T, dir string) []string { + t.Helper() + + entries, err := os.ReadDir(dir) + if err != nil && !os.IsNotExist(err) { + t.Fatalf("ReadDir(%q): %v", dir, err) + } + + names := make([]string, 0, len(entries)) + for _, e := range entries { + names = append(names, e.Name()) + } + + return names +} + +var unordered = []cmp.Option{ + cmpopts.SortSlices(func(a, b string) bool { return a < b }), + cmpopts.EquateEmpty(), +} + +func skipWithoutLeases(t *testing.T) { + t.Helper() + + if runtime.GOOS == "windows" { + t.Skip("run leases are not supported on Windows") + } +} + +func TestNewSessionID(t *testing.T) { + format := regexp.MustCompile(`^[0-9a-f]{16}$`) + + a, b := cr.NewSessionID(), cr.NewSessionID() + for _, id := range []string{a, b} { + if !format.MatchString(id) { + t.Errorf("NewSessionID() = %q, want 16 lowercase hex characters: reapers skip any other lease name", id) + } + } + + if a == b { + t.Errorf("NewSessionID() returned %q twice", a) + } +} + +func TestRunAcquire(t *testing.T) { + skipWithoutLeases(t) + + type want struct { + Containers map[string]container.ContainerState + Networks []string + Files []string + Err bool + } + + tests := map[string]struct { + reason string + + dead []string // runs that died, leaving their lease behind + live []string // runs that are still running + otherFiles []string // files in the lease directory that are not leases + host host // resources to add to hostContainers and hostNetworks, and failures + + want want + }{ + "NoDeadRunRemovesNothing": { + reason: "With no dead run to reclaim, acquiring a run changes nothing.", + live: []string{liveID}, + want: want{ + Containers: union(untouchedContainers, deadLeft, dead2Left), + Networks: append(slices.Clone(untouchedNetworks), "dead-net", "dead2-net", "orphan-net"), + Files: []string{liveID + ".lock", selfID + ".lock"}, + }, + }, + "ReapsOnlyTheDeadRunsResourcesPerPolicy": { + reason: "Every dead run's Remove and unlabelled containers, in both domains, are removed, its Stop one " + + "is stopped, then its network and lease go. A live run's, this run's, and anything without both " + + "of one domain's labels for a dead run are left alone.", + dead: []string{deadID, dead2ID}, + live: []string{liveID}, + want: want{ + Containers: union(untouchedContainers, deadReaped), + Networks: append(slices.Clone(untouchedNetworks), "orphan-net"), + Files: []string{liveID + ".lock", selfID + ".lock"}, + }, + }, + "OrphanIsKeptWithItsNetwork": { + reason: "A container the user asked to keep is left running, and its network with it. That counts as " + + "reclaimed, so the lease is dropped.", + dead: []string{orphanID}, + want: want{ + Containers: union(untouchedContainers, deadLeft, dead2Left), + Networks: append(slices.Clone(untouchedNetworks), "dead-net", "dead2-net", "orphan-net"), + Files: []string{selfID + ".lock"}, + }, + }, + "StoppedOrphanKeepsItsNetwork": { + reason: "Docker would remove the network of a stopped Orphan container, which then could not start " + + "again, so the network is left with it, and the lease is dropped.", + dead: []string{futureID}, + host: host{ + containers: []container.Summary{ + ctr("stopped-orphan", exited, cr.ContainerLabels(cr.DomainRender, futureID, cr.CleanupOrphan), "future-net"), + }, + networks: []network.Summary{net("future-net", cr.ManagedLabels(cr.DomainRender, futureID))}, + }, + want: want{ + Containers: union(untouchedContainers, deadLeft, dead2Left, map[string]container.ContainerState{"stopped-orphan": exited}), + Networks: append(slices.Clone(untouchedNetworks), "dead-net", "dead2-net", "orphan-net", "future-net"), + Files: []string{selfID + ".lock"}, + }, + }, + "UnknownCleanupPolicyIsKept": { + reason: "A container whose cleanup label holds a value this version does not know may have been asked " + + "to be kept, so it is left alone like Orphan, with its network, and the lease is dropped.", + dead: []string{futureID}, + host: host{ + containers: []container.Summary{ + ctr("future-fn", running, cr.ContainerLabels(cr.DomainRender, futureID, "orphan"), "future-net"), + }, + networks: []network.Summary{net("future-net", cr.ManagedLabels(cr.DomainRender, futureID))}, + }, + want: want{ + Containers: union(untouchedContainers, deadLeft, dead2Left, map[string]container.ContainerState{"future-fn": running}), + Networks: append(slices.Clone(untouchedNetworks), "dead-net", "dead2-net", "orphan-net", "future-net"), + Files: []string{selfID + ".lock"}, + }, + }, + "ContainerRemovalFailureKeepsNetworkAndLease": { + reason: "A container that could not be removed may still hold its network, and the next run should " + + "retry, so the network and lease are kept and Acquire reports it.", + dead: []string{deadID}, + host: host{cannotRemove: "dead-render"}, + want: want{ + Containers: union(untouchedContainers, deadReaped, map[string]container.ContainerState{"dead-render": running}, dead2Left), + Networks: append(slices.Clone(untouchedNetworks), "dead-net", "dead2-net", "orphan-net"), + Files: []string{deadID + ".lock", selfID + ".lock"}, + Err: true, + }, + }, + "ResourcesAlreadyGoneCountAsReaped": { + reason: "A container or network that no longer exists by the time it is removed, e.g. removed by a " + + "concurrent reaper, is already reclaimed.", + dead: []string{deadID}, + host: host{vanished: map[string]bool{"dead-fn": true, "dead-stop": true, "dead-net": true}}, + want: want{ + Containers: union(untouchedContainers, dead2Left), + Networks: append(slices.Clone(untouchedNetworks), "dead2-net", "orphan-net"), + Files: []string{selfID + ".lock"}, + }, + }, + "IgnoresFilesThatAreNotLeases": { + reason: "Only <16 lowercase hex>.lock files are leases; anything else in the directory is never acted on.", + otherFiles: []string{ + "notes.txt", + "not-a-run-id.lock", + strings.ToUpper(deadID) + ".lock", + deadID + "-123.tmp", + }, + want: want{ + Containers: union(untouchedContainers, deadLeft, dead2Left), + Networks: append(slices.Clone(untouchedNetworks), "dead-net", "dead2-net", "orphan-net"), + Files: []string{ + deadID + "-123.tmp", strings.ToUpper(deadID) + ".lock", "not-a-run-id.lock", "notes.txt", + selfID + ".lock", + }, + }, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + + h := tt.host + h.containers = append(hostContainers(), tt.host.containers...) + h.networks = append(hostNetworks(), tt.host.networks...) + + // Live runs started before any run died, so had nothing to reclaim. + for _, id := range tt.live { + startRun(t, dir, id, &h) + } + + for _, id := range tt.dead { + leaveLease(t, dir, id) + } + + for _, f := range tt.otherFiles { + if err := os.WriteFile(filepath.Join(dir, f), nil, 0o600); err != nil { + t.Fatal(err) + } + } + + r := cr.NewRun(dir, selfID, true, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + err := r.Acquire(t.Context()) + t.Cleanup(func() { _ = r.Release(context.Background(), false, "") }) + + got := want{Files: dirEntries(t, dir), Err: err != nil} + got.Containers, got.Networks = h.left() + + if diff := cmp.Diff(tt.want, got, unordered...); diff != "" { + t.Errorf("\n%s\nAcquire(...): -want, +got:\n%s", tt.reason, diff) + } + }) + } +} + +// TestRunRemovesStaleTempLeases checks that a reaper removes the temporary +// lease file a run that died while taking its lease left, once it is old +// enough that no live run can still be renaming it, and nothing else. +func TestRunRemovesStaleTempLeases(t *testing.T) { + skipWithoutLeases(t) + + dir := t.TempDir() + stale := time.Now().Add(-time.Hour) + + files := map[string]bool{ // name -> stale + deadID + "-123.tmp": true, + dead2ID + "-456.tmp": false, + "notes-123.tmp": true, + } + for f, old := range files { + p := filepath.Join(dir, f) + if err := os.WriteFile(p, nil, 0o600); err != nil { + t.Fatal(err) + } + if old { + if err := os.Chtimes(p, stale, stale); err != nil { + t.Fatal(err) + } + } + } + + h := &host{} + startRun(t, dir, selfID, h) + + want := []string{dead2ID + "-456.tmp", "notes-123.tmp", selfID + ".lock"} + if diff := cmp.Diff(want, dirEntries(t, dir), unordered...); diff != "" { + t.Errorf("Acquire(...): lease directory, -want, +got:\n%s", diff) + } +} + +// TestRunConcurrentReapersOfOneDeadRun checks that two runs reaping one dead +// run at once both succeed, leaving the dead run reclaimed once, by one of +// them, and its lease gone. A flock conflicts per open file, so two goroutines contend for +// a lease as two processes do. Repeated, since the interleaving varies. +func TestRunConcurrentReapersOfOneDeadRun(t *testing.T) { + skipWithoutLeases(t) + + for range 50 { + dir := t.TempDir() + leaveLease(t, dir, deadID) + + h := &host{ + containers: []container.Summary{ + ctr("dead-fn", running, cr.ContainerLabels(cr.DomainRender, deadID, cr.CleanupRemove), "dead-net"), + ctr("dead-stop", running, cr.ContainerLabels(cr.DomainRender, deadID, cr.CleanupStop), "dead-net"), + ctr("dead-server", running, cr.ContainerLabels(cr.DomainSimulate, deadID, cr.CleanupRemove)), + }, + networks: []network.Summary{net("dead-net", cr.ManagedLabels(cr.DomainRender, deadID))}, + } + var mu sync.Mutex + finished := map[string]int{} + newClient := func() (cr.RunClient, error) { return serialized(&mu, h.client(), finished), nil } + + ids := []string{cr.NewSessionID(), cr.NewSessionID()} + errs := make([]error, len(ids)) + var wg sync.WaitGroup + for i, id := range ids { + r := cr.NewRun(dir, id, true, logging.NewNopLogger(), cr.WithRunClient(newClient)) + t.Cleanup(func() { _ = r.Release(context.Background(), false, "") }) + wg.Go(func() { errs[i] = r.Acquire(t.Context()) }) + } + wg.Wait() + + type state struct { + Errs []error + Containers map[string]container.ContainerState + Networks []string + Files []string + Finished map[string]int + } + got := state{Errs: errs, Files: dirEntries(t, dir), Finished: finished} + got.Containers, got.Networks = h.left() + want := state{ + Errs: []error{nil, nil}, + Containers: map[string]container.ContainerState{"dead-stop": exited}, + Files: []string{ids[0] + ".lock", ids[1] + ".lock"}, + // Only one reaper takes the dead run's lease and reclaims it. + Finished: map[string]int{"dead-fn": 1, "dead-stop": 1, "dead-server": 1, "dead-net": 1}, + } + if diff := cmp.Diff(want, got, append(unordered, cmpopts.EquateErrors())...); diff != "" { + t.Fatalf("two concurrent Acquire(...): -want, +got:\n%s", diff) + } + } +} + +// serialized makes a fake client's calls run one at a time under mu, for a +// host shared by concurrent runs, and counts in finished each container or +// network a stop or remove succeeded on. +func serialized(mu *sync.Mutex, c *fake.Client, finished map[string]int) *fake.Client { + count := func(id string, err error) error { + if err == nil { + finished[id]++ + } + return err + } + + return &fake.Client{ + MockContainerList: func(ctx context.Context, o client.ContainerListOptions) (client.ContainerListResult, error) { + mu.Lock() + defer mu.Unlock() + return c.MockContainerList(ctx, o) + }, + MockContainerStop: func(ctx context.Context, id string, o client.ContainerStopOptions) (client.ContainerStopResult, error) { + mu.Lock() + defer mu.Unlock() + res, err := c.MockContainerStop(ctx, id, o) + return res, count(id, err) + }, + MockContainerRemove: func(ctx context.Context, id string, o client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + mu.Lock() + defer mu.Unlock() + res, err := c.MockContainerRemove(ctx, id, o) + return res, count(id, err) + }, + MockNetworkList: func(ctx context.Context, o client.NetworkListOptions) (client.NetworkListResult, error) { + mu.Lock() + defer mu.Unlock() + return c.MockNetworkList(ctx, o) + }, + MockNetworkRemove: func(ctx context.Context, id string, o client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) { + mu.Lock() + defer mu.Unlock() + res, err := c.MockNetworkRemove(ctx, id, o) + return res, count(id, err) + }, + } +} + +func TestRunWithoutLease(t *testing.T) { + skipWithoutLeases(t) + + t.Run("SuppliedIDIsNeverLeased", func(t *testing.T) { + dir := t.TempDir() + h := &host{containers: hostContainers(), networks: hostNetworks()} + leaveLease(t, dir, deadID) + + // A supplied ID that happens to be valid hex is still not leased. + r := cr.NewRun(dir, selfID, false, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + if err := r.Acquire(t.Context()); err != nil { + t.Fatalf("Acquire(...): unexpected error: %v", err) + } + + if got := dirEntries(t, dir); len(got) != 0 { + t.Errorf("Acquire(...) of a supplied ID: lease directory holds %v, want nothing (no lease of its own, dead lease reaped)", got) + } + + if err := r.Release(t.Context(), true, ""); err != nil { + t.Errorf("Release(...): unexpected error: %v", err) + } + }) + + t.Run("NoLeaseDirDisablesLeaseAndReap", func(t *testing.T) { + // A dead run's lease in the working directory, which an empty lease + // directory would resolve to, is left alone, along with its resources. + dir := t.TempDir() + t.Chdir(dir) + leaveLease(t, dir, deadID) + h := &host{containers: hostContainers(), networks: hostNetworks()} + + r := cr.NewRun("", cr.NewSessionID(), true, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + if err := r.Acquire(t.Context()); err != nil { + t.Fatalf("Acquire(...): unexpected error: %v", err) + } + + if err := r.Release(t.Context(), true, ""); err != nil { + t.Errorf("Release(...): unexpected error: %v", err) + } + + if got := dirEntries(t, dir); !slices.Equal(got, []string{deadID + ".lock"}) { + t.Errorf("Acquire/Release(...) without a lease dir: working directory holds %v, want only the dead lease", got) + } + + if got, _ := h.left(); got["dead-fn"] != exited { + t.Errorf("Acquire/Release(...) without a lease dir: reaped a dead run's container, left %v", got) + } + }) +} + +func TestRunRelease(t *testing.T) { + skipWithoutLeases(t) + + type want struct { + Networks []string + Files []string + Err bool + } + + tests := map[string]struct { + reason string + clean bool + sweep string + host host + want want + }{ + "CleanDropsLease": { + reason: "A run that removed everything drops its lease file, and leaves networks alone without a sweep.", + clean: true, + want: want{ + Networks: []string{"live-net", "self-net", "self-net-2", "self-sim-net"}, + }, + }, + "NotCleanKeepsLease": { + reason: "A run that left something behind keeps its lease file, unlocked, so the next run reaps it.", + want: want{ + Networks: []string{"live-net", "self-net", "self-net-2", "self-sim-net"}, + Files: []string{selfID + ".lock"}, + }, + }, + "SweepRemovesOnlyThisRunsNetworksInItsDomain": { + reason: "A sweep removes this run's networks in the domain it is given, and no other run's or domain's.", + clean: true, + sweep: cr.DomainRender, + want: want{ + Networks: []string{"live-net", "self-sim-net"}, + }, + }, + "SweepExcusesNetworkOfKeptContainer": { + reason: "A network that this run's Orphan or unknown-policy container is attached to, running or stopped, is left, and does not keep the lease.", + clean: true, + sweep: cr.DomainRender, + host: host{containers: []container.Summary{ + ctr("orphan", exited, cr.ContainerLabels(cr.DomainRender, selfID, cr.CleanupOrphan), "self-net"), + ctr("future", running, cr.ContainerLabels(cr.DomainRender, selfID, cr.Cleanup("Future")), "self-net-2"), + }}, + want: want{ + Networks: []string{"live-net", "self-net", "self-net-2", "self-sim-net"}, + }, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + + h := tt.host + h.networks = []network.Summary{ + net("self-net", cr.ManagedLabels(cr.DomainRender, selfID)), + net("self-net-2", cr.ManagedLabels(cr.DomainRender, selfID)), + net("self-sim-net", cr.ManagedLabels(cr.DomainSimulate, selfID)), + net("live-net", cr.ManagedLabels(cr.DomainRender, liveID)), + } + + r := cr.NewRun(dir, selfID, true, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + if err := r.Acquire(t.Context()); err != nil { + t.Fatalf("Acquire(...): unexpected error: %v", err) + } + + err := r.Release(t.Context(), tt.clean, tt.sweep) + + _, nets := h.left() + got := want{Networks: nets, Files: dirEntries(t, dir), Err: err != nil} + + if diff := cmp.Diff(tt.want, got, unordered...); diff != "" { + t.Errorf("\n%s\nRelease(...): -want, +got:\n%s", tt.reason, diff) + } + }) + } +} + +// TestRunReclaimsARunThatDied pins reaping end to end, with generated IDs and +// the leases runs really take: a later run leaves a run's resources alone +// while that run is alive, and removes them once its lock is gone. +func TestRunReclaimsARunThatDied(t *testing.T) { + skipWithoutLeases(t) + + type left struct { + Containers map[string]container.ContainerState + Networks []string + } + + dir := t.TempDir() + h := &host{} + + first := cr.NewRun(dir, cr.NewSessionID(), true, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + if err := first.Acquire(t.Context()); err != nil { + t.Fatalf("first Acquire(...): unexpected error: %v", err) + } + + h.containers = []container.Summary{ctr("fn", running, cr.ContainerLabels(cr.DomainRender, first.ID(), cr.CleanupRemove), "net")} + h.networks = []network.Summary{net("net", cr.ManagedLabels(cr.DomainRender, first.ID()))} + + startRun(t, dir, cr.NewSessionID(), h) + + got := map[string]left{} + got["WhileAlive"] = func() left { c, n := h.left(); return left{c, n} }() + + // The first run dies: its lock is dropped and its lease file stays. + _ = first.Release(t.Context(), false, "") + + startRun(t, dir, cr.NewSessionID(), h) + + got["AfterItDied"] = func() left { c, n := h.left(); return left{c, n} }() + + want := map[string]left{ + "WhileAlive": {Containers: map[string]container.ContainerState{"fn": running}, Networks: []string{"net"}}, + "AfterItDied": {}, + } + if diff := cmp.Diff(want, got, unordered...); diff != "" { + t.Errorf("what later runs left of a run, -want, +got:\n%s", diff) + } +} + +func TestRunReclaimsInAGlobLikeDir(t *testing.T) { + skipWithoutLeases(t) + + type left struct { + Containers map[string]container.ContainerState + Networks []string + } + + // Glob syntax in the lease dir must not hide its leases. + dir := filepath.Join(t.TempDir(), "[x]", "runs") + h := &host{} + + dead := cr.NewRun(dir, cr.NewSessionID(), true, logging.NewNopLogger(), cr.WithRunClient(h.newClient)) + if err := dead.Acquire(t.Context()); err != nil { + t.Fatalf("Acquire(...): unexpected error: %v", err) + } + + h.containers = []container.Summary{ctr("fn", running, cr.ContainerLabels(cr.DomainRender, dead.ID(), cr.CleanupRemove), "net")} + h.networks = []network.Summary{net("net", cr.ManagedLabels(cr.DomainRender, dead.ID()))} + + _ = dead.Release(t.Context(), false, "") + + startRun(t, dir, cr.NewSessionID(), h) + + c, n := h.left() + if diff := cmp.Diff(left{}, left{c, n}, unordered...); diff != "" { + t.Errorf("what a later run left of a dead run, -want, +got:\n%s", diff) + } +} diff --git a/internal/diffserver/manager.go b/internal/diffserver/manager.go index 418e4462..6e143c36 100644 --- a/internal/diffserver/manager.go +++ b/internal/diffserver/manager.go @@ -101,7 +101,7 @@ const ( // labelDomain prefixes the labels simulate applies to the Docker containers // it creates. -const labelDomain = "simulate.crossplane.io" +const labelDomain = containerruntime.DomainSimulate // Labels applied to the Docker containers simulate creates, mirroring // render's, so leftovers (a crash, or an Orphan cleanup policy) can be diff --git a/internal/docker/docker.go b/internal/docker/docker.go index 9ca70b70..c92df184 100644 --- a/internal/docker/docker.go +++ b/internal/docker/docker.go @@ -336,6 +336,15 @@ type runContainerConfig struct { hostConfig *container.HostConfig networkConfig *network.NetworkingConfig stdin []byte + cli client.APIClient +} + +// RunWithClient makes RunContainer talk to Docker through cli, which it never +// closes, instead of a client built from the environment. +func RunWithClient(cli client.APIClient) RunContainerOption { + return func(cfg *runContainerConfig) { + cfg.cli = cli + } } // RunWithCommand sets the command to run in the container. @@ -433,9 +442,13 @@ func RunContainer(ctx context.Context, img string, opts ...RunContainerOption) ( opt(cfg) } - cli, err := NewClient() - if err != nil { - return nil, nil, err + cli := cfg.cli + if cli == nil { + c, err := NewClient() + if err != nil { + return nil, nil, err + } + cli = c } // Pull the image if it's not already present. diff --git a/pkg/render/engine.go b/pkg/render/engine.go index cbec0994..0e4da451 100644 --- a/pkg/render/engine.go +++ b/pkg/render/engine.go @@ -99,11 +99,11 @@ type Engine interface { // EngineFlags contains flags for configuring the render engine. It is embedded // by render command structs to provide shared engine configuration. type EngineFlags struct { - CrossplaneVersion string `help:"Version of the Crossplane image to use for rendering. Defaults to the latest stable version." placeholder:"VERSION" xor:"crossplane-selector"` - CrossplaneImage string `help:"Override the full Crossplane Docker image reference for rendering." placeholder:"IMAGE" xor:"crossplane-selector"` - CrossplaneBinary string `help:"Path to a local crossplane binary to use instead of Docker. Ignores --crossplane-docker-network." placeholder:"PATH" type:"existingfile" xor:"crossplane-selector"` - CrossplaneDockerNetwork string `env:"CROSSPLANE_DOCKER_NETWORK" help:"The docker network to start the crossplane container in (also $$${env}). Ignored with --crossplane-binary."` - SessionID string `help:"${session_id_help=Session ID to apply as the render.crossplane.io/session label on Docker containers and networks that render creates. Defaults to a random per-process ID.}" placeholder:"ID"` + CrossplaneVersion string `help:"Version of the Crossplane image to use for rendering. Defaults to the latest stable version." placeholder:"VERSION" xor:"crossplane-selector"` + CrossplaneImage string `help:"Override the full Crossplane Docker image reference for rendering." placeholder:"IMAGE" xor:"crossplane-selector"` + CrossplaneBinary string `help:"Path to a local crossplane binary to use instead of Docker. Ignores --crossplane-docker-network." placeholder:"PATH" type:"existingfile" xor:"crossplane-selector"` + CrossplaneDockerNetwork string `env:"CROSSPLANE_DOCKER_NETWORK" help:"The docker network to start the crossplane container in (also $$${env}). Ignored with --crossplane-binary."` + SessionID string `help:"${session_id_help=Session ID to apply as the render.crossplane.io/session label on Docker containers and networks that render creates. Defaults to a random per-process ID. A supplied ID isn't leased, so its resources aren't reclaimed after a crash.}" placeholder:"ID"` } // AfterApply warns when --crossplane-docker-network is passed on the command diff --git a/pkg/render/engine_docker.go b/pkg/render/engine_docker.go index 02e8d8ba..27f40673 100644 --- a/pkg/render/engine_docker.go +++ b/pkg/render/engine_docker.go @@ -23,6 +23,7 @@ import ( "strings" "time" + "github.com/moby/moby/client" "google.golang.org/protobuf/proto" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" @@ -73,18 +74,25 @@ type dockerRenderEngine struct { runner containerRunner // networks creates and removes the temporary Docker network Setup owns. - // Production callers leave it nil and Setup builds a real client from the - // environment only when it needs to create a network. Tests substitute a - // fake to exercise the create-network branch without a real Docker - // daemon. + // When nil, Setup builds a client from the environment, only when it + // needs to create a network. A Session sets it from WithDockerClient. networks networkClient + + // dockerClient, when set, is the Docker client the render container is + // run through, and the daemon CheckContextSupport checks, instead of the + // environment's. A Session sets it, and networks, from WithDockerClient. + dockerClient client.APIClient } func (e *dockerRenderEngine) CheckContextSupport() error { if runtime.GOOS == "windows" { return errors.New("context handling via --context-values/--context-files/--include-context is not supported on Windows") } - if host := os.Getenv("DOCKER_HOST"); host != "" && !strings.HasPrefix(host, "unix://") { + host := os.Getenv("DOCKER_HOST") + if e.dockerClient != nil { + host = e.dockerClient.DaemonHost() + } + if host != "" && !strings.HasPrefix(host, "unix://") { return errors.New("context handling via --context-values/--context-files/--include-context requires a local Docker daemon or Crossplane controller binary") } @@ -156,7 +164,9 @@ func (e *dockerRenderEngine) Setup(ctx context.Context, fns []pkgv1.Function) (f // partial response AND a non-nil error containing stderr — letting callers // recover the partial output (e.g. RequiredResources) and iterate. func (e *dockerRenderEngine) Render(ctx context.Context, req *renderv1alpha1.RenderRequest) (*renderv1alpha1.RenderResponse, error) { - // Update any localhost function addresses if needed. + // Update any localhost function addresses if needed, on a copy: the + // caller's request may be rendering concurrently. + req = proto.CloneOf(req) if cinput := req.GetComposite(); cinput != nil { cinput.Functions = RewriteAddressesForDocker(cinput.GetFunctions()) } @@ -180,6 +190,9 @@ func (e *dockerRenderEngine) Render(ctx context.Context, req *renderv1alpha1.Ren if e.network != "" { opts = append(opts, docker.RunWithNetworkName(e.network)) } + if e.dockerClient != nil { + opts = append(opts, docker.RunWithClient(e.dockerClient)) + } e.log.Debug("Running crossplane internal render in Docker", "image", e.image, "network", e.network) diff --git a/pkg/render/engine_docker_test.go b/pkg/render/engine_docker_test.go index 571015c0..bb8e211b 100644 --- a/pkg/render/engine_docker_test.go +++ b/pkg/render/engine_docker_test.go @@ -248,6 +248,44 @@ func TestDockerRenderEngineRender(t *testing.T) { } } +// TestDockerRenderEngineRenderLeavesRequest checks that Render rewrites +// function addresses on its own copy, so concurrent Renders of one request +// don't race on it. +func TestDockerRenderEngineRenderLeavesRequest(t *testing.T) { + fns := func() []*renderv1alpha1.FunctionInput { + return []*renderv1alpha1.FunctionInput{{Name: "fn", Address: "localhost:9443"}} + } + reqs := map[string]func() *renderv1alpha1.RenderRequest{ + "Composite": func() *renderv1alpha1.RenderRequest { + return &renderv1alpha1.RenderRequest{Input: &renderv1alpha1.RenderRequest_Composite{Composite: &renderv1alpha1.CompositeInput{Functions: fns()}}} + }, + "Operation": func() *renderv1alpha1.RenderRequest { + return &renderv1alpha1.RenderRequest{Input: &renderv1alpha1.RenderRequest_Operation{Operation: &renderv1alpha1.OperationInput{Functions: fns()}}} + }, + } + + for name, newReq := range reqs { + t.Run(name, func(t *testing.T) { + e := &dockerRenderEngine{ + image: "test-image", + log: logging.NewNopLogger(), + runner: &mockContainerRunner{MockRun: func(context.Context, string, ...docker.RunContainerOption) ([]byte, []byte, error) { + return nil, nil, nil + }}, + } + + req := newReq() + if _, err := e.Render(t.Context(), req); err != nil { + t.Fatalf("Render(...): unexpected error: %v", err) + } + + if diff := cmp.Diff(newReq(), req, protocmp.Transform()); diff != "" { + t.Errorf("Render(...): want the caller's request unchanged, -want, +got:\n%s", diff) + } + }) + } +} + func TestDockerRenderEngineSetup(t *testing.T) { // These cases all exercise the early-return branch of Setup — where // e.network is already non-empty, either because NewEngineFromFlags diff --git a/pkg/render/render.go b/pkg/render/render.go index 9e9da0e7..48cb91aa 100644 --- a/pkg/render/render.go +++ b/pkg/render/render.go @@ -246,11 +246,11 @@ func OverrideFunctionAnnotations(fns []pkgv1.Function, annotations []string) err } // WarnCleanupFailure writes a short warning about err to w. It does nothing if -// err is nil. Callers use it to surface function runtime cleanup failures that +// err is nil. Callers use it to surface render session cleanup failures that // would otherwise only be visible with --verbose. func WarnCleanupFailure(w io.Writer, err error) { if err == nil { return } - _, _ = fmt.Fprintf(w, "warning: failed to clean up function runtimes; some resources (e.g. containers) may need to be removed manually: %v\n", err) + _, _ = fmt.Fprintf(w, "warning: failed to clean up render session; some resources (e.g. containers or networks) may need to be removed manually: %v\n", err) } diff --git a/pkg/render/runtime_docker.go b/pkg/render/runtime_docker.go index 17c79413..359a17c4 100644 --- a/pkg/render/runtime_docker.go +++ b/pkg/render/runtime_docker.go @@ -35,7 +35,7 @@ const FunctionPort = 9443 // labelDomain prefixes the labels render applies to the Docker resources it // creates. -const labelDomain = "render.crossplane.io" +const labelDomain = containerruntime.DomainRender // Annotations that can be used to configure the Docker runtime. const ( @@ -319,6 +319,7 @@ func (r *RuntimeDocker) Start(ctx context.Context) (RuntimeContext, error) { Port: FunctionPort, Env: r.Env, Labels: functionContainerLabels(r.Session, r.Cleanup), + Domain: labelDomain, BindAddress: r.BindAddress, Target: r.Target, Network: r.Network, diff --git a/pkg/render/runtime_docker_test.go b/pkg/render/runtime_docker_test.go index 4e3b62e3..06f17fe9 100644 --- a/pkg/render/runtime_docker_test.go +++ b/pkg/render/runtime_docker_test.go @@ -708,3 +708,45 @@ func TestRuntimeDockerStartLabels(t *testing.T) { }) } } + +func TestRuntimeDockerNamedContainer(t *testing.T) { + const ( + name = "shared-fn" + dockerNetwork = "render-net" + ) + + // A running Orphan container found by name is shared, and this run's + // Remove policy never stops or removes it: ContainerStop and + // ContainerRemove are nil, so calling either panics. + cli := &fake.Client{ + MockContainerInspect: func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + return client.ContainerInspectResult{Container: container.InspectResponse{ + ID: "old", + Name: "/" + name, + State: &container.State{Running: true}, + Config: &container.Config{Labels: functionContainerLabels("other-session", AnnotationValueRuntimeDockerCleanupOrphan)}, + NetworkSettings: &container.NetworkSettings{Networks: map[string]*network.EndpointSettings{dockerNetwork: {}}}, + }}, nil + }, + MockContainerStart: startContainer("old"), + } + r := &RuntimeDocker{ + Image: "xpkg.crossplane.io/crossplane-contrib/function-dummy:v0.1.0", + Name: name, + Cleanup: AnnotationValueRuntimeDockerCleanupRemove, + Session: "this-session", + PullPolicy: AnnotationValueRuntimeDockerPullPolicyNever, + Keychain: authn.NewMultiKeychain(), + Network: dockerNetwork, + log: logging.NewNopLogger(), + dockerClient: cli, + } + + rctx, err := r.Start(t.Context()) + if err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + if err := rctx.Stop(t.Context()); err != nil { + t.Errorf("Stop(...): unexpected error: %v", err) + } +} diff --git a/pkg/render/session.go b/pkg/render/session.go new file mode 100644 index 00000000..cfadf10c --- /dev/null +++ b/pkg/render/session.go @@ -0,0 +1,382 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package render + +import ( + "context" + "maps" + "slices" + "sync" + "sync/atomic" + + "github.com/moby/moby/client" + + "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" + + "github.com/crossplane/cli/v2/internal/containerruntime" + renderv1alpha1 "github.com/crossplane/cli/v2/proto/render/v1alpha1" +) + +// errSessionClosed is returned by a Session used after Close. +var errSessionClosed = errors.New("render session is closed") + +// A Session owns everything one run of render creates: a render engine, the +// environment its Setup creates (such as a Docker network), every function +// runtime it starts, and the run's lease. It starts each function once and +// renders any number of times against them. +// +// Order: the first Start or Render takes the run's lease, reclaims what dead +// runs left behind, then runs the engine's Setup, all before any Render +// reaches the engine. Close stops every runtime, then runs the Setup +// cleanups, then releases the lease. +// +// Concurrency: Start and Render are safe for concurrent use. Starting is +// serialized, so concurrent Starts that need the same function start it once. +// Rendering is not: once the first Setup has run, Render calls the engine +// without waiting for any Start. Close must not run while a Render is in +// flight; a Start or Render that begins after Close returns an error. +// +// Only a Session with a generated ID is leased. A supplied ID may be shared +// with other processes, so its Session neither leases it nor, at Close, +// removes networks labelled with it. +type Session struct { + flags EngineFlags + log logging.Logger + engine Engine + run *containerruntime.Run + warn func(error) + + // sweep is whether Close removes networks left labelled with this + // session: only when the engine creates its own network and the session + // ID belongs to this run alone. + sweep bool + + leaseDir string + newEngine func(*EngineFlags, logging.Logger) Engine + // docker, if set, is the Docker client for everything the Session does + // on Docker. When nil, each piece builds its own from the environment. + docker client.APIClient + + // mu guards everything below, and serializes Setup and starting runtimes. + mu sync.Mutex + // addrs maps the name of every started function to its address. + addrs map[string]string + // runtimes holds every started runtime, so Close stops each of them. + runtimes []startedRuntime + cleanups []func() + // leaked is whether a failed Start may have left a Docker container + // behind, so Close keeps the lease for a later run to reclaim. + leaked bool + + // closed is set, under mu, by Close. Render reads it without mu. + closed atomic.Bool + + // setUp is set, under mu, once the engine's first Setup succeeded. Render + // reads it without mu, which orders what Setup wrote before the render. + setUp atomic.Bool +} + +// A SessionOption configures a Session. +type SessionOption func(*Session) + +// WithLeaseDir makes the Session lease its run in dir and reclaim what dead +// runs leased there left behind. The default, "", disables both. A Session +// with a supplied ID still reclaims dead runs, but takes no lease. +func WithLeaseDir(dir string) SessionOption { + return func(s *Session) { + s.leaseDir = dir + } +} + +// WithEngineFactory makes the Session build its engine with f. The default +// is NewEngineFromFlags. +func WithEngineFactory(f func(*EngineFlags, logging.Logger) Engine) SessionOption { + return func(s *Session) { + s.newEngine = f + } +} + +// WithWarn makes the Session report lease and reclaim failures to f. They +// never stop the Session. The default logs them at Info. f is called with +// the Session's lock held, so f must not call the Session. +func WithWarn(f func(error)) SessionOption { + return func(s *Session) { + s.warn = f + } +} + +// WithDockerClient makes the Session do everything it does on Docker through +// c, instead of clients built from the environment: create and remove the +// render network, run the render container, start and stop every function +// container, and reclaim what dead runs left behind. The render network and +// container are the Docker engine's, as NewEngineFromFlags builds it; another +// engine from WithEngineFactory keeps its own clients: if it uses Docker, it +// must use c's daemon too, or its network and the function containers end up +// on different daemons. Leases don't record their daemon, so a Session can +// retire the lease of a dead run against another daemon that shares its lease +// directory. The Session never closes c; the caller does, after Close. A nil +// c is the default. +func WithDockerClient(c client.APIClient) SessionOption { + return func(s *Session) { + s.docker = c + } +} + +// NewSession returns a Session that renders with an engine built from a copy +// of f. When f.SessionID is empty, the Session generates an ID, and leases it +// only when WithLeaseDir is set; a supplied ID is used as is and never +// leased, since other processes may share it. NewSession does no I/O. +func NewSession(f EngineFlags, log logging.Logger, opts ...SessionOption) *Session { + if log == nil { + log = logging.NewNopLogger() + } + + s := &Session{ + flags: f, + log: log, + newEngine: NewEngineFromFlags, + addrs: make(map[string]string), + } + s.warn = func(err error) { + s.log.Info("Problem managing this run's Docker resources", "error", err) + } + + for _, o := range opts { + o(s) + } + + leased := s.flags.SessionID == "" + if leased { + s.flags.SessionID = containerruntime.NewSessionID() + } + + var runOpts []containerruntime.RunOption + if s.docker != nil { + runOpts = append(runOpts, containerruntime.WithRunClient(func() (containerruntime.RunClient, error) { + return sharedRunClient{s.docker}, nil + })) + } + s.run = containerruntime.NewRun(s.leaseDir, s.flags.SessionID, leased, log, runOpts...) + s.engine = s.newEngine(&s.flags, log) + + e, isDocker := s.engine.(*dockerRenderEngine) + if isDocker && s.docker != nil { + e.networks = s.docker + e.dockerClient = s.docker + } + s.sweep = isDocker && s.flags.CrossplaneDockerNetwork == "" && leased + + return s +} + +// ID is the session ID the Session labels its Docker resources with. +func (s *Session) ID() string { + return s.flags.SessionID +} + +// CheckContextSupport reports whether the Session's engine supports context +// injection and collection. +func (s *Session) CheckContextSupport() error { + return s.engine.CheckContextSupport() +} + +// Start ensures each of fns is running, starting the ones this Session has +// not started yet, and returns the addresses of fns only. Functions are +// identified by name. The first call, even with no fns, runs the engine's +// Setup. Start copies each function's annotations before Setup adds to them, +// so the caller's are untouched. +func (s *Session) Start(ctx context.Context, fns []pkgv1.Function) (map[string]string, error) { + s.mu.Lock() + defer s.mu.Unlock() + + if s.closed.Load() { + return nil, errSessionClosed + } + + // Functions sharing a name within fns all start, and the last one's + // address is used, as with StartFunctionRuntimes. + batch := make([]pkgv1.Function, 0, len(fns)) + for _, fn := range fns { + if _, ok := s.addrs[fn.GetName()]; ok { + continue + } + + // fn is a copy, but shares its annotation map with the caller's. + fn.SetAnnotations(maps.Clone(fn.GetAnnotations())) + batch = append(batch, fn) + } + + if err := s.setupLocked(ctx, batch); err != nil { + return nil, err + } + + if len(batch) > 0 { + docker := false + fa, err := startFunctionRuntimes(ctx, s.log, batch, func(fn pkgv1.Function, log logging.Logger) (Runtime, error) { + rt, err := s.getRuntime(fn, log) + if _, ok := rt.(*RuntimeDocker); ok { + docker = true + } + return rt, err + }) + if err != nil { + // A Docker start can fail after creating its container, which + // then is in no runtime Close stops. + s.leaked = s.leaked || docker + // The Setup cleanup is kept: Close still runs it. + return nil, errors.Wrap(err, "cannot start function runtimes") + } + + maps.Copy(s.addrs, fa.addrs) + s.runtimes = append(s.runtimes, fa.runtimes...) + } + + // Copied, because the caller reads them after mu is released. + out := make(map[string]string, len(fns)) + for _, fn := range fns { + if a, ok := s.addrs[fn.GetName()]; ok { + out[fn.GetName()] = a + } + } + + return out, nil +} + +// getRuntime gets fn's runtime as GetRuntime does, on the Session's Docker +// client when it has one. +func (s *Session) getRuntime(fn pkgv1.Function, log logging.Logger) (Runtime, error) { + rt, err := GetRuntime(fn, log) + if err != nil { + return nil, err + } + if rd, ok := rt.(*RuntimeDocker); ok && s.docker != nil { + rd.dockerClient = s.docker + } + return rt, nil +} + +// Render runs req through the Session's engine. It waits for the first +// Setup, running it if no Start has, then renders without holding the +// Session's lock. After Close it returns an error without rendering. See +// Engine.Render for the partial response it may return with an error. +func (s *Session) Render(ctx context.Context, req *renderv1alpha1.RenderRequest) (*renderv1alpha1.RenderResponse, error) { + if s.closed.Load() { + return nil, errSessionClosed + } + + if !s.setUp.Load() { + s.mu.Lock() + err := s.setupLocked(ctx, nil) + s.mu.Unlock() + + if err != nil { + return nil, err + } + } + + return s.engine.Render(ctx, req) +} + +// setupLocked takes the run's lease and reclaims dead runs, the first time, +// then integrates fns into the engine's environment. Once the first Setup +// succeeded, it does nothing for no fns. The caller holds mu. +func (s *Session) setupLocked(ctx context.Context, fns []pkgv1.Function) error { + if s.closed.Load() { + return errSessionClosed + } + + if s.setUp.Load() && len(fns) == 0 { + return nil + } + + // Idempotent: only the first call does anything. + if err := s.run.Acquire(ctx); err != nil { + s.warn(err) + } + + cleanup, err := s.engine.Setup(ctx, fns) + if err != nil { + // Returned as is, as before Sessions: the engine's errors say what + // failed. + return err + } + + s.cleanups = append(s.cleanups, cleanup) + s.setUp.Store(true) + + return nil +} + +// Close stops every runtime the Session started, each per its own cleanup +// policy and concurrently, then runs the engine's Setup cleanups in reverse +// order, then releases the run's lease. Teardown is detached from ctx's +// cancellation, so it can follow an interrupted render. The lease is kept, +// so a later run reclaims what is left, when a runtime cannot be stopped or +// a failed Start may have left a container. +// Close is idempotent; Start or Render after Close returns an error. Close +// must not run while a Render is in flight. +func (s *Session) Close(ctx context.Context) error { + s.mu.Lock() + defer s.mu.Unlock() + + if s.closed.Swap(true) { + return nil + } + + // No deadline here: a Docker runtime's Stop bounds each of its requests + // (containerruntime.StopTimeout, then RemoveTimeout), as the network + // cleanup and the sweep bound theirs, so a hung daemon cannot hold Close. + // The other runtimes' Stops do nothing. + ctx = context.WithoutCancel(ctx) + + errs := make([]error, len(s.runtimes)) + + var wg sync.WaitGroup + for i, rt := range s.runtimes { + wg.Go(func() { + if err := rt.Stop(ctx); err != nil { + errs[i] = errors.Wrapf(err, "cannot stop function %q runtime (target %q)", rt.name, rt.Target) + } + }) + } + wg.Wait() + + for _, c := range slices.Backward(s.cleanups) { + c() + } + + stopErr := errors.Join(errs...) + + // Without a Setup, the engine created no network to sweep. + sweep := "" + if s.sweep && s.setUp.Load() { + sweep = labelDomain + } + relErr := s.run.Release(ctx, stopErr == nil && !s.leaked, sweep) + + return errors.Join(stopErr, relErr) +} + +// sharedRunClient is the Session's Docker client as the Run uses it. It hides +// the client's Close, so the Run, which closes the clients it gets, leaves the +// caller's open. +type sharedRunClient struct { + containerruntime.RunClient +} diff --git a/pkg/render/session_integration_test.go b/pkg/render/session_integration_test.go new file mode 100644 index 00000000..4ff5424e --- /dev/null +++ b/pkg/render/session_integration_test.go @@ -0,0 +1,87 @@ +//go:build integration + +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package render + +import ( + "testing" + + "github.com/moby/moby/client" + + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + + "github.com/crossplane/cli/v2/internal/containerruntime" +) + +// TestSessionNetworkOnDocker pins, against a real Docker daemon, that a +// Session's Start with no functions creates its network on the daemon and its +// Close removes it. Both the engine's cleanup and Close's sweep remove it, so +// this does not tell which one did. It needs no image. Like the +// containerruntime integration test, it fails rather than skips without a +// Docker daemon. +// +// If the test dies without its cleanups (a -timeout panic, a SIGKILL), remove +// the network it leaked, using the session ID it logged, with +// +// docker network rm $(docker network ls -q --filter name=crossplane-render- --filter label=render.crossplane.io/session=) +func TestSessionNetworkOnDocker(t *testing.T) { + ctx := t.Context() + + cli, err := client.New(client.FromEnv) + if err != nil { + t.Fatalf("client.New(...): %v", err) + } + t.Cleanup(func() { _ = cli.Close() }) + + if _, err := cli.Ping(ctx, client.PingOptions{}); err != nil { + t.Fatalf("this test needs a Docker daemon: Ping(...): %v", err) + } + + s := NewSession(EngineFlags{}, logging.NewNopLogger(), WithLeaseDir(t.TempDir()), WithDockerClient(cli)) + t.Cleanup(func() { _ = s.Close(ctx) }) + t.Logf("session %s", s.ID()) + + if _, err := s.Start(ctx, nil); err != nil { + t.Fatalf("Start(ctx, nil): %v", err) + } + + f := make(client.Filters) + for k, v := range containerruntime.ManagedLabels(containerruntime.DomainRender, s.ID()) { + f = f.Add("label", k+"="+v) + } + + ns, err := cli.NetworkList(ctx, client.NetworkListOptions{Filters: f}) + if err != nil { + t.Fatalf("NetworkList(...): %v", err) + } + if len(ns.Items) != 1 { + t.Fatalf("networks labelled with the session's ID after Start: got %d, want 1", len(ns.Items)) + } + + if err := s.Close(ctx); err != nil { + t.Errorf("Close(...): %v", err) + } + + ns, err = cli.NetworkList(ctx, client.NetworkListOptions{Filters: f}) + if err != nil { + t.Fatalf("NetworkList(...): %v", err) + } + if len(ns.Items) != 0 { + t.Errorf("networks labelled with the session's ID after Close: got %d, want 0", len(ns.Items)) + } +} diff --git a/pkg/render/session_test.go b/pkg/render/session_test.go index c766a407..8818286a 100644 --- a/pkg/render/session_test.go +++ b/pkg/render/session_test.go @@ -17,13 +17,32 @@ limitations under the License. package render import ( + "context" + "fmt" + "io" + "maps" + "net" + "os" + "path/filepath" + "runtime" + "slices" + "strings" + "sync" "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" + "github.com/moby/moby/client" + "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" + + "github.com/crossplane/cli/v2/internal/containerruntime" + renderv1alpha1 "github.com/crossplane/cli/v2/proto/render/v1alpha1" ) func TestManagedLabelsSession(t *testing.T) { @@ -145,3 +164,554 @@ func TestGetRuntimeDockerSession(t *testing.T) { t.Errorf("GetRuntimeDocker(...): Session: want %q, got %q", "my-session", rd.Session) } } + +// recorder records events from concurrent goroutines. +type recorder struct { + mu sync.Mutex + events []string +} + +func (r *recorder) add(e string) { + r.mu.Lock() + defer r.mu.Unlock() + r.events = append(r.events, e) +} + +func (r *recorder) get() []string { + r.mu.Lock() + defer r.mu.Unlock() + return slices.Clone(r.events) +} + +func namedFn(name string, annotations map[string]string) pkgv1.Function { + fn := pkgv1.Function{} + fn.SetName(name) + fn.SetAnnotations(annotations) + return fn +} + +// devFn returns a Function named name on the Development runtime, which +// starts nothing and listens on name:9443. +func devFn(name string, annotations map[string]string) pkgv1.Function { + a := map[string]string{ + AnnotationKeyRuntime: string(AnnotationValueRuntimeDevelopment), + AnnotationKeyRuntimeDevelopmentTarget: name + ":9443", + } + maps.Copy(a, annotations) + return namedFn(name, a) +} + +// starts counts, by function name, the runtimes s started. +func starts(s *Session) map[string]int { + s.mu.Lock() + defer s.mu.Unlock() + n := make(map[string]int) + for _, rt := range s.runtimes { + n[rt.name]++ + } + return n +} + +// recordStops makes each runtime s started record its stop in rec, then +// return err. +func recordStops(s *Session, rec *recorder, err error) { + s.mu.Lock() + defer s.mu.Unlock() + for i := range s.runtimes { + name := s.runtimes[i].name + s.runtimes[i].Stop = func(context.Context) error { + rec.add("stop:" + name) + return err + } + } +} + +// newTestSession returns a Session on engine, leasing in a temporary +// directory. +func newTestSession(t *testing.T, engine Engine, opts ...SessionOption) (*Session, string) { + t.Helper() + dir := t.TempDir() + opts = append([]SessionOption{ + WithLeaseDir(dir), + WithEngineFactory(func(*EngineFlags, logging.Logger) Engine { return engine }), + WithWarn(func(err error) { t.Errorf("unexpected warning: %v", err) }), + }, opts...) + s := NewSession(EngineFlags{}, logging.NewNopLogger(), opts...) + return s, dir +} + +func TestSessionStartsEachFunctionOnceConcurrently(t *testing.T) { + s, _ := newTestSession(t, &MockEngine{}) + + names := []string{"a", "b", "c", "d", "e"} + + var wg sync.WaitGroup + for i := range 16 { + wg.Go(func() { + fns := []pkgv1.Function{devFn(names[i%5], nil), devFn(names[(i+1)%5], nil)} + addrs, err := s.Start(t.Context(), fns) + if err != nil { + t.Errorf("Start(...): unexpected error: %v", err) + return + } + want := map[string]string{names[i%5]: names[i%5] + ":9443", names[(i+1)%5]: names[(i+1)%5] + ":9443"} + if diff := cmp.Diff(want, addrs); diff != "" { + t.Errorf("Start(...): -want addresses, +got addresses:\n%s", diff) + } + if _, err := s.Render(t.Context(), &renderv1alpha1.RenderRequest{}); err != nil { + t.Errorf("Render(...): unexpected error: %v", err) + } + }) + } + wg.Wait() + + got := starts(s) + for _, n := range names { + if got[n] != 1 { + t.Errorf("function %q: want 1 start, got %d", n, got[n]) + } + } +} + +func TestSessionSetupOnceBeforeRender(t *testing.T) { + cases := map[string]struct { + reason string + start bool + }{ + "StartWithNoFunctions": { + reason: "A Start with no functions should still run Setup, once, before the first Render.", + start: true, + }, + "RenderWithoutStart": { + reason: "A Render with no Start before it should run Setup itself, once.", + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + rec := &recorder{} + engine := &MockEngine{ + MockSetup: func(context.Context, []pkgv1.Function) (func(), error) { + rec.add("setup") + return func() {}, nil + }, + MockRender: func(context.Context, *renderv1alpha1.RenderRequest) (*renderv1alpha1.RenderResponse, error) { + rec.add("render") + return &renderv1alpha1.RenderResponse{}, nil + }, + } + s, _ := newTestSession(t, engine) + + if tc.start { + if _, err := s.Start(t.Context(), nil); err != nil { + t.Fatalf("\n%s\nStart(...): unexpected error: %v", tc.reason, err) + } + if _, err := s.Start(t.Context(), nil); err != nil { + t.Fatalf("\n%s\nStart(...): unexpected error: %v", tc.reason, err) + } + } + for range 2 { + if _, err := s.Render(t.Context(), &renderv1alpha1.RenderRequest{}); err != nil { + t.Fatalf("\n%s\nRender(...): unexpected error: %v", tc.reason, err) + } + } + + if diff := cmp.Diff([]string{"setup", "render", "render"}, rec.get()); diff != "" { + t.Errorf("\n%s\n-want events, +got events:\n%s", tc.reason, diff) + } + }) + } +} + +func TestSessionClose(t *testing.T) { + rec := &recorder{} + var dir string + + leaseHeld := func() string { + matches, _ := filepath.Glob(filepath.Join(dir, "*.lock")) + return fmt.Sprintf("lease held: %t", len(matches) == 1) + } + + engine := &MockEngine{ + MockSetup: func(_ context.Context, fns []pkgv1.Function) (func(), error) { + n := fns[0].GetName() + return func() { rec.add(fmt.Sprintf("cleanup %s (%s)", n, leaseHeld())) }, nil + }, + } + s, dir := newTestSession(t, engine) + + if _, err := s.Start(t.Context(), []pkgv1.Function{devFn("a", nil)}); err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + // Two functions named b in one Start both start, and both stop. + if _, err := s.Start(t.Context(), []pkgv1.Function{devFn("a", nil), devFn("b", nil), devFn("b", nil)}); err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + if diff := cmp.Diff(map[string]int{"a": 1, "b": 2}, starts(s)); diff != "" { + t.Errorf("Start(...): want each function started once per Start that names it first: -want starts, +got starts:\n%s", diff) + } + recordStops(s, rec, nil) + + if err := s.Close(t.Context()); err != nil { + t.Fatalf("Close(...): unexpected error: %v", err) + } + rec.add(leaseHeld()) + + got := rec.get() + // Runtimes stop concurrently, so their order is not fixed. + slices.Sort(got[0:3]) + want := []string{ + "stop:a", "stop:b", "stop:b", + "cleanup b (lease held: true)", "cleanup a (lease held: true)", + "lease held: false", + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Close(...): want runtimes stopped, then Setup cleanups LIFO, then the lease released: -want events, +got events:\n%s", diff) + } + + if err := s.Close(t.Context()); err != nil { + t.Errorf("Close(...): a second Close: unexpected error: %v", err) + } + if diff := cmp.Diff(want, rec.get(), cmpopts.SortSlices(func(a, b string) bool { return a < b })); diff != "" { + t.Errorf("Close(...): a second Close should do nothing: -want events, +got events:\n%s", diff) + } + + if _, err := s.Start(t.Context(), nil); !errors.Is(err, errSessionClosed) { + t.Errorf("Start(...) after Close: want %v, got %v", errSessionClosed, err) + } +} + +func TestSessionStartFailureKeepsSetupCleanup(t *testing.T) { + rec := &recorder{} + engine := &MockEngine{ + MockSetup: func(context.Context, []pkgv1.Function) (func(), error) { + return func() { rec.add("cleanup") }, nil + }, + } + s, dir := newTestSession(t, engine) + + // An unknown runtime cannot be started. + fn := namedFn("a", map[string]string{AnnotationKeyRuntime: "Unknown"}) + _, err := s.Start(t.Context(), []pkgv1.Function{fn}) + if err == nil || !strings.Contains(err.Error(), "cannot start function runtimes") || !strings.Contains(err.Error(), "unknown runtime") { + t.Fatalf("Start(...): want a failure to start the unknown runtime, got %v", err) + } + if err := s.Close(t.Context()); err != nil { + t.Fatalf("Close(...): unexpected error: %v", err) + } + + if diff := cmp.Diff([]string{"cleanup"}, rec.get()); diff != "" { + t.Errorf("Close(...): a failed start should keep the Setup cleanup: -want events, +got events:\n%s", diff) + } + if matches, _ := filepath.Glob(filepath.Join(dir, "*.lock")); len(matches) != 0 { + t.Errorf("Close(...): want the lease released, got lease files %v", matches) + } +} + +func TestSessionCloseStopFailureKeepsLease(t *testing.T) { + errBoom := errors.New("boom") + s, dir := newTestSession(t, &MockEngine{}) + + if _, err := s.Start(t.Context(), []pkgv1.Function{devFn("a", nil)}); err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + recordStops(s, &recorder{}, errBoom) + if err := s.Close(t.Context()); !errors.Is(err, errBoom) { + t.Errorf("Close(...): want an error wrapping %v, got %v", errBoom, err) + } + + if matches, _ := filepath.Glob(filepath.Join(dir, "*.lock")); len(matches) != 1 { + t.Errorf("Close(...): a runtime that cannot be stopped should keep the lease for a later run to reclaim, got lease files %v", matches) + } +} + +// TestSessionDockerStartFailureKeepsLease checks that a container created by a +// failed Start, which no runtime stops, keeps the lease for a later run. +func TestSessionDockerStartFailureKeepsLease(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows has no run leases") + } + t.Setenv("DOCKER_CONFIG", t.TempDir()) // No credential helpers. + + errBoom := errors.New("boom") + cli := &fakeDocker{host: "unix:///var/run/docker.sock", startErr: errBoom} + s, dir := newTestSession(t, &MockEngine{}, WithDockerClient(cli)) + + fn := namedFn("fn", nil) + fn.Spec.Package = "fn-image" + if _, err := s.Start(t.Context(), []pkgv1.Function{fn}); !errors.Is(err, errBoom) { + t.Fatalf("Start(...): want an error wrapping %v, got %v", errBoom, err) + } + if err := s.Close(t.Context()); err != nil { + t.Fatalf("Close(...): unexpected error: %v", err) + } + + type state struct { + Containers []string + Leases int + } + matches, _ := filepath.Glob(filepath.Join(dir, "*.lock")) + got := state{Containers: cli.left().Containers, Leases: len(matches)} + want := state{Containers: []string{"id-fn-image"}, Leases: 1} + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Start and Close: want the created container left and its lease kept for a later run: -want, +got:\n%s", diff) + } +} + +// TestSessionSweepsOnlyItsOwnNetworks checks that Close removes the networks +// labelled with its session only when its session ID is its own. The seeded +// network is not the engine's own, so only the sweep can remove it. +func TestSessionSweepsOnlyItsOwnNetworks(t *testing.T) { + cases := map[string]struct { + reason string + flags EngineFlags + want []string + }{ + "OwnNetwork": { + reason: "A Docker engine on a network it created, under a generated ID, should sweep its session's networks.", + }, + "SuppliedID": { + reason: "A supplied ID may be shared with another process, so its networks should not be swept.", + flags: EngineFlags{SessionID: "0123456789abcdef"}, + want: []string{"stray"}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + cli := &fakeDocker{} + s := NewSession(tc.flags, logging.NewNopLogger(), WithDockerClient(cli)) + cli.networks = []network.Summary{{Network: network.Network{ID: "stray", Labels: managedLabels(s.ID())}}} + + if _, err := s.Start(t.Context(), nil); err != nil { + t.Fatalf("\n%s\nStart(...): unexpected error: %v", tc.reason, err) + } + if err := s.Close(t.Context()); err != nil { + t.Fatalf("\n%s\nClose(...): unexpected error: %v", tc.reason, err) + } + + if diff := cmp.Diff(tc.want, cli.left().Networks, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("\n%s\nClose(...): -want networks left, +got networks left:\n%s", tc.reason, diff) + } + }) + } +} + +// TestSessionWithDockerClient checks that a Session reclaims a dead run, runs +// a function and the render container, and cleans up after itself through the +// client WithDockerClient supplies, and that it never closes that client. The +// environment points at no daemon, so a path that builds its own client fails. +// CheckContextSupport judges the supplied client's daemon too. +func TestSessionWithDockerClient(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows has no run leases, so no dead run to reclaim") + } + t.Setenv("DOCKER_HOST", "unix://"+filepath.Join(t.TempDir(), "no-docker.sock")) + t.Setenv("DOCKER_CONFIG", t.TempDir()) // No credential helpers. + + dir := t.TempDir() + deadID := containerruntime.NewSessionID() + if err := os.WriteFile(filepath.Join(dir, deadID+".lock"), nil, 0o600); err != nil { + t.Fatal(err) + } + + cli := &fakeDocker{ + host: "tcp://remote:2376", + containers: []container.Summary{{ID: "dead-fn", State: container.StateRunning, Labels: managedLabels(deadID)}}, + networks: []network.Summary{{Network: network.Network{ID: "dead-net", Labels: managedLabels(deadID)}}}, + } + s := NewSession(EngineFlags{CrossplaneImage: "render-image"}, logging.NewNopLogger(), + WithDockerClient(cli), + WithLeaseDir(dir), + WithWarn(func(err error) { t.Errorf("unexpected warning: %v", err) }), + ) + + // The environment's daemon is local, the supplied client's remote. + if err := s.CheckContextSupport(); err == nil { + t.Errorf("CheckContextSupport(): want an error for the supplied client's remote daemon, got nil") + } + + fn := namedFn("fn", nil) + fn.Spec.Package = "fn-image" + + if _, err := s.Start(t.Context(), []pkgv1.Function{fn}); err != nil { + t.Fatalf("Start(...): unexpected error: %v", err) + } + if _, err := s.Render(t.Context(), &renderv1alpha1.RenderRequest{}); err != nil { + t.Fatalf("Render(...): unexpected error: %v", err) + } + if err := s.Close(t.Context()); err != nil { + t.Fatalf("Close(...): unexpected error: %v", err) + } + + if diff := cmp.Diff(leftover{}, cli.left(), cmpopts.EquateEmpty()); diff != "" { + t.Errorf("Start, Render and Close: want the dead run and the Session's own resources gone, and the client not closed: -want, +got:\n%s", diff) + } +} + +// fakeDocker is a Docker client that keeps the containers and networks it +// serves, for the Docker calls a Session makes. Any other call panics. +type fakeDocker struct { + client.APIClient + + host string + // startErr, if set, is what ContainerStart returns. + startErr error + + mu sync.Mutex + closed bool + network string + networks []network.Summary + containers []container.Summary +} + +// leftover is what a fakeDocker still holds. +type leftover struct { + Containers []string + Networks []string + Closed bool +} + +func (f *fakeDocker) left() leftover { + f.mu.Lock() + defer f.mu.Unlock() + l := leftover{Closed: f.closed} + for _, c := range f.containers { + l.Containers = append(l.Containers, c.ID) + } + for _, n := range f.networks { + l.Networks = append(l.Networks, n.ID) + } + return l +} + +func (f *fakeDocker) DaemonHost() string { return f.host } + +func (f *fakeDocker) Close() error { + f.mu.Lock() + defer f.mu.Unlock() + f.closed = true + return nil +} + +// hasLabels reports whether labels has every key=value label filters selects +// on. +func hasLabels(filters client.Filters, labels map[string]string) bool { + for l := range filters["label"] { + k, v, _ := strings.Cut(l, "=") + if labels[k] != v { + return false + } + } + return true +} + +// ContainerList lists the containers the fake created and has not removed. +func (f *fakeDocker) ContainerList(_ context.Context, o client.ContainerListOptions) (client.ContainerListResult, error) { + f.mu.Lock() + defer f.mu.Unlock() + var res client.ContainerListResult + for _, c := range f.containers { + if hasLabels(o.Filters, c.Labels) { + res.Items = append(res.Items, c) + } + } + return res, nil +} + +// NetworkList lists the networks the fake created and has not removed. +func (f *fakeDocker) NetworkList(_ context.Context, o client.NetworkListOptions) (client.NetworkListResult, error) { + f.mu.Lock() + defer f.mu.Unlock() + var res client.NetworkListResult + for _, n := range f.networks { + if hasLabels(o.Filters, n.Labels) { + res.Items = append(res.Items, n) + } + } + return res, nil +} + +func (f *fakeDocker) NetworkCreate(_ context.Context, name string, o client.NetworkCreateOptions) (client.NetworkCreateResult, error) { + f.mu.Lock() + f.network = name + f.networks = append(f.networks, network.Summary{Network: network.Network{ID: "net-id", Name: name, Labels: o.Labels}}) + f.mu.Unlock() + return client.NetworkCreateResult{ID: "net-id"}, nil +} + +func (f *fakeDocker) NetworkRemove(_ context.Context, id string, _ client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.networks = slices.DeleteFunc(f.networks, func(n network.Summary) bool { return n.ID == id }) + return client.NetworkRemoveResult{}, nil +} + +func (f *fakeDocker) ImageInspect(_ context.Context, _ string, _ ...client.ImageInspectOption) (client.ImageInspectResult, error) { + return client.ImageInspectResult{}, nil +} + +// ContainerCreate records the container, running and attached to the networks +// it was created on, so ContainerList lists it until it is removed. +func (f *fakeDocker) ContainerCreate(_ context.Context, o client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + id := "id-" + o.Config.Image + c := container.Summary{ID: id, State: container.StateRunning, Labels: o.Config.Labels, NetworkSettings: &container.NetworkSettingsSummary{ + Networks: map[string]*network.EndpointSettings{}, + }} + if o.NetworkingConfig != nil { + for n := range o.NetworkingConfig.EndpointsConfig { + c.NetworkSettings.Networks[n] = &network.EndpointSettings{} + } + } + f.mu.Lock() + f.containers = append(f.containers, c) + f.mu.Unlock() + return client.ContainerCreateResult{ID: id}, nil +} + +func (f *fakeDocker) ContainerStart(_ context.Context, _ string, _ client.ContainerStartOptions) (client.ContainerStartResult, error) { + return client.ContainerStartResult{}, f.startErr +} + +// ContainerInspect reports the container attached to the network the fake +// created last. +func (f *fakeDocker) ContainerInspect(_ context.Context, id string, _ client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + f.mu.Lock() + defer f.mu.Unlock() + return client.ContainerInspectResult{Container: container.InspectResponse{ + Name: "/" + id, + NetworkSettings: &container.NetworkSettings{Networks: map[string]*network.EndpointSettings{f.network: {}}}, + }}, nil +} + +func (f *fakeDocker) ContainerStop(_ context.Context, _ string, _ client.ContainerStopOptions) (client.ContainerStopResult, error) { + return client.ContainerStopResult{}, nil +} + +func (f *fakeDocker) ContainerRemove(_ context.Context, id string, _ client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.containers = slices.DeleteFunc(f.containers, func(c container.Summary) bool { return c.ID == id }) + return client.ContainerRemoveResult{}, nil +} + +// ContainerAttach returns a connection that discards stdin and has no output. +func (f *fakeDocker) ContainerAttach(_ context.Context, _ string, _ client.ContainerAttachOptions) (client.ContainerAttachResult, error) { + return client.ContainerAttachResult{HijackedResponse: client.NewHijackedResponse(discardConn{}, "")}, nil +} + +// ContainerWait reports that the container exited successfully. +func (f *fakeDocker) ContainerWait(_ context.Context, _ string, _ client.ContainerWaitOptions) client.ContainerWaitResult { + res := make(chan container.WaitResponse, 1) + res <- container.WaitResponse{} + return client.ContainerWaitResult{Result: res} +} + +// discardConn is a connection that discards writes and reads nothing. +type discardConn struct { + net.Conn +} + +func (discardConn) Read([]byte) (int, error) { return 0, io.EOF } +func (discardConn) Write(b []byte) (int, error) { return len(b), nil } +func (discardConn) Close() error { return nil }