Переглянути джерело

Fix live-restore w/ restart policies + volume refs

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 <cpuguy83@gmail.com>
Brian Goff 2 роки тому
батько
коміт
4c0e0979b4
3 змінених файлів з 78 додано та 0 видалено
  1. 3 0
      daemon/daemon.go
  2. 57 0
      integration/daemon/daemon_test.go
  3. 18 0
      integration/daemon/main_test.go

+ 3 - 0
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")
 			}

+ 57 - 0
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")
+	})
+}

+ 18 - 0
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())
 }