From 0a87dc9f71b4d132bbf9f3157f269f88e1ac4052 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 30 Sep 2022 22:30:58 +0000 Subject: [PATCH] Fix live-restore w/ restart policies + volume refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change restarting the daemon in live-restore with running containers + a restart policy meant that volume refs were not restored. This specifically happens when the container is still running *and* there is a restart policy that would make sure the container was running again on restart. The bug allows volumes to be removed even though containers are referencing them. 😱 Signed-off-by: Brian Goff (cherry picked from commit 4c0e0979b4f54972f8b89200078d691c68bd7f3e) Signed-off-by: Brian Goff --- daemon/daemon.go | 3 ++ integration/daemon/daemon_test.go | 57 +++++++++++++++++++++++++++++++ integration/daemon/main_test.go | 18 ++++++++++ 3 files changed, 78 insertions(+) diff --git a/daemon/daemon.go b/daemon/daemon.go index 54b45d5f6b..3b0e48e66f 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -519,6 +519,9 @@ func (daemon *Daemon) restore() error { } } + if err := daemon.prepareMountPoints(c); err != nil { + log.WithError(err).Error("failed to prepare mount points for container") + } if err := daemon.containerStart(c, "", "", true); err != nil { log.WithError(err).Error("failed to start container") } diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go index 72ffdd1dbf..42e4155269 100644 --- a/integration/daemon/daemon_test.go +++ b/integration/daemon/daemon_test.go @@ -14,7 +14,10 @@ import ( "testing" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/mount" + "github.com/docker/docker/api/types/volume" "github.com/docker/docker/daemon/config" + "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -378,3 +381,57 @@ func TestDaemonProxy(t *testing.T) { assert.Assert(t, !strings.Contains(string(logs), userPass), "logs should not contain the non-sanitized proxy URL: %s", string(logs)) }) } + +func TestLiveRestore(t *testing.T) { + skip.If(t, runtime.GOOS == "windows", "cannot start multiple daemons on windows") + + t.Run("volume references", testLiveRestoreVolumeReferences) +} + +func testLiveRestoreVolumeReferences(t *testing.T) { + t.Parallel() + + d := daemon.New(t) + d.StartWithBusybox(t, "--live-restore", "--iptables=false") + defer func() { + d.Stop(t) + d.Cleanup(t) + }() + + c := d.NewClientT(t) + ctx := context.Background() + + runTest := func(t *testing.T, policy string) { + t.Run(policy, func(t *testing.T) { + volName := "test-live-restore-volume-references-" + policy + _, err := c.VolumeCreate(ctx, volume.CreateOptions{Name: volName}) + assert.NilError(t, err) + + // Create a container that uses the volume + m := mount.Mount{ + Type: mount.TypeVolume, + Source: volName, + Target: "/foo", + } + cID := container.Run(ctx, t, c, container.WithMount(m), container.WithCmd("top"), container.WithRestartPolicy(policy)) + defer c.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true}) + + // Stop the daemon + d.Restart(t, "--live-restore", "--iptables=false") + + // Try to remove the volume + err = c.VolumeRemove(ctx, volName, false) + assert.ErrorContains(t, err, "volume is in use") + + _, err = c.VolumeInspect(ctx, volName) + assert.NilError(t, err) + }) + } + + t.Run("restartPolicy", func(t *testing.T) { + runTest(t, "always") + runTest(t, "unless-stopped") + runTest(t, "on-failure") + runTest(t, "no") + }) +} diff --git a/integration/daemon/main_test.go b/integration/daemon/main_test.go index 74a342e298..6fb7250a0f 100644 --- a/integration/daemon/main_test.go +++ b/integration/daemon/main_test.go @@ -1,10 +1,28 @@ package daemon // import "github.com/docker/docker/integration/daemon" import ( + "fmt" "os" "testing" + + "github.com/docker/docker/testutil/environment" ) +var testEnv *environment.Execution + func TestMain(m *testing.M) { + var err error + testEnv, err = environment.New() + if err != nil { + fmt.Println(err) + os.Exit(1) + } + err = environment.EnsureFrozenImagesLinux(testEnv) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + testEnv.Print() os.Exit(m.Run()) }