From feaae223964c1d263e724b5ebdd4542c58664610 Mon Sep 17 00:00:00 2001 From: David Levy Date: Fri, 17 Apr 2026 11:40:54 -0500 Subject: [PATCH 1/5] fix: detect silent wget failures in container DownloadFile DownloadFile discarded wget's exit code, so failed downloads (e.g. unreachable host) produced no error. The caller then tried to restore a missing or empty .bak file, yielding a confusing 'volume is empty' message from SQL Server. Add ExecInspect to runCmdInContainer to capture the exit code. Check wget's exit code in DownloadFile and panic with stderr context on failure. Also adds mkdir -p for idempotent directory creation. Fixes #566 --- internal/container/controller.go | 35 ++++++++++++++++++--------- internal/container/controller_test.go | 12 +++++++-- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/internal/container/controller.go b/internal/container/controller.go index 4838eecc..7a039099 100644 --- a/internal/container/controller.go +++ b/internal/container/controller.go @@ -250,23 +250,27 @@ func (c Controller) DownloadFile(id string, src string, destFolder string) { panic("Must pass in non-empty destFolder") } - cmd := []string{"mkdir", destFolder} + cmd := []string{"mkdir", "-p", destFolder} c.runCmdInContainer(id, cmd) _, file := filepath.Split(src) - - // Wget the .bak file from the http src, and place it in /var/opt/sql/backup - cmd = []string{ - "wget", - "-O", - destFolder + "/" + file, // not using filepath.Join here, this is in the *nix container. always / - src, + if file == "" { + panic("src URL has no filename: " + src) + } + dest := destFolder + "/" + file // not using filepath.Join here, this is in the *nix container. always / + + cmd = []string{"wget", "-O", dest, src} + _, stderr, exitCode := c.runCmdInContainer(id, cmd) + if exitCode != 0 { + msg := "download failed: " + src + if len(stderr) > 0 { + msg += "\nwget output: " + string(stderr) + } + panic(msg) } - - c.runCmdInContainer(id, cmd) } -func (c Controller) runCmdInContainer(id string, cmd []string) ([]byte, []byte) { +func (c Controller) runCmdInContainer(id string, cmd []string) ([]byte, []byte, int) { trace("Running command in container: " + strings.Join(cmd, " ")) response, err := c.cli.ExecCreate( @@ -308,7 +312,14 @@ func (c Controller) runCmdInContainer(id string, cmd []string) ([]byte, []byte) trace("Stdout: " + string(stdout)) trace("Stderr: " + string(stderr)) - return stdout, stderr + inspect, err := c.cli.ExecInspect( + context.Background(), + response.ID, + client.ExecInspectOptions{}, + ) + checkErr(err) + + return stdout, stderr, inspect.ExitCode } // ContainerRunning returns true if the container with the given ID is running. diff --git a/internal/container/controller_test.go b/internal/container/controller_test.go index 347d7aec..0b84e05a 100644 --- a/internal/container/controller_test.go +++ b/internal/container/controller_test.go @@ -5,10 +5,11 @@ package container import ( "fmt" - "github.com/stretchr/testify/assert" "net/http" "net/http/httptest" "testing" + + "github.com/stretchr/testify/assert" ) func TestController_ListTags(t *testing.T) { @@ -54,7 +55,7 @@ func TestController_EnsureImage(t *testing.T) { })) defer ts.Close() - c.DownloadFile(id, ts.URL, "test.txt") + c.DownloadFile(id, ts.URL+"/test.bak", "/tmp") err = c.ContainerStop(id) checkErr(err) @@ -187,3 +188,10 @@ func TestController_DownloadFileNeg3(t *testing.T) { c.DownloadFile("not_blank", "not_blank", "") }) } + +func TestController_DownloadFileNoFilename(t *testing.T) { + c := NewController() + assert.Panics(t, func() { + c.DownloadFile("not_blank", "http://host:9999/", "/tmp") + }) +} From 8b46b680e20b919950a9af10ba39ea5d1a876cb4 Mon Sep 17 00:00:00 2001 From: David Levy Date: Fri, 17 Apr 2026 23:59:17 -0500 Subject: [PATCH 2/5] fix: container test uses host.docker.internal for host networking - Add ExtraHosts host-gateway mapping to ContainerRun so containers can resolve host.docker.internal to the Docker host - Update TestController_EnsureImage to replace 127.0.0.1/localhost with host.docker.internal so wget inside the container can reach the host's httptest server in CI --- internal/container/controller.go | 1 + internal/container/controller_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/container/controller.go b/internal/container/controller.go index 7a039099..defe6268 100644 --- a/internal/container/controller.go +++ b/internal/container/controller.go @@ -81,6 +81,7 @@ func (c Controller) ContainerRun( unitTestFailure bool, ) string { hostConfig := &container.HostConfig{ + ExtraHosts: []string{"host.docker.internal:host-gateway"}, PortBindings: network.PortMap{ network.MustParsePort("1433/tcp"): []network.PortBinding{ { diff --git a/internal/container/controller_test.go b/internal/container/controller_test.go index 0b84e05a..7ed0fab9 100644 --- a/internal/container/controller_test.go +++ b/internal/container/controller_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -55,7 +56,12 @@ func TestController_EnsureImage(t *testing.T) { })) defer ts.Close() - c.DownloadFile(id, ts.URL+"/test.bak", "/tmp") + // Replace 127.0.0.1/localhost with host.docker.internal so the + // container can reach the host's httptest server. + tsURL := strings.Replace(ts.URL, "127.0.0.1", "host.docker.internal", 1) + tsURL = strings.Replace(tsURL, "localhost", "host.docker.internal", 1) + + c.DownloadFile(id, tsURL+"/test.bak", "/tmp") err = c.ContainerStop(id) checkErr(err) From b7323b8093d1326de12f802c5dfc080d20a04cdd Mon Sep 17 00:00:00 2001 From: David Levy Date: Sat, 18 Apr 2026 00:08:08 -0500 Subject: [PATCH 3/5] fix: bind httptest server to 0.0.0.0 for container reachability httptest.NewServer binds to 127.0.0.1, which is unreachable from the Docker bridge network even with host.docker.internal. Use NewUnstartedServer with a 0.0.0.0 listener so the container can connect via 172.17.0.1. --- internal/container/controller_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/container/controller_test.go b/internal/container/controller_test.go index 7ed0fab9..cbbf43a0 100644 --- a/internal/container/controller_test.go +++ b/internal/container/controller_test.go @@ -5,6 +5,7 @@ package container import ( "fmt" + "net" "net/http" "net/http/httptest" "strings" @@ -51,9 +52,15 @@ func TestController_EnsureImage(t *testing.T) { c.ContainerExists(id) c.ContainerFiles(id, "*.mdf") - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Bind to 0.0.0.0 so the container can reach the server via the + // Docker bridge network (host.docker.internal resolves to 172.17.0.1). + ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("test")) })) + l, err := net.Listen("tcp", "0.0.0.0:0") + checkErr(err) + ts.Listener = l + ts.Start() defer ts.Close() // Replace 127.0.0.1/localhost with host.docker.internal so the From d65cf0c1ea292ad38903f112057e4c363e07b906 Mon Sep 17 00:00:00 2001 From: David Levy Date: Sat, 18 Apr 2026 00:11:40 -0500 Subject: [PATCH 4/5] fix: construct test URL from listener port instead of string replacement Use net.Listen tcp4 + net.SplitHostPort to extract the port and build the host.docker.internal URL directly. Avoids fragile string replacements that miss IPv6 [::] addresses on some CI runners. --- internal/container/controller_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/container/controller_test.go b/internal/container/controller_test.go index cbbf43a0..24decfe5 100644 --- a/internal/container/controller_test.go +++ b/internal/container/controller_test.go @@ -8,7 +8,6 @@ import ( "net" "net/http" "net/http/httptest" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -57,16 +56,16 @@ func TestController_EnsureImage(t *testing.T) { ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("test")) })) - l, err := net.Listen("tcp", "0.0.0.0:0") + l, err := net.Listen("tcp4", "0.0.0.0:0") checkErr(err) ts.Listener = l ts.Start() defer ts.Close() - // Replace 127.0.0.1/localhost with host.docker.internal so the - // container can reach the host's httptest server. - tsURL := strings.Replace(ts.URL, "127.0.0.1", "host.docker.internal", 1) - tsURL = strings.Replace(tsURL, "localhost", "host.docker.internal", 1) + // Build URL from listener port so it works regardless of whether + // the OS returns 127.0.0.1, localhost, or [::] in ts.URL. + _, tsPort, _ := net.SplitHostPort(ts.Listener.Addr().String()) + tsURL := fmt.Sprintf("http://host.docker.internal:%s", tsPort) c.DownloadFile(id, tsURL+"/test.bak", "/tmp") From 893ffd556a9561ca15f9eb99d1ddff15f8892e93 Mon Sep 17 00:00:00 2001 From: David Levy Date: Mon, 20 Apr 2026 16:08:09 -0500 Subject: [PATCH 5/5] fix: validate URL and check mkdir exit code before wget --- internal/container/controller.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/container/controller.go b/internal/container/controller.go index defe6268..82e18ead 100644 --- a/internal/container/controller.go +++ b/internal/container/controller.go @@ -81,6 +81,9 @@ func (c Controller) ContainerRun( unitTestFailure bool, ) string { hostConfig := &container.HostConfig{ + // On Linux Docker Engine, host.docker.internal is not set by default + // (Docker Desktop sets it automatically). Add it so containers can + // reach host services, e.g. for bak-file restore from a local HTTP server. ExtraHosts: []string{"host.docker.internal:host-gateway"}, PortBindings: network.PortMap{ network.MustParsePort("1433/tcp"): []network.PortBinding{ @@ -251,15 +254,18 @@ func (c Controller) DownloadFile(id string, src string, destFolder string) { panic("Must pass in non-empty destFolder") } - cmd := []string{"mkdir", "-p", destFolder} - c.runCmdInContainer(id, cmd) - _, file := filepath.Split(src) if file == "" { panic("src URL has no filename: " + src) } dest := destFolder + "/" + file // not using filepath.Join here, this is in the *nix container. always / + cmd := []string{"mkdir", "-p", destFolder} + _, _, mkdirExit := c.runCmdInContainer(id, cmd) + if mkdirExit != 0 { + panic("mkdir failed for " + destFolder) + } + cmd = []string{"wget", "-O", dest, src} _, stderr, exitCode := c.runCmdInContainer(id, cmd) if exitCode != 0 { @@ -313,6 +319,9 @@ func (c Controller) runCmdInContainer(id string, cmd []string) ([]byte, []byte, trace("Stdout: " + string(stdout)) trace("Stderr: " + string(stderr)) + // ExecInspect may rarely return Running:true after output is drained + // (moby/moby#42408). In practice the race window is negligible for + // short-lived commands like mkdir and wget. inspect, err := c.cli.ExecInspect( context.Background(), response.ID,