Skip to content

Commit

Permalink
Restore active mount counts on live-restore
Browse files Browse the repository at this point in the history
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>
(cherry picked from commit 647c2a6)
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
cpuguy83 authored and thaJeztah committed Jun 28, 2023
1 parent 6eb4d7f commit 1fed2b0
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 0 deletions.
12 changes: 12 additions & 0 deletions daemon/mounts.go
Expand Up @@ -5,16 +5,28 @@ import (
"fmt"
"strings"

"github.com/containerd/containerd/log"
mounttypes "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/container"
volumesservice "github.com/docker/docker/volume/service"
"github.com/sirupsen/logrus"
)

func (daemon *Daemon) prepareMountPoints(container *container.Container) error {
alive := container.IsRunning()
for _, config := range container.MountPoints {
if err := daemon.lazyInitializeVolume(container.ID, config); err != nil {
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
}
Expand Down
7 changes: 7 additions & 0 deletions daemon/volumes.go
Expand Up @@ -21,6 +21,8 @@ import (
"github.com/sirupsen/logrus"
)

var _ volume.LiveRestorer = (*volumeWrapper)(nil)

type mounts []container.Mount

// Len returns the number of mounts. Used in sorting.
Expand Down Expand Up @@ -257,6 +259,7 @@ func (daemon *Daemon) VolumesService() *service.VolumesService {
type volumeMounter interface {
Mount(ctx context.Context, v *volumetypes.Volume, ref string) (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 {
Expand Down Expand Up @@ -291,3 +294,7 @@ func (v *volumeWrapper) CreatedAt() (time.Time, error) {
func (v *volumeWrapper) Status() map[string]interface{} {
return v.v.Status
}

func (v *volumeWrapper) LiveRestoreVolume(ctx context.Context, ref string) error {
return v.s.LiveRestoreVolume(ctx, v.v, ref)
}
36 changes: 36 additions & 0 deletions integration/daemon/daemon_test.go
Expand Up @@ -400,6 +400,42 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
runTest(t, "on-failure")
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) {
Expand Down
28 changes: 28 additions & 0 deletions volume/local/local.go
Expand Up @@ -4,13 +4,15 @@
package local // import "github.com/docker/docker/volume/local"

import (
"context"
"encoding/json"
"os"
"path/filepath"
"reflect"
"strings"
"sync"

"github.com/containerd/containerd/log"
"github.com/docker/docker/daemon/names"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/pkg/idtools"
Expand All @@ -35,6 +37,8 @@ var (
// 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.
volumeNameRegex = names.RestrictedNamePattern

_ volume.LiveRestorer = (*localVolume)(nil)
)

type activeMount struct {
Expand Down Expand Up @@ -296,14 +300,17 @@ func (v *localVolume) CachedPath() string {
func (v *localVolume) Mount(id string) (string, error) {
v.m.Lock()
defer v.m.Unlock()
logger := log.G(context.TODO()).WithField("volume", v.name)
if v.needsMount() {
if !v.active.mounted {
logger.Debug("Mounting volume")
if err := v.mount(); err != nil {
return "", errdefs.System(err)
}
v.active.mounted = true
}
v.active.count++
logger.WithField("active mounts", v.active).Debug("Decremented active mount count")
}
if err := v.postMount(); err != nil {
return "", err
Expand All @@ -316,19 +323,22 @@ func (v *localVolume) Mount(id string) (string, error) {
func (v *localVolume) Unmount(id string) error {
v.m.Lock()
defer v.m.Unlock()
logger := log.G(context.TODO()).WithField("volume", v.name)

// Always decrement the count, even if the unmount fails
// Essentially docker doesn't care if this fails, it will send an error, but
// ultimately there's nothing that can be done. If we don't decrement the count
// this volume can never be removed until a daemon restart occurs.
if v.needsMount() {
v.active.count--
logger.WithField("active mounts", v.active).Debug("Decremented active mount count")
}

if v.active.count > 0 {
return nil
}

logger.Debug("Unmounting volume")
return v.unmount()
}

Expand Down Expand Up @@ -369,6 +379,24 @@ func (v *localVolume) saveOpts() error {
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
func getAddress(opts string) string {
for _, opt := range strings.Split(opts, ",") {
Expand Down
29 changes: 29 additions & 0 deletions volume/mounts/mounts.go
@@ -1,17 +1,20 @@
package mounts // import "github.com/docker/docker/volume/mounts"

import (
"context"
"fmt"
"os"
"path/filepath"
"syscall"

"github.com/containerd/containerd/log"
mounttypes "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/pkg/idtools"
"github.com/docker/docker/pkg/stringid"
"github.com/docker/docker/volume"
"github.com/opencontainers/selinux/go-selinux/label"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

// MountPoint is the intersection point between a volume and a container. It
Expand Down Expand Up @@ -164,6 +167,32 @@ func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.Identity, checkFun
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.
func (m *MountPoint) Path() string {
if m.Volume != nil {
Expand Down
15 changes: 15 additions & 0 deletions volume/service/service.go
Expand Up @@ -274,3 +274,18 @@ func (s *VolumesService) List(ctx context.Context, filter filters.Args) (volumes
func (s *VolumesService) Shutdown() error {
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 changes: 9 additions & 0 deletions volume/service/store.go
Expand Up @@ -24,6 +24,8 @@ const (
volumeDataDir = "volumes"
)

var _ volume.LiveRestorer = (*volumeWrapper)(nil)

type volumeWrapper struct {
volume.Volume
labels map[string]string
Expand Down Expand Up @@ -67,6 +69,13 @@ func (v volumeWrapper) CachedPath() string {
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
type StoreOpt func(store *VolumeStore) error

Expand Down
10 changes: 10 additions & 0 deletions volume/volume.go
@@ -1,6 +1,7 @@
package volume // import "github.com/docker/docker/volume"

import (
"context"
"time"
)

Expand Down Expand Up @@ -60,6 +61,15 @@ type Volume 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`)
type DetailedVolume interface {
Labels() map[string]string
Expand Down

0 comments on commit 1fed2b0

Please sign in to comment.