Skip to content
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

Add support for go-getter modes #2781

Merged
merged 6 commits into from
Jul 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ IMPROVEMENTS:
[GH-2610]
* client: Fingerprint all routable addresses on an interface including IPv6
addresses [GH-2536]
* client/artifact: Allow specifying a go-getter mode [GH-2781]
* client/artifact: Support non-Amazon S3-compatible sources [GH-2781]
* client/template: Support reading env vars from templates [GH-2654]
* config: Support Unix socket addresses for Consul [GH-2622]
* discovery: Advertise driver-specified IP address and port [GH-2709]
Expand Down
23 changes: 22 additions & 1 deletion api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package api

import (
"fmt"
"path"

"path/filepath"
"strings"
"time"

Expand Down Expand Up @@ -323,12 +326,30 @@ func (t *Task) Canonicalize(tg *TaskGroup, job *Job) {
type TaskArtifact struct {
GetterSource *string `mapstructure:"source"`
GetterOptions map[string]string `mapstructure:"options"`
GetterMode *string `mapstructure:"mode"`
RelativeDest *string `mapstructure:"destination"`
}

func (a *TaskArtifact) Canonicalize() {
if a.GetterMode == nil {
a.GetterMode = helper.StringToPtr("any")
}
if a.GetterSource == nil {
// Shouldn't be possible, but we don't want to panic
a.GetterSource = helper.StringToPtr("")
}
if a.RelativeDest == nil {
a.RelativeDest = helper.StringToPtr("local/")
switch *a.GetterMode {
case "file":
// File mode should default to local/filename
dest := *a.GetterSource
dest = path.Base(dest)
dest = filepath.Join("local", dest)
a.RelativeDest = &dest
default:
// Default to a directory
a.RelativeDest = helper.StringToPtr("local/")
}
}
}

Expand Down
14 changes: 14 additions & 0 deletions api/tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,17 @@ func TestTask_Constrain(t *testing.T) {
t.Fatalf("expect: %#v, got: %#v", expect, task.Constraints)
}
}

func TestTask_Artifact(t *testing.T) {
a := TaskArtifact{
GetterSource: helper.StringToPtr("http://localhost/foo.txt"),
GetterMode: helper.StringToPtr("file"),
}
a.Canonicalize()
if *a.GetterMode != "file" {
t.Errorf("expected file but found %q", *a.GetterMode)
}
if *a.RelativeDest != "local/foo.txt" {
t.Errorf("expected local/foo.txt but found %q", *a.RelativeDest)
}
}
16 changes: 13 additions & 3 deletions client/getter/getter.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type EnvReplacer interface {
}

// getClient returns a client that is suitable for Nomad downloading artifacts.
func getClient(src, dst string) *gg.Client {
func getClient(src string, mode gg.ClientMode, dst string) *gg.Client {
lock.Lock()
defer lock.Unlock()

Expand All @@ -50,7 +50,7 @@ func getClient(src, dst string) *gg.Client {
return &gg.Client{
Src: src,
Dst: dst,
Mode: gg.ClientModeAny,
Mode: mode,
Getters: getters,
}
}
Expand Down Expand Up @@ -97,7 +97,17 @@ func GetArtifact(taskEnv EnvReplacer, artifact *structs.TaskArtifact, taskDir st

// Download the artifact
dest := filepath.Join(taskDir, artifact.RelativeDest)
if err := getClient(url, dest).Get(); err != nil {

// Convert from string getter mode to go-getter const
mode := gg.ClientModeAny
switch artifact.GetterMode {
case structs.GetterModeFile:
mode = gg.ClientModeFile
case structs.GetterModeDir:
mode = gg.ClientModeDir
}

if err := getClient(url, mode, dest).Get(); err != nil {
return newGetError(url, err, true)
}

Expand Down
1 change: 1 addition & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
structsTask.Artifacts[k] = &structs.TaskArtifact{
GetterSource: *ta.GetterSource,
GetterOptions: ta.GetterOptions,
GetterMode: *ta.GetterMode,
RelativeDest: *ta.RelativeDest,
}
}
Expand Down
8 changes: 6 additions & 2 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"net/http"
"net/http/httptest"
"reflect"
"strings"
"testing"
"time"

Expand All @@ -12,6 +13,7 @@ import (
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/kr/pretty"
)

func TestHTTP_JobsList(t *testing.T) {
Expand Down Expand Up @@ -993,6 +995,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
GetterOptions: map[string]string{
"a": "b",
},
GetterMode: helper.StringToPtr("dir"),
RelativeDest: helper.StringToPtr("dest"),
},
},
Expand Down Expand Up @@ -1178,6 +1181,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
GetterOptions: map[string]string{
"a": "b",
},
GetterMode: "dir",
RelativeDest: "dest",
},
},
Expand Down Expand Up @@ -1213,7 +1217,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {

structsJob := ApiJobToStructJob(apiJob)

if !reflect.DeepEqual(expected, structsJob) {
t.Fatalf("bad %#v", structsJob)
if diff := pretty.Diff(expected, structsJob); len(diff) > 0 {
t.Fatalf("bad:\n%s", strings.Join(diff, "\n"))
}
}
1 change: 1 addition & 0 deletions jobspec/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,7 @@ func parseArtifacts(result *[]*api.TaskArtifact, list *ast.ObjectList) error {
valid := []string{
"source",
"options",
"mode",
"destination",
}
if err := checkHCLKeys(o.Val, valid); err != nil {
Expand Down
1 change: 1 addition & 0 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ func TestParse(t *testing.T) {
GetterOptions: map[string]string{
"checksum": "md5:ff1cc0d3432dad54d607c1505fb7245c",
},
GetterMode: helper.StringToPtr("file"),
},
},
Vault: &api.Vault{
Expand Down
1 change: 1 addition & 0 deletions jobspec/test-fixtures/basic.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ job "binstore-storagelocker" {
artifact {
source = "http://bar.com/artifact"
destination = "test/foo/"
mode = "file"

options {
checksum = "md5:ff1cc0d3432dad54d607c1505fb7245c"
Expand Down
14 changes: 14 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2469,6 +2469,7 @@ func TestTaskDiff(t *testing.T) {
GetterOptions: map[string]string{
"bar": "baz",
},
GetterMode: "dir",
RelativeDest: "bar",
},
},
Expand All @@ -2487,6 +2488,7 @@ func TestTaskDiff(t *testing.T) {
GetterOptions: map[string]string{
"bam": "baz",
},
GetterMode: "file",
RelativeDest: "bam",
},
},
Expand All @@ -2498,6 +2500,12 @@ func TestTaskDiff(t *testing.T) {
Type: DiffTypeAdded,
Name: "Artifact",
Fields: []*FieldDiff{
{
Type: DiffTypeAdded,
Name: "GetterMode",
Old: "",
New: "file",
},
{
Type: DiffTypeAdded,
Name: "GetterOptions[bam]",
Expand All @@ -2522,6 +2530,12 @@ func TestTaskDiff(t *testing.T) {
Type: DiffTypeDeleted,
Name: "Artifact",
Fields: []*FieldDiff{
{
Type: DiffTypeDeleted,
Name: "GetterMode",
Old: "dir",
New: "",
},
{
Type: DiffTypeDeleted,
Name: "GetterOptions[bar]",
Expand Down
19 changes: 19 additions & 0 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ const (
ProtocolVersion = "protocol"
APIMajorVersion = "api.major"
APIMinorVersion = "api.minor"

GetterModeAny = "any"
GetterModeFile = "file"
GetterModeDir = "dir"
)

// RPCInfo is used to describe common information about query
Expand Down Expand Up @@ -3405,6 +3409,10 @@ type TaskArtifact struct {
// go-getter.
GetterOptions map[string]string

// GetterMode is the go-getter.ClientMode for fetching resources.
// Defaults to "any" but can be set to "file" or "dir".
GetterMode string

// RelativeDest is the download destination given relative to the task's
// directory.
RelativeDest string
Expand Down Expand Up @@ -3453,6 +3461,17 @@ func (ta *TaskArtifact) Validate() error {
mErr.Errors = append(mErr.Errors, fmt.Errorf("source must be specified"))
}

switch ta.GetterMode {
case "":
// Default to any
ta.GetterMode = GetterModeAny
case GetterModeAny, GetterModeFile, GetterModeDir:
// Ok
default:
mErr.Errors = append(mErr.Errors, fmt.Errorf("invalid artifact mode %q; must be one of: %s, %s, %s",
ta.GetterMode, GetterModeAny, GetterModeFile, GetterModeDir))
}

escaped, err := PathEscapesAllocDir("task", ta.RelativeDest)
if err != nil {
mErr.Errors = append(mErr.Errors, fmt.Errorf("invalid destination path: %v", err))
Expand Down
19 changes: 16 additions & 3 deletions vendor/github.com/hashicorp/go-getter/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/github.com/hashicorp/go-getter/appveyor.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading