diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go index f4565c9616..5ce70d8418 100644 --- a/integration/daemon/daemon_test.go +++ b/integration/daemon/daemon_test.go @@ -1,14 +1,17 @@ package daemon // import "github.com/docker/docker/integration/daemon" import ( + "bytes" "context" "fmt" + "io" "net/http" "net/http/httptest" "os" "os/exec" "path/filepath" "runtime" + "strings" "syscall" "testing" @@ -16,7 +19,9 @@ import ( "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/volume" "github.com/docker/docker/daemon/config" + "github.com/docker/docker/errdefs" "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/pkg/stdcopy" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -431,9 +436,28 @@ func testLiveRestoreVolumeReferences(t *testing.T) { Source: v.Name, Target: "/foo", } - cID := container.Run(ctx, t, c, container.WithMount(m), container.WithCmd("top")) + + const testContent = "hello" + cID := container.Run(ctx, t, c, container.WithMount(m), container.WithCmd("sh", "-c", "echo "+testContent+">>/foo/test.txt; sleep infinity")) defer c.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true}) + // Wait until container creates a file in the volume. + poll.WaitOn(t, func(t poll.LogT) poll.Result { + stat, err := c.ContainerStatPath(ctx, cID, "/foo/test.txt") + if err != nil { + if errdefs.IsNotFound(err) { + return poll.Continue("file doesn't yet exist") + } + return poll.Error(err) + } + + if int(stat.Size) != len(testContent)+1 { + return poll.Error(fmt.Errorf("unexpected test file size: %d", stat.Size)) + } + + return poll.Success() + }) + d.Restart(t, "--live-restore", "--iptables=false") // Try to remove the volume @@ -441,6 +465,32 @@ func testLiveRestoreVolumeReferences(t *testing.T) { err = c.VolumeRemove(ctx, v.Name, false) assert.ErrorContains(t, err, "volume is in use") + t.Run("volume still mounted", func(t *testing.T) { + skip.If(t, testEnv.IsRootless(), "restarted rootless daemon has a new mount namespace and it won't have the previous mounts") + + // Check if a new container with the same volume has access to the previous content. + // This fails if the volume gets unmounted at startup. + cID2 := container.Run(ctx, t, c, container.WithMount(m), container.WithCmd("cat", "/foo/test.txt")) + defer c.ContainerRemove(ctx, cID2, types.ContainerRemoveOptions{Force: true}) + + poll.WaitOn(t, container.IsStopped(ctx, c, cID2)) + + inspect, err := c.ContainerInspect(ctx, cID2) + if assert.Check(t, err) { + assert.Check(t, is.Equal(inspect.State.ExitCode, 0), "volume doesn't have the same file") + } + + logs, err := c.ContainerLogs(ctx, cID2, types.ContainerLogsOptions{ShowStdout: true}) + assert.NilError(t, err) + defer logs.Close() + + var stdoutBuf bytes.Buffer + _, err = stdcopy.StdCopy(&stdoutBuf, io.Discard, logs) + assert.NilError(t, err) + + assert.Check(t, is.Equal(strings.TrimSpace(stdoutBuf.String()), testContent)) + }) + // Remove that container which should free the references in the volume err = c.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true}) assert.NilError(t, err) diff --git a/volume/local/local.go b/volume/local/local.go index 1417941cca..b4f3a3669a 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -83,13 +83,18 @@ func New(scope string, rootIdentity idtools.Identity) (*Root, error) { quotaCtl: r.quotaCtl, } - // unmount anything that may still be mounted (for example, from an - // unclean shutdown). This is a no-op on windows - unmount(v.path) - if err := v.loadOpts(); err != nil { return nil, err } + + if err := v.restoreIfMounted(); err != nil { + log.G(context.TODO()).WithFields(log.Fields{ + "volume": v.name, + "path": v.path, + "error": err, + }).Warn("restoreIfMounted failed") + } + r.volumes[name] = v } @@ -338,6 +343,10 @@ func (v *localVolume) Unmount(id string) error { return nil } + if !v.active.mounted { + return nil + } + logger.Debug("Unmounting volume") return v.unmount() } diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go index 2db5f8ef09..0b534653b9 100644 --- a/volume/local/local_unix.go +++ b/volume/local/local_unix.go @@ -99,10 +99,6 @@ func (v *localVolume) setOpts(opts map[string]string) error { return v.saveOpts() } -func unmount(path string) { - _ = mount.Unmount(path) -} - func (v *localVolume) needsMount() bool { if v.opts == nil { return false @@ -163,6 +159,29 @@ func (v *localVolume) unmount() error { return nil } +// restoreIfMounted restores the mounted status if the _data directory is already mounted. +func (v *localVolume) restoreIfMounted() error { + if v.needsMount() { + // Check if the _data directory is already mounted. + mounted, err := mountinfo.Mounted(v.path) + if err != nil { + return fmt.Errorf("failed to determine if volume _data path is already mounted: %w", err) + } + + if mounted { + // Mark volume as mounted, but don't increment active count. If + // any container needs this, the refcount will be incremented + // by the live-restore (if enabled). + // In other case, refcount will be zero but the volume will + // already be considered as mounted when Mount is called, and + // only the refcount will be incremented. + v.active.mounted = true + } + } + + return nil +} + func (v *localVolume) CreatedAt() (time.Time, error) { fileInfo, err := os.Stat(v.rootPath) if err != nil { diff --git a/volume/local/local_windows.go b/volume/local/local_windows.go index 43b89b3cb1..11723b02f3 100644 --- a/volume/local/local_windows.go +++ b/volume/local/local_windows.go @@ -43,6 +43,11 @@ func (v *localVolume) postMount() error { return nil } +// restoreIfMounted is a no-op on Windows (because mounts are not supported). +func (v *localVolume) restoreIfMounted() error { + return nil +} + func (v *localVolume) CreatedAt() (time.Time, error) { fileInfo, err := os.Stat(v.rootPath) if err != nil {