From a1b57f5cb221e599c9e7a72ed31e0990bedd2424 Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Mon, 7 Sep 2026 04:50:02 +0000 Subject: [PATCH 1/2] fix(build): always give go-dockerclient an output stream The vendored fsouza/go-dockerclient rejects BuildImage calls with a nil OutputStream before sending anything to the daemon, but dockercrtclient only set one when showBuildLogs was requested. A plain 'mint build' without --show-blogs therefore failed immediately with 'missing output stream'. Always buffer the build output into ref.buildLog, the same pattern slimbuilder.go already uses. The regression test drives the real BuildImage against the local Docker daemon (skipped when no daemon is reachable) with exactly the options shape buildFatImage constructs. Fixes #87 Signed-off-by: Eljees <3.14hell@gmail.com> --- .../docker/dockercrtclient/dockercrtclient.go | 6 +- pkg/crt/docker/dockercrtclient/repro_test.go | 70 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 pkg/crt/docker/dockercrtclient/repro_test.go diff --git a/pkg/crt/docker/dockercrtclient/dockercrtclient.go b/pkg/crt/docker/dockercrtclient/dockercrtclient.go index e1be5da20..77fac875f 100644 --- a/pkg/crt/docker/dockercrtclient/dockercrtclient.go +++ b/pkg/crt/docker/dockercrtclient/dockercrtclient.go @@ -106,7 +106,11 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err if options.OutputStream != nil { buildOptions.OutputStream = options.OutputStream - } else if ref.showBuildLogs { + } else { + // go-dockerclient's BuildImage() unconditionally rejects a nil + // OutputStream (see vendor/github.com/fsouza/go-dockerclient/image.go, + // ErrMissingOutputStream), so a destination must always be provided, + // not just when showBuildLogs is requested (mintoolkit/mint#87). ref.buildLog.Reset() buildOptions.OutputStream = &ref.buildLog } diff --git a/pkg/crt/docker/dockercrtclient/repro_test.go b/pkg/crt/docker/dockercrtclient/repro_test.go new file mode 100644 index 000000000..722ede1ff --- /dev/null +++ b/pkg/crt/docker/dockercrtclient/repro_test.go @@ -0,0 +1,70 @@ +package dockercrtclient + +import ( + "os" + "path/filepath" + "strings" + "testing" + + docker "github.com/fsouza/go-dockerclient" + + "github.com/mintoolkit/mint/pkg/imagebuilder" +) + +// TestBuildImage_MissingOutputStream_Issue87 reproduces mintoolkit/mint#87 +// ("Missing output stream"): building a Dockerfile-based ("fat") image +// through the internal build engine (dockercrtclient), WITHOUT passing an +// explicit OutputStream and WITHOUT --show-blogs (showBuildLogs=false), +// must not immediately fail with "missing output stream" coming from the +// vendored fsouza/go-dockerclient BuildImage(), which requires a non-nil +// OutputStream unconditionally. +func TestBuildImage_MissingOutputStream_Issue87(t *testing.T) { + client, err := docker.NewClientFromEnv() + if err != nil { + t.Fatalf("docker.NewClientFromEnv: %v", err) + } + if err := client.Ping(); err != nil { + t.Skipf("no docker daemon reachable, skipping live repro: %v", err) + } + + dir := t.TempDir() + dockerfile := "FROM scratch\nCOPY hello.txt /hello.txt\n" + if err := os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte(dockerfile), 0o644); err != nil { + t.Fatalf("write Dockerfile: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "hello.txt"), []byte("hello\n"), 0o644); err != nil { + t.Fatalf("write hello.txt: %v", err) + } + + b := NewBuilder(client, false) // showBuildLogs=false, matches the user's repro command (no --show-blogs) + + opts := imagebuilder.DockerfileBuildOptions{ + Dockerfile: "Dockerfile", + BuildContext: dir, + ImagePath: "el-mint87-repro:latest", + // OutputStream intentionally left nil - this is the exact shape + // buildFatImage() in pkg/app/master/command/build/image.go constructs. + } + + buildErr := b.BuildImage(opts) + defer client.RemoveImage("el-mint87-repro:latest") + + if buildErr != nil { + if strings.Contains(buildErr.Error(), "missing output stream") { + t.Fatalf("REPRODUCED issue #87: BuildImage failed with %q (options.OutputStream was nil and showBuildLogs=false, "+ + "so dockercrtclient never set buildOptions.OutputStream, and vendor/github.com/fsouza/go-dockerclient "+ + "image.go:538-540 rejects the whole build before running anything)", buildErr) + } + t.Fatalf("BuildImage failed for an unrelated reason: %v", buildErr) + } + + // GREEN-path sanity: image must actually exist and the build log must be + // non-empty even though showBuildLogs was false (fix always buffers into + // ref.buildLog, matching the pattern already used by slimbuilder.go:267). + if _, err := client.InspectImage("el-mint87-repro:latest"); err != nil { + t.Fatalf("image was not built despite BuildImage returning nil error: %v", err) + } + if b.BuildOutputLog() == "" { + t.Fatalf("expected non-empty build log to be captured even with showBuildLogs=false") + } +} From 7d1b2f7620c5adf830e145b253421d82666c74ca Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Tue, 8 Sep 2026 13:04:11 +0000 Subject: [PATCH 2/2] review: reset the build log on every build, isolate the test image Reset ref.buildLog unconditionally so BuildOutputLog() always reflects the current build, including when the caller supplies its own OutputStream -- previously the buffer was only reset on the showBuildLogs path, so a custom-stream build left the previous build's output readable. The regression test now builds a uniquely named image and removes it only when it actually created one, so it cannot touch a pre-existing image or collide with a parallel run. Signed-off-by: Eljees <3.14hell@gmail.com> --- .../docker/dockercrtclient/dockercrtclient.go | 12 ++++--- pkg/crt/docker/dockercrtclient/repro_test.go | 34 ++++++++++++------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/pkg/crt/docker/dockercrtclient/dockercrtclient.go b/pkg/crt/docker/dockercrtclient/dockercrtclient.go index 77fac875f..2ede2eed4 100644 --- a/pkg/crt/docker/dockercrtclient/dockercrtclient.go +++ b/pkg/crt/docker/dockercrtclient/dockercrtclient.go @@ -104,14 +104,16 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err } } + // go-dockerclient's BuildImage() unconditionally rejects a nil OutputStream + // (see vendor/github.com/fsouza/go-dockerclient/image.go, + // ErrMissingOutputStream), so a destination must always be provided, not + // just when showBuildLogs is requested (mintoolkit/mint#87). The buffer is + // reset on every build so BuildOutputLog() can never return the output of + // a previous build, including when the caller supplies its own stream. + ref.buildLog.Reset() if options.OutputStream != nil { buildOptions.OutputStream = options.OutputStream } else { - // go-dockerclient's BuildImage() unconditionally rejects a nil - // OutputStream (see vendor/github.com/fsouza/go-dockerclient/image.go, - // ErrMissingOutputStream), so a destination must always be provided, - // not just when showBuildLogs is requested (mintoolkit/mint#87). - ref.buildLog.Reset() buildOptions.OutputStream = &ref.buildLog } diff --git a/pkg/crt/docker/dockercrtclient/repro_test.go b/pkg/crt/docker/dockercrtclient/repro_test.go index 722ede1ff..ad2c7f4f2 100644 --- a/pkg/crt/docker/dockercrtclient/repro_test.go +++ b/pkg/crt/docker/dockercrtclient/repro_test.go @@ -1,10 +1,12 @@ package dockercrtclient import ( + "fmt" "os" "path/filepath" "strings" "testing" + "time" docker "github.com/fsouza/go-dockerclient" @@ -21,7 +23,7 @@ import ( func TestBuildImage_MissingOutputStream_Issue87(t *testing.T) { client, err := docker.NewClientFromEnv() if err != nil { - t.Fatalf("docker.NewClientFromEnv: %v", err) + t.Skipf("no docker client available, skipping live repro: %v", err) } if err := client.Ping(); err != nil { t.Skipf("no docker daemon reachable, skipping live repro: %v", err) @@ -36,35 +38,43 @@ func TestBuildImage_MissingOutputStream_Issue87(t *testing.T) { t.Fatalf("write hello.txt: %v", err) } - b := NewBuilder(client, false) // showBuildLogs=false, matches the user's repro command (no --show-blogs) + // Unique per run so the test can never overwrite or delete an image that + // happens to exist on the developer's machine, and so parallel runs on the + // same daemon don't collide. + imageTag := fmt.Sprintf("mint-issue87-repro-%d:test", time.Now().UnixNano()) + + b := NewBuilder(client, false) // showBuildLogs=false, matches the report (no --show-blogs) opts := imagebuilder.DockerfileBuildOptions{ Dockerfile: "Dockerfile", BuildContext: dir, - ImagePath: "el-mint87-repro:latest", + ImagePath: imageTag, // OutputStream intentionally left nil - this is the exact shape // buildFatImage() in pkg/app/master/command/build/image.go constructs. } buildErr := b.BuildImage(opts) - defer client.RemoveImage("el-mint87-repro:latest") + if buildErr == nil { + // Only clean up an image this test actually created. + t.Cleanup(func() { + if err := client.RemoveImage(imageTag); err != nil { + t.Logf("could not remove test image %s: %v", imageTag, err) + } + }) + } if buildErr != nil { if strings.Contains(buildErr.Error(), "missing output stream") { - t.Fatalf("REPRODUCED issue #87: BuildImage failed with %q (options.OutputStream was nil and showBuildLogs=false, "+ - "so dockercrtclient never set buildOptions.OutputStream, and vendor/github.com/fsouza/go-dockerclient "+ - "image.go:538-540 rejects the whole build before running anything)", buildErr) + t.Fatalf("BuildImage failed with %q: options.OutputStream was nil and showBuildLogs=false, "+ + "so no output destination was set and go-dockerclient rejected the build before running it", buildErr) } t.Fatalf("BuildImage failed for an unrelated reason: %v", buildErr) } - // GREEN-path sanity: image must actually exist and the build log must be - // non-empty even though showBuildLogs was false (fix always buffers into - // ref.buildLog, matching the pattern already used by slimbuilder.go:267). - if _, err := client.InspectImage("el-mint87-repro:latest"); err != nil { + if _, err := client.InspectImage(imageTag); err != nil { t.Fatalf("image was not built despite BuildImage returning nil error: %v", err) } if b.BuildOutputLog() == "" { - t.Fatalf("expected non-empty build log to be captured even with showBuildLogs=false") + t.Fatalf("expected the build log to be captured even with showBuildLogs=false") } }