Merge pull request #21637 from tonistiigi/refactor-cleanup

Fix container mount cleanup issues

daemon/container_operations.go

@@ -720,7 +720,7 @@ func (daemon *Daemon) releaseNetwork(container *container.Container) {
 
 	sb, err := daemon.netController.SandboxByID(sid)
 	if err != nil {
-		logrus.Errorf("error locating sandbox id %s: %v", sid, err)
+		logrus.Warnf("error locating sandbox id %s: %v", sid, err)
 		return
 	}
 

daemon/daemon.go

@@ -309,10 +309,6 @@ func (daemon *Daemon) restore() error {
 				mapLock.Lock()
 				restartContainers[c] = make(chan struct{})
 				mapLock.Unlock()
-			} else if !c.IsRunning() && !c.IsPaused() {
-				if mountid, err := daemon.layerStore.GetMountID(c.ID); err == nil {
-					daemon.cleanupMountsByID(mountid)
-				}
 			}
 
 			// if c.hostConfig.Links is nil (not just empty), then it is using the old sqlite links and needs to be migrated

daemon/daemon_linux.go

@@ -5,7 +5,7 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/Sirupsen/logrus"
@@ -28,91 +28,53 @@ func (daemon *Daemon) cleanupMountsFromReaderByID(reader io.Reader, id string, u
 		return nil
 	}
 	var errors []string
-	mountRoot := ""
-	shmSuffix := "/" + id + "/shm"
-	mergedSuffix := "/" + id + "/merged"
+
+	regexps := getCleanPatterns(id)
 	sc := bufio.NewScanner(reader)
 	for sc.Scan() {
-		line := sc.Text()
-		fields := strings.Fields(line)
-		if strings.HasPrefix(fields[4], daemon.root) {
-			logrus.Debugf("Mount base: %v", fields[4])
-			mnt := fields[4]
-			if strings.HasSuffix(mnt, shmSuffix) || strings.HasSuffix(mnt, mergedSuffix) {
-				logrus.Debugf("Unmounting %v", mnt)
-				if err := unmount(mnt); err != nil {
-					logrus.Error(err)
-					errors = append(errors, err.Error())
+		if fields := strings.Fields(sc.Text()); len(fields) >= 4 {
+			if mnt := fields[4]; strings.HasPrefix(mnt, daemon.root) {
+				for _, p := range regexps {
+					if p.MatchString(mnt) {
+						if err := unmount(mnt); err != nil {
+							logrus.Error(err)
+							errors = append(errors, err.Error())
+						}
+					}
 				}
-			} else if mountBase := filepath.Base(mnt); mountBase == id {
-				mountRoot = mnt
 			}
 		}
 	}
 
-	if mountRoot != "" {
-		logrus.Debugf("Unmounting %v", mountRoot)
-		if err := unmount(mountRoot); err != nil {
-			logrus.Error(err)
-			errors = append(errors, err.Error())
-		}
-	}
-
 	if err := sc.Err(); err != nil {
 		return err
 	}
 
 	if len(errors) > 0 {
-		return fmt.Errorf("Error cleaningup mounts:\n%v", strings.Join(errors, "\n"))
+		return fmt.Errorf("Error cleaning up mounts:\n%v", strings.Join(errors, "\n"))
 	}
 
-	logrus.Debugf("Cleaning up old container shm/mqueue/rootfs mounts: done.")
+	logrus.Debugf("Cleaning up old mountid %v: done.", id)
 	return nil
 }
 
 // cleanupMounts umounts shm/mqueue mounts for old containers
 func (daemon *Daemon) cleanupMounts() error {
-	logrus.Debugf("Cleaning up old container shm/mqueue/rootfs mounts: start.")
-	f, err := os.Open("/proc/self/mountinfo")
-	if err != nil {
-		return err
-	}
-	defer f.Close()
-
-	return daemon.cleanupMountsFromReader(f, mount.Unmount)
+	return daemon.cleanupMountsByID("")
 }
 
-func (daemon *Daemon) cleanupMountsFromReader(reader io.Reader, unmount func(target string) error) error {
-	if daemon.root == "" {
-		return nil
+func getCleanPatterns(id string) (regexps []*regexp.Regexp) {
+	var patterns []string
+	if id == "" {
+		id = "[0-9a-f]{64}"
+		patterns = append(patterns, "containers/"+id+"/shm")
 	}
-	sc := bufio.NewScanner(reader)
-	var errors []string
-	for sc.Scan() {
-		line := sc.Text()
-		fields := strings.Fields(line)
-		if strings.HasPrefix(fields[4], daemon.root) {
-			logrus.Debugf("Mount base: %v", fields[4])
-			mnt := fields[4]
-			mountBase := filepath.Base(mnt)
-			if mountBase == "shm" || mountBase == "merged" {
-				logrus.Debugf("Unmounting %v", mnt)
-				if err := unmount(mnt); err != nil {
-					logrus.Error(err)
-					errors = append(errors, err.Error())
-				}
-			}
+	patterns = append(patterns, "aufs/mnt/"+id+"$", "overlay/"+id+"/merged$", "zfs/graph/"+id+"$")
+	for _, p := range patterns {
+		r, err := regexp.Compile(p)
+		if err == nil {
+			regexps = append(regexps, r)
 		}
 	}
-
-	if err := sc.Err(); err != nil {
-		return err
-	}
-
-	if len(errors) > 0 {
-		return fmt.Errorf("Error cleaningup mounts:\n%v", strings.Join(errors, "\n"))
-	}
-
-	logrus.Debugf("Cleaning up old container shm/mqueue/rootfs mounts: done.")
-	return nil
+	return
 }

daemon/daemon_linux_test.go

@@ -59,7 +59,7 @@ func TestCleanupMounts(t *testing.T) {
 		return nil
 	}
 
-	d.cleanupMountsFromReader(strings.NewReader(mountsFixture), unmount)
+	d.cleanupMountsFromReaderByID(strings.NewReader(mountsFixture), "", unmount)
 
 	if unmounted != 1 {
 		t.Fatalf("Expected to unmount the shm (and the shm only)")
@@ -97,7 +97,7 @@ func TestNotCleanupMounts(t *testing.T) {
 		return nil
 	}
 	mountInfo := `234 232 0:59 / /dev/shm rw,nosuid,nodev,noexec,relatime - tmpfs shm rw,size=65536k`
-	d.cleanupMountsFromReader(strings.NewReader(mountInfo), unmount)
+	d.cleanupMountsFromReaderByID(strings.NewReader(mountInfo), "", unmount)
 	if unmounted {
 		t.Fatalf("Expected not to clean up /dev/shm")
 	}

daemon/start.go

@@ -174,8 +174,10 @@ func (daemon *Daemon) Cleanup(container *container.Container) {
 		daemon.unregisterExecCommand(container, eConfig)
 	}
 
-	if err := container.UnmountVolumes(false, daemon.LogVolumeEvent); err != nil {
-		logrus.Warnf("%s cleanup: Failed to umount volumes: %v", container.ID, err)
+	if container.BaseFS != "" {
+		if err := container.UnmountVolumes(false, daemon.LogVolumeEvent); err != nil {
+			logrus.Warnf("%s cleanup: Failed to umount volumes: %v", container.ID, err)
+		}
 	}
 	container.CancelAttachContext()
 }

libcontainerd/client_shutdownrestore_linux.go

@@ -31,8 +31,10 @@ func (clnt *client) Restore(containerID string, options ...CreateOption) error {
 			select {
 			case <-time.After(2 * time.Second):
 			case <-w.wait():
+				return nil
 			}
 		case <-w.wait():
+			return nil
 		}
 	}
 	return clnt.setExited(containerID)