From 03cfefbd2f96f8212dd8911b563f54b255def8b3 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Fri, 2 Oct 2026 13:41:31 -0400 Subject: [PATCH] feat(render): label docker containers and networks created by render Docker resources created by crossplane render carried no labels, so leftovers after a crash, kill -9, a CI timeout, or a deliberate Orphan cleanup policy could not be reliably identified or swept. Apply render.crossplane.io/managed-by=crossplane to Function containers, the render engine container and the temporary render network, and record the effective cleanup policy on Function containers as render.crossplane.io/cleanup. The label keys are exported constants, and a new docker.RunWithLabels option plumbs labels into docker.RunContainer. Fixes #401 Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Jonathan Ogilvie --- cmd/crossplane/render/engine_docker.go | 1 + cmd/crossplane/render/network.go | 1 + cmd/crossplane/render/network_test.go | 4 +- cmd/crossplane/render/runtime_docker.go | 34 +++++++ cmd/crossplane/render/runtime_docker_test.go | 100 +++++++++++++++++++ internal/docker/docker.go | 11 ++ internal/docker/docker_test.go | 31 ++++++ 7 files changed, 180 insertions(+), 2 deletions(-) diff --git a/cmd/crossplane/render/engine_docker.go b/cmd/crossplane/render/engine_docker.go index af5ed8fc..4eecbf5a 100644 --- a/cmd/crossplane/render/engine_docker.go +++ b/cmd/crossplane/render/engine_docker.go @@ -170,6 +170,7 @@ func (e *dockerRenderEngine) Render(ctx context.Context, req *renderv1alpha1.Ren // Let the container access any functions running in development mode on // the host. docker.RunWithExtraHosts([]string{"host.docker.internal:host-gateway"}), + docker.RunWithLabels(managedLabels()), } if e.network != "" { opts = append(opts, docker.RunWithNetworkName(e.network)) diff --git a/cmd/crossplane/render/network.go b/cmd/crossplane/render/network.go index 8ef17d68..3dfa2346 100644 --- a/cmd/crossplane/render/network.go +++ b/cmd/crossplane/render/network.go @@ -72,6 +72,7 @@ func createRenderNetwork(ctx context.Context, cli networkClient) (string, string resp, err := cli.NetworkCreate(ctx, name, client.NetworkCreateOptions{ Driver: "bridge", + Labels: managedLabels(), }) if err != nil { return "", "", errors.Wrapf(err, "cannot create Docker network %q", name) diff --git a/cmd/crossplane/render/network_test.go b/cmd/crossplane/render/network_test.go index 843f16f4..315fa708 100644 --- a/cmd/crossplane/render/network_test.go +++ b/cmd/crossplane/render/network_test.go @@ -102,13 +102,13 @@ const renderNetworkPrefix = "crossplane-render-" // createRenderNetworkReturns returns a MockNetworkCreate that returns the // supplied network ID and error. It returns an error instead when it is not -// asked to create a render bridge network. +// asked to create a render bridge network labeled as managed by render. func createRenderNetworkReturns(id string, err error) func(context.Context, string, client.NetworkCreateOptions) (client.NetworkCreateResult, error) { return func(_ context.Context, name string, options client.NetworkCreateOptions) (client.NetworkCreateResult, error) { if !strings.HasPrefix(name, renderNetworkPrefix) { return client.NetworkCreateResult{}, errors.Errorf("NetworkCreate(...): name %q does not have prefix %q", name, renderNetworkPrefix) } - if diff := cmp.Diff(client.NetworkCreateOptions{Driver: "bridge"}, options); diff != "" { + if diff := cmp.Diff(client.NetworkCreateOptions{Driver: "bridge", Labels: map[string]string{LabelKeyManagedBy: LabelValueManagedByCrossplane}}, options); diff != "" { return client.NetworkCreateResult{}, errors.Errorf("NetworkCreate(...): -want options, +got options:\n%s", diff) } return client.NetworkCreateResult{ID: id}, err diff --git a/cmd/crossplane/render/runtime_docker.go b/cmd/crossplane/render/runtime_docker.go index 27b02cec..329e3b53 100644 --- a/cmd/crossplane/render/runtime_docker.go +++ b/cmd/crossplane/render/runtime_docker.go @@ -82,6 +82,36 @@ const ( AnnotationKeyRuntimeDockerNetwork = "render.crossplane.io/runtime-docker-network" ) +// Labels that render applies to the Docker resources it creates, so that +// leftovers (e.g. after a crash or with an Orphan cleanup policy) can be +// identified and swept. +const ( + // LabelKeyManagedBy is applied to every Docker container and network + // created by render. Its value is always LabelValueManagedByCrossplane. + LabelKeyManagedBy = "render.crossplane.io/managed-by" + + // LabelValueManagedByCrossplane is the value of LabelKeyManagedBy. + LabelValueManagedByCrossplane = "crossplane" + + // LabelKeyCleanup is applied to Function containers and records the + // effective DockerCleanup policy the container was created with. + LabelKeyCleanup = "render.crossplane.io/cleanup" +) + +// managedLabels returns the labels applied to every Docker resource render +// creates. +func managedLabels() map[string]string { + return map[string]string{LabelKeyManagedBy: LabelValueManagedByCrossplane} +} + +// functionContainerLabels returns the labels applied to a Function container +// created with the supplied cleanup policy. +func functionContainerLabels(cleanup DockerCleanup) map[string]string { + l := managedLabels() + l[LabelKeyCleanup] = string(cleanup) + return l +} + // DockerCleanup specifies what Docker should do with a Function container after // it has been run. type DockerCleanup string @@ -323,6 +353,10 @@ func (r *RuntimeDocker) createContainer(ctx context.Context, cli containerClient Cmd: []string{"--insecure"}, ExposedPorts: network.PortSet{port: struct{}{}}, Env: r.Env, + // Labels are only set at creation time. A pre-existing named container + // that is reused is not relabeled, since Docker can't change labels on + // an existing container. + Labels: functionContainerLabels(r.Cleanup), } hcfg := &container.HostConfig{} var ncfg *network.NetworkingConfig diff --git a/cmd/crossplane/render/runtime_docker_test.go b/cmd/crossplane/render/runtime_docker_test.go index 2fbfd96e..52f0bf98 100644 --- a/cmd/crossplane/render/runtime_docker_test.go +++ b/cmd/crossplane/render/runtime_docker_test.go @@ -647,3 +647,103 @@ func TestRuntimeDockerStop(t *testing.T) { }) } } + +func TestRuntimeDockerStartLabels(t *testing.T) { + const ( + containerID = "container-id" + containerName = "fn-container" + dockerNetwork = "render-net" + ) + + // createLabeledContainer returns a MockContainerCreate that creates a + // container, or returns an error when the container config doesn't carry + // the supplied labels. + createLabeledContainer := func(labels map[string]string) func(context.Context, client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + return func(_ context.Context, options client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + if options.Config == nil { + return client.ContainerCreateResult{}, errors.New("ContainerCreate(...): called without a container config") + } + if diff := cmp.Diff(labels, options.Config.Labels); diff != "" { + return client.ContainerCreateResult{}, errors.Errorf("ContainerCreate(...): -want labels, +got labels:\n%s", diff) + } + return client.ContainerCreateResult{ID: containerID}, nil + } + } + + type args struct { + cleanup DockerCleanup + cli *mockContainerClient + } + type want struct { + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "Stop": { + reason: "Function containers should be marked as managed by render and record the Stop policy.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupStop, + cli: &mockContainerClient{ + MockContainerCreate: createLabeledContainer(map[string]string{ + LabelKeyManagedBy: LabelValueManagedByCrossplane, + LabelKeyCleanup: string(AnnotationValueRuntimeDockerCleanupStop), + }), + MockContainerStart: startContainer(containerID), + MockContainerInspect: inspectContainerOnNetwork(containerID, containerName, dockerNetwork), + }, + }, + }, + "Remove": { + reason: "Function containers should be marked as managed by render and record the Remove policy.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupRemove, + cli: &mockContainerClient{ + MockContainerCreate: createLabeledContainer(map[string]string{ + LabelKeyManagedBy: LabelValueManagedByCrossplane, + LabelKeyCleanup: string(AnnotationValueRuntimeDockerCleanupRemove), + }), + MockContainerStart: startContainer(containerID), + MockContainerInspect: inspectContainerOnNetwork(containerID, containerName, dockerNetwork), + }, + }, + }, + "Orphan": { + reason: "Function containers should record the Orphan policy so sweeps can skip them.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupOrphan, + cli: &mockContainerClient{ + MockContainerCreate: createLabeledContainer(map[string]string{ + LabelKeyManagedBy: LabelValueManagedByCrossplane, + LabelKeyCleanup: string(AnnotationValueRuntimeDockerCleanupOrphan), + }), + MockContainerStart: startContainer(containerID), + MockContainerInspect: inspectContainerOnNetwork(containerID, containerName, dockerNetwork), + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := &RuntimeDocker{ + Image: "xpkg.crossplane.io/crossplane-contrib/function-dummy:v0.1.0", + Cleanup: tc.args.cleanup, + PullPolicy: AnnotationValueRuntimeDockerPullPolicyIfNotPresent, + Keychain: authn.NewMultiKeychain(), + Network: dockerNetwork, + log: logging.NewNopLogger(), + dockerClient: tc.args.cli, + } + + _, err := r.Start(t.Context()) + + if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("\n%s\nStart(...): -want error, +got error:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/internal/docker/docker.go b/internal/docker/docker.go index 0aab565d..4805c4fb 100644 --- a/internal/docker/docker.go +++ b/internal/docker/docker.go @@ -26,6 +26,7 @@ import ( "encoding/json" "fmt" "io" + "maps" "path" "path/filepath" "slices" @@ -377,6 +378,16 @@ func RunWithExtraHosts(hosts []string) RunContainerOption { } } +// RunWithLabels adds labels to the container. +func RunWithLabels(labels map[string]string) RunContainerOption { + return func(cfg *runContainerConfig) { + if cfg.containerConfig.Labels == nil { + cfg.containerConfig.Labels = map[string]string{} + } + maps.Copy(cfg.containerConfig.Labels, labels) + } +} + // RunWithBindMount adds a bind mount to the container. func RunWithBindMount(hostPath, containerPath string) RunContainerOption { return func(cfg *runContainerConfig) { diff --git a/internal/docker/docker_test.go b/internal/docker/docker_test.go index e194799f..6f24c178 100644 --- a/internal/docker/docker_test.go +++ b/internal/docker/docker_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" @@ -136,3 +137,33 @@ func TestStartAndAttach(t *testing.T) { }) } } + +func TestRunWithLabels(t *testing.T) { + cases := map[string]struct { + reason string + existing map[string]string + labels map[string]string + want map[string]string + }{ + "NilLabels": { + reason: "Labels should be set when the container config has none.", + labels: map[string]string{"a": "1"}, + want: map[string]string{"a": "1"}, + }, + "MergeLabels": { + reason: "Labels should be merged into existing labels.", + existing: map[string]string{"a": "1"}, + labels: map[string]string{"b": "2"}, + want: map[string]string{"a": "1", "b": "2"}, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + cfg := &runContainerConfig{containerConfig: &container.Config{Labels: tc.existing}} + RunWithLabels(tc.labels)(cfg) + if diff := cmp.Diff(tc.want, cfg.containerConfig.Labels); diff != "" { + t.Errorf("\n%s\nRunWithLabels(...): -want, +got:\n%s", tc.reason, diff) + } + }) + } +}