From 46ce8dd02073d7d049a635582e1735ea4656dab6 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 30 Aug 2016 21:38:16 -0700 Subject: [PATCH] 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"