Skip to content

Commit 647c2a6

Browse files
cpuguy83cpuguy83
committed
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>
1 parent 7f8b1cd commit 647c2a6

8 files changed

Lines changed: 145 additions & 0 deletions

File tree

‎daemon/mounts.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,28 @@ import (
55
"fmt"
66
"strings"
77

8+
"github.com/containerd/containerd/log"
89
mounttypes "github.com/docker/docker/api/types/mount"
910
"github.com/docker/docker/container"
1011
volumesservice "github.com/docker/docker/volume/service"
12+
"github.com/sirupsen/logrus"
1113
)
1214

1315
func (daemon *Daemon) prepareMountPoints(container *container.Container) error {
16+
alive := container.IsRunning()
1417
for _, config := range container.MountPoints {
1518
if err := daemon.lazyInitializeVolume(container.ID, config); err != nil {
1619
return err
1720
}
21+
if alive {
22+
log.G(context.TODO()).WithFields(logrus.Fields{
23+
"container": container.ID,
24+
"volume": config.Volume.Name(),
25+
}).Debug("Live-restoring volume for alive container")
26+
if err := config.LiveRestore(context.TODO()); err != nil {
27+
return err
28+
}
29+
}
1830
}
1931
return nil
2032
}

‎daemon/volumes.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"github.com/pkg/errors"
2222
)
2323

24+
var _ volume.LiveRestorer = (*volumeWrapper)(nil)
25+
2426
type mounts []container.Mount
2527

2628
// Len returns the number of mounts. Used in sorting.
@@ -257,6 +259,7 @@ func (daemon *Daemon) VolumesService() *service.VolumesService {
257259
type volumeMounter interface {
258260
Mount(ctx context.Context, v *volumetypes.Volume, ref string) (string, error)
259261
Unmount(ctx context.Context, v *volumetypes.Volume, ref string) error
262+
LiveRestoreVolume(ctx context.Context, v *volumetypes.Volume, ref string) error
260263
}
261264

262265
type volumeWrapper struct {
@@ -291,3 +294,7 @@ func (v *volumeWrapper) CreatedAt() (time.Time, error) {
291294
func (v *volumeWrapper) Status() map[string]interface{} {
292295
return v.v.Status
293296
}
297+
298+
func (v *volumeWrapper) LiveRestoreVolume(ctx context.Context, ref string) error {
299+
return v.s.LiveRestoreVolume(ctx, v.v, ref)
300+
}

‎integration/daemon/daemon_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,42 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
400400
runTest(t, "on-failure")
401401
runTest(t, "no")
402402
})
403+
404+
// Make sure that the local volume driver's mount ref count is restored
405+
// Addresses https://github.com/moby/moby/issues/44422
406+
t.Run("local volume with mount options", func(t *testing.T) {
407+
v, err := c.VolumeCreate(ctx, volume.CreateOptions{
408+
Driver: "local",
409+
Name: "test-live-restore-volume-references-local",
410+
DriverOpts: map[string]string{
411+
"type": "tmpfs",
412+
"device": "tmpfs",
413+
},
414+
})
415+
assert.NilError(t, err)
416+
m := mount.Mount{
417+
Type: mount.TypeVolume,
418+
Source: v.Name,
419+
Target: "/foo",
420+
}
421+
cID := container.Run(ctx, t, c, container.WithMount(m), container.WithCmd("top"))
422+
defer c.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true})
423+
424+
d.Restart(t, "--live-restore", "--iptables=false")
425+
426+
// Try to remove the volume
427+
// This should fail since its used by a container
428+
err = c.VolumeRemove(ctx, v.Name, false)
429+
assert.ErrorContains(t, err, "volume is in use")
430+
431+
// Remove that container which should free the references in the volume
432+
err = c.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true})
433+
assert.NilError(t, err)
434+
435+
// Now we should be able to remove the volume
436+
err = c.VolumeRemove(ctx, v.Name, false)
437+
assert.NilError(t, err)
438+
})
403439
}
404440

405441
func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {

‎volume/local/local.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/docker/docker/quota"
2020
"github.com/docker/docker/volume"
2121
"github.com/pkg/errors"
22+
"github.com/sirupsen/logrus"
2223
)
2324

2425
const (
@@ -36,6 +37,8 @@ var (
3637
// This name is used to create the bind directory, so we need to avoid characters that
3738
// would make the path to escape the root directory.
3839
volumeNameRegex = names.RestrictedNamePattern
40+
41+
_ volume.LiveRestorer = (*localVolume)(nil)
3942
)
4043

4144
type activeMount struct {
@@ -297,14 +300,17 @@ func (v *localVolume) CachedPath() string {
297300
func (v *localVolume) Mount(id string) (string, error) {
298301
v.m.Lock()
299302
defer v.m.Unlock()
303+
logger := log.G(context.TODO()).WithField("volume", v.name)
300304
if v.needsMount() {
301305
if !v.active.mounted {
306+
logger.Debug("Mounting volume")
302307
if err := v.mount(); err != nil {
303308
return "", errdefs.System(err)
304309
}
305310
v.active.mounted = true
306311
}
307312
v.active.count++
313+
logger.WithField("active mounts", v.active).Debug("Decremented active mount count")
308314
}
309315
if err := v.postMount(); err != nil {
310316
return "", err
@@ -317,19 +323,22 @@ func (v *localVolume) Mount(id string) (string, error) {
317323
func (v *localVolume) Unmount(id string) error {
318324
v.m.Lock()
319325
defer v.m.Unlock()
326+
logger := log.G(context.TODO()).WithField("volume", v.name)
320327

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

329337
if v.active.count > 0 {
330338
return nil
331339
}
332340

341+
logger.Debug("Unmounting volume")
333342
return v.unmount()
334343
}
335344

@@ -370,6 +379,24 @@ func (v *localVolume) saveOpts() error {
370379
return nil
371380
}
372381

382+
// LiveRestoreVolume restores reference counts for mounts
383+
// It is assumed that the volume is already mounted since this is only called for active, live-restored containers.
384+
func (v *localVolume) LiveRestoreVolume(ctx context.Context, _ string) error {
385+
v.m.Lock()
386+
defer v.m.Unlock()
387+
388+
if !v.needsMount() {
389+
return nil
390+
}
391+
v.active.count++
392+
v.active.mounted = true
393+
log.G(ctx).WithFields(logrus.Fields{
394+
"volume": v.name,
395+
"active mounts": v.active,
396+
}).Debugf("Live restored volume")
397+
return nil
398+
}
399+
373400
// getAddress finds out address/hostname from options
374401
func getAddress(opts string) string {
375402
for _, opt := range strings.Split(opts, ",") {

‎volume/mounts/mounts.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
package mounts // import "github.com/docker/docker/volume/mounts"
22

33
import (
4+
"context"
45
"fmt"
56
"os"
67
"path/filepath"
78
"syscall"
89

10+
"github.com/containerd/containerd/log"
911
mounttypes "github.com/docker/docker/api/types/mount"
1012
"github.com/docker/docker/pkg/idtools"
1113
"github.com/docker/docker/pkg/stringid"
1214
"github.com/docker/docker/volume"
1315
"github.com/opencontainers/selinux/go-selinux/label"
1416
"github.com/pkg/errors"
17+
"github.com/sirupsen/logrus"
1518
)
1619

1720
// 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
164167
return m.Source, nil
165168
}
166169

170+
func (m *MountPoint) LiveRestore(ctx context.Context) error {
171+
if m.Volume == nil {
172+
logrus.Debug("No volume to restore")
173+
return nil
174+
}
175+
176+
lrv, ok := m.Volume.(volume.LiveRestorer)
177+
if !ok {
178+
log.G(ctx).WithField("volume", m.Volume.Name()).Debugf("Volume does not support live restore: %T", m.Volume)
179+
return nil
180+
}
181+
182+
id := m.ID
183+
if id == "" {
184+
id = stringid.GenerateRandomID()
185+
}
186+
187+
if err := lrv.LiveRestoreVolume(ctx, id); err != nil {
188+
return errors.Wrapf(err, "error while restoring volume '%s'", m.Source)
189+
}
190+
191+
m.ID = id
192+
m.active++
193+
return nil
194+
}
195+
167196
// Path returns the path of a volume in a mount point.
168197
func (m *MountPoint) Path() string {
169198
if m.Volume != nil {

‎volume/service/service.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,3 +274,18 @@ func (s *VolumesService) List(ctx context.Context, filter filters.Args) (volumes
274274
func (s *VolumesService) Shutdown() error {
275275
return s.vs.Shutdown()
276276
}
277+
278+
// LiveRestoreVolume passes through the LiveRestoreVolume call to the volume if it is implemented
279+
// otherwise it is a no-op.
280+
func (s *VolumesService) LiveRestoreVolume(ctx context.Context, vol *volumetypes.Volume, ref string) error {
281+
v, err := s.vs.Get(ctx, vol.Name, opts.WithGetDriver(vol.Driver))
282+
if err != nil {
283+
return err
284+
}
285+
rlv, ok := v.(volume.LiveRestorer)
286+
if !ok {
287+
log.G(ctx).WithField("volume", vol.Name).Debugf("volume does not implement LiveRestoreVolume: %T", v)
288+
return nil
289+
}
290+
return rlv.LiveRestoreVolume(ctx, ref)
291+
}

‎volume/service/store.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ const (
2424
volumeDataDir = "volumes"
2525
)
2626

27+
var _ volume.LiveRestorer = (*volumeWrapper)(nil)
28+
2729
type volumeWrapper struct {
2830
volume.Volume
2931
labels map[string]string
@@ -67,6 +69,13 @@ func (v volumeWrapper) CachedPath() string {
6769
return v.Volume.Path()
6870
}
6971

72+
func (v volumeWrapper) LiveRestoreVolume(ctx context.Context, ref string) error {
73+
if vv, ok := v.Volume.(volume.LiveRestorer); ok {
74+
return vv.LiveRestoreVolume(ctx, ref)
75+
}
76+
return nil
77+
}
78+
7079
// StoreOpt sets options for a VolumeStore
7180
type StoreOpt func(store *VolumeStore) error
7281

‎volume/volume.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package volume // import "github.com/docker/docker/volume"
22

33
import (
4+
"context"
45
"time"
56
)
67

@@ -60,6 +61,15 @@ type Volume interface {
6061
Status() map[string]interface{}
6162
}
6263

64+
// LiveRestorer is an optional interface that can be implemented by a volume driver
65+
// It is used to restore any resources that are necessary for a volume to be used by a live-restored container
66+
type LiveRestorer interface {
67+
// LiveRestoreVolume allows a volume driver which implements this interface to restore any necessary resources (such as reference counting)
68+
// This is called only after the daemon is restarted with live-restored containers
69+
// It is called once per live-restored container.
70+
LiveRestoreVolume(_ context.Context, ref string) error
71+
}
72+
6373
// DetailedVolume wraps a Volume with user-defined labels, options, and cluster scope (e.g., `local` or `global`)
6474
type DetailedVolume interface {
6575
Labels() map[string]string

0 commit comments

Comments
 (0)