From 607e7f2ddee02daa2265ee337216a10c9bb7aae1 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 4 Jan 2019 16:05:40 -0500 Subject: [PATCH] remove always false parameter Simplify allocDir.Build() function to avoid depending on client/structs, and remove a parameter that's always set to `false`. The motivation here is to avoid a dependency cycle between drivers/cstructs and alloc_dir. --- client/allocdir/alloc_dir_test.go | 13 ++++++------ client/allocdir/task_dir.go | 21 +++++++------------ client/allocdir/task_dir_test.go | 5 ++--- .../allocrunner/taskrunner/task_dir_hook.go | 2 +- 4 files changed, 16 insertions(+), 25 deletions(-) diff --git a/client/allocdir/alloc_dir_test.go b/client/allocdir/alloc_dir_test.go index ac9bad946..6c39950b8 100644 --- a/client/allocdir/alloc_dir_test.go +++ b/client/allocdir/alloc_dir_test.go @@ -14,7 +14,6 @@ import ( "syscall" "testing" - cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/structs" "github.com/stretchr/testify/require" @@ -112,11 +111,11 @@ func TestAllocDir_MountSharedAlloc(t *testing.T) { // Build 2 task dirs td1 := d.NewTaskDir(t1.Name) - if err := td1.Build(false, nil, cstructs.FSIsolationChroot); err != nil { + if err := td1.Build(true, nil); err != nil { t.Fatalf("error build task=%q dir: %v", t1.Name, err) } td2 := d.NewTaskDir(t2.Name) - if err := td2.Build(false, nil, cstructs.FSIsolationChroot); err != nil { + if err := td2.Build(true, nil); err != nil { t.Fatalf("error build task=%q dir: %v", t2.Name, err) } @@ -157,11 +156,11 @@ func TestAllocDir_Snapshot(t *testing.T) { // Build 2 task dirs td1 := d.NewTaskDir(t1.Name) - if err := td1.Build(false, nil, cstructs.FSIsolationImage); err != nil { + if err := td1.Build(false, nil); err != nil { t.Fatalf("error build task=%q dir: %v", t1.Name, err) } td2 := d.NewTaskDir(t2.Name) - if err := td2.Build(false, nil, cstructs.FSIsolationImage); err != nil { + if err := td2.Build(false, nil); err != nil { t.Fatalf("error build task=%q dir: %v", t2.Name, err) } @@ -249,7 +248,7 @@ func TestAllocDir_Move(t *testing.T) { defer d2.Destroy() td1 := d1.NewTaskDir(t1.Name) - if err := td1.Build(false, nil, cstructs.FSIsolationImage); err != nil { + if err := td1.Build(false, nil); err != nil { t.Fatalf("TaskDir.Build() faild: %v", err) } @@ -345,7 +344,7 @@ func TestAllocDir_ReadAt_SecretDir(t *testing.T) { defer d.Destroy() td := d.NewTaskDir(t1.Name) - if err := td.Build(false, nil, cstructs.FSIsolationImage); err != nil { + if err := td.Build(false, nil); err != nil { t.Fatalf("TaskDir.Build() failed: %v", err) } diff --git a/client/allocdir/task_dir.go b/client/allocdir/task_dir.go index 9c4a602ae..9d99f9717 100644 --- a/client/allocdir/task_dir.go +++ b/client/allocdir/task_dir.go @@ -7,7 +7,6 @@ import ( "path/filepath" hclog "github.com/hashicorp/go-hclog" - cstructs "github.com/hashicorp/nomad/client/structs" ) // TaskDir contains all of the paths relevant to a task. All paths are on the @@ -75,7 +74,7 @@ func (t *TaskDir) Copy() *TaskDir { // Build default directories and permissions in a task directory. chrootCreated // allows skipping chroot creation if the caller knows it has already been // done. -func (t *TaskDir) Build(chrootCreated bool, chroot map[string]string, fsi cstructs.FSIsolation) error { +func (t *TaskDir) Build(createChroot bool, chroot map[string]string) error { if err := os.MkdirAll(t.Dir, 0777); err != nil { return err } @@ -110,7 +109,7 @@ func (t *TaskDir) Build(chrootCreated bool, chroot map[string]string, fsi cstruc // Image based isolation will bind the shared alloc dir in the driver. // If there's no isolation the task will use the host path to the // shared alloc dir. - if fsi == cstructs.FSIsolationChroot { + if createChroot { // If the path doesn't exist OR it exists and is empty, link it empty, _ := pathEmpty(t.SharedTaskDir) if !pathExists(t.SharedTaskDir) || empty { @@ -130,8 +129,8 @@ func (t *TaskDir) Build(chrootCreated bool, chroot map[string]string, fsi cstruc } // Build chroot if chroot filesystem isolation is going to be used - if fsi == cstructs.FSIsolationChroot { - if err := t.buildChroot(chrootCreated, chroot); err != nil { + if createChroot { + if err := t.buildChroot(chroot); err != nil { return err } } @@ -142,15 +141,9 @@ func (t *TaskDir) Build(chrootCreated bool, chroot map[string]string, fsi cstruc // buildChroot takes a mapping of absolute directory or file paths on the host // to their intended, relative location within the task directory. This // attempts hardlink and then defaults to copying. If the path exists on the -// host and can't be embedded an error is returned. If chrootCreated is true -// skip expensive embedding operations and only ephemeral operations (eg -// mounting /dev) are done. -func (t *TaskDir) buildChroot(chrootCreated bool, entries map[string]string) error { - if !chrootCreated { - // Link/copy chroot entries - return t.embedDirs(entries) - } - return nil +// host and can't be embedded an error is returned. +func (t *TaskDir) buildChroot(entries map[string]string) error { + return t.embedDirs(entries) } func (t *TaskDir) embedDirs(entries map[string]string) error { diff --git a/client/allocdir/task_dir_test.go b/client/allocdir/task_dir_test.go index a4c1b7dc9..b39c275f2 100644 --- a/client/allocdir/task_dir_test.go +++ b/client/allocdir/task_dir_test.go @@ -6,7 +6,6 @@ import ( "path/filepath" "testing" - cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/helper/testlog" ) @@ -104,7 +103,7 @@ func TestTaskDir_NonRoot_Image(t *testing.T) { t.Fatalf("Build() failed: %v", err) } - if err := td.Build(false, nil, cstructs.FSIsolationImage); err != nil { + if err := td.Build(false, nil); err != nil { t.Fatalf("TaskDir.Build failed: %v", err) } } @@ -127,7 +126,7 @@ func TestTaskDir_NonRoot(t *testing.T) { t.Fatalf("Build() failed: %v", err) } - if err := td.Build(false, nil, cstructs.FSIsolationNone); err != nil { + if err := td.Build(false, nil); err != nil { t.Fatalf("TaskDir.Build failed: %v", err) } diff --git a/client/allocrunner/taskrunner/task_dir_hook.go b/client/allocrunner/taskrunner/task_dir_hook.go index 42c5ecc25..9b18c8628 100644 --- a/client/allocrunner/taskrunner/task_dir_hook.go +++ b/client/allocrunner/taskrunner/task_dir_hook.go @@ -44,7 +44,7 @@ func (h *taskDirHook) Prestart(ctx context.Context, req *interfaces.TaskPrestart // Build the task directory structure fsi := h.runner.driverCapabilities.FSIsolation - err := h.runner.taskDir.Build(false, chroot, fsi) + err := h.runner.taskDir.Build(fsi == cstructs.FSIsolationChroot, chroot) if err != nil { return err }