diff --git a/client/alloc_runner.go b/client/alloc_runner.go index 181600df0..e2705a341 100644 --- a/client/alloc_runner.go +++ b/client/alloc_runner.go @@ -12,7 +12,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/structs" cstructs "github.com/hashicorp/nomad/client/structs" @@ -52,20 +51,17 @@ type AllocRunner struct { dirtyCh chan struct{} - ctx *driver.ExecContext - ctxLock sync.Mutex + ctx *driver.ExecContext + ctxLock sync.Mutex + tasks map[string]*TaskRunner + taskStates map[string]*structs.TaskState + restored map[string]struct{} + taskLock sync.RWMutex - 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.SecretDirectory - destroy bool destroyCh chan struct{} destroyLock sync.Mutex @@ -84,13 +80,12 @@ 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.SecretDirectory) *AllocRunner { + alloc *structs.Allocation) *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), @@ -131,8 +126,6 @@ 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,11 +386,9 @@ func (r *AllocRunner) Run() { // Create the execution context r.ctxLock.Lock() 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.CreateFor) + allocDir := allocdir.NewAllocDir(filepath.Join(r.config.AllocDir, r.alloc.ID), r.Alloc().Resources.DiskMB) if err := allocDir.Build(tg.Tasks); err != nil { - r.logger.Printf("[ERR] client: failed to build task directories: %v", err) + r.logger.Printf("[WARN] 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/alloc_runner_test.go b/client/alloc_runner_test.go index a479cd91d..7e72fae2a 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -12,7 +12,6 @@ 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" ) @@ -26,7 +25,7 @@ func (m *MockAllocStateUpdater) Update(alloc *structs.Allocation) { m.Allocs = append(m.Allocs, alloc) } -func testAllocRunnerFromAlloc(t *testing.T, alloc *structs.Allocation, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { +func testAllocRunnerFromAlloc(alloc *structs.Allocation, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { logger := testLogger() conf := config.DefaultConfig() conf.StateDir = os.TempDir() @@ -36,17 +35,17 @@ func testAllocRunnerFromAlloc(t *testing.T, alloc *structs.Allocation, restarts *alloc.Job.LookupTaskGroup(alloc.TaskGroup).RestartPolicy = structs.RestartPolicy{Attempts: 0} alloc.Job.Type = structs.JobTypeBatch } - ar := NewAllocRunner(logger, conf, upd.Update, alloc, secretdir.NewTestSecretDir(t)) + ar := NewAllocRunner(logger, conf, upd.Update, alloc) return upd, ar } -func testAllocRunner(t *testing.T, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { - return testAllocRunnerFromAlloc(t, mock.Alloc(), restarts) +func testAllocRunner(restarts bool) (*MockAllocStateUpdater, *AllocRunner) { + return testAllocRunnerFromAlloc(mock.Alloc(), restarts) } func TestAllocRunner_SimpleRun(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(t, false) + upd, ar := testAllocRunner(false) go ar.Run() defer ar.Destroy() @@ -83,7 +82,7 @@ func TestAllocRunner_RetryArtifact(t *testing.T) { } alloc.Job.TaskGroups[0].Tasks = append(alloc.Job.TaskGroups[0].Tasks, badtask) - upd, ar := testAllocRunnerFromAlloc(t, alloc, true) + upd, ar := testAllocRunnerFromAlloc(alloc, true) go ar.Run() defer ar.Destroy() @@ -119,7 +118,7 @@ func TestAllocRunner_RetryArtifact(t *testing.T) { func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(t, false) + upd, ar := testAllocRunner(false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -208,7 +207,7 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { func TestAllocRunner_DiskExceeded_Destroy(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(t, false) + upd, ar := testAllocRunner(false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -314,7 +313,7 @@ func TestAllocRunner_DiskExceeded_Destroy(t *testing.T) { } func TestAllocRunner_Destroy(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(t, false) + upd, ar := testAllocRunner(false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -366,7 +365,7 @@ func TestAllocRunner_Destroy(t *testing.T) { func TestAllocRunner_Update(t *testing.T) { ctestutil.ExecCompatible(t) - _, ar := testAllocRunner(t, false) + _, ar := testAllocRunner(false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -392,7 +391,7 @@ func TestAllocRunner_Update(t *testing.T) { func TestAllocRunner_SaveRestoreState(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(t, false) + upd, ar := testAllocRunner(false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -414,7 +413,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}, ar.secretDir) + &structs.Allocation{ID: ar.alloc.ID}) err = ar2.RestoreState() if err != nil { t.Fatalf("err: %v", err) @@ -442,7 +441,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(t, false) + upd, ar := testAllocRunner(false) ar.logger = prefixedTestLogger("ar1: ") // Ensure task takes some time @@ -486,7 +485,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}, ar.secretDir) + &structs.Allocation{ID: ar.alloc.ID}) ar2.logger = prefixedTestLogger("ar2: ") err = ar2.RestoreState() if err != nil { @@ -548,7 +547,7 @@ func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) { func TestAllocRunner_TaskFailed_KillTG(t *testing.T) { ctestutil.ExecCompatible(t) - upd, ar := testAllocRunner(t, false) + upd, ar := testAllocRunner(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 fd51577dc..6d0a41ee0 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -46,24 +46,11 @@ 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 CreateSecretDirFn func(allocID, task string) (path string, err error) - type AllocDir struct { - // AllocID is the allocation ID for this directory - AllocID string - - // 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 // of this allocation. It will be purged on alloc destroy. AllocDir string @@ -122,30 +109,20 @@ 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. 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 { +// the allocation directory and maxSize as the maximum allowed size in megabytes. +func NewAllocDir(allocDir string, maxSize int) *AllocDir { d := &AllocDir{ - AllocID: allocID, AllocDir: allocDir, MaxCheckDiskInterval: maxCheckDiskInterval, MinCheckDiskInterval: minCheckDiskInterval, 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 -} - // Tears down previously build directory structure. func (d *AllocDir) Destroy() error { @@ -168,7 +145,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.unmount(taskAlloc); err != nil { + if err := d.unmountSharedDir(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 { @@ -177,18 +154,6 @@ 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) } @@ -258,24 +223,6 @@ func (d *AllocDir) Build(tasks []*structs.Task) error { return err } } - - // Get the secret directory - 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 for task %q: %v", t.Name, err) - } - - if err := d.dropDirPermissions(taskSecret); err != nil { - return err - } - } } return nil @@ -385,7 +332,7 @@ func (d *AllocDir) MountSharedDir(task string) error { } taskLoc := filepath.Join(taskDir, SharedAllocName) - if err := d.mount(d.SharedDir, taskLoc); err != nil { + if err := d.mountSharedDir(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 318d5bb15..2cfdd38c3 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 src and dest directory -// must be on the same filesystem. -func (d *AllocDir) mount(src, dest string) error { - return syscall.Link(src, dest) +// 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) } -func (d *AllocDir) unmount(dir string) error { +func (d *AllocDir) unmountSharedDir(dir string) error { return syscall.Unlink(dir) } diff --git a/client/allocdir/alloc_dir_freebsd.go b/client/allocdir/alloc_dir_freebsd.go index 8e5d4eaec..a4d3801db 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 src and dest directory -// must be on the same filesystem. -func (d *AllocDir) mount(src, dest string) error { - return syscall.Link(src, dest) +// 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) } -func (d *AllocDir) unmount(dir string) error { +func (d *AllocDir) unmountSharedDir(dir string) error { return syscall.Unlink(dir) } diff --git a/client/allocdir/alloc_dir_linux.go b/client/allocdir/alloc_dir_linux.go index bf5570d41..9b2c67035 100644 --- a/client/allocdir/alloc_dir_linux.go +++ b/client/allocdir/alloc_dir_linux.go @@ -9,15 +9,17 @@ import ( "github.com/hashicorp/go-multierror" ) -func (d *AllocDir) mount(src, dest string) error { - if err := os.MkdirAll(dest, 0777); err != nil { +// 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 { return err } - return syscall.Mount(src, dest, "", syscall.MS_BIND, "") + return syscall.Mount(d.SharedDir, taskDir, "", syscall.MS_BIND, "") } -func (d *AllocDir) unmount(dir string) error { +func (d *AllocDir) unmountSharedDir(dir string) error { return syscall.Unmount(dir, 0) } diff --git a/client/allocdir/alloc_dir_test.go b/client/allocdir/alloc_dir_test.go index c2682e5d4..aa0f5e49b 100644 --- a/client/allocdir/alloc_dir_test.go +++ b/client/allocdir/alloc_dir_test.go @@ -52,12 +52,7 @@ func TestAllocDir_BuildAlloc(t *testing.T) { } defer os.RemoveAll(tmp) - secretDirFn := func(allocID, task string) (string, error) { - return ioutil.TempDir("", "") - } - - allocID := "123" - d := NewAllocDir(allocID, tmp, structs.DefaultResources().DiskMB, secretDirFn) + d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) defer d.Destroy() tasks := []*structs.Task{t1, t2} if err := d.Build(tasks); err != nil { @@ -78,10 +73,6 @@ 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) - } } } @@ -92,7 +83,7 @@ func TestAllocDir_LogDir(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir("123", tmp, structs.DefaultResources().DiskMB, TestCreateSecretDirFn) + d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) defer d.Destroy() expected := filepath.Join(d.AllocDir, SharedAllocName, LogDirName) @@ -108,7 +99,7 @@ func TestAllocDir_EmbedNonExistent(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir("123", tmp, structs.DefaultResources().DiskMB, TestCreateSecretDirFn) + d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) defer d.Destroy() tasks := []*structs.Task{t1, t2} if err := d.Build(tasks); err != nil { @@ -130,7 +121,7 @@ func TestAllocDir_EmbedDirs(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir("123", tmp, structs.DefaultResources().DiskMB, TestCreateSecretDirFn) + d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) defer d.Destroy() tasks := []*structs.Task{t1, t2} if err := d.Build(tasks); err != nil { @@ -191,7 +182,7 @@ func TestAllocDir_MountSharedAlloc(t *testing.T) { } defer os.RemoveAll(tmp) - d := NewAllocDir("123", tmp, structs.DefaultResources().DiskMB, TestCreateSecretDirFn) + d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) 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 deleted file mode 100644 index 199640a03..000000000 --- a/client/allocdir/alloc_dir_testing.go +++ /dev/null @@ -1,8 +0,0 @@ -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_unix.go b/client/allocdir/alloc_dir_unix.go index 0807505c4..339e59d5d 100644 --- a/client/allocdir/alloc_dir_unix.go +++ b/client/allocdir/alloc_dir_unix.go @@ -14,17 +14,11 @@ import ( ) var ( - // SharedAllocContainerPath is the path inside container for mounted - // directory shared across tasks in a task group. + //Path inside container for mounted directory shared across tasks in a task group. SharedAllocContainerPath = filepath.Join("/", SharedAllocName) - // TaskLocalContainer is the path inside a container for mounted directory - // for local storage. + //Path inside 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/allocdir/alloc_dir_windows.go b/client/allocdir/alloc_dir_windows.go index 951a30789..112fe9b63 100644 --- a/client/allocdir/alloc_dir_windows.go +++ b/client/allocdir/alloc_dir_windows.go @@ -1,42 +1,26 @@ package allocdir import ( + "errors" "os" "path/filepath" - "syscall" ) var ( - // SharedAllocContainerPath is the path inside container for mounted - // directory shared across tasks in a task group. + //Path inside container for mounted directory that is shared across tasks in a task group. SharedAllocContainerPath = filepath.Join("c:\\", SharedAllocName) - // TaskLocalContainer is the path inside a container for mounted directory - // for local storage. + //Path inside 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 { return fileCopy(src, dst, perm) } -// 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 os.Symlink(src, dest) -} - -func (d *AllocDir) unmount(dir string) error { - p, err := syscall.UTF16PtrFromString(dir) - if err != nil { - return err - } - - return syscall.RemoveDirectory(p) +// The windows version does nothing currently. +func (d *AllocDir) mountSharedDir(dir string) error { + return errors.New("Mount on Windows not supported.") } // The windows version does nothing currently. @@ -44,6 +28,11 @@ func (d *AllocDir) dropDirPermissions(path string) error { return nil } +// The windows version does nothing currently. +func (d *AllocDir) unmountSharedDir(dir string) error { + return nil +} + // MountSpecialDirs mounts the dev and proc file system on the chroot of the // task. It's a no-op on windows. func (d *AllocDir) MountSpecialDirs(taskDir string) error { diff --git a/client/client.go b/client/client.go index 074ecba0a..45c51fdbe 100644 --- a/client/client.go +++ b/client/client.go @@ -22,7 +22,6 @@ 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/client/vaultclient" "github.com/hashicorp/nomad/command/agent/consul" @@ -80,9 +79,6 @@ 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 @@ -113,8 +109,6 @@ type Client struct { connPool *nomad.ConnPool - secretDir secretdir.SecretDirectory - // 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, @@ -197,7 +191,7 @@ func NewClient(cfg *config.Config, consulSyncer *consul.Syncer, logger *log.Logg } // Setup the reserved resources - c.reserveResources() + c.reservePorts() // Store the config copy before restoring state but after it has been // initialized. @@ -296,14 +290,6 @@ 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 } @@ -361,10 +347,6 @@ func (c *Client) Shutdown() error { <-ar.WaitCh() } c.allocLock.Unlock() - - if err := c.secretDir.Destroy(); err != nil { - return err - } } c.shutdown = true @@ -458,9 +440,6 @@ 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 } @@ -490,7 +469,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, c.secretDir) + ar := NewAllocRunner(c.logger, c.configCopy, c.updateAllocStatus, alloc) c.configLock.RUnlock() c.allocLock.Lock() c.allocs[id] = ar @@ -626,24 +605,10 @@ 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.Lock() - defer c.configLock.Unlock() + c.configLock.RLock() + defer c.configLock.RUnlock() global := c.config.GloballyReservedPorts if len(global) == 0 { return @@ -1319,7 +1284,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, c.secretDir) + ar := NewAllocRunner(c.logger, c.configCopy, c.updateAllocStatus, alloc) c.configLock.RUnlock() go ar.Run() diff --git a/client/client_test.go b/client/client_test.go index 91d260d08..ba98dc054 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -13,7 +13,6 @@ 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" @@ -151,23 +150,6 @@ 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 - secretUsage := 10 - expected := c.Node().Reserved.MemoryMB + secretUsage - tsd.MemoryUsed = secretUsage - - 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/docker.go b/client/driver/docker.go index c2fdc92c2..6c1390aa1 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -381,7 +381,6 @@ 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 60adf9c7d..2ad38835a 100644 --- a/client/driver/driver.go +++ b/client/driver/driver.go @@ -153,7 +153,6 @@ 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/driver_test.go b/client/driver/driver_test.go index 0f2ad758b..b8b1c2889 100644 --- a/client/driver/driver_test.go +++ b/client/driver/driver_test.go @@ -79,9 +79,7 @@ func testConfig() *config.Config { 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.TestCreateSecretDirFn) + allocDir := allocdir.NewAllocDir(filepath.Join(cfg.AllocDir, structs.GenerateUUID()), task.Resources.DiskMB) allocDir.Build([]*structs.Task{task}) alloc := mock.Alloc() execCtx := NewExecContext(allocDir, alloc.ID) diff --git a/client/driver/env/env.go b/client/driver/env/env.go index e2ff660b8..1ac9b7510 100644 --- a/client/driver/env/env.go +++ b/client/driver/env/env.go @@ -21,10 +21,6 @@ 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" @@ -83,7 +79,6 @@ type TaskEnvironment struct { JobMeta map[string]string AllocDir string TaskDir string - SecretDir string CpuLimit int MemLimit int TaskName string @@ -158,9 +153,6 @@ 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 { @@ -257,16 +249,6 @@ 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 8925c0339..c673555cc 100644 --- a/client/driver/executor/executor_linux.go +++ b/client/driver/executor/executor_linux.go @@ -240,7 +240,6 @@ 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 { diff --git a/client/driver/executor/executor_linux_test.go b/client/driver/executor/executor_linux_test.go index 2dc6808df..c3006071a 100644 --- a/client/driver/executor/executor_linux_test.go +++ b/client/driver/executor/executor_linux_test.go @@ -81,24 +81,7 @@ func TestExecutor_IsolationAndConstraints(t *testing.T) { t.Fatalf("file %v hasn't been removed", memLimits) } - expected := `/: -alloc/ -bin/ -dev/ -etc/ -lib/ -lib64/ -local/ -proc/ -secrets/ -tmp/ -usr/ - -/etc/: -ld.so.cache -ld.so.conf -ld.so.conf.d/` - + expected := "/:\nalloc/\nbin/\ndev/\netc/\nlib/\nlib64/\nlocal/\nproc/\ntmp/\nusr/\n\n/etc/:\nld.so.cache\nld.so.conf\nld.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 33f7f7216..50ec6353b 100644 --- a/client/driver/executor/executor_test.go +++ b/client/driver/executor/executor_test.go @@ -37,8 +37,7 @@ func mockAllocDir(t *testing.T) (*structs.Task, *allocdir.AllocDir) { alloc := mock.Alloc() task := alloc.Job.TaskGroups[0].Tasks[0] - path := filepath.Join(os.TempDir(), alloc.ID) - allocDir := allocdir.NewAllocDir(alloc.ID, path, task.Resources.DiskMB, allocdir.TestCreateSecretDirFn) + allocDir := allocdir.NewAllocDir(filepath.Join(os.TempDir(), alloc.ID), task.Resources.DiskMB) 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 fc68e4780..f8ed84243 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -376,7 +376,6 @@ 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 deleted file mode 100644 index de92f9c32..000000000 --- a/client/secretdir/secret_dir.go +++ /dev/null @@ -1,71 +0,0 @@ -package secretdir - -import ( - "fmt" - "os" - "path/filepath" -) - -type SecretDirectory interface { - MemoryUse() int - 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 -} - -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) - } - - if err := s.create(); 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 deleted file mode 100644 index ce8cf78e0..000000000 --- a/client/secretdir/secret_dir_test.go +++ /dev/null @@ -1,70 +0,0 @@ -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_testing.go b/client/secretdir/secret_dir_testing.go deleted file mode 100644 index 9c1d6885d..000000000 --- a/client/secretdir/secret_dir_testing.go +++ /dev/null @@ -1,53 +0,0 @@ -package secretdir - -import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" - "testing" -) - -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 { - 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) -} - -func (s *TestSecretDir) MemoryUse() int { return s.MemoryUsed } diff --git a/client/secretdir/secret_dir_universal.go b/client/secretdir/secret_dir_universal.go deleted file mode 100644 index 49e8501e3..000000000 --- a/client/secretdir/secret_dir_universal.go +++ /dev/null @@ -1,20 +0,0 @@ -// +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() error { - return os.MkdirAll(s.Dir, 0700) -} - -// destroy removes the secret dir -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 deleted file mode 100644 index 9592c9df8..000000000 --- a/client/secretdir/secret_dir_unix.go +++ /dev/null @@ -1,42 +0,0 @@ -// +build dragonfly freebsd linux netbsd openbsd solaris - -package secretdir - -import ( - "fmt" - "os" - "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() error { - if err := os.MkdirAll(s.Dir, 0700); err != nil { - return err - } - - var flags uintptr - flags = syscall.MS_NOEXEC - options := fmt.Sprintf("size=%dm", SecretDirTmpfsSize) - 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) -} - -// MemoryUse returns the memory used by the SecretDir -func (s *SecretDir) MemoryUse() int { - return SecretDirTmpfsSize -} diff --git a/client/task_runner_test.go b/client/task_runner_test.go index ffb626a5a..0fc7316c2 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -38,13 +38,13 @@ func (m *MockTaskStateUpdater) Update(name, state string, event *structs.TaskEve m.events = append(m.events, event) } -func testTaskRunner(t *testing.T, restarts bool) (*MockTaskStateUpdater, *TaskRunner) { - return testTaskRunnerFromAlloc(t, restarts, mock.Alloc()) +func testTaskRunner(restarts bool) (*MockTaskStateUpdater, *TaskRunner) { + return testTaskRunnerFromAlloc(restarts, mock.Alloc()) } // Creates a mock task runner using the first task in the first task group of // the passed allocation. -func testTaskRunnerFromAlloc(t *testing.T, restarts bool, alloc *structs.Allocation) (*MockTaskStateUpdater, *TaskRunner) { +func testTaskRunnerFromAlloc(restarts bool, alloc *structs.Allocation) (*MockTaskStateUpdater, *TaskRunner) { logger := testLogger() conf := config.DefaultConfig() conf.StateDir = os.TempDir() @@ -55,8 +55,7 @@ func testTaskRunnerFromAlloc(t *testing.T, restarts bool, alloc *structs.Allocat // we have a mock so that doesn't happen. task.Resources.Networks[0].ReservedPorts = []structs.Port{{"", 80}} - path := filepath.Join(conf.AllocDir, alloc.ID) - allocDir := allocdir.NewAllocDir(alloc.ID, path, task.Resources.DiskMB, allocdir.TestCreateSecretDirFn) + allocDir := allocdir.NewAllocDir(filepath.Join(conf.AllocDir, alloc.ID), task.Resources.DiskMB) allocDir.Build([]*structs.Task{task}) ctx := driver.NewExecContext(allocDir, alloc.ID) @@ -69,7 +68,7 @@ func testTaskRunnerFromAlloc(t *testing.T, restarts bool, alloc *structs.Allocat func TestTaskRunner_SimpleRun(t *testing.T) { ctestutil.ExecCompatible(t) - upd, tr := testTaskRunner(t, false) + upd, tr := testTaskRunner(false) tr.MarkReceived() go tr.Run() defer tr.Destroy(structs.NewTaskEvent(structs.TaskKilled)) @@ -104,7 +103,7 @@ func TestTaskRunner_SimpleRun(t *testing.T) { func TestTaskRunner_Destroy(t *testing.T) { ctestutil.ExecCompatible(t) - upd, tr := testTaskRunner(t, true) + upd, tr := testTaskRunner(true) tr.MarkReceived() defer tr.ctx.AllocDir.Destroy() @@ -166,7 +165,7 @@ func TestTaskRunner_Destroy(t *testing.T) { func TestTaskRunner_Update(t *testing.T) { ctestutil.ExecCompatible(t) - _, tr := testTaskRunner(t, false) + _, tr := testTaskRunner(false) // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" @@ -220,7 +219,7 @@ func TestTaskRunner_Update(t *testing.T) { func TestTaskRunner_SaveRestoreState(t *testing.T) { ctestutil.ExecCompatible(t) - upd, tr := testTaskRunner(t, false) + upd, tr := testTaskRunner(false) // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" @@ -270,7 +269,7 @@ func TestTaskRunner_Download_List(t *testing.T) { } task.Artifacts = []*structs.TaskArtifact{&artifact1, &artifact2} - upd, tr := testTaskRunnerFromAlloc(t, false, alloc) + upd, tr := testTaskRunnerFromAlloc(false, alloc) tr.MarkReceived() go tr.Run() defer tr.Destroy(structs.NewTaskEvent(structs.TaskKilled)) @@ -335,7 +334,7 @@ func TestTaskRunner_Download_Retries(t *testing.T) { Mode: structs.RestartPolicyModeFail, } - upd, tr := testTaskRunnerFromAlloc(t, true, alloc) + upd, tr := testTaskRunnerFromAlloc(true, alloc) tr.MarkReceived() go tr.Run() defer tr.Destroy(structs.NewTaskEvent(structs.TaskKilled)) @@ -385,9 +384,7 @@ func TestTaskRunner_Download_Retries(t *testing.T) { } func TestTaskRunner_Validate_UserEnforcement(t *testing.T) { - _, tr := testTaskRunner(t, false) - defer tr.Destroy(structs.NewTaskEvent(structs.TaskKilled)) - defer tr.ctx.AllocDir.Destroy() + _, tr := testTaskRunner(false) // Try to run as root with exec. tr.task.Driver = "exec"