Safely ensure {dev,proc,alloc} are mounted

If they're unmounted by a reboot they'll be properly remounted.

client/allocdir/alloc_dir.go

@@ -416,6 +416,21 @@ func pathExists(path string) bool {
 	return true
 }
 
+// pathEmpty returns true if a path exists, is listable, and is empty. If the
+// path does not exist or is not listable an error is returned.
+func pathEmpty(path string) (bool, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return false, err
+	}
+	defer f.Close()
+	entries, err := f.Readdir(1)
+	if err != nil && err != io.EOF {
+		return false, err
+	}
+	return len(entries) == 0, nil
+}
+
 // createDir creates a directory structure inside the basepath. This functions
 // preserves the permissions of each of the subdirectories in the relative path
 // by looking up the permissions in the host.

client/allocdir/alloc_dir_test.go

@@ -106,11 +106,11 @@ func TestAllocDir_MountSharedAlloc(t *testing.T) {
 
 	// Build 2 task dirs
 	td1 := d.NewTaskDir(t1.Name)
-	if err := td1.Build(nil, cstructs.FSIsolationChroot); err != nil {
+	if err := td1.Build(false, nil, cstructs.FSIsolationChroot); err != nil {
 		t.Fatalf("error build task=%q dir: %v", t1.Name, err)
 	}
 	td2 := d.NewTaskDir(t2.Name)
-	if err := td2.Build(nil, cstructs.FSIsolationChroot); err != nil {
+	if err := td2.Build(false, nil, cstructs.FSIsolationChroot); err != nil {
 		t.Fatalf("error build task=%q dir: %v", t2.Name, err)
 	}
 
@@ -151,11 +151,11 @@ func TestAllocDir_Snapshot(t *testing.T) {
 
 	// Build 2 task dirs
 	td1 := d.NewTaskDir(t1.Name)
-	if err := td1.Build(nil, cstructs.FSIsolationImage); err != nil {
+	if err := td1.Build(false, nil, cstructs.FSIsolationImage); err != nil {
 		t.Fatalf("error build task=%q dir: %v", t1.Name, err)
 	}
 	td2 := d.NewTaskDir(t2.Name)
-	if err := td2.Build(nil, cstructs.FSIsolationImage); err != nil {
+	if err := td2.Build(false, nil, cstructs.FSIsolationImage); err != nil {
 		t.Fatalf("error build task=%q dir: %v", t2.Name, err)
 	}
 
@@ -225,7 +225,7 @@ func TestAllocDir_Move(t *testing.T) {
 	defer d2.Destroy()
 
 	td1 := d1.NewTaskDir(t1.Name)
-	if err := td1.Build(nil, cstructs.FSIsolationImage); err != nil {
+	if err := td1.Build(false, nil, cstructs.FSIsolationImage); err != nil {
 		t.Fatalf("TaskDir.Build() faild: %v", err)
 	}
 
@@ -322,7 +322,7 @@ func TestAllocDir_ReadAt_SecretDir(t *testing.T) {
 	defer d.Destroy()
 
 	td := d.NewTaskDir(t1.Name)
-	if err := td.Build(nil, cstructs.FSIsolationImage); err != nil {
+	if err := td.Build(false, nil, cstructs.FSIsolationImage); err != nil {
 		t.Fatalf("TaskDir.Build() failed: %v", err)
 	}
 
@@ -334,7 +334,7 @@ func TestAllocDir_ReadAt_SecretDir(t *testing.T) {
 }
 
 func TestAllocDir_SplitPath(t *testing.T) {
-	dir, err := ioutil.TempDir("/tmp", "tmpdirtest")
+	dir, err := ioutil.TempDir("", "tmpdirtest")
 	if err != nil {
 		log.Fatal(err)
 	}
@@ -355,7 +355,7 @@ func TestAllocDir_SplitPath(t *testing.T) {
 }
 
 func TestAllocDir_CreateDir(t *testing.T) {
-	dir, err := ioutil.TempDir("/tmp", "tmpdirtest")
+	dir, err := ioutil.TempDir("", "tmpdirtest")
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -390,3 +390,38 @@ func TestAllocDir_CreateDir(t *testing.T) {
 		t.Fatalf("wrong file mode: %v, expected: %v", fi.Mode(), subdirMode.Mode())
 	}
 }
+
+func TestPathFuncs(t *testing.T) {
+	dir, err := ioutil.TempDir("", "nomadtest-pathfuncs")
+	if err != nil {
+		t.Fatalf("error creating temp dir: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	missingDir := filepath.Join(dir, "does-not-exist")
+
+	if !pathExists(dir) {
+		t.Errorf("%q exists", dir)
+	}
+	if pathExists(missingDir) {
+		t.Errorf("%q does not exist", missingDir)
+	}
+
+	if empty, err := pathEmpty(dir); err != nil || !empty {
+		t.Errorf("%q is empty and exists. empty=%v error=%v", empty, err)
+	}
+	if empty, err := pathEmpty(missingDir); err == nil || empty {
+		t.Errorf("%q is missing. empty=%v error=%v", empty, err)
+	}
+
+	filename := filepath.Join(dir, "just-some-file")
+	f, err := os.Create(filename)
+	if err != nil {
+		t.Fatalf("could not create %q: %v", filename, err)
+	}
+	f.Close()
+
+	if empty, err := pathEmpty(dir); err != nil || empty {
+		t.Errorf("%q is not empty. empty=%v error=%v", empty, err)
+	}
+}

client/allocdir/task_dir.go

@@ -57,8 +57,10 @@ func newTaskDir(logger *log.Logger, allocDir, taskName string) *TaskDir {
 	}
 }
 
-// Build default directories and permissions in a task directory.
-func (t *TaskDir) Build(chroot map[string]string, fsi cstructs.FSIsolation) error {
+// Build default directories and permissions in a task directory. If the built
+// boolean is true Build assumes the task dir has already been built and skips
+// expensive operations like chroot creation.
+func (t *TaskDir) Build(built bool, chroot map[string]string, fsi cstructs.FSIsolation) error {
 	if err := os.MkdirAll(t.Dir, 0777); err != nil {
 		return err
 	}
@@ -69,7 +71,7 @@ func (t *TaskDir) Build(chroot map[string]string, fsi cstructs.FSIsolation) erro
 	}
 
 	// Create a local directory that each task can use.
-	if err := os.MkdirAll(t.LocalDir, 0777); err != nil {
+	if err := os.Mkdir(t.LocalDir, 0777); err != nil {
 		return err
 	}
 
@@ -94,8 +96,12 @@ func (t *TaskDir) Build(chroot map[string]string, fsi cstructs.FSIsolation) erro
 	// If there's no isolation the task will use the host path to the
 	// shared alloc dir.
 	if fsi == cstructs.FSIsolationChroot {
-		if err := linkDir(t.SharedAllocDir, t.SharedTaskDir); err != nil {
-			return fmt.Errorf("Failed to mount shared directory for task: %v", err)
+		// If the path doesn't exist OR it exists and is empty, link it
+		empty, _ := pathEmpty(t.SharedTaskDir)
+		if !pathExists(t.SharedTaskDir) || empty {
+			if err := linkDir(t.SharedAllocDir, t.SharedTaskDir); err != nil {
+				return fmt.Errorf("Failed to mount shared directory for task: %v", err)
+			}
 		}
 	}
 
@@ -110,7 +116,7 @@ func (t *TaskDir) Build(chroot map[string]string, fsi cstructs.FSIsolation) erro
 
 	// Build chroot if chroot filesystem isolation is going to be used
 	if fsi == cstructs.FSIsolationChroot {
-		if err := t.buildChroot(chroot); err != nil {
+		if err := t.buildChroot(built, chroot); err != nil {
 			return err
 		}
 	}
@@ -122,10 +128,15 @@ func (t *TaskDir) Build(chroot map[string]string, fsi cstructs.FSIsolation) erro
 // to their intended, relative location within the task directory. This
 // attempts hardlink and then defaults to copying. If the path exists on the
 // host and can't be embedded an error is returned.
-func (t *TaskDir) buildChroot(entries map[string]string) error {
-	// Link/copy chroot entries
-	if err := t.embedDirs(entries); err != nil {
-		return err
+//
+// If built is true the chroot is assumed to have been built already and only
+// ephemeral operations (eg mounting /dev) are done.
+func (t *TaskDir) buildChroot(built bool, entries map[string]string) error {
+	if !built {
+		// Link/copy chroot entries
+		if err := t.embedDirs(entries); err != nil {
+			return err
+		}
 	}
 
 	// Mount special dirs

client/allocdir/task_dir_linux.go

@@ -15,10 +15,15 @@ func (t *TaskDir) mountSpecialDirs() error {
 	// Mount dev
 	dev := filepath.Join(t.Dir, "dev")
 	if !pathExists(dev) {
-		if err := os.MkdirAll(dev, 0777); err != nil {
+		if err := os.Mkdir(dev, 0777); err != nil {
 			return fmt.Errorf("Mkdir(%v) failed: %v", dev, err)
 		}
-
+	}
+	devEmpty, err := pathEmpty(dev)
+	if err != nil {
+		return fmt.Errorf("error listing %q: %v", dev, err)
+	}
+	if devEmpty {
 		if err := syscall.Mount("none", dev, "devtmpfs", syscall.MS_RDONLY, ""); err != nil {
 			return fmt.Errorf("Couldn't mount /dev to %v: %v", dev, err)
 		}
@@ -27,10 +32,15 @@ func (t *TaskDir) mountSpecialDirs() error {
 	// Mount proc
 	proc := filepath.Join(t.Dir, "proc")
 	if !pathExists(proc) {
-		if err := os.MkdirAll(proc, 0777); err != nil {
+		if err := os.Mkdir(proc, 0777); err != nil {
 			return fmt.Errorf("Mkdir(%v) failed: %v", proc, err)
 		}
-
+	}
+	procEmpty, err := pathEmpty(proc)
+	if err != nil {
+		return fmt.Errorf("error listing %q: %v", proc, err)
+	}
+	if procEmpty {
 		if err := syscall.Mount("none", proc, "proc", syscall.MS_RDONLY, ""); err != nil {
 			return fmt.Errorf("Couldn't mount /proc to %v: %v", proc, err)
 		}
@@ -39,25 +49,27 @@ func (t *TaskDir) mountSpecialDirs() error {
 	return nil
 }
 
-// unmountSpecialDirs unmounts the dev and proc file system from the chroot
+// unmountSpecialDirs unmounts the dev and proc file system from the chroot. No
+// error is returned if the directories do not exist or have already been
+// unmounted.
 func (t *TaskDir) unmountSpecialDirs() error {
 	errs := new(multierror.Error)
 	dev := filepath.Join(t.Dir, "dev")
 	if pathExists(dev) {
-		if err := syscall.Unmount(dev, 0); err != nil {
-			errs = multierror.Append(errs, fmt.Errorf("Failed to unmount dev (%v): %v", dev, err))
+		if err := unlinkDir(dev); err != nil {
+			errs = multierror.Append(errs, fmt.Errorf("Failed to unmount dev %q: %v", dev, err))
 		} else if err := os.RemoveAll(dev); err != nil {
-			errs = multierror.Append(errs, fmt.Errorf("Failed to delete dev directory (%v): %v", dev, err))
+			errs = multierror.Append(errs, fmt.Errorf("Failed to delete dev directory %q: %v", dev, err))
 		}
 	}
 
 	// Unmount proc.
 	proc := filepath.Join(t.Dir, "proc")
 	if pathExists(proc) {
-		if err := syscall.Unmount(proc, 0); err != nil {
-			errs = multierror.Append(errs, fmt.Errorf("Failed to unmount proc (%v): %v", proc, err))
+		if err := unlinkDir(proc); err != nil {
+			errs = multierror.Append(errs, fmt.Errorf("Failed to unmount proc %q: %v", proc, err))
 		} else if err := os.RemoveAll(proc); err != nil {
-			errs = multierror.Append(errs, fmt.Errorf("Failed to delete proc directory (%v): %v", dev, err))
+			errs = multierror.Append(errs, fmt.Errorf("Failed to delete proc directory %q: %v", dev, err))
 		}
 	}
 

client/allocdir/task_dir_linux_test.go

@@ -0,0 +1,72 @@
+package allocdir
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"golang.org/x/sys/unix"
+)
+
+// TestLinuxSpecialDirs ensures mounting /dev and /proc works.
+func TestLinuxSpecialDirs(t *testing.T) {
+	if unix.Geteuid() != 0 {
+		t.Skip("Must be run as root")
+	}
+
+	allocDir, err := ioutil.TempDir("", "nomadtest-specialdirs")
+	if err != nil {
+		t.Fatalf("unable to create tempdir for test: %v", err)
+	}
+	defer os.RemoveAll(allocDir)
+
+	td := newTaskDir(testLogger(), allocDir, "test")
+
+	// Despite the task dir not existing, unmountSpecialDirs should *not*
+	// return an error
+	if err := td.unmountSpecialDirs(); err != nil {
+		t.Fatalf("error removing nonexistent special dirs: %v", err)
+	}
+
+	// Mounting special dirs in a nonexistent task dir *should* return an
+	// error
+	if err := td.mountSpecialDirs(); err == nil {
+		t.Fatalf("expected mounting in a nonexistent task dir %q to fail", td.Dir)
+	}
+
+	// Create the task dir like TaskDir.Build would
+	if err := os.MkdirAll(td.Dir, 0777); err != nil {
+		t.Fatalf("error creating task dir %q: %v", td.Dir, err)
+	}
+
+	// Mounting special dirs should now work and contain files
+	if err := td.mountSpecialDirs(); err != nil {
+		t.Fatalf("error mounting special dirs in %q: %v", td.Dir, err)
+	}
+	if empty, err := pathEmpty(filepath.Join(td.Dir, "dev")); empty || err != nil {
+		t.Fatalf("expected dev to be populated but found: empty=%v error=%v", empty, err)
+	}
+	if empty, err := pathEmpty(filepath.Join(td.Dir, "proc")); empty || err != nil {
+		t.Fatalf("expected proc to be populated but found: empty=%v error=%v", empty, err)
+	}
+
+	// Remounting again should be fine
+	if err := td.mountSpecialDirs(); err != nil {
+		t.Fatalf("error remounting special dirs in %q: %v", td.Dir, err)
+	}
+
+	// Now unmount
+	if err := td.unmountSpecialDirs(); err != nil {
+		t.Fatalf("error unmounting special dirs in %q: %v", td.Dir, err)
+	}
+	if pathExists(filepath.Join(td.Dir, "dev")) {
+		t.Fatalf("dev was not removed from %q", td.Dir)
+	}
+	if pathExists(filepath.Join(td.Dir, "proc")) {
+		t.Fatalf("proc was not removed from %q", td.Dir)
+	}
+	if err := td.unmountSpecialDirs(); err != nil {
+		t.Fatalf("error re-unmounting special dirs in %q: %v", td.Dir, err)
+	}
+}

client/allocdir/task_dir_test.go

@@ -103,7 +103,7 @@ func TestTaskDir_NonRoot_Image(t *testing.T) {
 		t.Fatalf("Build() failed: %v", err)
 	}
 
-	if err := td.Build(nil, cstructs.FSIsolationImage); err != nil {
+	if err := td.Build(false, nil, cstructs.FSIsolationImage); err != nil {
 		t.Fatalf("TaskDir.Build failed: %v", err)
 	}
 }
@@ -126,7 +126,7 @@ func TestTaskDir_NonRoot(t *testing.T) {
 		t.Fatalf("Build() failed: %v", err)
 	}
 
-	if err := td.Build(nil, cstructs.FSIsolationNone); err != nil {
+	if err := td.Build(false, nil, cstructs.FSIsolationNone); err != nil {
 		t.Fatalf("TaskDir.Build failed: %v", err)
 	}
 

client/task_runner.go

@@ -285,7 +285,7 @@ func (r *TaskRunner) RestoreState() error {
 
 		// In the case it fails, we relaunch the task in the Run() method.
 		if err != nil {
-			r.logger.Printf("[ERR] client: failed to open handle to task '%s' for alloc '%s': %v",
+			r.logger.Printf("[ERR] client: failed to open handle to task %q for alloc %q: %v",
 				r.task.Name, r.alloc.ID, err)
 			return nil
 		}
@@ -1208,11 +1208,7 @@ func (r *TaskRunner) startTask() error {
 // to call multiple times as its state is persisted.
 func (r *TaskRunner) buildTaskDir(fsi cstructs.FSIsolation) error {
 	r.persistLock.Lock()
-	if r.taskDirBuilt {
-		// Already built! Nothing to do.
-		r.persistLock.Unlock()
-		return nil
-	}
+	built := r.taskDirBuilt
 	r.persistLock.Unlock()
 
 	r.setState(structs.TaskStatePending, structs.NewTaskEvent(structs.TaskSetup).
@@ -1222,7 +1218,7 @@ func (r *TaskRunner) buildTaskDir(fsi cstructs.FSIsolation) error {
 	if len(r.config.ChrootEnv) > 0 {
 		chroot = r.config.ChrootEnv
 	}
-	if err := r.taskDir.Build(chroot, fsi); err != nil {
+	if err := r.taskDir.Build(built, chroot, fsi); err != nil {
 		return err
 	}