diff --git a/cmd/crossplane/render/engine_docker.go b/cmd/crossplane/render/engine_docker.go index 9beb3e8c..af5ed8fc 100644 --- a/cmd/crossplane/render/engine_docker.go +++ b/cmd/crossplane/render/engine_docker.go @@ -21,6 +21,7 @@ import ( "os" "runtime" "strings" + "time" "google.golang.org/protobuf/proto" @@ -48,6 +49,10 @@ func (realContainerRunner) Run(ctx context.Context, img string, opts ...docker.R return docker.RunContainer(ctx, img, opts...) } +// networkRemoveTimeout bounds how long Setup's cleanup waits to remove the +// temporary render network. +const networkRemoveTimeout = 30 * time.Second + // dockerRenderEngine executes crossplane internal render in a Docker container. type dockerRenderEngine struct { // image is the Crossplane Docker image reference. @@ -63,6 +68,13 @@ type dockerRenderEngine struct { // (exit-3 partial output, *docker.ContainerExitError vs non-exit errors) // without a real Docker daemon. 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. + networks networkClient } func (e *dockerRenderEngine) CheckContextSupport() error { @@ -95,7 +107,16 @@ func (e *dockerRenderEngine) Setup(ctx context.Context, fns []pkgv1.Function) (f return func() {}, nil } - networkID, networkName, err := createRenderNetwork(ctx) + cli := e.networks + if cli == nil { + c, err := newNetworkClient() + if err != nil { + return func() {}, errors.Wrap(err, "cannot create Docker network for rendering") + } + cli = c + } + + networkID, networkName, err := createRenderNetwork(ctx, cli) if err != nil { return func() {}, errors.Wrap(err, "cannot create Docker network for rendering") } @@ -103,8 +124,17 @@ func (e *dockerRenderEngine) Setup(ctx context.Context, fns []pkgv1.Function) (f injectNetworkAnnotation(fns, networkName) - cleanup := func() { //nolint:contextcheck // Detached context for cleanup. - _ = removeRenderNetwork(context.Background(), networkID) + cleanup := func() { + // Derive from ctx without its cancellation: cleanup typically runs after + // the caller's context is done, but must still be bounded so removal + // can't hang forever. + rctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), networkRemoveTimeout) + defer cancel() + if err := removeRenderNetwork(rctx, cli, networkID); err != nil { + // The cleanup signature can't return the error, so log it rather than + // silently leaking the network (e.g. a container is still attached). + e.log.Info("Cannot remove Docker network used for rendering", "network", networkName, "id", networkID, "error", err) + } } return cleanup, nil diff --git a/cmd/crossplane/render/engine_docker_test.go b/cmd/crossplane/render/engine_docker_test.go index 73fcdddb..970a7a21 100644 --- a/cmd/crossplane/render/engine_docker_test.go +++ b/cmd/crossplane/render/engine_docker_test.go @@ -23,10 +23,13 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/moby/moby/client" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/structpb" + "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" @@ -222,9 +225,8 @@ func TestDockerRenderEngineSetup(t *testing.T) { // call on the same engine stored its created network there. The branch // must annotate the supplied functions so their containers join the // network, never create a second network, and always return a no-op - // cleanup. The create-new-network branch is not covered here because it - // depends on a live Docker daemon; the broader render command tests - // exercise it integration-style. + // cleanup. The create-new-network branch is covered separately by + // TestDockerRenderEngineSetupCreatesNetwork. // // The MultiBatchAnnotatesAdditionalFunctions case simulates the // in-process multi-composition use case from crossplane/cli#96: a @@ -346,6 +348,170 @@ func TestDockerRenderEngineSetup(t *testing.T) { } } +func TestDockerRenderEngineSetupCreatesNetwork(t *testing.T) { + // When e.network is unset, Setup must create a temporary network through + // the engine's network client, record its name, annotate the supplied + // functions to join it, and return a cleanup that removes it through the + // same client. + errBoom := errors.New("boom") + + type args struct { + create func(ctx context.Context, name string, options client.NetworkCreateOptions) (client.NetworkCreateResult, error) + // removeErr is what NetworkRemove returns. When nil, NetworkRemove + // returns the error of the context it is called with, so cleanup + // logs an error if it removes the network with a cancelled context. + removeErr error + // cancel cancels Setup's context before calling cleanup. + cancel bool + } + type want struct { + err error + // created is whether Setup should create a network, annotate the + // functions to join it, and return a cleanup that removes it. + created bool + // logErr is the error cleanup should log when it cannot remove the + // network. When nil, cleanup must log nothing. + logErr error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "CreatesNetwork": { + reason: "Setup should create a network, annotate the functions to join it, and return a cleanup that removes it.", + args: args{ + create: createRenderNetworkReturns("network-id", nil), + }, + want: want{ + created: true, + }, + }, + "CleanupLogsRemoveError": { + reason: "The cleanup can't return an error, so it should log a failure to remove the network, with the network's name and ID, rather than discard it.", + args: args{ + create: createRenderNetworkReturns("network-id", nil), + removeErr: errBoom, + }, + want: want{ + created: true, + logErr: errBoom, + }, + }, + "CleanupSurvivesCancelledContext": { + reason: "The cleanup typically runs after Setup's context is done, so it should still remove the network with a live context.", + args: args{ + create: createRenderNetworkReturns("network-id", nil), + cancel: true, + }, + want: want{ + created: true, + }, + }, + "NetworkCreateError": { + reason: "Setup should return an error and a no-op cleanup, leaving the functions unannotated, when it cannot create the network.", + args: args{ + create: createRenderNetworkReturns("", errBoom), + }, + want: want{ + err: errBoom, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + // Cleanup discards NetworkRemove's result, so record whether it + // was called. A cleanup that shouldn't remove anything leaves + // MockNetworkRemove nil. + removed := false + cli := &mockNetworkClient{MockNetworkCreate: tc.args.create} + if tc.want.created { + cli.MockNetworkRemove = func(ctx context.Context, networkID string, _ client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) { + if diff := cmp.Diff("network-id", networkID); diff != "" { + t.Errorf("\n%s\nNetworkRemove(...): -want network ID, +got network ID:\n%s", tc.reason, diff) + } + removed = true + if tc.args.removeErr != nil { + return client.NetworkRemoveResult{}, tc.args.removeErr + } + return client.NetworkRemoveResult{}, ctx.Err() + } + } + log := newRecordingLogger() + e := &dockerRenderEngine{log: log, networks: cli} + fns := []pkgv1.Function{functionWithAnnotations(nil)} + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + cleanup, err := e.Setup(ctx, fns) + + if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("\n%s\nSetup(...): -want error, +got error:\n%s", tc.reason, diff) + } + + wantFns := []pkgv1.Function{functionWithAnnotations(nil)} + if tc.want.created { + if !strings.HasPrefix(e.network, renderNetworkPrefix) { + t.Errorf("\n%s\nSetup(...): e.network = %q, want a network with prefix %q", tc.reason, e.network, renderNetworkPrefix) + } + wantFns = []pkgv1.Function{functionWithAnnotations(map[string]string{AnnotationKeyRuntimeDockerNetwork: e.network})} + } else if e.network != "" { + t.Errorf("\n%s\nSetup(...): e.network = %q, want it unset", tc.reason, e.network) + } + if diff := cmp.Diff(wantFns, fns); diff != "" { + t.Errorf("\n%s\nSetup(...): -want fns, +got fns:\n%s", tc.reason, diff) + } + + if tc.args.cancel { + cancel() + } + cleanup() + + if diff := cmp.Diff(tc.want.created, removed); diff != "" { + t.Errorf("\n%s\nSetup(...) cleanup: -want network removed, +got network removed:\n%s", tc.reason, diff) + } + var wantLog []logEntry + if tc.want.logErr != nil { + wantLog = []logEntry{{ + Msg: "Cannot remove Docker network used for rendering", + KV: []any{"network", e.network, "id", "network-id", "error", tc.want.logErr}, + }} + } + if diff := cmp.Diff(wantLog, *log.entries, cmpopts.EquateErrors(), cmpopts.EquateEmpty()); diff != "" { + t.Errorf("\n%s\nSetup(...) cleanup: -want log, +got log:\n%s", tc.reason, diff) + } + }) + } +} + +// recordingLogger is a logging.Logger that records every Info and Debug call. +type recordingLogger struct { + entries *[]logEntry + kv []any +} + +type logEntry struct { + Msg string + KV []any +} + +var _ logging.Logger = recordingLogger{} + +func newRecordingLogger() recordingLogger { return recordingLogger{entries: &[]logEntry{}} } + +func (l recordingLogger) Info(msg string, kv ...any) { + *l.entries = append(*l.entries, logEntry{Msg: msg, KV: append(append([]any{}, l.kv...), kv...)}) +} + +func (l recordingLogger) Debug(msg string, kv ...any) { l.Info(msg, kv...) } + +func (l recordingLogger) WithValues(kv ...any) logging.Logger { + return recordingLogger{entries: l.entries, kv: append(append([]any{}, l.kv...), kv...)} +} + // nonExitError is a stand-in for non-*ContainerExitError failures (e.g. image // pull errors) returned by docker.RunContainer. type nonExitError struct{ msg string } diff --git a/cmd/crossplane/render/network.go b/cmd/crossplane/render/network.go index 454b122a..8ef17d68 100644 --- a/cmd/crossplane/render/network.go +++ b/cmd/crossplane/render/network.go @@ -46,15 +46,28 @@ func (f *EngineFlags) SetDefaultCrossplaneDockerNetwork(fns []pkgv1.Function) { } } -// createRenderNetwork creates a temporary Docker bridge network for render. -// Function containers and the Crossplane render container join this network so -// they can reach each other. Returns the network ID and name. -func createRenderNetwork(ctx context.Context) (string, string, error) { +// networkClient is the subset of the Docker client the render engine uses to +// manage its temporary Docker network. +type networkClient interface { + NetworkCreate(ctx context.Context, name string, options client.NetworkCreateOptions) (client.NetworkCreateResult, error) + NetworkRemove(ctx context.Context, networkID string, options client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) +} + +var _ networkClient = (*client.Client)(nil) + +// newNetworkClient returns a real Docker client built from the environment. +func newNetworkClient() (networkClient, error) { cli, err := docker.NewClient() if err != nil { - return "", "", errors.Wrap(err, "cannot create Docker client") + return nil, errors.Wrap(err, "cannot create Docker client") } + return cli, nil +} +// createRenderNetwork creates a temporary Docker bridge network for render. +// Function containers and the Crossplane render container join this network so +// they can reach each other. Returns the network ID and name. +func createRenderNetwork(ctx context.Context, cli networkClient) (string, string, error) { name := fmt.Sprintf("crossplane-render-%s", rand.String(8)) resp, err := cli.NetworkCreate(ctx, name, client.NetworkCreateOptions{ @@ -68,11 +81,7 @@ func createRenderNetwork(ctx context.Context) (string, string, error) { } // removeRenderNetwork removes a temporary Docker network. -func removeRenderNetwork(ctx context.Context, networkID string) error { - cli, err := docker.NewClient() - if err != nil { - return errors.Wrap(err, "cannot create Docker client") - } - _, err = cli.NetworkRemove(ctx, networkID, client.NetworkRemoveOptions{}) +func removeRenderNetwork(ctx context.Context, cli networkClient, networkID string) error { + _, err := cli.NetworkRemove(ctx, networkID, client.NetworkRemoveOptions{}) return errors.Wrap(err, "cannot remove Docker network") } diff --git a/cmd/crossplane/render/network_test.go b/cmd/crossplane/render/network_test.go index fde13f0f..843f16f4 100644 --- a/cmd/crossplane/render/network_test.go +++ b/cmd/crossplane/render/network_test.go @@ -1,9 +1,15 @@ package render import ( + "context" + "strings" "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/moby/moby/client" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1" ) @@ -74,3 +80,152 @@ func TestSetDefaultCrossplaneDockerNetwork(t *testing.T) { }) } } + +type mockNetworkClient struct { + MockNetworkCreate func(ctx context.Context, name string, options client.NetworkCreateOptions) (client.NetworkCreateResult, error) + MockNetworkRemove func(ctx context.Context, networkID string, options client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) +} + +func (m *mockNetworkClient) NetworkCreate(ctx context.Context, name string, options client.NetworkCreateOptions) (client.NetworkCreateResult, error) { + return m.MockNetworkCreate(ctx, name, options) +} + +func (m *mockNetworkClient) NetworkRemove(ctx context.Context, networkID string, options client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) { + return m.MockNetworkRemove(ctx, networkID, options) +} + +var _ networkClient = &mockNetworkClient{} + +// renderNetworkPrefix is the prefix of the temporary network name +// createRenderNetwork generates. +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. +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 != "" { + return client.NetworkCreateResult{}, errors.Errorf("NetworkCreate(...): -want options, +got options:\n%s", diff) + } + return client.NetworkCreateResult{ID: id}, err + } +} + +func TestCreateRenderNetwork(t *testing.T) { + errBoom := errors.New("boom") + + type args struct { + cli networkClient + } + type want struct { + id string + // namePrefix is a prefix the returned network name must have. The + // rest of the name is random. + namePrefix string + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "CreatesBridgeNetwork": { + reason: "createRenderNetwork should create a uniquely named bridge network through the supplied client and return its ID and name.", + args: args{ + cli: &mockNetworkClient{MockNetworkCreate: createRenderNetworkReturns("network-id", nil)}, + }, + want: want{ + id: "network-id", + namePrefix: renderNetworkPrefix, + }, + }, + "NetworkCreateError": { + reason: "createRenderNetwork should return an error when the client cannot create the network.", + args: args{ + cli: &mockNetworkClient{MockNetworkCreate: createRenderNetworkReturns("", errBoom)}, + }, + want: want{ + err: errBoom, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + id, networkName, err := createRenderNetwork(t.Context(), tc.args.cli) + + if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("\n%s\ncreateRenderNetwork(...): -want error, +got error:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.id, id); diff != "" { + t.Errorf("\n%s\ncreateRenderNetwork(...): -want ID, +got ID:\n%s", tc.reason, diff) + } + if !strings.HasPrefix(networkName, tc.want.namePrefix) { + t.Errorf("\n%s\ncreateRenderNetwork(...): name %q does not have prefix %q", tc.reason, networkName, tc.want.namePrefix) + } + }) + } +} + +func TestRemoveRenderNetwork(t *testing.T) { + errBoom := errors.New("boom") + + // removeNetworkReturns returns a MockNetworkRemove that returns the + // supplied error, or an error of its own when asked to remove any network + // other than network-id. + removeNetworkReturns := func(err error) func(context.Context, string, client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) { + return func(_ context.Context, networkID string, _ client.NetworkRemoveOptions) (client.NetworkRemoveResult, error) { + if diff := cmp.Diff("network-id", networkID); diff != "" { + return client.NetworkRemoveResult{}, errors.Errorf("NetworkRemove(...): -want network ID, +got network ID:\n%s", diff) + } + return client.NetworkRemoveResult{}, err + } + } + + type args struct { + cli networkClient + networkID string + } + type want struct { + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "RemovesNetwork": { + reason: "removeRenderNetwork should remove the network with the supplied ID through the supplied client.", + args: args{ + cli: &mockNetworkClient{MockNetworkRemove: removeNetworkReturns(nil)}, + networkID: "network-id", + }, + }, + "NetworkRemoveError": { + reason: "removeRenderNetwork should return an error when the client cannot remove the network.", + args: args{ + cli: &mockNetworkClient{MockNetworkRemove: removeNetworkReturns(errBoom)}, + networkID: "network-id", + }, + want: want{ + err: errBoom, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := removeRenderNetwork(t.Context(), tc.args.cli, tc.args.networkID) + + if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("\n%s\nremoveRenderNetwork(...): -want error, +got error:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/cmd/crossplane/render/runtime_docker.go b/cmd/crossplane/render/runtime_docker.go index 169fe3f5..8972945b 100644 --- a/cmd/crossplane/render/runtime_docker.go +++ b/cmd/crossplane/render/runtime_docker.go @@ -161,8 +161,33 @@ type RuntimeDocker struct { // and is reached via host port bindings. When set, the container joins // the specified network and is reached via its Docker hostname on port 9443. Network string + + // dockerClient manages the Function's container. Production callers leave + // it nil and Start builds a real client from the environment. Tests + // substitute a fake to exercise container lifecycle and cleanup handling + // without a real Docker daemon. + dockerClient containerClient +} + +// containerClient is the subset of the Docker client RuntimeDocker uses to +// pull images and manage a Function's container. +type containerClient interface { + pullClient + containerCleanupClient + ContainerInspect(ctx context.Context, containerID string, options client.ContainerInspectOptions) (client.ContainerInspectResult, error) + ContainerCreate(ctx context.Context, options client.ContainerCreateOptions) (client.ContainerCreateResult, error) + ContainerStart(ctx context.Context, containerID string, options client.ContainerStartOptions) (client.ContainerStartResult, error) +} + +// containerCleanupClient is the subset of the Docker client RuntimeDocker's +// stop function uses to clean up a Function's container. +type containerCleanupClient 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) } +var _ containerClient = (*client.Client)(nil) + // GetDockerPullPolicy extracts PullPolicy configuration from the supplied // Function. func GetDockerPullPolicy(fn pkgv1.Function) (DockerPullPolicy, error) { @@ -250,7 +275,7 @@ func GetRuntimeDocker(fn pkgv1.Function, log logging.Logger) (*RuntimeDocker, er var _ Runtime = &RuntimeDocker{} -func (r *RuntimeDocker) findContainer(ctx context.Context, cli *client.Client) (string, error) { +func (r *RuntimeDocker) findContainer(ctx context.Context, cli containerClient) (string, error) { if r.Name == "" { return "", nil } @@ -266,7 +291,7 @@ func (r *RuntimeDocker) findContainer(ctx context.Context, cli *client.Client) ( return inspect.Container.ID, nil } -func (r *RuntimeDocker) createContainer(ctx context.Context, cli *client.Client) (string, error) { +func (r *RuntimeDocker) createContainer(ctx context.Context, cli containerClient) (string, error) { r.log.Debug("Starting Docker container runtime setup", "image", r.Image) // Let Docker automatically allocate an available port on the bind address. @@ -377,7 +402,7 @@ func (r *RuntimeDocker) createContainer(ctx context.Context, cli *client.Client) } // startContainer ensures the container is running and returns its address. -func (r *RuntimeDocker) startContainer(ctx context.Context, cli *client.Client, containerID string) (string, error) { +func (r *RuntimeDocker) startContainer(ctx context.Context, cli containerClient, containerID 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") @@ -457,9 +482,13 @@ func (r *RuntimeDocker) getPullOptions() (client.ImagePullOptions, error) { // Start a Function as a Docker container. func (r *RuntimeDocker) Start(ctx context.Context) (RuntimeContext, error) { - cli, err := client.New(client.FromEnv) - if err != nil { - return RuntimeContext{}, errors.Wrap(err, "cannot create Docker client using environment variables") + cli := r.dockerClient + if cli == nil { + c, err := client.New(client.FromEnv) + if err != nil { + return RuntimeContext{}, errors.Wrap(err, "cannot create Docker client using environment variables") + } + cli = c } // Try to find an existing container with the supplied container name. diff --git a/cmd/crossplane/render/runtime_docker_test.go b/cmd/crossplane/render/runtime_docker_test.go index a05a6079..68c2e90c 100644 --- a/cmd/crossplane/render/runtime_docker_test.go +++ b/cmd/crossplane/render/runtime_docker_test.go @@ -18,13 +18,20 @@ package render import ( "context" + "io" + "net" + "strconv" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/google/go-containerregistry/pkg/authn" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/network" "github.com/moby/moby/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/logging" pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1" @@ -40,6 +47,95 @@ func (m *mockPullClient) ImagePull(ctx context.Context, ref string, options clie var _ pullClient = &mockPullClient{} +type mockContainerClient struct { + MockImagePull func(ctx context.Context, ref string, options client.ImagePullOptions) (client.ImagePullResponse, error) + MockContainerInspect func(ctx context.Context, containerID string, options client.ContainerInspectOptions) (client.ContainerInspectResult, error) + MockContainerCreate func(ctx context.Context, options client.ContainerCreateOptions) (client.ContainerCreateResult, error) + 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) +} + +func (m *mockContainerClient) ImagePull(ctx context.Context, ref string, options client.ImagePullOptions) (client.ImagePullResponse, error) { + return m.MockImagePull(ctx, ref, options) +} + +func (m *mockContainerClient) ContainerInspect(ctx context.Context, containerID string, options client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + return m.MockContainerInspect(ctx, containerID, options) +} + +func (m *mockContainerClient) ContainerCreate(ctx context.Context, options client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + return m.MockContainerCreate(ctx, options) +} + +func (m *mockContainerClient) ContainerStart(ctx context.Context, containerID string, options client.ContainerStartOptions) (client.ContainerStartResult, error) { + return m.MockContainerStart(ctx, containerID, options) +} + +func (m *mockContainerClient) ContainerStop(ctx context.Context, containerID string, options client.ContainerStopOptions) (client.ContainerStopResult, error) { + return m.MockContainerStop(ctx, containerID, options) +} + +func (m *mockContainerClient) ContainerRemove(ctx context.Context, containerID string, options client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + return m.MockContainerRemove(ctx, containerID, options) +} + +var _ containerClient = &mockContainerClient{} + +// imagePullDone is a MockImagePull that reports an image pull that has +// already completed. +func imagePullDone(context.Context, string, client.ImagePullOptions) (client.ImagePullResponse, error) { + return pullDoneResponse{}, nil +} + +// pullDoneResponse is an ImagePullResponse whose body is empty. Only the +// io.ReadCloser methods PullImage uses are implemented; the embedded +// interface is nil. +type pullDoneResponse struct { + client.ImagePullResponse +} + +func (pullDoneResponse) Read([]byte) (int, error) { return 0, io.EOF } + +func (pullDoneResponse) Close() error { return nil } + +// createContainerReturns returns a MockContainerCreate that creates a +// container with the supplied ID. +func createContainerReturns(id string) func(context.Context, client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + return func(context.Context, client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + return client.ContainerCreateResult{ID: id}, nil + } +} + +// startContainer returns a MockContainerStart that starts the container with +// the supplied ID, and returns an error for any other container. +func startContainer(id string) func(context.Context, string, client.ContainerStartOptions) (client.ContainerStartResult, error) { + return func(_ context.Context, containerID string, _ client.ContainerStartOptions) (client.ContainerStartResult, error) { + if diff := cmp.Diff(id, containerID); diff != "" { + return client.ContainerStartResult{}, errors.Errorf("ContainerStart(...): -want container ID, +got container ID:\n%s", diff) + } + return client.ContainerStartResult{}, nil + } +} + +// inspectContainerOnNetwork returns a MockContainerInspect that reports the +// container with the supplied ID as running with the supplied name, attached +// to the supplied Docker network. It returns an error for any other container. +func inspectContainerOnNetwork(id, name, networkName string) func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + return func(_ context.Context, containerID string, _ client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + if diff := cmp.Diff(id, containerID); diff != "" { + return client.ContainerInspectResult{}, errors.Errorf("ContainerInspect(...): -want container ID, +got container ID:\n%s", diff) + } + return client.ContainerInspectResult{Container: container.InspectResponse{ + ID: id, + Name: "/" + name, + NetworkSettings: &container.NetworkSettings{ + Networks: map[string]*network.EndpointSettings{networkName: {}}, + }, + }}, nil + } +} + func TestGetRuntimeDocker(t *testing.T) { type args struct { fn pkgv1.Function @@ -246,3 +342,263 @@ func TestGetRuntimeDocker(t *testing.T) { }) } } + +func TestRuntimeDockerStart(t *testing.T) { + const ( + image = "xpkg.crossplane.io/crossplane-contrib/function-dummy:v0.1.0" + containerID = "container-id" + containerName = "fn-container" + dockerNetwork = "render-net" + ) + + errBoom := errors.New("boom") + + type args struct { + pullPolicy DockerPullPolicy + cli *mockContainerClient + } + type want struct { + rctx RuntimeContext + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "CreatesAndStartsContainer": { + reason: "Start should create, start, and inspect the Function's container through the injected client, without pulling an image that is present.", + args: args{ + pullPolicy: AnnotationValueRuntimeDockerPullPolicyIfNotPresent, + cli: &mockContainerClient{ + MockContainerCreate: createContainerReturns(containerID), + MockContainerStart: startContainer(containerID), + MockContainerInspect: inspectContainerOnNetwork(containerID, containerName, dockerNetwork), + }, + }, + want: want{ + rctx: RuntimeContext{Target: net.JoinHostPort(containerName, strconv.Itoa(FunctionPort))}, + }, + }, + "PullsImage": { + reason: "Start should pull the Function's image through the injected client when the pull policy is Always.", + args: args{ + pullPolicy: AnnotationValueRuntimeDockerPullPolicyAlways, + cli: &mockContainerClient{ + MockImagePull: func(ctx context.Context, ref string, options client.ImagePullOptions) (client.ImagePullResponse, error) { + if diff := cmp.Diff(image, ref); diff != "" { + return nil, errors.Errorf("ImagePull(...): -want ref, +got ref:\n%s", diff) + } + return imagePullDone(ctx, ref, options) + }, + MockContainerCreate: createContainerReturns(containerID), + MockContainerStart: startContainer(containerID), + MockContainerInspect: inspectContainerOnNetwork(containerID, containerName, dockerNetwork), + }, + }, + want: want{ + rctx: RuntimeContext{Target: net.JoinHostPort(containerName, strconv.Itoa(FunctionPort))}, + }, + }, + "PullError": { + reason: "Start should return an error, without creating a container, when the injected client cannot pull the image.", + args: args{ + pullPolicy: AnnotationValueRuntimeDockerPullPolicyAlways, + cli: &mockContainerClient{ + MockImagePull: func(context.Context, string, client.ImagePullOptions) (client.ImagePullResponse, error) { + return nil, errBoom + }, + }, + }, + want: want{ + err: errBoom, + }, + }, + "StartError": { + reason: "Start should return an error when the injected client cannot start the container.", + args: args{ + pullPolicy: AnnotationValueRuntimeDockerPullPolicyIfNotPresent, + cli: &mockContainerClient{ + MockContainerCreate: createContainerReturns(containerID), + MockContainerStart: func(context.Context, string, client.ContainerStartOptions) (client.ContainerStartResult, error) { + return client.ContainerStartResult{}, errBoom + }, + }, + }, + want: want{ + err: errBoom, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := &RuntimeDocker{ + Image: image, + Cleanup: AnnotationValueRuntimeDockerCleanupRemove, + PullPolicy: tc.args.pullPolicy, + Keychain: authn.NewMultiKeychain(), + Network: dockerNetwork, + log: logging.NewNopLogger(), + dockerClient: tc.args.cli, + } + + rctx, 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) + } + if diff := cmp.Diff(tc.want.rctx, rctx, cmpopts.IgnoreFields(RuntimeContext{}, "Stop")); diff != "" { + t.Errorf("\n%s\nStart(...): -want, +got:\n%s", tc.reason, diff) + } + }) + } +} + +func TestRuntimeDockerStop(t *testing.T) { + const ( + containerID = "container-id" + containerName = "fn-container" + dockerNetwork = "render-net" + ) + + errStop := errors.New("stop boom") + errRemove := errors.New("remove boom") + + type args struct { + cleanup DockerCleanup + // stop and remove are the cleanup client's ContainerStop and + // ContainerRemove. A nil mock must not be called. The test checks + // the arguments each non-nil mock is called with. + stop func(ctx context.Context, containerID string, options client.ContainerStopOptions) (client.ContainerStopResult, error) + remove func(ctx context.Context, containerID string, options client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) + } + type want struct { + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "Stop": { + reason: "The Stop cleanup policy should stop the container and leave it in place.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupStop, + stop: func(context.Context, string, client.ContainerStopOptions) (client.ContainerStopResult, error) { + return client.ContainerStopResult{}, nil + }, + }, + }, + "StopError": { + reason: "The Stop cleanup policy should return an error when the container cannot be stopped.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupStop, + stop: func(context.Context, string, client.ContainerStopOptions) (client.ContainerStopResult, error) { + return client.ContainerStopResult{}, errStop + }, + }, + want: want{ + err: errStop, + }, + }, + "Remove": { + reason: "The Remove cleanup policy should stop the container, then remove it.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupRemove, + stop: func(context.Context, string, client.ContainerStopOptions) (client.ContainerStopResult, error) { + return client.ContainerStopResult{}, nil + }, + remove: func(context.Context, string, client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + return client.ContainerRemoveResult{}, nil + }, + }, + }, + "RemoveStopError": { + reason: "The Remove cleanup policy should stop the container before removing it, and return an error without removing it when it cannot be stopped.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupRemove, + stop: func(context.Context, string, client.ContainerStopOptions) (client.ContainerStopResult, error) { + return client.ContainerStopResult{}, errStop + }, + }, + want: want{ + err: errStop, + }, + }, + "RemoveError": { + reason: "The Remove cleanup policy should return an error when the container cannot be removed.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupRemove, + stop: func(context.Context, string, client.ContainerStopOptions) (client.ContainerStopResult, error) { + return client.ContainerStopResult{}, nil + }, + remove: func(context.Context, string, client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + return client.ContainerRemoveResult{}, errRemove + }, + }, + want: want{ + err: errRemove, + }, + }, + "Orphan": { + reason: "The Orphan cleanup policy should leave the container running without calling Docker.", + args: args{ + cleanup: AnnotationValueRuntimeDockerCleanupOrphan, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + cli := &mockContainerClient{ + MockContainerCreate: createContainerReturns(containerID), + MockContainerStart: startContainer(containerID), + MockContainerInspect: inspectContainerOnNetwork(containerID, containerName, dockerNetwork), + } + if stop := tc.args.stop; stop != nil { + cli.MockContainerStop = func(ctx context.Context, id string, options client.ContainerStopOptions) (client.ContainerStopResult, error) { + if diff := cmp.Diff(containerID, id); diff != "" { + t.Errorf("\n%s\nContainerStop(...): -want container ID, +got container ID:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(client.ContainerStopOptions{}, options); diff != "" { + t.Errorf("\n%s\nContainerStop(...): -want options, +got options:\n%s", tc.reason, diff) + } + return stop(ctx, id, options) + } + } + if remove := tc.args.remove; remove != nil { + cli.MockContainerRemove = func(ctx context.Context, id string, options client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + if diff := cmp.Diff(containerID, id); diff != "" { + t.Errorf("\n%s\nContainerRemove(...): -want container ID, +got container ID:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(client.ContainerRemoveOptions{}, options); diff != "" { + t.Errorf("\n%s\nContainerRemove(...): -want options, +got options:\n%s", tc.reason, diff) + } + return remove(ctx, id, options) + } + } + 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: cli, + } + + rctx, err := r.Start(t.Context()) + if err != nil { + t.Fatalf("\n%s\nStart(...): unexpected error: %v", tc.reason, err) + } + + err = rctx.Stop(t.Context()) + if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("\n%s\nStop(...): -want error, +got error:\n%s", tc.reason, diff) + } + }) + } +}