From a78d56a37ef114f5bbd207c6124e2e2f0af88ad8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 19 Nov 2022 17:17:02 +0100 Subject: [PATCH] manager/state/raft/storage: fix TestCreateOpenInvalidDirFails This test failed on macOS, but passed in CI: --- FAIL: TestCreateOpenInvalidDirFails (0.00s) walwrap_test.go:205: Error Trace: walwrap_test.go:205 Error: An error is expected but got nil. Test: TestCreateOpenInvalidDirFails time="2022-11-19T14:36:00Z" level=info msg="repaired WAL error" error="unexpected EOF" FAIL It looks like this test was always broken; this test was added in 3e2cebe76b8c7e1ae67b89de3b5fa063234edc85 and running the test from that commit and with the Go version used at the time showed the test was failing; git checkout 3e2cebe76b8c7e1ae67b89de3b5fa063234edc85 docker run -it --rm -v $(pwd):/go/src/github.com/docker/swarmkit -w /go/src/github.com/docker/swarmkit golang:1.7 sh -c 'go test -v -run TestCreateOpenInvalidDirFails ./manager/state/raft/storage/' === RUN TestCreateOpenInvalidDirFails --- FAIL: TestCreateOpenInvalidDirFails (0.01s) Error Trace: walwrap_test.go:193 Error: An error is expected but got nil. FAIL exit status 1 FAIL github.com/docker/swarmkit/manager/state/raft/storage 0.031s It took some digging to understand why, but when debugging the error in CI (this part of the test _expected_ an error), it became apparent: --- FAIL: TestCreateOpenInvalidDirFails (0.00s) walwrap_test.go:207: Error Trace: walwrap_test.go:207 Error: Received unexpected error: mkdir /not: permission denied Test: TestCreateOpenInvalidDirFails time="2022-11-19T16:55:42Z" level=info msg="repaired WAL error" error="unexpected EOF" So what happened was that; - `walCryptor.Create()` is called, which calls `wal.Create()` (from go.etcd.io/etcd/server/v3/wal/wal.go). - `wal.Create()` creates a new `.tmp` directory at the same location as the specified path (see https://github.com/etcd-io/etcd/blob/server/v3.5.1/server/wal/wal.go#L110-L127) - in our case, the directory passed was `/not/existing/directory`, so it tries to create `/not/existing/directory.tmp`. - which fails, because CI doesn't run as `root` and doesn't have permissions to create the path. On macOS, running the test inside a container failed, as the tests are run as `root` inside the container. Similarly, when updating the test to use `t.TempDir()`, and using `/not/existing/directory` within that directory, the test failed as well (as there's no permissions issue when creating the `.tmp` directory inside the temp-dir). So, in short, the test was broken from the start; calling `was.Create()` using a non-existing directory is allowed, and not an error condition. (If we _do_ want it to be an error-condition, we should implement code to disallow this). This patch removes the broken part from the test, and updates the remaining part to use `t.TempDir()` and a directory inside that's certain to be empty. Also renaming the test to `TestOpenInvalidDirFails`, as the test now only covers the `Open` case. Signed-off-by: Sebastiaan van Stijn --- manager/state/raft/storage/walwrap_test.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/manager/state/raft/storage/walwrap_test.go b/manager/state/raft/storage/walwrap_test.go index feb505e12a..a2f169e930 100644 --- a/manager/state/raft/storage/walwrap_test.go +++ b/manager/state/raft/storage/walwrap_test.go @@ -196,19 +196,18 @@ func TestSaveEncryptionFails(t *testing.T) { require.Empty(t, ents) } -// If the underlying WAL returns an error when opening or creating, the error -// is propagated up. -func TestCreateOpenInvalidDirFails(t *testing.T) { +// If the underlying WAL returns an error when opening, the error is +// propagated up. +func TestOpenInvalidDirFails(t *testing.T) { c := NewWALFactory(encryption.NoopCrypter, encryption.NoopCrypter) - _, err := c.Create("/not/existing/directory", []byte("metadata")) - require.Error(t, err) - - tempDir, err := os.MkdirTemp("", "test-migrate") - require.NoError(t, err) - defer os.RemoveAll(tempDir) - - _, err = c.Open(tempDir, walpb.Snapshot{}) // invalid because no WAL file + tempDir := t.TempDir() + // using a subdirectory, as some of the code uses "filepath.Dir()" + // and we want to be sure we scope everything to inside the tempDir + // we created. + emptyDir := filepath.Join(tempDir, "empty_dir") + require.NoError(t, os.Mkdir(emptyDir, 0o700)) + _, err := c.Open(emptyDir, walpb.Snapshot{}) // invalid because no WAL file require.Error(t, err) }