Skip to content

Commit

Permalink
Fix live-restore w/ restart policies + volume refs
Browse files Browse the repository at this point in the history
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>
(cherry picked from commit 4c0e097)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
  • Loading branch information
cpuguy83 committed Oct 3, 2022
1 parent f219cb5 commit 66ddb7f
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 1 deletion.
4 changes: 3 additions & 1 deletion daemon/daemon.go
Expand Up @@ -532,7 +532,9 @@ func (daemon *Daemon) restore() error {
}
}

// Make sure networks are available before starting
if err := daemon.prepareMountPoints(c); err != nil {
log.WithError(err).Error("failed to prepare mount points for container")
}
daemon.waitForNetworks(c)
if err := daemon.containerStart(c, "", "", true); err != nil {
log.WithError(err).Error("failed to start container")
Expand Down
69 changes: 69 additions & 0 deletions integration/daemon/daemon_test.go
@@ -0,0 +1,69 @@
package daemon // import "github.com/docker/docker/integration/daemon"

import (
"context"
"runtime"
"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/integration/internal/container"
"github.com/docker/docker/testutil/daemon"
"gotest.tools/v3/assert"
"gotest.tools/v3/skip"
)

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.VolumeCreateBody{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")
})
}
28 changes: 28 additions & 0 deletions integration/daemon/main_test.go
@@ -0,0 +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())
}

0 comments on commit 66ddb7f

Please sign in to comment.