Browse Source

Restore active mount counts on live-restore

When live-restoring a container the volume driver needs be notified that
there is an active mount for the volume.
Before this change the count is zero until the container stops and the
uint64 overflows pretty much making it so the volume can never be
removed until another daemon restart.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Brian Goff 2 years ago
parent
commit
647c2a6cdd

+ 12 - 0
daemon/mounts.go

@@ -5,16 +5,28 @@ import (
 	"fmt"
 	"fmt"
 	"strings"
 	"strings"
 
 
+	"github.com/containerd/containerd/log"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/container"
 	volumesservice "github.com/docker/docker/volume/service"
 	volumesservice "github.com/docker/docker/volume/service"
+	"github.com/sirupsen/logrus"
 )
 )
 
 
 func (daemon *Daemon) prepareMountPoints(container *container.Container) error {
 func (daemon *Daemon) prepareMountPoints(container *container.Container) error {
+	alive := container.IsRunning()
 	for _, config := range container.MountPoints {
 	for _, config := range container.MountPoints {
 		if err := daemon.lazyInitializeVolume(container.ID, config); err != nil {
 		if err := daemon.lazyInitializeVolume(container.ID, config); err != nil {
 			return err
 			return err
 		}
 		}
+		if alive {
+			log.G(context.TODO()).WithFields(logrus.Fields{
+				"container": container.ID,
+				"volume":    config.Volume.Name(),
+			}).Debug("Live-restoring volume for alive container")
+			if err := config.LiveRestore(context.TODO()); err != nil {
+				return err
+			}
+		}
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 7 - 0
daemon/volumes.go

@@ -21,6 +21,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/pkg/errors"
 )
 )
 
 
+var _ volume.LiveRestorer = (*volumeWrapper)(nil)
+
 type mounts []container.Mount
 type mounts []container.Mount
 
 
 // Len returns the number of mounts. Used in sorting.
 // Len returns the number of mounts. Used in sorting.
@@ -257,6 +259,7 @@ func (daemon *Daemon) VolumesService() *service.VolumesService {
 type volumeMounter interface {
 type volumeMounter interface {
 	Mount(ctx context.Context, v *volumetypes.Volume, ref string) (string, error)
 	Mount(ctx context.Context, v *volumetypes.Volume, ref string) (string, error)
 	Unmount(ctx context.Context, v *volumetypes.Volume, ref string) error
 	Unmount(ctx context.Context, v *volumetypes.Volume, ref string) error
+	LiveRestoreVolume(ctx context.Context, v *volumetypes.Volume, ref string) error
 }
 }
 
 
 type volumeWrapper struct {
 type volumeWrapper struct {
@@ -291,3 +294,7 @@ func (v *volumeWrapper) CreatedAt() (time.Time, error) {
 func (v *volumeWrapper) Status() map[string]interface{} {
 func (v *volumeWrapper) Status() map[string]interface{} {
 	return v.v.Status
 	return v.v.Status
 }
 }
+
+func (v *volumeWrapper) LiveRestoreVolume(ctx context.Context, ref string) error {
+	return v.s.LiveRestoreVolume(ctx, v.v, ref)
+}

+ 36 - 0
integration/daemon/daemon_test.go

@@ -400,6 +400,42 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
 		runTest(t, "on-failure")
 		runTest(t, "on-failure")
 		runTest(t, "no")
 		runTest(t, "no")
 	})
 	})
+
+	// Make sure that the local volume driver's mount ref count is restored
+	// Addresses https://github.com/moby/moby/issues/44422
+	t.Run("local volume with mount options", func(t *testing.T) {
+		v, err := c.VolumeCreate(ctx, volume.CreateOptions{
+			Driver: "local",
+			Name:   "test-live-restore-volume-references-local",
+			DriverOpts: map[string]string{
+				"type":   "tmpfs",
+				"device": "tmpfs",
+			},
+		})
+		assert.NilError(t, err)
+		m := mount.Mount{
+			Type:   mount.TypeVolume,
+			Source: v.Name,
+			Target: "/foo",
+		}
+		cID := container.Run(ctx, t, c, container.WithMount(m), container.WithCmd("top"))
+		defer c.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true})
+
+		d.Restart(t, "--live-restore", "--iptables=false")
+
+		// Try to remove the volume
+		// This should fail since its used by a container
+		err = c.VolumeRemove(ctx, v.Name, false)
+		assert.ErrorContains(t, err, "volume is in use")
+
+		// Remove that container which should free the references in the volume
+		err = c.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true})
+		assert.NilError(t, err)
+
+		// Now we should be able to remove the volume
+		err = c.VolumeRemove(ctx, v.Name, false)
+		assert.NilError(t, err)
+	})
 }
 }
 
 
 func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {
 func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {

+ 27 - 0
volume/local/local.go

@@ -19,6 +19,7 @@ import (
 	"github.com/docker/docker/quota"
 	"github.com/docker/docker/quota"
 	"github.com/docker/docker/volume"
 	"github.com/docker/docker/volume"
 	"github.com/pkg/errors"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 )
 
 
 const (
 const (
@@ -36,6 +37,8 @@ var (
 	// This name is used to create the bind directory, so we need to avoid characters that
 	// This name is used to create the bind directory, so we need to avoid characters that
 	// would make the path to escape the root directory.
 	// would make the path to escape the root directory.
 	volumeNameRegex = names.RestrictedNamePattern
 	volumeNameRegex = names.RestrictedNamePattern
+
+	_ volume.LiveRestorer = (*localVolume)(nil)
 )
 )
 
 
 type activeMount struct {
 type activeMount struct {
@@ -297,14 +300,17 @@ func (v *localVolume) CachedPath() string {
 func (v *localVolume) Mount(id string) (string, error) {
 func (v *localVolume) Mount(id string) (string, error) {
 	v.m.Lock()
 	v.m.Lock()
 	defer v.m.Unlock()
 	defer v.m.Unlock()
+	logger := log.G(context.TODO()).WithField("volume", v.name)
 	if v.needsMount() {
 	if v.needsMount() {
 		if !v.active.mounted {
 		if !v.active.mounted {
+			logger.Debug("Mounting volume")
 			if err := v.mount(); err != nil {
 			if err := v.mount(); err != nil {
 				return "", errdefs.System(err)
 				return "", errdefs.System(err)
 			}
 			}
 			v.active.mounted = true
 			v.active.mounted = true
 		}
 		}
 		v.active.count++
 		v.active.count++
+		logger.WithField("active mounts", v.active).Debug("Decremented active mount count")
 	}
 	}
 	if err := v.postMount(); err != nil {
 	if err := v.postMount(); err != nil {
 		return "", err
 		return "", err
@@ -317,6 +323,7 @@ func (v *localVolume) Mount(id string) (string, error) {
 func (v *localVolume) Unmount(id string) error {
 func (v *localVolume) Unmount(id string) error {
 	v.m.Lock()
 	v.m.Lock()
 	defer v.m.Unlock()
 	defer v.m.Unlock()
+	logger := log.G(context.TODO()).WithField("volume", v.name)
 
 
 	// Always decrement the count, even if the unmount fails
 	// Always decrement the count, even if the unmount fails
 	// Essentially docker doesn't care if this fails, it will send an error, but
 	// Essentially docker doesn't care if this fails, it will send an error, but
@@ -324,12 +331,14 @@ func (v *localVolume) Unmount(id string) error {
 	// this volume can never be removed until a daemon restart occurs.
 	// this volume can never be removed until a daemon restart occurs.
 	if v.needsMount() {
 	if v.needsMount() {
 		v.active.count--
 		v.active.count--
+		logger.WithField("active mounts", v.active).Debug("Decremented active mount count")
 	}
 	}
 
 
 	if v.active.count > 0 {
 	if v.active.count > 0 {
 		return nil
 		return nil
 	}
 	}
 
 
+	logger.Debug("Unmounting volume")
 	return v.unmount()
 	return v.unmount()
 }
 }
 
 
@@ -370,6 +379,24 @@ func (v *localVolume) saveOpts() error {
 	return nil
 	return nil
 }
 }
 
 
+// LiveRestoreVolume restores reference counts for mounts
+// It is assumed that the volume is already mounted since this is only called for active, live-restored containers.
+func (v *localVolume) LiveRestoreVolume(ctx context.Context, _ string) error {
+	v.m.Lock()
+	defer v.m.Unlock()
+
+	if !v.needsMount() {
+		return nil
+	}
+	v.active.count++
+	v.active.mounted = true
+	log.G(ctx).WithFields(logrus.Fields{
+		"volume":        v.name,
+		"active mounts": v.active,
+	}).Debugf("Live restored volume")
+	return nil
+}
+
 // getAddress finds out address/hostname from options
 // getAddress finds out address/hostname from options
 func getAddress(opts string) string {
 func getAddress(opts string) string {
 	for _, opt := range strings.Split(opts, ",") {
 	for _, opt := range strings.Split(opts, ",") {

+ 29 - 0
volume/mounts/mounts.go

@@ -1,17 +1,20 @@
 package mounts // import "github.com/docker/docker/volume/mounts"
 package mounts // import "github.com/docker/docker/volume/mounts"
 
 
 import (
 import (
+	"context"
 	"fmt"
 	"fmt"
 	"os"
 	"os"
 	"path/filepath"
 	"path/filepath"
 	"syscall"
 	"syscall"
 
 
+	"github.com/containerd/containerd/log"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/volume"
 	"github.com/docker/docker/volume"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 )
 
 
 // MountPoint is the intersection point between a volume and a container. It
 // MountPoint is the intersection point between a volume and a container. It
@@ -164,6 +167,32 @@ func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.Identity, checkFun
 	return m.Source, nil
 	return m.Source, nil
 }
 }
 
 
+func (m *MountPoint) LiveRestore(ctx context.Context) error {
+	if m.Volume == nil {
+		logrus.Debug("No volume to restore")
+		return nil
+	}
+
+	lrv, ok := m.Volume.(volume.LiveRestorer)
+	if !ok {
+		log.G(ctx).WithField("volume", m.Volume.Name()).Debugf("Volume does not support live restore: %T", m.Volume)
+		return nil
+	}
+
+	id := m.ID
+	if id == "" {
+		id = stringid.GenerateRandomID()
+	}
+
+	if err := lrv.LiveRestoreVolume(ctx, id); err != nil {
+		return errors.Wrapf(err, "error while restoring volume '%s'", m.Source)
+	}
+
+	m.ID = id
+	m.active++
+	return nil
+}
+
 // Path returns the path of a volume in a mount point.
 // Path returns the path of a volume in a mount point.
 func (m *MountPoint) Path() string {
 func (m *MountPoint) Path() string {
 	if m.Volume != nil {
 	if m.Volume != nil {

+ 15 - 0
volume/service/service.go

@@ -274,3 +274,18 @@ func (s *VolumesService) List(ctx context.Context, filter filters.Args) (volumes
 func (s *VolumesService) Shutdown() error {
 func (s *VolumesService) Shutdown() error {
 	return s.vs.Shutdown()
 	return s.vs.Shutdown()
 }
 }
+
+// LiveRestoreVolume passes through the LiveRestoreVolume call to the volume if it is implemented
+// otherwise it is a no-op.
+func (s *VolumesService) LiveRestoreVolume(ctx context.Context, vol *volumetypes.Volume, ref string) error {
+	v, err := s.vs.Get(ctx, vol.Name, opts.WithGetDriver(vol.Driver))
+	if err != nil {
+		return err
+	}
+	rlv, ok := v.(volume.LiveRestorer)
+	if !ok {
+		log.G(ctx).WithField("volume", vol.Name).Debugf("volume does not implement LiveRestoreVolume: %T", v)
+		return nil
+	}
+	return rlv.LiveRestoreVolume(ctx, ref)
+}

+ 9 - 0
volume/service/store.go

@@ -24,6 +24,8 @@ const (
 	volumeDataDir = "volumes"
 	volumeDataDir = "volumes"
 )
 )
 
 
+var _ volume.LiveRestorer = (*volumeWrapper)(nil)
+
 type volumeWrapper struct {
 type volumeWrapper struct {
 	volume.Volume
 	volume.Volume
 	labels  map[string]string
 	labels  map[string]string
@@ -67,6 +69,13 @@ func (v volumeWrapper) CachedPath() string {
 	return v.Volume.Path()
 	return v.Volume.Path()
 }
 }
 
 
+func (v volumeWrapper) LiveRestoreVolume(ctx context.Context, ref string) error {
+	if vv, ok := v.Volume.(volume.LiveRestorer); ok {
+		return vv.LiveRestoreVolume(ctx, ref)
+	}
+	return nil
+}
+
 // StoreOpt sets options for a VolumeStore
 // StoreOpt sets options for a VolumeStore
 type StoreOpt func(store *VolumeStore) error
 type StoreOpt func(store *VolumeStore) error
 
 

+ 10 - 0
volume/volume.go

@@ -1,6 +1,7 @@
 package volume // import "github.com/docker/docker/volume"
 package volume // import "github.com/docker/docker/volume"
 
 
 import (
 import (
+	"context"
 	"time"
 	"time"
 )
 )
 
 
@@ -60,6 +61,15 @@ type Volume interface {
 	Status() map[string]interface{}
 	Status() map[string]interface{}
 }
 }
 
 
+// LiveRestorer is an optional interface that can be implemented by a volume driver
+// It is used to restore any resources that are necessary for a volume to be used by a live-restored container
+type LiveRestorer interface {
+	// LiveRestoreVolume allows a volume driver which implements this interface to restore any necessary resources (such as reference counting)
+	// This is called only after the daemon is restarted with live-restored containers
+	// It is called once per live-restored container.
+	LiveRestoreVolume(_ context.Context, ref string) error
+}
+
 // DetailedVolume wraps a Volume with user-defined labels, options, and cluster scope (e.g., `local` or `global`)
 // DetailedVolume wraps a Volume with user-defined labels, options, and cluster scope (e.g., `local` or `global`)
 type DetailedVolume interface {
 type DetailedVolume interface {
 	Labels() map[string]string
 	Labels() map[string]string