-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drivers: avoid referencing client/structs package #5157
Changes from all commits
607e7f2
2831088
694e301
c0162fa
800a352
34ee0ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This func seems like a useless wrapper now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, I liked having it around, as the function name and comment is useful. |
||
return t.embedDirs(entries) | ||
} | ||
|
||
func (t *TaskDir) embedDirs(entries map[string]string) error { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why get rid of this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I guess because it's only called /w false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed -
buildChroot
andtaskdir.Build()
[1] always is called with false, so didn't see the value.[1] https://github.com/hashicorp/nomad/pull/5157/files/34ee0ba6b931f64dd5a97ca9df0c44506c17f07e#diff-301b205830d382e5f5ce9d97cd3e3404L78
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍