From 0ebdff2bb736f51552580fd4e1ec1e4969c767de Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 29 Aug 2016 15:35:14 -0700 Subject: [PATCH 1/5] Secret dir, hello world --- client/alloc_runner.go | 25 ++++++--- client/allocdir/alloc_dir.go | 43 ++++++++++++++- client/allocdir/alloc_dir_darwin.go | 10 ++-- client/allocdir/alloc_dir_freebsd.go | 10 ++-- client/allocdir/alloc_dir_linux.go | 10 ++-- client/allocdir/alloc_dir_windows.go | 6 +- client/client.go | 22 +++++++- client/secretdir/secret_dir.go | 67 +++++++++++++++++++++++ client/secretdir/secret_dir_test.go | 70 ++++++++++++++++++++++++ client/secretdir/secret_dir_universal.go | 15 +++++ client/secretdir/secret_dir_unix.go | 31 +++++++++++ client/secretdir/secret_dir_unix_test.go | 1 + 12 files changed, 277 insertions(+), 33 deletions(-) create mode 100644 client/secretdir/secret_dir.go create mode 100644 client/secretdir/secret_dir_test.go create mode 100644 client/secretdir/secret_dir_universal.go create mode 100644 client/secretdir/secret_dir_unix.go create mode 100644 client/secretdir/secret_dir_unix_test.go diff --git a/client/alloc_runner.go b/client/alloc_runner.go index e2705a341..5ec8f2eb3 100644 --- a/client/alloc_runner.go +++ b/client/alloc_runner.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver" + "github.com/hashicorp/nomad/client/secretdir" "github.com/hashicorp/nomad/nomad/structs" cstructs "github.com/hashicorp/nomad/client/structs" @@ -51,17 +52,20 @@ type AllocRunner struct { dirtyCh chan struct{} - ctx *driver.ExecContext - ctxLock sync.Mutex - tasks map[string]*TaskRunner - taskStates map[string]*structs.TaskState - restored map[string]struct{} - taskLock sync.RWMutex + ctx *driver.ExecContext + ctxLock sync.Mutex + tasks map[string]*TaskRunner + restored map[string]struct{} + taskLock sync.RWMutex + + taskStates map[string]*structs.TaskState taskStatusLock sync.RWMutex updateCh chan *structs.Allocation + secretDir *secretdir.SecretDir + destroy bool destroyCh chan struct{} destroyLock sync.Mutex @@ -80,12 +84,13 @@ type allocRunnerState struct { // NewAllocRunner is used to create a new allocation context func NewAllocRunner(logger *log.Logger, config *config.Config, updater AllocStateUpdater, - alloc *structs.Allocation) *AllocRunner { + alloc *structs.Allocation, secretDir *secretdir.SecretDir) *AllocRunner { ar := &AllocRunner{ config: config, updater: updater, logger: logger, alloc: alloc, + secretDir: secretDir, dirtyCh: make(chan struct{}, 1), tasks: make(map[string]*TaskRunner), taskStates: copyTaskStates(alloc.TaskStates), @@ -386,9 +391,11 @@ func (r *AllocRunner) Run() { // Create the execution context r.ctxLock.Lock() if r.ctx == nil { - allocDir := allocdir.NewAllocDir(filepath.Join(r.config.AllocDir, r.alloc.ID), r.Alloc().Resources.DiskMB) + path := filepath.Join(r.config.AllocDir, r.alloc.ID) + size := r.Alloc().Resources.DiskMB + allocDir := allocdir.NewAllocDir(r.alloc.ID, path, size, r.secretDir) if err := allocDir.Build(tg.Tasks); err != nil { - r.logger.Printf("[WARN] client: failed to build task directories: %v", err) + r.logger.Printf("[ERR] client: failed to build task directories: %v", err) r.setStatus(structs.AllocClientStatusFailed, fmt.Sprintf("failed to build task dirs for '%s'", alloc.TaskGroup)) r.ctxLock.Unlock() return diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go index 6d0a41ee0..209297e83 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -14,6 +14,7 @@ import ( "gopkg.in/tomb.v1" "github.com/hashicorp/go-multierror" + "github.com/hashicorp/nomad/client/secretdir" "github.com/hashicorp/nomad/nomad/structs" "github.com/hpcloud/tail/watch" ) @@ -46,11 +47,21 @@ var ( // regardless of driver. TaskLocal = "local" + // TaskSecrets is the the name of the secret directory inside each task + // directory + TaskSecrets = "secrets" + // TaskDirs is the set of directories created in each tasks directory. TaskDirs = []string{"tmp"} ) type AllocDir struct { + // AllocID is the allocation ID for this directory + AllocID string + + // SecretDir is used to build the secret directory for the allocation + SecretDir *secretdir.SecretDir + // AllocDir is the directory used for storing any state // of this allocation. It will be purged on alloc destroy. AllocDir string @@ -110,8 +121,10 @@ type AllocDirFS interface { // NewAllocDir initializes the AllocDir struct with allocDir as base path for // the allocation directory and maxSize as the maximum allowed size in megabytes. -func NewAllocDir(allocDir string, maxSize int) *AllocDir { +func NewAllocDir(allocID, allocDir string, maxSize int, sdir *secretdir.SecretDir) *AllocDir { d := &AllocDir{ + AllocID: allocID, + SecretDir: sdir, AllocDir: allocDir, MaxCheckDiskInterval: maxCheckDiskInterval, MinCheckDiskInterval: minCheckDiskInterval, @@ -145,7 +158,7 @@ func (d *AllocDir) UnmountAll() error { // Check if the directory has the shared alloc mounted. taskAlloc := filepath.Join(dir, SharedAllocName) if d.pathExists(taskAlloc) { - if err := d.unmountSharedDir(taskAlloc); err != nil { + if err := d.unmount(taskAlloc); err != nil { mErr.Errors = append(mErr.Errors, fmt.Errorf("failed to unmount shared alloc dir %q: %v", taskAlloc, err)) } else if err := os.RemoveAll(taskAlloc); err != nil { @@ -154,6 +167,18 @@ func (d *AllocDir) UnmountAll() error { } } + // Remove the secrets dir + taskSecrets := filepath.Join(dir, TaskSecrets) + if d.pathExists(taskSecrets) { + if err := d.unmount(taskSecrets); err != nil { + mErr.Errors = append(mErr.Errors, + fmt.Errorf("failed to unmount secrets dir %q: %v", taskSecrets, err)) + } else if err := os.RemoveAll(taskSecrets); err != nil { + mErr.Errors = append(mErr.Errors, + fmt.Errorf("failed to delete secrets dir %q: %v", taskSecrets, err)) + } + } + // Unmount dev/ and proc/ have been mounted. d.unmountSpecialDirs(dir) } @@ -223,6 +248,18 @@ func (d *AllocDir) Build(tasks []*structs.Task) error { return err } } + + // Get the secret directory + sdir, err := d.SecretDir.CreateFor(d.AllocID, t.Name) + if err != nil { + return fmt.Errorf("Creating secret directory for task %q failed: %v", t.Name, err) + } + + // Mount the secret directory + taskSecret := filepath.Join(taskDir, TaskSecrets) + if err := d.mount(sdir, taskSecret); err != nil { + return fmt.Errorf("failed to mount secret directory: %v", err) + } } return nil @@ -332,7 +369,7 @@ func (d *AllocDir) MountSharedDir(task string) error { } taskLoc := filepath.Join(taskDir, SharedAllocName) - if err := d.mountSharedDir(taskLoc); err != nil { + if err := d.mount(d.SharedDir, taskLoc); err != nil { return fmt.Errorf("Failed to mount shared directory for task %v: %v", task, err) } diff --git a/client/allocdir/alloc_dir_darwin.go b/client/allocdir/alloc_dir_darwin.go index 2cfdd38c3..318d5bb15 100644 --- a/client/allocdir/alloc_dir_darwin.go +++ b/client/allocdir/alloc_dir_darwin.go @@ -4,13 +4,13 @@ import ( "syscall" ) -// Hardlinks the shared directory. As a side-effect the shared directory and -// task directory must be on the same filesystem. -func (d *AllocDir) mountSharedDir(dir string) error { - return syscall.Link(d.SharedDir, dir) +// Hardlinks the shared directory. As a side-effect the src and dest directory +// must be on the same filesystem. +func (d *AllocDir) mount(src, dest string) error { + return syscall.Link(src, dest) } -func (d *AllocDir) unmountSharedDir(dir string) error { +func (d *AllocDir) unmount(dir string) error { return syscall.Unlink(dir) } diff --git a/client/allocdir/alloc_dir_freebsd.go b/client/allocdir/alloc_dir_freebsd.go index a4d3801db..8e5d4eaec 100644 --- a/client/allocdir/alloc_dir_freebsd.go +++ b/client/allocdir/alloc_dir_freebsd.go @@ -4,13 +4,13 @@ import ( "syscall" ) -// Hardlinks the shared directory. As a side-effect the shared directory and -// task directory must be on the same filesystem. -func (d *AllocDir) mountSharedDir(dir string) error { - return syscall.Link(d.SharedDir, dir) +// Hardlinks the shared directory. As a side-effect the src and dest directory +// must be on the same filesystem. +func (d *AllocDir) mount(src, dest string) error { + return syscall.Link(src, dest) } -func (d *AllocDir) unmountSharedDir(dir string) error { +func (d *AllocDir) unmount(dir string) error { return syscall.Unlink(dir) } diff --git a/client/allocdir/alloc_dir_linux.go b/client/allocdir/alloc_dir_linux.go index 9b2c67035..bf5570d41 100644 --- a/client/allocdir/alloc_dir_linux.go +++ b/client/allocdir/alloc_dir_linux.go @@ -9,17 +9,15 @@ import ( "github.com/hashicorp/go-multierror" ) -// Bind mounts the shared directory into the task directory. Must be root to -// run. -func (d *AllocDir) mountSharedDir(taskDir string) error { - if err := os.MkdirAll(taskDir, 0777); err != nil { +func (d *AllocDir) mount(src, dest string) error { + if err := os.MkdirAll(dest, 0777); err != nil { return err } - return syscall.Mount(d.SharedDir, taskDir, "", syscall.MS_BIND, "") + return syscall.Mount(src, dest, "", syscall.MS_BIND, "") } -func (d *AllocDir) unmountSharedDir(dir string) error { +func (d *AllocDir) unmount(dir string) error { return syscall.Unmount(dir, 0) } diff --git a/client/allocdir/alloc_dir_windows.go b/client/allocdir/alloc_dir_windows.go index 112fe9b63..d37c073e2 100644 --- a/client/allocdir/alloc_dir_windows.go +++ b/client/allocdir/alloc_dir_windows.go @@ -19,17 +19,17 @@ func (d *AllocDir) linkOrCopy(src, dst string, perm os.FileMode) error { } // The windows version does nothing currently. -func (d *AllocDir) mountSharedDir(dir string) error { +func (d *AllocDir) mount(src, dest string) error { return errors.New("Mount on Windows not supported.") } // The windows version does nothing currently. -func (d *AllocDir) dropDirPermissions(path string) error { +func (d *AllocDir) unmount(dir string) error { return nil } // The windows version does nothing currently. -func (d *AllocDir) unmountSharedDir(dir string) error { +func (d *AllocDir) dropDirPermissions(path string) error { return nil } diff --git a/client/client.go b/client/client.go index 8379fd4bf..f7a86c0ea 100644 --- a/client/client.go +++ b/client/client.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/nomad/client/driver" "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/client/rpcproxy" + "github.com/hashicorp/nomad/client/secretdir" "github.com/hashicorp/nomad/client/stats" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/nomad" @@ -77,6 +78,9 @@ const ( // allocSyncRetryIntv is the interval on which we retry updating // the status of the allocation allocSyncRetryIntv = 5 * time.Second + + // secretDir is the secrets directory nested under the state directory + secretDir = "secrets" ) // ClientStatsReporter exposes all the APIs related to resource usage of a Nomad @@ -107,6 +111,8 @@ type Client struct { connPool *nomad.ConnPool + secretDir *secretdir.SecretDir + // lastHeartbeatFromQuorum is an atomic int32 acting as a bool. When // true, the last heartbeat message had a leader. When false (0), // the last heartbeat did not include the RPC address of the leader, @@ -275,6 +281,14 @@ func (c *Client) init() error { } c.logger.Printf("[INFO] client: using alloc directory %v", c.config.AllocDir) + + // Initialize the secret directory + sdir, err := secretdir.NewSecretDir(filepath.Join(c.config.StateDir, secretDir)) + if err != nil { + return err + } + c.secretDir = sdir + return nil } @@ -327,6 +341,10 @@ func (c *Client) Shutdown() error { <-ar.WaitCh() } c.allocLock.Unlock() + + if err := c.secretDir.Destroy(); err != nil { + return err + } } c.shutdown = true @@ -449,7 +467,7 @@ func (c *Client) restoreState() error { id := entry.Name() alloc := &structs.Allocation{ID: id} c.configLock.RLock() - ar := NewAllocRunner(c.logger, c.configCopy, c.updateAllocStatus, alloc) + ar := NewAllocRunner(c.logger, c.configCopy, c.updateAllocStatus, alloc, c.secretDir) c.configLock.RUnlock() c.allocLock.Lock() c.allocs[id] = ar @@ -1264,7 +1282,7 @@ func (c *Client) updateAlloc(exist, update *structs.Allocation) error { // addAlloc is invoked when we should add an allocation func (c *Client) addAlloc(alloc *structs.Allocation) error { c.configLock.RLock() - ar := NewAllocRunner(c.logger, c.configCopy, c.updateAllocStatus, alloc) + ar := NewAllocRunner(c.logger, c.configCopy, c.updateAllocStatus, alloc, c.secretDir) c.configLock.RUnlock() go ar.Run() diff --git a/client/secretdir/secret_dir.go b/client/secretdir/secret_dir.go new file mode 100644 index 000000000..0251b07b7 --- /dev/null +++ b/client/secretdir/secret_dir.go @@ -0,0 +1,67 @@ +package secretdir + +import ( + "fmt" + "os" + "path/filepath" +) + +const () + +type SecretDir struct { + // Dir is the path to the secret directory + Dir string +} + +func NewSecretDir(dir string) (*SecretDir, error) { + s := &SecretDir{ + Dir: dir, + } + + if err := s.init(); err != nil { + return nil, err + } + + return s, nil +} + +// init checks the secret directory exists and if it doesn't creates the secret +// directory +func (s *SecretDir) init() error { + if _, err := os.Stat(s.Dir); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to stat secret dir: %v", err) + } + + // TODO this shouldn't be hardcoded + if err := s.create(32); err != nil { + return fmt.Errorf("failed to create secret dir: %v", err) + } + + return nil +} + +// Destroy is used to destroy the secret dir +func (s *SecretDir) Destroy() error { + return s.destroy() +} + +func (s *SecretDir) getPathFor(allocID, task string) string { + return filepath.Join(s.Dir, fmt.Sprintf("%s-%s", allocID, task)) +} + +// CreateFor creates a secret directory for the given allocation and task. If +// the directory couldn't be created an error is returned, otherwise the path +// is. +func (s *SecretDir) CreateFor(allocID, task string) (string, error) { + path := s.getPathFor(allocID, task) + if err := os.Mkdir(path, 0777); err != nil { + return "", err + } + return path, nil +} + +// Remove deletes the secret directory for the given allocation and task +func (s *SecretDir) Remove(allocID, task string) error { + path := s.getPathFor(allocID, task) + return os.RemoveAll(path) +} diff --git a/client/secretdir/secret_dir_test.go b/client/secretdir/secret_dir_test.go new file mode 100644 index 000000000..ce8cf78e0 --- /dev/null +++ b/client/secretdir/secret_dir_test.go @@ -0,0 +1,70 @@ +package secretdir + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" +) + +func TestSecretDir_CreateDestroy(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Failed to make tmpdir: %v", err) + } + defer os.RemoveAll(tmpdir) + + path := filepath.Join(tmpdir, "secret") + sdir, err := NewSecretDir(path) + if err != nil { + t.Fatalf("Failed to create SecretDir: %v", err) + } + + // Check the folder exists + if _, err := os.Stat(path); err != nil { + t.Fatalf("Stating path failed: %v", err) + } + + if err := sdir.Destroy(); err != nil { + t.Fatalf("Destroying failed: %v", err) + } + + // Check the folder doesn't exists + if _, err := os.Stat(path); err == nil || !os.IsNotExist(err) { + t.Fatalf("path err: %v", err) + } +} + +func TestSecretDir_CreateFor_Remove(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Failed to make tmpdir: %v", err) + } + defer os.RemoveAll(tmpdir) + + path := filepath.Join(tmpdir, "secret") + sdir, err := NewSecretDir(path) + if err != nil { + t.Fatalf("Failed to create SecretDir: %v", err) + } + + alloc, task := "123", "foo" + taskDir, err := sdir.CreateFor(alloc, task) + if err != nil { + t.Fatalf("CreateFor failed: %v", err) + } + + // Check the folder exists + if _, err := os.Stat(taskDir); err != nil { + t.Fatalf("Stating path failed: %v", err) + } + + if err := sdir.Remove(alloc, task); err != nil { + t.Fatalf("Destroying failed: %v", err) + } + + // Check the folder doesn't exists + if _, err := os.Stat(taskDir); err == nil || !os.IsNotExist(err) { + t.Fatalf("path err: %v", err) + } +} diff --git a/client/secretdir/secret_dir_universal.go b/client/secretdir/secret_dir_universal.go new file mode 100644 index 000000000..5f7778a1f --- /dev/null +++ b/client/secretdir/secret_dir_universal.go @@ -0,0 +1,15 @@ +// +build !dragonfly,!freebsd,!linux,!netbsd,!openbsd,!solaris + +package secretdir + +import "os" + +// create creates a normal folder at the secret dir path +func (s *SecretDir) create(sizeMB int) error { + return os.MkdirAll(s.Dir, 0700) +} + +// destroy removes the secret dir +func (s *SecretDir) destroy() error { + return os.RemoveAll(s.Dir) +} diff --git a/client/secretdir/secret_dir_unix.go b/client/secretdir/secret_dir_unix.go new file mode 100644 index 000000000..384b3767e --- /dev/null +++ b/client/secretdir/secret_dir_unix.go @@ -0,0 +1,31 @@ +// +build dragonfly freebsd linux netbsd openbsd solaris + +package secretdir + +import ( + "fmt" + "os" + "syscall" +) + +// create creates a tmpfs folder at the secret dir path +func (s *SecretDir) create(sizeMB int64) error { + if err := os.MkdirAll(s.Dir, 0700); err != nil { + return err + } + + var flags uintptr + flags = syscall.MS_NOEXEC + options := fmt.Sprintf("size=%dm", sizeMB) + err := syscall.Mount("tmpfs", s.Dir, "tmpfs", flags, options) + return os.NewSyscallError("mount", err) +} + +// destroy unmounts the tmpfs folder and deletes it +func (s *SecretDir) destroy() error { + if err := syscall.Unmount(s.Dir, 0); err != nil { + return err + } + + return os.RemoveAll(s.Dir) +} diff --git a/client/secretdir/secret_dir_unix_test.go b/client/secretdir/secret_dir_unix_test.go new file mode 100644 index 000000000..7946158c2 --- /dev/null +++ b/client/secretdir/secret_dir_unix_test.go @@ -0,0 +1 @@ +package secretdir From 46ce8dd02073d7d049a635582e1735ea4656dab6 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 30 Aug 2016 21:38:16 -0700 Subject: [PATCH 2/5] Interface + tests --- client/alloc_runner.go | 9 ++-- client/alloc_runner_test.go | 31 ++++++------ client/allocdir/alloc_dir.go | 32 ++++++++----- client/driver/driver_test.go | 8 +++- client/driver/executor/executor_linux_test.go | 19 +++++++- client/driver/executor/executor_test.go | 6 ++- client/driver/rkt_test.go | 1 + client/secretdir/secret_dir.go | 6 +++ client/secretdir/secret_dir_testing.go | 48 +++++++++++++++++++ client/task_runner_test.go | 28 ++++++----- 10 files changed, 143 insertions(+), 45 deletions(-) create mode 100644 client/secretdir/secret_dir_testing.go diff --git a/client/alloc_runner.go b/client/alloc_runner.go index 5ec8f2eb3..6fb2566b3 100644 --- a/client/alloc_runner.go +++ b/client/alloc_runner.go @@ -64,7 +64,7 @@ type AllocRunner struct { updateCh chan *structs.Allocation - secretDir *secretdir.SecretDir + secretDir secretdir.SecretDirectory destroy bool destroyCh chan struct{} @@ -84,7 +84,7 @@ type allocRunnerState struct { // NewAllocRunner is used to create a new allocation context func NewAllocRunner(logger *log.Logger, config *config.Config, updater AllocStateUpdater, - alloc *structs.Allocation, secretDir *secretdir.SecretDir) *AllocRunner { + alloc *structs.Allocation, secretDir secretdir.SecretDirectory) *AllocRunner { ar := &AllocRunner{ config: config, updater: updater, @@ -131,6 +131,8 @@ func (r *AllocRunner) RestoreState() error { } if r.ctx == nil { snapshotErrors.Errors = append(snapshotErrors.Errors, fmt.Errorf("alloc_runner snapshot includes a nil context")) + } else { + r.ctx.AllocDir.SetSecretDirFn(r.secretDir.CreateFor) } if e := snapshotErrors.ErrorOrNil(); e != nil { return e @@ -393,7 +395,8 @@ func (r *AllocRunner) Run() { if r.ctx == nil { path := filepath.Join(r.config.AllocDir, r.alloc.ID) size := r.Alloc().Resources.DiskMB - allocDir := allocdir.NewAllocDir(r.alloc.ID, path, size, r.secretDir) + allocDir := allocdir.NewAllocDir(r.alloc.ID, path, size) + allocDir.SetSecretDirFn(r.secretDir.CreateFor) if err := allocDir.Build(tg.Tasks); err != nil { r.logger.Printf("[ERR] client: failed to build task directories: %v", err) r.setStatus(structs.AllocClientStatusFailed, fmt.Sprintf("failed to build task dirs for '%s'", alloc.TaskGroup)) diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index 7e72fae2a..a479cd91d 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/nomad/testutil" "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/secretdir" ctestutil "github.com/hashicorp/nomad/client/testutil" ) @@ -25,7 +26,7 @@ func (m *MockAllocStateUpdater) Update(alloc *structs.Allocation) { m.Allocs = append(m.Allocs, alloc) } -func testAllocRunnerFromAlloc(alloc *structs.Allocation, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { +func testAllocRunnerFromAlloc(t *testing.T, alloc *structs.Allocation, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { logger := testLogger() conf := config.DefaultConfig() conf.StateDir = os.TempDir() @@ -35,17 +36,17 @@ func testAllocRunnerFromAlloc(alloc *structs.Allocation, restarts bool) (*MockAl *alloc.Job.LookupTaskGroup(alloc.TaskGroup).RestartPolicy = structs.RestartPolicy{Attempts: 0} alloc.Job.Type = structs.JobTypeBatch } - ar := NewAllocRunner(logger, conf, upd.Update, alloc) + ar := NewAllocRunner(logger, conf, upd.Update, alloc, secretdir.NewTestSecretDir(t)) return upd, ar } -func testAllocRunner(restarts bool) (*MockAllocStateUpdater, *AllocRunner) { - return testAllocRunnerFromAlloc(mock.Alloc(), restarts) +func testAllocRunner(t *testing.T, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { + return testAllocRunnerFromAlloc(t, mock.Alloc(), restarts) } func TestAllocRunner_SimpleRun(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) go ar.Run() defer ar.Destroy() @@ -82,7 +83,7 @@ func TestAllocRunner_RetryArtifact(t *testing.T) { } alloc.Job.TaskGroups[0].Tasks = append(alloc.Job.TaskGroups[0].Tasks, badtask) - upd, ar := testAllocRunnerFromAlloc(alloc, true) + upd, ar := testAllocRunnerFromAlloc(t, alloc, true) go ar.Run() defer ar.Destroy() @@ -118,7 +119,7 @@ func TestAllocRunner_RetryArtifact(t *testing.T) { func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -207,7 +208,7 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { func TestAllocRunner_DiskExceeded_Destroy(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -313,7 +314,7 @@ func TestAllocRunner_DiskExceeded_Destroy(t *testing.T) { } func TestAllocRunner_Destroy(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -365,7 +366,7 @@ func TestAllocRunner_Destroy(t *testing.T) { func TestAllocRunner_Update(t *testing.T) { ctestutil.ExecCompatible(t) - _, ar := testAllocRunner(false) + _, ar := testAllocRunner(t, false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -391,7 +392,7 @@ func TestAllocRunner_Update(t *testing.T) { func TestAllocRunner_SaveRestoreState(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -413,7 +414,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { // Create a new alloc runner ar2 := NewAllocRunner(ar.logger, ar.config, upd.Update, - &structs.Allocation{ID: ar.alloc.ID}) + &structs.Allocation{ID: ar.alloc.ID}, ar.secretDir) err = ar2.RestoreState() if err != nil { t.Fatalf("err: %v", err) @@ -441,7 +442,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) ar.logger = prefixedTestLogger("ar1: ") // Ensure task takes some time @@ -485,7 +486,7 @@ func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) { // Create a new alloc runner ar2 := NewAllocRunner(ar.logger, ar.config, upd.Update, - &structs.Allocation{ID: ar.alloc.ID}) + &structs.Allocation{ID: ar.alloc.ID}, ar.secretDir) ar2.logger = prefixedTestLogger("ar2: ") err = ar2.RestoreState() if err != nil { @@ -547,7 +548,7 @@ func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) { func TestAllocRunner_TaskFailed_KillTG(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Create two tasks in the task group task := ar.alloc.Job.TaskGroups[0].Tasks[0] diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go index 209297e83..9a45e119e 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -14,7 +14,6 @@ import ( "gopkg.in/tomb.v1" "github.com/hashicorp/go-multierror" - "github.com/hashicorp/nomad/client/secretdir" "github.com/hashicorp/nomad/nomad/structs" "github.com/hpcloud/tail/watch" ) @@ -55,12 +54,14 @@ var ( TaskDirs = []string{"tmp"} ) +type CreateSecretDirFn func(allocID, task string) (path string, err error) + type AllocDir struct { // AllocID is the allocation ID for this directory AllocID string - // SecretDir is used to build the secret directory for the allocation - SecretDir *secretdir.SecretDir + // TODO + createSecretDirFn CreateSecretDirFn // AllocDir is the directory used for storing any state // of this allocation. It will be purged on alloc destroy. @@ -121,10 +122,9 @@ type AllocDirFS interface { // NewAllocDir initializes the AllocDir struct with allocDir as base path for // the allocation directory and maxSize as the maximum allowed size in megabytes. -func NewAllocDir(allocID, allocDir string, maxSize int, sdir *secretdir.SecretDir) *AllocDir { +func NewAllocDir(allocID, allocDir string, maxSize int) *AllocDir { d := &AllocDir{ AllocID: allocID, - SecretDir: sdir, AllocDir: allocDir, MaxCheckDiskInterval: maxCheckDiskInterval, MinCheckDiskInterval: minCheckDiskInterval, @@ -136,6 +136,10 @@ func NewAllocDir(allocID, allocDir string, maxSize int, sdir *secretdir.SecretDi return d } +func (d *AllocDir) SetSecretDirFn(fn CreateSecretDirFn) { + d.createSecretDirFn = fn +} + // Tears down previously build directory structure. func (d *AllocDir) Destroy() error { @@ -250,15 +254,17 @@ func (d *AllocDir) Build(tasks []*structs.Task) error { } // Get the secret directory - sdir, err := d.SecretDir.CreateFor(d.AllocID, t.Name) - if err != nil { - return fmt.Errorf("Creating secret directory for task %q failed: %v", t.Name, err) - } + if d.createSecretDirFn != nil { + sdir, err := d.createSecretDirFn(d.AllocID, t.Name) + if err != nil { + return fmt.Errorf("Creating secret directory for task %q failed: %v", t.Name, err) + } - // Mount the secret directory - taskSecret := filepath.Join(taskDir, TaskSecrets) - if err := d.mount(sdir, taskSecret); err != nil { - return fmt.Errorf("failed to mount secret directory: %v", err) + // Mount the secret directory + taskSecret := filepath.Join(taskDir, TaskSecrets) + if err := d.mount(sdir, taskSecret); err != nil { + return fmt.Errorf("failed to mount secret directory: %v", err) + } } } diff --git a/client/driver/driver_test.go b/client/driver/driver_test.go index b8b1c2889..757e3cc04 100644 --- a/client/driver/driver_test.go +++ b/client/driver/driver_test.go @@ -2,6 +2,7 @@ package driver import ( "io" + "io/ioutil" "log" "math/rand" "os" @@ -79,8 +80,13 @@ func testConfig() *config.Config { func testDriverContexts(task *structs.Task) (*DriverContext, *ExecContext) { cfg := testConfig() - allocDir := allocdir.NewAllocDir(filepath.Join(cfg.AllocDir, structs.GenerateUUID()), task.Resources.DiskMB) + id := structs.GenerateUUID() + path := filepath.Join(cfg.AllocDir, id) + allocDir := allocdir.NewAllocDir(id, path, task.Resources.DiskMB) allocDir.Build([]*structs.Task{task}) + allocDir.SetSecretDirFn(func(a, b string) (string, error) { + return ioutil.TempDir("", "") + }) alloc := mock.Alloc() execCtx := NewExecContext(allocDir, alloc.ID) diff --git a/client/driver/executor/executor_linux_test.go b/client/driver/executor/executor_linux_test.go index c3006071a..2dc6808df 100644 --- a/client/driver/executor/executor_linux_test.go +++ b/client/driver/executor/executor_linux_test.go @@ -81,7 +81,24 @@ func TestExecutor_IsolationAndConstraints(t *testing.T) { t.Fatalf("file %v hasn't been removed", memLimits) } - expected := "/:\nalloc/\nbin/\ndev/\netc/\nlib/\nlib64/\nlocal/\nproc/\ntmp/\nusr/\n\n/etc/:\nld.so.cache\nld.so.conf\nld.so.conf.d/" + expected := `/: +alloc/ +bin/ +dev/ +etc/ +lib/ +lib64/ +local/ +proc/ +secrets/ +tmp/ +usr/ + +/etc/: +ld.so.cache +ld.so.conf +ld.so.conf.d/` + file := filepath.Join(ctx.AllocDir.LogDir(), "web.stdout.0") output, err := ioutil.ReadFile(file) if err != nil { diff --git a/client/driver/executor/executor_test.go b/client/driver/executor/executor_test.go index 50ec6353b..fa119da52 100644 --- a/client/driver/executor/executor_test.go +++ b/client/driver/executor/executor_test.go @@ -37,7 +37,11 @@ func mockAllocDir(t *testing.T) (*structs.Task, *allocdir.AllocDir) { alloc := mock.Alloc() task := alloc.Job.TaskGroups[0].Tasks[0] - allocDir := allocdir.NewAllocDir(filepath.Join(os.TempDir(), alloc.ID), task.Resources.DiskMB) + path := filepath.Join(os.TempDir(), alloc.ID) + allocDir := allocdir.NewAllocDir(alloc.ID, path, task.Resources.DiskMB) + allocDir.SetSecretDirFn(func(a, b string) (string, error) { + return ioutil.TempDir("", "") + }) if err := allocDir.Build([]*structs.Task{task}); err != nil { log.Panicf("allocDir.Build() failed: %v", err) } diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index f8ed84243..fc68e4780 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -376,6 +376,7 @@ func TestRktTaskValidate(t *testing.T) { "dns_servers": []string{"8.8.8.8", "8.8.4.4"}, "dns_search_domains": []string{"example.com", "example.org", "example.net"}, }, + Resources: basicResources, } driverCtx, execCtx := testDriverContexts(task) defer execCtx.AllocDir.Destroy() diff --git a/client/secretdir/secret_dir.go b/client/secretdir/secret_dir.go index 0251b07b7..8bb92eb14 100644 --- a/client/secretdir/secret_dir.go +++ b/client/secretdir/secret_dir.go @@ -8,6 +8,12 @@ import ( const () +type SecretDirectory interface { + Destroy() error + CreateFor(allocID, task string) (path string, err error) + Remove(allocID, task string) error +} + type SecretDir struct { // Dir is the path to the secret directory Dir string diff --git a/client/secretdir/secret_dir_testing.go b/client/secretdir/secret_dir_testing.go new file mode 100644 index 000000000..b7434d2b4 --- /dev/null +++ b/client/secretdir/secret_dir_testing.go @@ -0,0 +1,48 @@ +package secretdir + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" +) + +type TestSecretDir struct { + // Dir is the path to the secret directory + Dir string +} + +func NewTestSecretDir(t *testing.T) *TestSecretDir { + tmp, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Failed to make tmp dir: %v", err) + } + + s := &TestSecretDir{ + Dir: tmp, + } + + return s +} + +func (s *TestSecretDir) Destroy() error { + return os.RemoveAll(s.Dir) +} + +func (s *TestSecretDir) getPathFor(allocID, task string) string { + return filepath.Join(s.Dir, fmt.Sprintf("%s-%s", allocID, task)) +} + +func (s *TestSecretDir) CreateFor(allocID, task string) (string, error) { + path := s.getPathFor(allocID, task) + if err := os.Mkdir(path, 0777); err != nil { + return "", err + } + return path, nil +} + +func (s *TestSecretDir) Remove(allocID, task string) error { + path := s.getPathFor(allocID, task) + return os.RemoveAll(path) +} diff --git a/client/task_runner_test.go b/client/task_runner_test.go index 0fc7316c2..8975d0c67 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver" + "github.com/hashicorp/nomad/client/secretdir" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" @@ -38,13 +39,13 @@ func (m *MockTaskStateUpdater) Update(name, state string, event *structs.TaskEve m.events = append(m.events, event) } -func testTaskRunner(restarts bool) (*MockTaskStateUpdater, *TaskRunner) { - return testTaskRunnerFromAlloc(restarts, mock.Alloc()) +func testTaskRunner(t *testing.T, restarts bool) (*MockTaskStateUpdater, *TaskRunner) { + return testTaskRunnerFromAlloc(t, restarts, mock.Alloc()) } // Creates a mock task runner using the first task in the first task group of // the passed allocation. -func testTaskRunnerFromAlloc(restarts bool, alloc *structs.Allocation) (*MockTaskStateUpdater, *TaskRunner) { +func testTaskRunnerFromAlloc(t *testing.T, restarts bool, alloc *structs.Allocation) (*MockTaskStateUpdater, *TaskRunner) { logger := testLogger() conf := config.DefaultConfig() conf.StateDir = os.TempDir() @@ -55,7 +56,10 @@ func testTaskRunnerFromAlloc(restarts bool, alloc *structs.Allocation) (*MockTas // we have a mock so that doesn't happen. task.Resources.Networks[0].ReservedPorts = []structs.Port{{"", 80}} - allocDir := allocdir.NewAllocDir(filepath.Join(conf.AllocDir, alloc.ID), task.Resources.DiskMB) + path := filepath.Join(conf.AllocDir, alloc.ID) + allocDir := allocdir.NewAllocDir(alloc.ID, path, task.Resources.DiskMB) + sdir := secretdir.NewTestSecretDir(t) + allocDir.SetSecretDirFn(sdir.CreateFor) allocDir.Build([]*structs.Task{task}) ctx := driver.NewExecContext(allocDir, alloc.ID) @@ -68,7 +72,7 @@ func testTaskRunnerFromAlloc(restarts bool, alloc *structs.Allocation) (*MockTas func TestTaskRunner_SimpleRun(t *testing.T) { ctestutil.ExecCompatible(t) - upd, tr := testTaskRunner(false) + upd, tr := testTaskRunner(t, false) tr.MarkReceived() go tr.Run() defer tr.Destroy(structs.NewTaskEvent(structs.TaskKilled)) @@ -103,7 +107,7 @@ func TestTaskRunner_SimpleRun(t *testing.T) { func TestTaskRunner_Destroy(t *testing.T) { ctestutil.ExecCompatible(t) - upd, tr := testTaskRunner(true) + upd, tr := testTaskRunner(t, true) tr.MarkReceived() defer tr.ctx.AllocDir.Destroy() @@ -165,7 +169,7 @@ func TestTaskRunner_Destroy(t *testing.T) { func TestTaskRunner_Update(t *testing.T) { ctestutil.ExecCompatible(t) - _, tr := testTaskRunner(false) + _, tr := testTaskRunner(t, false) // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" @@ -219,7 +223,7 @@ func TestTaskRunner_Update(t *testing.T) { func TestTaskRunner_SaveRestoreState(t *testing.T) { ctestutil.ExecCompatible(t) - upd, tr := testTaskRunner(false) + upd, tr := testTaskRunner(t, false) // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" @@ -269,7 +273,7 @@ func TestTaskRunner_Download_List(t *testing.T) { } task.Artifacts = []*structs.TaskArtifact{&artifact1, &artifact2} - upd, tr := testTaskRunnerFromAlloc(false, alloc) + upd, tr := testTaskRunnerFromAlloc(t, false, alloc) tr.MarkReceived() go tr.Run() defer tr.Destroy(structs.NewTaskEvent(structs.TaskKilled)) @@ -334,7 +338,7 @@ func TestTaskRunner_Download_Retries(t *testing.T) { Mode: structs.RestartPolicyModeFail, } - upd, tr := testTaskRunnerFromAlloc(true, alloc) + upd, tr := testTaskRunnerFromAlloc(t, true, alloc) tr.MarkReceived() go tr.Run() defer tr.Destroy(structs.NewTaskEvent(structs.TaskKilled)) @@ -384,7 +388,9 @@ func TestTaskRunner_Download_Retries(t *testing.T) { } func TestTaskRunner_Validate_UserEnforcement(t *testing.T) { - _, tr := testTaskRunner(false) + _, tr := testTaskRunner(t, false) + defer tr.Destroy(structs.NewTaskEvent(structs.TaskKilled)) + defer tr.ctx.AllocDir.Destroy() // Try to run as root with exec. tr.task.Driver = "exec" From dfab22cd9fd36aaf163161f481b99d3cce5cfb2e Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 31 Aug 2016 13:56:11 -0700 Subject: [PATCH 3/5] environment variables --- client/allocdir/alloc_dir_unix.go | 10 ++++++++-- client/driver/docker.go | 1 + client/driver/driver.go | 1 + client/driver/env/env.go | 18 ++++++++++++++++++ client/driver/executor/executor_linux.go | 1 + 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/client/allocdir/alloc_dir_unix.go b/client/allocdir/alloc_dir_unix.go index 339e59d5d..0807505c4 100644 --- a/client/allocdir/alloc_dir_unix.go +++ b/client/allocdir/alloc_dir_unix.go @@ -14,11 +14,17 @@ import ( ) var ( - //Path inside container for mounted directory shared across tasks in a task group. + // SharedAllocContainerPath is the path inside container for mounted + // directory shared across tasks in a task group. SharedAllocContainerPath = filepath.Join("/", SharedAllocName) - //Path inside container for mounted directory for local storage. + // TaskLocalContainer is the path inside a container for mounted directory + // for local storage. TaskLocalContainerPath = filepath.Join("/", TaskLocal) + + // TaskSecretsContainerPath is the path inside a container for mounted + // secrets directory + TaskSecretsContainerPath = filepath.Join("/", TaskSecrets) ) func (d *AllocDir) linkOrCopy(src, dst string, perm os.FileMode) error { diff --git a/client/driver/docker.go b/client/driver/docker.go index 6c1390aa1..c2fdc92c2 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -381,6 +381,7 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, // Set environment variables. d.taskEnv.SetAllocDir(allocdir.SharedAllocContainerPath) d.taskEnv.SetTaskLocalDir(allocdir.TaskLocalContainerPath) + d.taskEnv.SetTaskLocalDir(allocdir.TaskSecretsContainerPath) config := &docker.Config{ Image: driverConfig.ImageName, diff --git a/client/driver/driver.go b/client/driver/driver.go index 112626585..82fc0b142 100644 --- a/client/driver/driver.go +++ b/client/driver/driver.go @@ -156,6 +156,7 @@ func GetTaskEnv(allocDir *allocdir.AllocDir, node *structs.Node, } env.SetTaskLocalDir(filepath.Join(taskdir, allocdir.TaskLocal)) + env.SetSecretDir(filepath.Join(taskdir, allocdir.TaskSecrets)) } if task.Resources != nil { diff --git a/client/driver/env/env.go b/client/driver/env/env.go index 1ac9b7510..e2ff660b8 100644 --- a/client/driver/env/env.go +++ b/client/driver/env/env.go @@ -21,6 +21,10 @@ const ( // removed. TaskLocalDir = "NOMAD_TASK_DIR" + // SecretDir is the environment variable with the path to the tasks secret + // directory where it can store sensitive data. + SecretDir = "NOMAD_SECRET_DIR" + // MemLimit is the environment variable with the tasks memory limit in MBs. MemLimit = "NOMAD_MEMORY_LIMIT" @@ -79,6 +83,7 @@ type TaskEnvironment struct { JobMeta map[string]string AllocDir string TaskDir string + SecretDir string CpuLimit int MemLimit int TaskName string @@ -153,6 +158,9 @@ func (t *TaskEnvironment) Build() *TaskEnvironment { if t.TaskDir != "" { t.TaskEnv[TaskLocalDir] = t.TaskDir } + if t.SecretDir != "" { + t.TaskEnv[SecretDir] = t.SecretDir + } // Build the resource limits if t.MemLimit != 0 { @@ -249,6 +257,16 @@ func (t *TaskEnvironment) ClearTaskLocalDir() *TaskEnvironment { return t } +func (t *TaskEnvironment) SetSecretDir(dir string) *TaskEnvironment { + t.SecretDir = dir + return t +} + +func (t *TaskEnvironment) ClearSecretDir() *TaskEnvironment { + t.SecretDir = "" + return t +} + func (t *TaskEnvironment) SetMemLimit(limit int) *TaskEnvironment { t.MemLimit = limit return t diff --git a/client/driver/executor/executor_linux.go b/client/driver/executor/executor_linux.go index c673555cc..8925c0339 100644 --- a/client/driver/executor/executor_linux.go +++ b/client/driver/executor/executor_linux.go @@ -240,6 +240,7 @@ func (e *UniversalExecutor) configureChroot() error { e.ctx.TaskEnv. SetAllocDir(filepath.Join("/", allocdir.SharedAllocName)). SetTaskLocalDir(filepath.Join("/", allocdir.TaskLocal)). + SetSecretDir(filepath.Join("/", allocdir.TaskSecrets)). Build() if e.cmd.SysProcAttr == nil { From 0b07ef93c1c5b00629ae87a567e4db4c89efa37c Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 31 Aug 2016 17:57:06 -0700 Subject: [PATCH 4/5] Address comments and reserve --- client/alloc_runner.go | 3 +-- client/allocdir/alloc_dir.go | 18 ++++++++++++++---- client/allocdir/alloc_dir_test.go | 19 ++++++++++++++----- client/allocdir/alloc_dir_testing.go | 8 ++++++++ client/allocdir/alloc_dir_windows.go | 10 +++++----- client/client.go | 22 ++++++++++++++++++---- client/client_test.go | 17 +++++++++++++++++ client/driver/driver_test.go | 6 +----- client/driver/executor/executor_test.go | 5 +---- client/secretdir/secret_dir.go | 6 ++---- client/secretdir/secret_dir_testing.go | 5 +++++ client/secretdir/secret_dir_universal.go | 7 ++++++- client/secretdir/secret_dir_unix.go | 15 +++++++++++++-- client/secretdir/secret_dir_unix_test.go | 1 - client/task_runner_test.go | 5 +---- 15 files changed, 106 insertions(+), 41 deletions(-) create mode 100644 client/allocdir/alloc_dir_testing.go delete mode 100644 client/secretdir/secret_dir_unix_test.go diff --git a/client/alloc_runner.go b/client/alloc_runner.go index 6fb2566b3..181600df0 100644 --- a/client/alloc_runner.go +++ b/client/alloc_runner.go @@ -395,8 +395,7 @@ func (r *AllocRunner) Run() { if r.ctx == nil { path := filepath.Join(r.config.AllocDir, r.alloc.ID) size := r.Alloc().Resources.DiskMB - allocDir := allocdir.NewAllocDir(r.alloc.ID, path, size) - allocDir.SetSecretDirFn(r.secretDir.CreateFor) + allocDir := allocdir.NewAllocDir(r.alloc.ID, path, size, r.secretDir.CreateFor) if err := allocDir.Build(tg.Tasks); err != nil { r.logger.Printf("[ERR] client: failed to build task directories: %v", err) r.setStatus(structs.AllocClientStatusFailed, fmt.Sprintf("failed to build task dirs for '%s'", alloc.TaskGroup)) diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go index 9a45e119e..fd51577dc 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -60,7 +60,8 @@ type AllocDir struct { // AllocID is the allocation ID for this directory AllocID string - // TODO + // createSecretDirFn is used to create a secret directory and retrieve the + // path to it so that it can be mounted in the task directory. createSecretDirFn CreateSecretDirFn // AllocDir is the directory used for storing any state @@ -121,8 +122,10 @@ type AllocDirFS interface { } // NewAllocDir initializes the AllocDir struct with allocDir as base path for -// the allocation directory and maxSize as the maximum allowed size in megabytes. -func NewAllocDir(allocID, allocDir string, maxSize int) *AllocDir { +// the allocation directory and maxSize as the maximum allowed size in +// megabytes. The secretDirFn is used to create secret directories and get their +// path which will then be mounted into the task directory +func NewAllocDir(allocID, allocDir string, maxSize int, secretDirFn CreateSecretDirFn) *AllocDir { d := &AllocDir{ AllocID: allocID, AllocDir: allocDir, @@ -131,11 +134,14 @@ func NewAllocDir(allocID, allocDir string, maxSize int) *AllocDir { CheckDiskMaxEnforcePeriod: checkDiskMaxEnforcePeriod, TaskDirs: make(map[string]string), MaxSize: maxSize, + createSecretDirFn: secretDirFn, } d.SharedDir = filepath.Join(d.AllocDir, SharedAllocName) return d } +// SetSecretDirFn is used to set the function used to create secret +// directories. func (d *AllocDir) SetSecretDirFn(fn CreateSecretDirFn) { d.createSecretDirFn = fn } @@ -263,7 +269,11 @@ func (d *AllocDir) Build(tasks []*structs.Task) error { // Mount the secret directory taskSecret := filepath.Join(taskDir, TaskSecrets) if err := d.mount(sdir, taskSecret); err != nil { - return fmt.Errorf("failed to mount secret directory: %v", err) + return fmt.Errorf("failed to mount secret directory for task %q: %v", t.Name, err) + } + + if err := d.dropDirPermissions(taskSecret); err != nil { + return err } } } diff --git a/client/allocdir/alloc_dir_test.go b/client/allocdir/alloc_dir_test.go index aa0f5e49b..c2682e5d4 100644 --- a/client/allocdir/alloc_dir_test.go +++ b/client/allocdir/alloc_dir_test.go @@ -52,7 +52,12 @@ func TestAllocDir_BuildAlloc(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) + secretDirFn := func(allocID, task string) (string, error) { + return ioutil.TempDir("", "") + } + + allocID := "123" + d := NewAllocDir(allocID, tmp, structs.DefaultResources().DiskMB, secretDirFn) defer d.Destroy() tasks := []*structs.Task{t1, t2} if err := d.Build(tasks); err != nil { @@ -73,6 +78,10 @@ func TestAllocDir_BuildAlloc(t *testing.T) { if _, err := os.Stat(tDir); os.IsNotExist(err) { t.Fatalf("Build(%v) didn't create TaskDir %v", tasks, tDir) } + + if _, err := os.Stat(filepath.Join(tDir, TaskSecrets)); os.IsNotExist(err) { + t.Fatalf("Build(%v) didn't create secret dir %v", tasks) + } } } @@ -83,7 +92,7 @@ func TestAllocDir_LogDir(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) + d := NewAllocDir("123", tmp, structs.DefaultResources().DiskMB, TestCreateSecretDirFn) defer d.Destroy() expected := filepath.Join(d.AllocDir, SharedAllocName, LogDirName) @@ -99,7 +108,7 @@ func TestAllocDir_EmbedNonExistent(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) + d := NewAllocDir("123", tmp, structs.DefaultResources().DiskMB, TestCreateSecretDirFn) defer d.Destroy() tasks := []*structs.Task{t1, t2} if err := d.Build(tasks); err != nil { @@ -121,7 +130,7 @@ func TestAllocDir_EmbedDirs(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) + d := NewAllocDir("123", tmp, structs.DefaultResources().DiskMB, TestCreateSecretDirFn) defer d.Destroy() tasks := []*structs.Task{t1, t2} if err := d.Build(tasks); err != nil { @@ -182,7 +191,7 @@ func TestAllocDir_MountSharedAlloc(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) + d := NewAllocDir("123", tmp, structs.DefaultResources().DiskMB, TestCreateSecretDirFn) defer d.Destroy() tasks := []*structs.Task{t1, t2} if err := d.Build(tasks); err != nil { diff --git a/client/allocdir/alloc_dir_testing.go b/client/allocdir/alloc_dir_testing.go new file mode 100644 index 000000000..199640a03 --- /dev/null +++ b/client/allocdir/alloc_dir_testing.go @@ -0,0 +1,8 @@ +package allocdir + +import "io/ioutil" + +// TestCreateSecretDirFn is used to create a secret dir suitable for testing +func TestCreateSecretDirFn(_, _ string) (string, error) { + return ioutil.TempDir("", "") +} diff --git a/client/allocdir/alloc_dir_windows.go b/client/allocdir/alloc_dir_windows.go index d37c073e2..a26496b38 100644 --- a/client/allocdir/alloc_dir_windows.go +++ b/client/allocdir/alloc_dir_windows.go @@ -1,9 +1,9 @@ package allocdir import ( - "errors" "os" "path/filepath" + "syscall" ) var ( @@ -18,14 +18,14 @@ func (d *AllocDir) linkOrCopy(src, dst string, perm os.FileMode) error { return fileCopy(src, dst, perm) } -// The windows version does nothing currently. +// Hardlinks the shared directory. As a side-effect the src and dest directory +// must be on the same filesystem. func (d *AllocDir) mount(src, dest string) error { - return errors.New("Mount on Windows not supported.") + return syscall.Link(src, dest) } -// The windows version does nothing currently. func (d *AllocDir) unmount(dir string) error { - return nil + return syscall.Unlink(dir) } // The windows version does nothing currently. diff --git a/client/client.go b/client/client.go index f7a86c0ea..95f405b73 100644 --- a/client/client.go +++ b/client/client.go @@ -111,7 +111,7 @@ type Client struct { connPool *nomad.ConnPool - secretDir *secretdir.SecretDir + secretDir secretdir.SecretDirectory // lastHeartbeatFromQuorum is an atomic int32 acting as a bool. When // true, the last heartbeat message had a leader. When false (0), @@ -192,7 +192,7 @@ func NewClient(cfg *config.Config, consulSyncer *consul.Syncer, logger *log.Logg } // Setup the reserved resources - c.reservePorts() + c.reserveResources() // Store the config copy before restoring state but after it has been // initialized. @@ -603,10 +603,24 @@ func (c *Client) setupNode() error { return nil } +// reserveResources is used to reserve resources on the Node that will be +// registered with the Server. +func (c *Client) reserveResources() { + c.reservePorts() + + // Add the memory consumed by the secret directory + c.configLock.Lock() + if c.config.Node.Reserved == nil { + c.config.Node.Reserved = new(structs.Resources) + } + c.config.Node.Reserved.MemoryMB += c.secretDir.MemoryUse() + c.configLock.Unlock() +} + // reservePorts is used to reserve ports on the fingerprinted network devices. func (c *Client) reservePorts() { - c.configLock.RLock() - defer c.configLock.RUnlock() + c.configLock.Lock() + defer c.configLock.Unlock() global := c.config.GloballyReservedPorts if len(global) == 0 { return diff --git a/client/client_test.go b/client/client_test.go index 1bc984bc3..0326b5ce3 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/secretdir" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/nomad" "github.com/hashicorp/nomad/nomad/mock" @@ -149,6 +150,22 @@ func TestClient_RPC_Passthrough(t *testing.T) { }) } +func TestClient_ReserveSecretDir(t *testing.T) { + c := testClient(t, nil) + defer c.Shutdown() + + tsd := secretdir.NewTestSecretDir(t) + c.secretDir = tsd + expected := 10 + tsd.MemoryUsed = expected + + c.reserveResources() + res := c.Node().Reserved + if res == nil || res.MemoryMB != expected { + t.Fatalf("Unexpected reserved memory: %v", res) + } +} + func TestClient_Fingerprint(t *testing.T) { c := testClient(t, nil) defer c.Shutdown() diff --git a/client/driver/driver_test.go b/client/driver/driver_test.go index 757e3cc04..0f2ad758b 100644 --- a/client/driver/driver_test.go +++ b/client/driver/driver_test.go @@ -2,7 +2,6 @@ package driver import ( "io" - "io/ioutil" "log" "math/rand" "os" @@ -82,11 +81,8 @@ func testDriverContexts(task *structs.Task) (*DriverContext, *ExecContext) { cfg := testConfig() id := structs.GenerateUUID() path := filepath.Join(cfg.AllocDir, id) - allocDir := allocdir.NewAllocDir(id, path, task.Resources.DiskMB) + allocDir := allocdir.NewAllocDir(id, path, task.Resources.DiskMB, allocdir.TestCreateSecretDirFn) allocDir.Build([]*structs.Task{task}) - allocDir.SetSecretDirFn(func(a, b string) (string, error) { - return ioutil.TempDir("", "") - }) alloc := mock.Alloc() execCtx := NewExecContext(allocDir, alloc.ID) diff --git a/client/driver/executor/executor_test.go b/client/driver/executor/executor_test.go index fa119da52..33f7f7216 100644 --- a/client/driver/executor/executor_test.go +++ b/client/driver/executor/executor_test.go @@ -38,10 +38,7 @@ func mockAllocDir(t *testing.T) (*structs.Task, *allocdir.AllocDir) { task := alloc.Job.TaskGroups[0].Tasks[0] path := filepath.Join(os.TempDir(), alloc.ID) - allocDir := allocdir.NewAllocDir(alloc.ID, path, task.Resources.DiskMB) - allocDir.SetSecretDirFn(func(a, b string) (string, error) { - return ioutil.TempDir("", "") - }) + allocDir := allocdir.NewAllocDir(alloc.ID, path, task.Resources.DiskMB, allocdir.TestCreateSecretDirFn) if err := allocDir.Build([]*structs.Task{task}); err != nil { log.Panicf("allocDir.Build() failed: %v", err) } diff --git a/client/secretdir/secret_dir.go b/client/secretdir/secret_dir.go index 8bb92eb14..de92f9c32 100644 --- a/client/secretdir/secret_dir.go +++ b/client/secretdir/secret_dir.go @@ -6,9 +6,8 @@ import ( "path/filepath" ) -const () - type SecretDirectory interface { + MemoryUse() int Destroy() error CreateFor(allocID, task string) (path string, err error) Remove(allocID, task string) error @@ -38,8 +37,7 @@ func (s *SecretDir) init() error { return fmt.Errorf("failed to stat secret dir: %v", err) } - // TODO this shouldn't be hardcoded - if err := s.create(32); err != nil { + if err := s.create(); err != nil { return fmt.Errorf("failed to create secret dir: %v", err) } diff --git a/client/secretdir/secret_dir_testing.go b/client/secretdir/secret_dir_testing.go index b7434d2b4..9c1d6885d 100644 --- a/client/secretdir/secret_dir_testing.go +++ b/client/secretdir/secret_dir_testing.go @@ -11,6 +11,9 @@ import ( type TestSecretDir struct { // Dir is the path to the secret directory Dir string + + // MemoryUsed is returned when the MemoryUse function is called + MemoryUsed int } func NewTestSecretDir(t *testing.T) *TestSecretDir { @@ -46,3 +49,5 @@ func (s *TestSecretDir) Remove(allocID, task string) error { path := s.getPathFor(allocID, task) return os.RemoveAll(path) } + +func (s *TestSecretDir) MemoryUse() int { return s.MemoryUsed } diff --git a/client/secretdir/secret_dir_universal.go b/client/secretdir/secret_dir_universal.go index 5f7778a1f..49e8501e3 100644 --- a/client/secretdir/secret_dir_universal.go +++ b/client/secretdir/secret_dir_universal.go @@ -5,7 +5,7 @@ package secretdir import "os" // create creates a normal folder at the secret dir path -func (s *SecretDir) create(sizeMB int) error { +func (s *SecretDir) create() error { return os.MkdirAll(s.Dir, 0700) } @@ -13,3 +13,8 @@ func (s *SecretDir) create(sizeMB int) error { func (s *SecretDir) destroy() error { return os.RemoveAll(s.Dir) } + +// MemoryUse returns the memory used by the SecretDir +func (s *SecretDir) MemoryUse() int { + return 0 +} diff --git a/client/secretdir/secret_dir_unix.go b/client/secretdir/secret_dir_unix.go index 384b3767e..9592c9df8 100644 --- a/client/secretdir/secret_dir_unix.go +++ b/client/secretdir/secret_dir_unix.go @@ -8,15 +8,21 @@ import ( "syscall" ) +const ( + // SecretDirTmpfsSize is the size in MB of the in tmpfs backing the secret + // directory + SecretDirTmpfsSize = 32 +) + // create creates a tmpfs folder at the secret dir path -func (s *SecretDir) create(sizeMB int64) error { +func (s *SecretDir) create() error { if err := os.MkdirAll(s.Dir, 0700); err != nil { return err } var flags uintptr flags = syscall.MS_NOEXEC - options := fmt.Sprintf("size=%dm", sizeMB) + options := fmt.Sprintf("size=%dm", SecretDirTmpfsSize) err := syscall.Mount("tmpfs", s.Dir, "tmpfs", flags, options) return os.NewSyscallError("mount", err) } @@ -29,3 +35,8 @@ func (s *SecretDir) destroy() error { return os.RemoveAll(s.Dir) } + +// MemoryUse returns the memory used by the SecretDir +func (s *SecretDir) MemoryUse() int { + return SecretDirTmpfsSize +} diff --git a/client/secretdir/secret_dir_unix_test.go b/client/secretdir/secret_dir_unix_test.go deleted file mode 100644 index 7946158c2..000000000 --- a/client/secretdir/secret_dir_unix_test.go +++ /dev/null @@ -1 +0,0 @@ -package secretdir diff --git a/client/task_runner_test.go b/client/task_runner_test.go index 8975d0c67..ffb626a5a 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver" - "github.com/hashicorp/nomad/client/secretdir" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" @@ -57,9 +56,7 @@ func testTaskRunnerFromAlloc(t *testing.T, restarts bool, alloc *structs.Allocat task.Resources.Networks[0].ReservedPorts = []structs.Port{{"", 80}} path := filepath.Join(conf.AllocDir, alloc.ID) - allocDir := allocdir.NewAllocDir(alloc.ID, path, task.Resources.DiskMB) - sdir := secretdir.NewTestSecretDir(t) - allocDir.SetSecretDirFn(sdir.CreateFor) + allocDir := allocdir.NewAllocDir(alloc.ID, path, task.Resources.DiskMB, allocdir.TestCreateSecretDirFn) allocDir.Build([]*structs.Task{task}) ctx := driver.NewExecContext(allocDir, alloc.ID) From 0c050bd62badb10bb4b787fe0109a56e89076ab1 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 31 Aug 2016 21:38:04 -0700 Subject: [PATCH 5/5] Symlink on windows --- client/allocdir/alloc_dir_windows.go | 19 +++++++++++++++---- client/client.go | 3 +++ client/client_test.go | 5 +++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/client/allocdir/alloc_dir_windows.go b/client/allocdir/alloc_dir_windows.go index a26496b38..951a30789 100644 --- a/client/allocdir/alloc_dir_windows.go +++ b/client/allocdir/alloc_dir_windows.go @@ -7,11 +7,17 @@ import ( ) var ( - //Path inside container for mounted directory that is shared across tasks in a task group. + // SharedAllocContainerPath is the path inside container for mounted + // directory shared across tasks in a task group. SharedAllocContainerPath = filepath.Join("c:\\", SharedAllocName) - //Path inside container for mounted directory for local storage. + // TaskLocalContainer is the path inside a container for mounted directory + // for local storage. TaskLocalContainerPath = filepath.Join("c:\\", TaskLocal) + + // TaskSecretsContainerPath is the path inside a container for mounted + // secrets directory + TaskSecretsContainerPath = filepath.Join("c:\\", TaskSecrets) ) func (d *AllocDir) linkOrCopy(src, dst string, perm os.FileMode) error { @@ -21,11 +27,16 @@ func (d *AllocDir) linkOrCopy(src, dst string, perm os.FileMode) error { // Hardlinks the shared directory. As a side-effect the src and dest directory // must be on the same filesystem. func (d *AllocDir) mount(src, dest string) error { - return syscall.Link(src, dest) + return os.Symlink(src, dest) } func (d *AllocDir) unmount(dir string) error { - return syscall.Unlink(dir) + p, err := syscall.UTF16PtrFromString(dir) + if err != nil { + return err + } + + return syscall.RemoveDirectory(p) } // The windows version does nothing currently. diff --git a/client/client.go b/client/client.go index 95f405b73..e63d7b4aa 100644 --- a/client/client.go +++ b/client/client.go @@ -438,6 +438,9 @@ func (c *Client) GetAllocFS(allocID string) (allocdir.AllocDirFS, error) { if !ok { return nil, fmt.Errorf("alloc not found") } + if ar.ctx == nil { + return nil, fmt.Errorf("alloc dir not found") + } return ar.ctx.AllocDir, nil } diff --git a/client/client_test.go b/client/client_test.go index 0326b5ce3..c5cc3d38b 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -156,8 +156,9 @@ func TestClient_ReserveSecretDir(t *testing.T) { tsd := secretdir.NewTestSecretDir(t) c.secretDir = tsd - expected := 10 - tsd.MemoryUsed = expected + secretUsage := 10 + expected := c.Node().Reserved.MemoryMB + secretUsage + tsd.MemoryUsed = secretUsage c.reserveResources() res := c.Node().Reserved