diff --git a/internal/docker/docker.go b/internal/docker/docker.go index 3ae056a..7b0f9a2 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 0000000..04cfcb6 --- /dev/null +++ b/internal/docker/docker_test.go @@ -0,0 +1,105 @@ +/* +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" + "errors" + "strings" + "testing" + + "github.com/moby/moby/client" +) + +func TestStartAndAttach(t *testing.T) { + errStart := errors.New("start failed") + errAttach := errors.New("attach failed") + + cases := map[string]struct { + stdin bool + startErr error + attachErr error + wantCalls []string + wantErr string + wantOptions client.ContainerAttachOptions + }{ + "Success": { + stdin: true, + wantCalls: []string{"start", "attach"}, + wantOptions: client.ContainerAttachOptions{ + Stream: true, + Stdout: true, + Stderr: true, + Stdin: true, + Logs: true, + }, + }, + "StartFailureDoesNotAttach": { + startErr: errStart, + wantCalls: []string{"start"}, + wantErr: "failed to start container: start failed", + }, + "AttachFailure": { + attachErr: errAttach, + wantCalls: []string{"start", "attach"}, + wantErr: "failed to attach to container: attach failed", + wantOptions: client.ContainerAttachOptions{ + Stream: true, + Stdout: true, + Stderr: true, + Logs: true, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + calls := []string{} + var gotOptions client.ContainerAttachOptions + _, err := startAndAttach(context.Background(), "container-id", tc.stdin, runContainerCalls{ + start: func(_ context.Context, id string, _ client.ContainerStartOptions) error { + if id != "container-id" { + t.Errorf("start id = %q, want container-id", id) + } + calls = append(calls, "start") + return tc.startErr + }, + attach: func(_ context.Context, id string, opts client.ContainerAttachOptions) (client.ContainerAttachResult, error) { + if id != "container-id" { + t.Errorf("attach id = %q, want container-id", id) + } + calls = append(calls, "attach") + gotOptions = opts + return client.ContainerAttachResult{}, tc.attachErr + }, + }) + + if strings.Join(calls, ",") != strings.Join(tc.wantCalls, ",") { + t.Errorf("calls = %v, want %v", calls, tc.wantCalls) + } + if gotOptions != tc.wantOptions { + t.Errorf("attach options = %+v, want %+v", gotOptions, tc.wantOptions) + } + switch { + case tc.wantErr == "" && err != nil: + t.Fatalf("unexpected error: %v", err) + case tc.wantErr != "" && (err == nil || err.Error() != tc.wantErr): + t.Fatalf("error = %v, want %q", err, tc.wantErr) + } + }) + } +}