Skip to content

Commit

Permalink
Fix daemon proxy test for "reload sanitized"
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
cpuguy83 authored and thaJeztah committed Jul 31, 2023
1 parent 8197752 commit 476e788
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 15 deletions.
25 changes: 10 additions & 15 deletions integration/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
})
}

Expand Down
40 changes: 40 additions & 0 deletions testutil/daemon/daemon.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package daemon // import "github.com/docker/docker/testutil/daemon"

import (
"bufio"
"context"
"encoding/json"
"io"
"net/http"
"os"
"os/exec"
Expand All @@ -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.
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 476e788

Please sign in to comment.