diff --git a/internal/docker/docker.go b/internal/docker/docker.go index 3ae056af..7b0f9a27 100644 --- a/internal/docker/docker.go +++ b/internal/docker/docker.go @@ -454,24 +454,23 @@ func RunContainer(ctx context.Context, img string, opts ...RunContainerOption) ( _, _ = cli.ContainerRemove(context.Background(), resp.ID, client.ContainerRemoveOptions{Force: true}) }() - // Attach before starting so we don't miss any output. Docker - // multiplexes stdout/stderr with 8-byte frame headers when the + // Start before attaching. Podman's Docker-compatible API rejects attach for + // a created container, while Docker supports both orderings. Request logs + // when attaching so output written between start and attach is not lost. + // Docker multiplexes stdout/stderr with 8-byte frame headers when the // container is not using a TTY. - attach, err := cli.ContainerAttach(ctx, resp.ID, client.ContainerAttachOptions{ - Stream: true, - Stdout: true, - Stderr: true, - Stdin: cfg.stdin != nil, + attach, err := startAndAttach(ctx, resp.ID, cfg.stdin != nil, runContainerCalls{ + start: func(ctx context.Context, id string, opts client.ContainerStartOptions) error { + _, err := cli.ContainerStart(ctx, id, opts) + return err + }, + attach: cli.ContainerAttach, }) if err != nil { - return nil, nil, errors.Wrap(err, "failed to attach to container") + return nil, nil, err } defer attach.Close() - if _, err := cli.ContainerStart(ctx, resp.ID, client.ContainerStartOptions{}); err != nil { - return nil, nil, errors.Wrap(err, "failed to start container") - } - // Write stdin data if provided, then close the write side so the // container sees EOF. if cfg.stdin != nil { @@ -507,6 +506,29 @@ func RunContainer(ctx context.Context, img string, opts ...RunContainerOption) ( return stdout.Bytes(), stderr.Bytes(), nil } +type runContainerCalls struct { + start func(context.Context, string, client.ContainerStartOptions) error + attach func(context.Context, string, client.ContainerAttachOptions) (client.ContainerAttachResult, error) +} + +// startAndAttach starts a container before attaching to its streams. Podman's +// Docker-compatible API does not support attaching to a created container. The +// Logs option ensures output produced between these two calls is replayed. +func startAndAttach(ctx context.Context, id string, stdin bool, calls runContainerCalls) (client.ContainerAttachResult, error) { + if err := calls.start(ctx, id, client.ContainerStartOptions{}); err != nil { + return client.ContainerAttachResult{}, errors.Wrap(err, "failed to start container") + } + + rsp, err := calls.attach(ctx, id, client.ContainerAttachOptions{ + Stream: true, + Stdout: true, + Stderr: true, + Stdin: stdin, + Logs: true, + }) + return rsp, errors.Wrap(err, "failed to attach to container") +} + // CopyFromContainer copies files from a container to an afero filesystem. func CopyFromContainer(ctx context.Context, cid, basePath string, fs afero.Fs) error { cli, err := NewClient() diff --git a/internal/docker/docker_test.go b/internal/docker/docker_test.go new file mode 100644 index 00000000..8a0f7592 --- /dev/null +++ b/internal/docker/docker_test.go @@ -0,0 +1,125 @@ +/* +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 docker + +import ( + "context" + "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" +) + +func TestStartAndAttach(t *testing.T) { + t.Parallel() + + errStart := errors.New("start failed") + errAttach := errors.New("attach failed") + + type args struct { + stdin bool + startErr error + attachErr error + } + type want struct { + calls []string + err error + options client.ContainerAttachOptions + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "Success": { + reason: "A container must be started before attaching, and attach must replay logs while streaming all configured channels.", + args: args{stdin: true}, + want: want{ + calls: []string{"start", "attach"}, + options: client.ContainerAttachOptions{ + Stream: true, + Stdout: true, + Stderr: true, + Stdin: true, + Logs: true, + }, + }, + }, + "StartFailureDoesNotAttach": { + reason: "Attaching cannot succeed when starting the container fails, so the start error must be preserved and attach must not be attempted.", + args: args{startErr: errStart}, + want: want{ + calls: []string{"start"}, + err: errStart, + }, + }, + "AttachFailure": { + reason: "An attach failure after a successful start must be preserved for callers.", + args: args{attachErr: errAttach}, + want: want{ + calls: []string{"start", "attach"}, + err: errAttach, + options: client.ContainerAttachOptions{ + Stream: true, + Stdout: true, + Stderr: true, + Logs: true, + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + calls := []string{} + var gotOptions client.ContainerAttachOptions + _, err := startAndAttach(t.Context(), "container-id", tc.args.stdin, runContainerCalls{ + start: func(_ context.Context, id string, _ client.ContainerStartOptions) error { + if diff := cmp.Diff("container-id", id); diff != "" { + t.Errorf("%s\nstart container ID: -want, +got:\n%s", tc.reason, diff) + } + calls = append(calls, "start") + return tc.args.startErr + }, + attach: func(_ context.Context, id string, opts client.ContainerAttachOptions) (client.ContainerAttachResult, error) { + if diff := cmp.Diff("container-id", id); diff != "" { + t.Errorf("%s\nattach container ID: -want, +got:\n%s", tc.reason, diff) + } + calls = append(calls, "attach") + gotOptions = opts + return client.ContainerAttachResult{}, tc.args.attachErr + }, + }) + + if diff := cmp.Diff(tc.want.calls, calls); diff != "" { + t.Errorf("%s\nstartAndAttach(...) calls: -want, +got:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.options, gotOptions); diff != "" { + t.Errorf("%s\nstartAndAttach(...) attach options: -want, +got:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("%s\nstartAndAttach(...): -want error, +got error:\n%s", tc.reason, diff) + } + }) + } +}