diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go index b2821147c9..21632572a7 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" @@ -17,7 +20,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" @@ -432,9 +437,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 @@ -442,6 +466,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 fc0dd7a8c4..d4e58c7976 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -82,13 +82,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 } @@ -337,6 +342,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 c116eceb88..05bf89f36a 100644 --- a/volume/local/local_unix.go +++ b/volume/local/local_unix.go @@ -98,10 +98,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 @@ -162,6 +158,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 9eda81b514..78fdd45ccd 100644 --- a/volume/local/local_windows.go +++ b/volume/local/local_windows.go @@ -44,6 +44,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 {