Merge pull request #944 from hashicorp/f-artifact-location

Download artifacts to relative locations inside the task directory

api/tasks.go

@@ -99,6 +99,7 @@ type Task struct {
 type TaskArtifact struct {
 	GetterSource  string
 	GetterOptions map[string]string
+	RelativeDest  string
 }
 
 // NewTask creates and initializes a new Task.

client/driver/exec.go

@@ -113,7 +113,7 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle,
 	}, executorCtx)
 	if err != nil {
 		pluginClient.Kill()
-		return nil, fmt.Errorf("error starting process via the plugin: %v", err)
+		return nil, err
 	}
 	d.logger.Printf("[DEBUG] driver.exec: started process via plugin with pid: %v", ps.Pid)
 

client/driver/executor/executor.go

@@ -7,6 +7,7 @@ import (
 	"net"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"sync"
@@ -159,20 +160,36 @@ func (e *UniversalExecutor) LaunchCmd(command *ExecCommand, ctx *ExecutorContext
 	e.cmd.Stdout = e.lro
 	e.cmd.Stderr = e.lre
 
-	// setting the env, path and args for the command
 	e.ctx.TaskEnv.Build()
-	e.cmd.Env = ctx.TaskEnv.EnvList()
-	e.cmd.Path = ctx.TaskEnv.ReplaceEnv(command.Cmd)
-	e.cmd.Args = append([]string{e.cmd.Path}, ctx.TaskEnv.ParseAndReplace(command.Args)...)
 
-	// Ensure that the binary being started is executable.
-	if err := e.makeExecutable(e.cmd.Path); err != nil {
+	// Look up the binary path and make it executable
+	absPath, err := e.lookupBin(ctx.TaskEnv.ReplaceEnv(command.Cmd))
+	if err != nil {
+		return nil, err
+	}
+
+	if err := e.makeExecutable(absPath); err != nil {
 		return nil, err
 	}
 
-	// starting the process
+	// Determine the path to run as it may have to be relative to the chroot.
+	path := absPath
+	if e.command.FSIsolation {
+		rel, err := filepath.Rel(e.taskDir, absPath)
+		if err != nil {
+			return nil, err
+		}
+		path = rel
+	}
+
+	// Set the commands arguments
+	e.cmd.Path = path
+	e.cmd.Args = append([]string{path}, ctx.TaskEnv.ParseAndReplace(command.Args)...)
+	e.cmd.Env = ctx.TaskEnv.EnvList()
+
+	// Start the process
 	if err := e.cmd.Start(); err != nil {
-		return nil, fmt.Errorf("error starting command: %v", err)
+		return nil, err
 	}
 	go e.wait()
 	ic := &cstructs.IsolationConfig{Cgroup: e.groups}
@@ -328,8 +345,36 @@ func (e *UniversalExecutor) configureTaskDir() error {
 	return nil
 }
 
-// makeExecutablePosix makes the given file executable for root,group,others.
-func (e *UniversalExecutor) makeExecutablePosix(binPath string) error {
+// lookupBin looks for path to the binary to run by looking for the binary in
+// the following locations, in-order: task/local/, task/, based on host $PATH.
+// The return path is absolute.
+func (e *UniversalExecutor) lookupBin(bin string) (string, error) {
+	// Check in the local directory
+	local := filepath.Join(e.taskDir, allocdir.TaskLocal, bin)
+	if _, err := os.Stat(local); err == nil {
+		return local, nil
+	}
+
+	// Check at the root of the task's directory
+	root := filepath.Join(e.taskDir, bin)
+	if _, err := os.Stat(root); err == nil {
+		return root, nil
+	}
+
+	// Check the $PATH
+	if host, err := exec.LookPath(bin); err == nil {
+		return host, nil
+	}
+
+	return "", fmt.Errorf("binary %q could not be found", bin)
+}
+
+// makeExecutable makes the given file executable for root,group,others.
+func (e *UniversalExecutor) makeExecutable(binPath string) error {
+	if runtime.GOOS == "windows" {
+		return nil
+	}
+
 	fi, err := os.Stat(binPath)
 	if err != nil {
 		if os.IsNotExist(err) {

client/driver/executor/executor_basic.go

@@ -2,25 +2,7 @@
 
 package executor
 
-import (
-	"path/filepath"
-	"runtime"
-
-	cgroupConfig "github.com/opencontainers/runc/libcontainer/configs"
-)
-
-func (e *UniversalExecutor) makeExecutable(binPath string) error {
-	if runtime.GOOS == "windows" {
-		return nil
-	}
-
-	path := binPath
-	if !filepath.IsAbs(binPath) {
-		// The path must be relative the allocations directory.
-		path = filepath.Join(e.taskDir, binPath)
-	}
-	return e.makeExecutablePosix(path)
-}
+import cgroupConfig "github.com/opencontainers/runc/libcontainer/configs"
 
 func (e *UniversalExecutor) configureChroot() error {
 	return nil

client/driver/executor/executor_linux.go

@@ -36,18 +36,6 @@ var (
 	}
 )
 
-func (e *UniversalExecutor) makeExecutable(binPath string) error {
-	path := binPath
-	if e.command.FSIsolation {
-		// The path must be relative the chroot
-		path = filepath.Join(e.taskDir, binPath)
-	} else if !filepath.IsAbs(binPath) {
-		// The path must be relative the allocations directory.
-		path = filepath.Join(e.taskDir, binPath)
-	}
-	return e.makeExecutablePosix(path)
-}
-
 // configureIsolation configures chroot and creates cgroups
 func (e *UniversalExecutor) configureIsolation() error {
 	if e.command.FSIsolation {

client/driver/executor/executor_test.go

@@ -232,3 +232,38 @@ func TestExecutor_Start_Kill(t *testing.T) {
 		t.Fatalf("Command output incorrectly: want %v; got %v", expected, act)
 	}
 }
+
+func TestExecutor_MakeExecutable(t *testing.T) {
+	// Create a temp file
+	f, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+	defer os.Remove(f.Name())
+
+	// Set its permissions to be non-executable
+	f.Chmod(os.FileMode(0610))
+
+	// Make a fake exececutor
+	ctx := testExecutorContext(t)
+	defer ctx.AllocDir.Destroy()
+	executor := NewExecutor(log.New(os.Stdout, "", log.LstdFlags))
+
+	err = executor.(*UniversalExecutor).makeExecutable(f.Name())
+	if err != nil {
+		t.Fatalf("makeExecutable() failed: %v", err)
+	}
+
+	// Check the permissions
+	stat, err := f.Stat()
+	if err != nil {
+		t.Fatalf("Stat() failed: %v", err)
+	}
+
+	act := stat.Mode().Perm()
+	exp := os.FileMode(0755)
+	if act != exp {
+		t.Fatalf("expected permissions %v; got %v", err)
+	}
+}

client/driver/java.go

@@ -173,7 +173,7 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle,
 	}, executorCtx)
 	if err != nil {
 		pluginClient.Kill()
-		return nil, fmt.Errorf("error starting process via the plugin: %v", err)
+		return nil, err
 	}
 	d.logger.Printf("[DEBUG] driver.java: started process with pid: %v", ps.Pid)
 

client/driver/qemu.go

@@ -198,7 +198,7 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle,
 	ps, err := exec.LaunchCmd(&executor.ExecCommand{Cmd: args[0], Args: args[1:]}, executorCtx)
 	if err != nil {
 		pluginClient.Kill()
-		return nil, fmt.Errorf("error starting process via the plugin: %v", err)
+		return nil, err
 	}
 	d.logger.Printf("[INFO] Started new QemuVM: %s", vmID)
 

client/driver/raw_exec.go

@@ -103,7 +103,7 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandl
 	ps, err := exec.LaunchCmd(&executor.ExecCommand{Cmd: command, Args: driverConfig.Args}, executorCtx)
 	if err != nil {
 		pluginClient.Kill()
-		return nil, fmt.Errorf("error starting process via the plugin: %v", err)
+		return nil, err
 	}
 	d.logger.Printf("[DEBUG] driver.raw_exec: started process with pid: %v", ps.Pid)
 

client/driver/rkt.go

@@ -246,7 +246,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e
 	ps, err := execIntf.LaunchCmd(&executor.ExecCommand{Cmd: absPath, Args: cmdArgs}, executorCtx)
 	if err != nil {
 		pluginClient.Kill()
-		return nil, fmt.Errorf("error starting process via the plugin: %v", err)
+		return nil, err
 	}
 
 	d.logger.Printf("[DEBUG] driver.rkt: started ACI %q with: %v", img, cmdArgs)

client/getter/getter.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"log"
 	"net/url"
+	"path/filepath"
 	"sync"
 
 	gg "github.com/hashicorp/go-getter"
@@ -59,16 +60,17 @@ func getGetterUrl(artifact *structs.TaskArtifact) (string, error) {
 	return u.String(), nil
 }
 
-// GetArtifact downloads an artifact into the specified destination directory.
-func GetArtifact(artifact *structs.TaskArtifact, destDir string, logger *log.Logger) error {
+// GetArtifact downloads an artifact into the specified task directory.
+func GetArtifact(artifact *structs.TaskArtifact, taskDir string, logger *log.Logger) error {
 	url, err := getGetterUrl(artifact)
 	if err != nil {
 		return err
 	}
 
 	// Download the artifact
-	if err := getClient(url, destDir).Get(); err != nil {
-		return err
+	dest := filepath.Join(taskDir, artifact.RelativeDest)
+	if err := getClient(url, dest).Get(); err != nil {
+		return fmt.Errorf("GET error: %v", err)
 	}
 
 	return nil

client/getter/getter_test.go

@@ -21,11 +21,11 @@ func TestGetArtifact_FileAndChecksum(t *testing.T) {
 	defer ts.Close()
 
 	// Create a temp directory to download into
-	destDir, err := ioutil.TempDir("", "nomad-test")
+	taskDir, err := ioutil.TempDir("", "nomad-test")
 	if err != nil {
 		t.Fatalf("failed to make temp directory: %v", err)
 	}
-	defer os.RemoveAll(destDir)
+	defer os.RemoveAll(taskDir)
 
 	// Create the artifact
 	file := "test.sh"
@@ -38,13 +38,48 @@ func TestGetArtifact_FileAndChecksum(t *testing.T) {
 
 	// Download the artifact
 	logger := log.New(os.Stderr, "", log.LstdFlags)
-	if err := GetArtifact(artifact, destDir, logger); err != nil {
+	if err := GetArtifact(artifact, taskDir, logger); err != nil {
 		t.Fatalf("GetArtifact failed: %v", err)
 	}
 
 	// Verify artifact exists
-	if _, err := os.Stat(filepath.Join(destDir, file)); err != nil {
-		t.Fatalf("source path error: %s", err)
+	if _, err := os.Stat(filepath.Join(taskDir, file)); err != nil {
+		t.Fatalf("file not found: %s", err)
+	}
+}
+
+func TestGetArtifact_File_RelativeDest(t *testing.T) {
+	// Create the test server hosting the file to download
+	ts := httptest.NewServer(http.FileServer(http.Dir(filepath.Dir("./test-fixtures/"))))
+	defer ts.Close()
+
+	// Create a temp directory to download into
+	taskDir, err := ioutil.TempDir("", "nomad-test")
+	if err != nil {
+		t.Fatalf("failed to make temp directory: %v", err)
+	}
+	defer os.RemoveAll(taskDir)
+
+	// Create the artifact
+	file := "test.sh"
+	relative := "foo/"
+	artifact := &structs.TaskArtifact{
+		GetterSource: fmt.Sprintf("%s/%s", ts.URL, file),
+		GetterOptions: map[string]string{
+			"checksum": "md5:bce963762aa2dbfed13caf492a45fb72",
+		},
+		RelativeDest: relative,
+	}
+
+	// Download the artifact
+	logger := log.New(os.Stderr, "", log.LstdFlags)
+	if err := GetArtifact(artifact, taskDir, logger); err != nil {
+		t.Fatalf("GetArtifact failed: %v", err)
+	}
+
+	// Verify artifact was downloaded to the correct path
+	if _, err := os.Stat(filepath.Join(taskDir, relative, file)); err != nil {
+		t.Fatalf("file not found: %s", err)
 	}
 }
 
@@ -54,11 +89,11 @@ func TestGetArtifact_InvalidChecksum(t *testing.T) {
 	defer ts.Close()
 
 	// Create a temp directory to download into
-	destDir, err := ioutil.TempDir("", "nomad-test")
+	taskDir, err := ioutil.TempDir("", "nomad-test")
 	if err != nil {
 		t.Fatalf("failed to make temp directory: %v", err)
 	}
-	defer os.RemoveAll(destDir)
+	defer os.RemoveAll(taskDir)
 
 	// Create the artifact with an incorrect checksum
 	file := "test.sh"
@@ -71,7 +106,7 @@ func TestGetArtifact_InvalidChecksum(t *testing.T) {
 
 	// Download the artifact and expect an error
 	logger := log.New(os.Stderr, "", log.LstdFlags)
-	if err := GetArtifact(artifact, destDir, logger); err == nil {
+	if err := GetArtifact(artifact, taskDir, logger); err == nil {
 		t.Fatalf("GetArtifact should have failed")
 	}
 }
@@ -116,17 +151,17 @@ func TestGetArtifact_Archive(t *testing.T) {
 
 	// Create a temp directory to download into and create some of the same
 	// files that exist in the artifact to ensure they are overriden
-	destDir, err := ioutil.TempDir("", "nomad-test")
+	taskDir, err := ioutil.TempDir("", "nomad-test")
 	if err != nil {
 		t.Fatalf("failed to make temp directory: %v", err)
 	}
-	defer os.RemoveAll(destDir)
+	defer os.RemoveAll(taskDir)
 
 	create := map[string]string{
 		"exist/my.config": "to be replaced",
 		"untouched":       "existing top-level",
 	}
-	createContents(destDir, create, t)
+	createContents(taskDir, create, t)
 
 	file := "archive.tar.gz"
 	artifact := &structs.TaskArtifact{
@@ -137,7 +172,7 @@ func TestGetArtifact_Archive(t *testing.T) {
 	}
 
 	logger := log.New(os.Stderr, "", log.LstdFlags)
-	if err := GetArtifact(artifact, destDir, logger); err != nil {
+	if err := GetArtifact(artifact, taskDir, logger); err != nil {
 		t.Fatalf("GetArtifact failed: %v", err)
 	}
 
@@ -148,5 +183,5 @@ func TestGetArtifact_Archive(t *testing.T) {
 		"new/my.config":   "hello world\n",
 		"test.sh":         "sleep 1\n",
 	}
-	checkContents(destDir, expected, t)
+	checkContents(taskDir, expected, t)
 }