Fix rkt mounts and add arbitrary volume mounting

.travis.yml

@@ -17,6 +17,7 @@ before_install:
     - sudo apt-get update
     - sudo apt-get install -y docker-engine
     - sudo apt-get install -y qemu
+    - ./scripts/install_rkt.sh
     - ./scripts/install_consul.sh
     - ./scripts/install_vault.sh
 

CHANGELOG.md

@@ -13,6 +13,7 @@ IMPROVEMENTS:
   * driver: Export `NOMAD_JOB_NAME` environment variable [GH-1804]
   * driver/docker: Support Docker volumes [GH-1767]
   * driver/docker: Allow Docker logging to be configured [GH-1767]
+  * driver/rkt: Support rkt volumes (rkt >= 1.0.0 required) [GH-1812]
 
 BUG FIXES:
   * agent: Handle the SIGPIPE signal preventing panics on journalctl restarts

Vagrantfile

@@ -60,6 +60,7 @@ cd /opt/gopath/src/github.com/hashicorp/nomad && make bootstrap
 
 # Install rkt, consul and vault
 bash scripts/install_rkt.sh
+bash scripts/install_rkt_vagrant.sh
 bash scripts/install_consul.sh
 bash scripts/install_vault.sh
 

client/driver/docker.go

@@ -64,8 +64,9 @@ const (
 	dockerSELinuxLabelConfigOption = "docker.volumes.selinuxlabel"
 
 	// dockerVolumesConfigOption is the key for enabling the use of custom
-	// bind volumes.
-	dockerVolumesConfigOption = "docker.volumes.enabled"
+	// bind volumes to arbitrary host paths.
+	dockerVolumesConfigOption  = "docker.volumes.enabled"
+	dockerVolumesConfigDefault = true
 
 	// dockerPrivilegedConfigOption is the key for running containers in
 	// Docker's privileged mode.
@@ -369,8 +370,8 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool
 	node.Attributes[dockerDriverAttr] = "1"
 	node.Attributes["driver.docker.version"] = env.Get("Version")
 
-	// Advertise if this node supports Docker volumes (by default we do not)
-	if d.config.ReadBoolDefault(dockerVolumesConfigOption, false) {
+	// Advertise if this node supports Docker volumes
+	if d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault) {
 		node.Attributes["driver."+dockerVolumesConfigOption] = "1"
 	}
 
@@ -397,13 +398,30 @@ func (d *DockerDriver) containerBinds(driverConfig *DockerDriverConfig, alloc *a
 	secretDirBind := fmt.Sprintf("%s:%s", secret, allocdir.TaskSecretsContainerPath)
 	binds := []string{allocDirBind, taskLocalBind, secretDirBind}
 
-	volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, false)
-	if len(driverConfig.Volumes) > 0 && !volumesEnabled {
-		return nil, fmt.Errorf("%s is false; cannot use Docker Volumes: %+q", dockerVolumesConfigOption, driverConfig.Volumes)
-	}
+	volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault)
+	for _, userbind := range driverConfig.Volumes {
+		parts := strings.Split(userbind, ":")
+		if len(parts) < 2 {
+			return nil, fmt.Errorf("invalid docker volume: %q", userbind)
+		}
+
+		// Resolve dotted path segments
+		parts[0] = filepath.Clean(parts[0])
+
+		// Absolute paths aren't always supported
+		if filepath.IsAbs(parts[0]) {
+			if !volumesEnabled {
+				// Disallow mounting arbitrary absolute paths
+				return nil, fmt.Errorf("%s is false; cannot mount host paths: %+q", dockerVolumesConfigOption, userbind)
+			}
+			binds = append(binds, userbind)
+			continue
+		}
 
-	if len(driverConfig.Volumes) > 0 {
-		binds = append(binds, driverConfig.Volumes...)
+		// Relative paths are always allowed as they mount within a container
+		// Expand path relative to alloc dir
+		parts[0] = filepath.Join(shared, parts[0])
+		binds = append(binds, strings.Join(parts, ":"))
 	}
 
 	if selinuxLabel := d.config.Read(dockerSELinuxLabelConfigOption); selinuxLabel != "" {

client/driver/docker_test.go

@@ -5,7 +5,6 @@ import (
 	"io/ioutil"
 	"math/rand"
 	"os"
-	"path"
 	"path/filepath"
 	"reflect"
 	"runtime/debug"
@@ -944,28 +943,23 @@ done
 	}
 }
 
-func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver, *ExecContext, string, func()) {
+func setupDockerVolumes(t *testing.T, cfg *config.Config, hostpath string) (*structs.Task, Driver, *ExecContext, string, func()) {
 	if !testutil.DockerIsConnected(t) {
 		t.SkipNow()
 	}
 
-	tmpvol, err := ioutil.TempDir("", "nomadtest_dockerdriver_volumes")
-	if err != nil {
-		t.Fatalf("error creating temporary dir: %v", err)
-	}
-
 	randfn := fmt.Sprintf("test-%d", rand.Int())
-	hostpath := path.Join(tmpvol, randfn)
-	contpath := path.Join("/mnt/vol", randfn)
+	hostfile := filepath.Join(hostpath, randfn)
+	containerFile := filepath.Join("/mnt/vol", randfn)
 
 	task := &structs.Task{
 		Name: "ls",
 		Config: map[string]interface{}{
 			"image":   "busybox",
 			"load":    []string{"busybox.tar"},
 			"command": "touch",
-			"args":    []string{contpath},
-			"volumes": []string{fmt.Sprintf("%s:/mnt/vol", tmpvol)},
+			"args":    []string{containerFile},
+			"volumes": []string{fmt.Sprintf("%s:/mnt/vol", hostpath)},
 		},
 		LogConfig: &structs.LogConfig{
 			MaxFiles:      10,
@@ -980,7 +974,9 @@ func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver
 	execCtx := NewExecContext(allocDir, alloc.ID)
 	cleanup := func() {
 		execCtx.AllocDir.Destroy()
-		os.RemoveAll(tmpvol)
+		if filepath.IsAbs(hostpath) {
+			os.RemoveAll(hostpath)
+		}
 	}
 
 	taskEnv, err := GetTaskEnv(allocDir, cfg.Node, task, alloc, "")
@@ -993,26 +989,63 @@ func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver
 	driver := NewDockerDriver(driverCtx)
 	copyImage(execCtx, task, "busybox.tar", t)
 
-	return task, driver, execCtx, hostpath, cleanup
+	return task, driver, execCtx, hostfile, cleanup
 }
 
 func TestDockerDriver_VolumesDisabled(t *testing.T) {
 	cfg := testConfig()
+	cfg.Options = map[string]string{dockerVolumesConfigOption: "false"}
 
-	task, driver, execCtx, _, cleanup := setupDockerVolumes(t, cfg)
-	defer cleanup()
+	{
+		tmpvol, err := ioutil.TempDir("", "nomadtest_docker_volumesdisabled")
+		if err != nil {
+			t.Fatalf("error creating temporary dir: %v", err)
+		}
 
-	_, err := driver.Start(execCtx, task)
-	if err == nil {
-		t.Fatalf("Started driver successfully when volumes should have been disabled.")
+		task, driver, execCtx, _, cleanup := setupDockerVolumes(t, cfg, tmpvol)
+		defer cleanup()
+
+		if _, err := driver.Start(execCtx, task); err == nil {
+			t.Fatalf("Started driver successfully when volumes should have been disabled.")
+		}
 	}
+
+	// Relative paths should still be allowed
+	{
+		task, driver, execCtx, fn, cleanup := setupDockerVolumes(t, cfg, ".")
+		defer cleanup()
+
+		handle, err := driver.Start(execCtx, task)
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		defer handle.Kill()
+
+		select {
+		case res := <-handle.WaitCh():
+			if !res.Successful() {
+				t.Fatalf("unexpected err: %v", res)
+			}
+		case <-time.After(time.Duration(tu.TestMultiplier()*10) * time.Second):
+			t.Fatalf("timeout")
+		}
+
+		if _, err := ioutil.ReadFile(filepath.Join(execCtx.AllocDir.SharedDir, fn)); err != nil {
+			t.Fatalf("unexpected error reading %s: %v", fn, err)
+		}
+	}
+
 }
 
 func TestDockerDriver_VolumesEnabled(t *testing.T) {
 	cfg := testConfig()
-	cfg.Options = map[string]string{dockerVolumesConfigOption: "true"}
 
-	task, driver, execCtx, hostpath, cleanup := setupDockerVolumes(t, cfg)
+	tmpvol, err := ioutil.TempDir("", "nomadtest_docker_volumesenabled")
+	if err != nil {
+		t.Fatalf("error creating temporary dir: %v", err)
+	}
+
+	task, driver, execCtx, hostpath, cleanup := setupDockerVolumes(t, cfg, tmpvol)
 	defer cleanup()
 
 	handle, err := driver.Start(execCtx, task)

client/driver/rkt.go

@@ -21,7 +21,6 @@ import (
 	"github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/client/driver/executor"
 	dstructs "github.com/hashicorp/nomad/client/driver/structs"
-	"github.com/hashicorp/nomad/client/fingerprint"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/helper/discover"
 	"github.com/hashicorp/nomad/helper/fields"
@@ -38,19 +37,26 @@ const (
 	// minRktVersion is the earliest supported version of rkt. rkt added support
 	// for CPU and memory isolators in 0.14.0. We cannot support an earlier
 	// version to maintain an uniform interface across all drivers
-	minRktVersion = "0.14.0"
+	minRktVersion = "1.0.0"
 
 	// The key populated in the Node Attributes to indicate the presence of the
 	// Rkt driver
 	rktDriverAttr = "driver.rkt"
+
+	// rktVolumesConfigOption is the key for enabling the use of custom
+	// bind volumes.
+	rktVolumesConfigOption  = "rkt.volumes.enabled"
+	rktVolumesConfigDefault = true
+
+	// rktCmd is the command rkt is installed as.
+	rktCmd = "rkt"
 )
 
 // RktDriver is a driver for running images via Rkt
 // We attempt to chose sane defaults for now, with more configuration available
 // planned in the future
 type RktDriver struct {
 	DriverContext
-	fingerprint.StaticFingerprinter
 }
 
 type RktDriverConfig struct {
@@ -61,6 +67,7 @@ type RktDriverConfig struct {
 	DNSServers       []string `mapstructure:"dns_servers"`        // DNS Server for containers
 	DNSSearchDomains []string `mapstructure:"dns_search_domains"` // DNS Search domains for containers
 	Debug            bool     `mapstructure:"debug"`              // Enable debug option for rkt command
+	Volumes          []string `mapstructure:"volumes"`            // Host-Volumes to mount in, syntax: /path/to/host/directory:/destination/path/in/container
 }
 
 // rktHandle is returned from Start/Open as a handle to the PID
@@ -142,7 +149,7 @@ func (d *RktDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, e
 		return false, nil
 	}
 
-	outBytes, err := exec.Command("rkt", "version").Output()
+	outBytes, err := exec.Command(rktCmd, "version").Output()
 	if err != nil {
 		delete(node.Attributes, rktDriverAttr)
 		return false, nil
@@ -163,13 +170,22 @@ func (d *RktDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, e
 	minVersion, _ := version.NewVersion(minRktVersion)
 	currentVersion, _ := version.NewVersion(node.Attributes["driver.rkt.version"])
 	if currentVersion.LessThan(minVersion) {
-		// Do not allow rkt < 0.14.0
+		// Do not allow ancient rkt versions
 		d.logger.Printf("[WARN] driver.rkt: please upgrade rkt to a version >= %s", minVersion)
 		node.Attributes[rktDriverAttr] = "0"
 	}
+
+	// Advertise if this node supports rkt volumes
+	if d.config.ReadBoolDefault(rktVolumesConfigOption, rktVolumesConfigDefault) {
+		node.Attributes["driver."+rktVolumesConfigOption] = "1"
+	}
 	return true, nil
 }
 
+func (d *RktDriver) Periodic() (bool, time.Duration) {
+	return true, 15 * time.Second
+}
+
 // Run an existing Rkt image.
 func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, error) {
 	var driverConfig RktDriverConfig
@@ -198,7 +214,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e
 	insecure := false
 	if trustPrefix != "" {
 		var outBuf, errBuf bytes.Buffer
-		cmd := exec.Command("rkt", "trust", "--skip-fingerprint-review=true", fmt.Sprintf("--prefix=%s", trustPrefix), fmt.Sprintf("--debug=%t", debug))
+		cmd := exec.Command(rktCmd, "trust", "--skip-fingerprint-review=true", fmt.Sprintf("--prefix=%s", trustPrefix), fmt.Sprintf("--debug=%t", debug))
 		cmd.Stdout = &outBuf
 		cmd.Stderr = &errBuf
 		if err := cmd.Run(); err != nil {
@@ -211,15 +227,50 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e
 		insecure = true
 	}
 	cmdArgs = append(cmdArgs, "run")
-	cmdArgs = append(cmdArgs, fmt.Sprintf("--volume=%s,kind=host,source=%s", task.Name, ctx.AllocDir.SharedDir))
-	cmdArgs = append(cmdArgs, fmt.Sprintf("--mount=volume=%s,target=%s", task.Name, ctx.AllocDir.SharedDir))
+
+	// Mount /alloc
+	allocVolName := fmt.Sprintf("%s-%s-alloc", ctx.AllocID, task.Name)
+	cmdArgs = append(cmdArgs, fmt.Sprintf("--volume=%s,kind=host,source=%s", allocVolName, ctx.AllocDir.SharedDir))
+	cmdArgs = append(cmdArgs, fmt.Sprintf("--mount=volume=%s,target=%s", allocVolName, allocdir.SharedAllocContainerPath))
+
+	// Mount /local
+	localVolName := fmt.Sprintf("%s-%s-local", ctx.AllocID, task.Name)
+	cmdArgs = append(cmdArgs, fmt.Sprintf("--volume=%s,kind=host,source=%s", localVolName, filepath.Join(taskDir, allocdir.TaskLocal)))
+	cmdArgs = append(cmdArgs, fmt.Sprintf("--mount=volume=%s,target=%s", localVolName, allocdir.TaskLocalContainerPath))
+
+	// Mount /secrets
+	secretsVolName := fmt.Sprintf("%s-%s-secrets", ctx.AllocID, task.Name)
+	cmdArgs = append(cmdArgs, fmt.Sprintf("--volume=%s,kind=host,source=%s", secretsVolName, filepath.Join(taskDir, allocdir.TaskSecrets)))
+	cmdArgs = append(cmdArgs, fmt.Sprintf("--mount=volume=%s,target=%s", secretsVolName, allocdir.TaskSecretsContainerPath))
+
+	// Mount arbitrary volumes if enabled
+	if len(driverConfig.Volumes) > 0 {
+		if enabled := d.config.ReadBoolDefault(rktVolumesConfigOption, rktVolumesConfigDefault); !enabled {
+			return nil, fmt.Errorf("%s is false; cannot use rkt volumes: %+q", rktVolumesConfigOption, driverConfig.Volumes)
+		}
+
+		for i, rawvol := range driverConfig.Volumes {
+			parts := strings.Split(rawvol, ":")
+			if len(parts) != 2 {
+				return nil, fmt.Errorf("invalid rkt volume: %q", rawvol)
+			}
+			volName := fmt.Sprintf("%s-%s-%d", ctx.AllocID, task.Name, i)
+			cmdArgs = append(cmdArgs, fmt.Sprintf("--volume=%s,kind=host,source=%s", volName, parts[0]))
+			cmdArgs = append(cmdArgs, fmt.Sprintf("--mount=volume=%s,target=%s", volName, parts[1]))
+		}
+	}
+
 	cmdArgs = append(cmdArgs, img)
-	if insecure == true {
+	if insecure {
 		cmdArgs = append(cmdArgs, "--insecure-options=all")
 	}
 	cmdArgs = append(cmdArgs, fmt.Sprintf("--debug=%t", debug))
 
 	// Inject environment variables
+	d.taskEnv.SetAllocDir(allocdir.SharedAllocContainerPath)
+	d.taskEnv.SetTaskLocalDir(allocdir.TaskLocalContainerPath)
+	d.taskEnv.SetTaskLocalDir(allocdir.TaskSecretsContainerPath)
+	d.taskEnv.Build()
 	for k, v := range d.taskEnv.EnvMap() {
 		cmdArgs = append(cmdArgs, fmt.Sprintf("--set-env=%v=%v", k, v))
 	}
@@ -295,7 +346,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e
 		return nil, fmt.Errorf("failed to set executor context: %v", err)
 	}
 
-	absPath, err := GetAbsolutePath("rkt")
+	absPath, err := GetAbsolutePath(rktCmd)
 	if err != nil {
 		return nil, err
 	}