Repository: moby/moby (language: go)

Issue:
docker volume prune removes ALL volumes when live restore is enabled and unless-stopped restart policy is used
**Note : issue edited to include an additional restart of Docker daemon to really reproduce the issue from scratch.**

**Description**

When [live restore](https://docs.docker.com/config/containers/live-restore/) is enabled, stopping and restarting the daemon then running `docker volume prune` removes **all volumes** mounted in containers with `unless-stopped` policy.

It is very dangerous, as volumes used by running containers will just disappear with no warning.

**Steps to reproduce the issue:**

Here is a MWE from a fresh Debian 10.

1. Install Docker from the [Docker repository](https://docs.docker.com/engine/install/debian/#install-using-the-repository)
2. Add yourself into the `docker` group
3. Start the Docker daemon a first time :
```bash
sudo systemctl start docker
```
3. Edit `/etc/docker/daemon.json` with the following content to enable live restore :
```json
{
	"live-restore": true
}
```
5. Restart the Docker daemon to enable live restore :
```bash
$ sudo systemctl restart docker
```
6. Create a local volume (`pica`) :
```bash
$ docker volume create pica
```
7. Create a dummy container with `unless-stopped` policy and mount the volume :
```bash
$ docker run -d --volume pica:/pica --name pica --restart unless-stopped busybox sleep 10000
```
8. Check that the container is running :
```bash
$ docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES
32749a84d3a6        busybox             "sleep 10000"       17 seconds ago      Up 16 seconds                           pica
```
9. Restart the daemon. With live restore enabled, the container won't stop.
```bash
$ sudo systemctl restart docker
```
10. Check that the container is indeed still running :
```bash
$ docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED              STATUS              PORTS               NAMES
32749a84d3a6        busybox             "sleep 10000"       About a minute ago   Up 38 seconds                           pica
```
11. Prune volumes :
```bash
$ docker volume prune
WARNING! This will remove all local volumes not used by at least one container.
Are you sure you want to continue? [y/N] y
Deleted Volumes:
pica

Total reclaimed space: 0B
```

**Describe the results you received:**

The `pica` volume is **removed** whereas the `pica` container is **still running**.
Even after the removal, we can still execute a command in the container, showing that it is **really** still running :
```bash
 docker exec -it pica sh
/ # 
```

Excerpt of `docker inspect pica` :

```json
"Mounts": [
            {
                "Type": "volume",
                "Name": "pica",
                "Source": "/var/lib/docker/volumes/pica/_data",
                "Destination": "/pica",
                "Driver": "local",
                "Mode": "z",
                "RW": true,
                "Propagation": ""
            }
        ]
```

Obviously the volume has been removed, so :
```bash
$ docker volume inspect pica
[]
Error: No such volume: pica

$ sudo ls /var/lib/docker/volumes/pica/_data
ls: cannot access '/var/lib/docker/volumes/pica/_data': No such file or directory
```

**Describe the results you expected:**

Volumes mounted in running containers should not be removed when running `docker volume prune`.

**Additional information you deem important (e.g. issue happens only occasionally):**

With live restore alone or `unless-stopped` alone, `docker volume prune` will not remove volumes mounted in running containers.

If a container has been manually restarted after restarting the Docker daemon, the volume won't be removed.

**Output of `docker version`:**

```
Client: Docker Engine - Community
 Version:           19.03.13
 API version:       1.40
 Go version:        go1.13.15
 Git commit:        4484c46d9d
 Built:             Wed Sep 16 17:02:55 2020
 OS/Arch:           linux/amd64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          19.03.13
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       4484c46d9d
  Built:            Wed Sep 16 17:01:25 2020
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.3.7
  GitCommit:        8fba4e9a7d01810a393d5d25a3621dc101981175
 runc:
  Version:          1.0.0-rc10
  GitCommit:        dc9208a3303feef5b3839f4323d9beb36df0a9dd
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683
```

**Output of `docker info`:**

```
Client:
 Debug Mode: false

Server:
 Containers: 1
  Running: 1
  Paused: 0
  Stopped: 0
 Images: 1
 Server Version: 19.03.13
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 8fba4e9a7d01810a393d5d25a3621dc101981175
 runc version: dc9208a3303feef5b3839f4323d9beb36df0a9dd
 init version: fec3683
 Security Options:
  apparmor
  seccomp
   Profile: default
 Kernel Version: 4.19.0-5-cloud-amd64
 Operating System: Debian GNU/Linux 10 (buster)
 OSType: linux
 Architecture: x86_64
 CPUs: 1
 Total Memory: 1.904GiB
 Name: vps-5c256a04
 ID: D3BW:DYRD:D2UJ:5FAM:NOIR:CEPB:CDKT:DVBP:OFLO:WNFW:SPLK:LTCA
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: true

WARNING: No swap limit support
```

**Additional environment details (AWS, VirtualBox, physical, etc.):**

Tested with Debian 10 on virtual machines.



Discussion / hints:
Could reproduce this bug on a fresh Debian 10.6 VM, following the specified steps.
Thanks for reporting!

> Create a dummy container with unless-stopped policy and mount the volume :

is it only happening with that restart policy?


My initial suspicion would be that `dockerd` keeps reference counts for volumes and mounts in memory, and would do so when creating/starting/stopping containers. Possibly in the "live-restore" situation, it wouldn't have to recreate (or restart) that container, and thus doesn't increment the reference counts 🤔 

/cc @cpuguy83 



Thanks for your answer ! :grin: 

> is it only happening with that restart policy?

We only use `unless-stopped` policy, so I didn't initially check the other ones.

This issue occurs with `always` and `unless-stopped` policies, and does not occur with `no` and `on-failure` policies.

So even with live restore enabled, with a `no` policy, the container does **not** restart when `dockerd` restarts and `volume prune` **won't** remove the mounted volume.
> So even with live restore enabled, with a no policy, the container does not restart when dockerd restarts and volume prune won't remove the mounted volume.

Hmmm "interesting" 🤔 
This just happened here as well with the result of losing data. Very dangerous bug, any update on this? 
Can confirm still a problem :P fortunately had backups to roll back to, but would be nice to reconcile state to avoid pruning in-use volumes
I can confirm that this also can happen without `"live-restore": true` because happened to me 3 days ago.

Fix patch:
diff --git a/daemon/daemon.go b/daemon/daemon.go
index 09d9356d26927..272ab6bc5c945 100644
--- a/daemon/daemon.go
+++ b/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")
 			}


Test patch:
diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go
index 72ffdd1dbffeb..42e415526902a 100644
--- a/integration/daemon/daemon_test.go
+++ b/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")
+	})
+}
diff --git a/integration/daemon/main_test.go b/integration/daemon/main_test.go
index 74a342e298243..6fb7250a0fdb0 100644
--- a/integration/daemon/main_test.go
+++ b/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())
 }