From 476e788090feafb246f800e71e926aa76d671fb3 Mon Sep 17 00:00:00 2001
From: Brian Goff <cpuguy83@gmail.com>
Date: Fri, 28 Jul 2023 21:21:17 +0000
Subject: [PATCH] Fix daemon proxy test for "reload sanitized"

I noticed this was always being skipped because of race conditions
checking the logs.

This change adds a log scanner which will look through the logs line by
line rather than allocating a big buffer.
Additionally it adds a `poll.Check` which we can use to actually wait
for the desired log entry.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 integration/daemon/daemon_test.go | 25 ++++++++-----------
 testutil/daemon/daemon.go         | 40 +++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go
index 28c5c422cd41e..78d13d1256c23 100644
--- a/integration/daemon/daemon_test.go
+++ b/integration/daemon/daemon_test.go
@@ -22,6 +22,7 @@ import (
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/icmd"
+	"gotest.tools/v3/poll"
 	"gotest.tools/v3/skip"
 )
 
@@ -319,32 +320,26 @@ func TestDaemonProxy(t *testing.T) {
 
 	// Make sure values are sanitized when reloading the daemon-config
 	t.Run("reload sanitized", func(t *testing.T) {
+		t.Parallel()
+
+		ctx := context.Background()
 		const (
 			proxyRawURL = "https://" + userPass + "example.org"
 			proxyURL    = "https://xxxxx:xxxxx@example.org"
 		)
 
 		d := daemon.New(t)
-		d.Start(t, "--http-proxy", proxyRawURL, "--https-proxy", proxyRawURL, "--no-proxy", "example.com")
+		d.Start(t, "--iptables=false", "--http-proxy", proxyRawURL, "--https-proxy", proxyRawURL, "--no-proxy", "example.com")
 		defer d.Stop(t)
 		err := d.Signal(syscall.SIGHUP)
 		assert.NilError(t, err)
 
-		logs, err := d.ReadLogFile()
-		assert.NilError(t, err)
+		poll.WaitOn(t, d.PollCheckLogs(ctx, "Reloaded configuration:"))
+		poll.WaitOn(t, d.PollCheckLogs(ctx, proxyURL))
 
-		// FIXME: there appears to ba a race condition, which causes ReadLogFile
-		//        to not contain the full logs after signaling the daemon to reload,
-		//        causing the test to fail here. As a workaround, check if we
-		//        received the "reloaded" message after signaling, and only then
-		//        check that it's sanitized properly. For more details on this
-		//        issue, see https://github.com/moby/moby/pull/42835/files#r713120315
-		if !strings.Contains(string(logs), "Reloaded configuration:") {
-			t.Skip("Skipping test, because we did not find 'Reloaded configuration' in the logs")
-		}
-
-		assert.Assert(t, is.Contains(string(logs), proxyURL))
-		assert.Assert(t, !strings.Contains(string(logs), userPass), "logs should not contain the non-sanitized proxy URL: %s", string(logs))
+		ok, logs, err := d.ScanLogs(ctx, userPass)
+		assert.NilError(t, err)
+		assert.Assert(t, !ok, "logs should not contain the non-sanitized proxy URL: %s", logs)
 	})
 }
 
diff --git a/testutil/daemon/daemon.go b/testutil/daemon/daemon.go
index 4538bc72e91f4..2cd7a28b4d134 100644
--- a/testutil/daemon/daemon.go
+++ b/testutil/daemon/daemon.go
@@ -1,8 +1,10 @@
 package daemon // import "github.com/docker/docker/testutil/daemon"
 
 import (
+	"bufio"
 	"context"
 	"encoding/json"
+	"io"
 	"net/http"
 	"os"
 	"os/exec"
@@ -25,6 +27,7 @@ import (
 	"github.com/docker/go-connections/tlsconfig"
 	"github.com/pkg/errors"
 	"gotest.tools/v3/assert"
+	"gotest.tools/v3/poll"
 )
 
 // LogT is the subset of the testing.TB interface used by the daemon.
@@ -310,6 +313,43 @@ func (d *Daemon) TailLogsT(t LogT, n int) {
 	}
 }
 
+// PollCheckLogs is a poll.Check that checks the daemon logs for the passed in string (`contains`).
+func (d *Daemon) PollCheckLogs(ctx context.Context, contains string) poll.Check {
+	return func(t poll.LogT) poll.Result {
+		ok, _, err := d.ScanLogs(ctx, contains)
+		if err != nil {
+			return poll.Error(err)
+		}
+		if !ok {
+			return poll.Continue("waiting for %q in daemon logs", contains)
+		}
+		return poll.Success()
+	}
+}
+
+// ScanLogs scans the daemon logs for the passed in string (`contains`).
+// If the context is canceled, the function returns false but does not error out the test.
+func (d *Daemon) ScanLogs(ctx context.Context, contains string) (bool, string, error) {
+	stat, err := d.logFile.Stat()
+	if err != nil {
+		return false, "", err
+	}
+	rdr := io.NewSectionReader(d.logFile, 0, stat.Size())
+
+	scanner := bufio.NewScanner(rdr)
+	for scanner.Scan() {
+		if strings.Contains(scanner.Text(), contains) {
+			return true, scanner.Text(), nil
+		}
+		select {
+		case <-ctx.Done():
+			return false, "", ctx.Err()
+		default:
+		}
+	}
+	return false, "", scanner.Err()
+}
+
 // TailLogs tails N lines from the daemon logs
 func (d *Daemon) TailLogs(n int) ([][]byte, error) {
 	logF, err := os.Open(d.logFile.Name())