diff --git a/api/jobs.go b/api/jobs.go index bc1cdbd83..90cb11614 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -506,6 +506,9 @@ type JobValidateResponse struct { // ValidationErrors is a list of validation errors ValidationErrors []string + + // Error is a string version of any error that may have occured + Error string } // JobUpdateRequest is used to update a job diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index be5071f74..5ad4bde83 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/golang/snappy" - multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/nomad/structs" ) @@ -108,7 +107,7 @@ func (s *HTTPServer) jobPlan(resp http.ResponseWriter, req *http.Request, } s.parseRegion(req, &args.Region) - sJob := apiJobToStructJob(args.Job) + sJob := ApiJobToStructJob(args.Job) planReq := structs.JobPlanRequest{ Job: sJob, Diff: args.Diff, @@ -138,7 +137,7 @@ func (s *HTTPServer) ValidateJobRequest(resp http.ResponseWriter, req *http.Requ return nil, CodedError(400, "Job must be specified") } - job := apiJobToStructJob(validateRequest.Job) + job := ApiJobToStructJob(validateRequest.Job) args := structs.JobValidateRequest{ Job: job, WriteRequest: structs.WriteRequest{ @@ -149,18 +148,7 @@ func (s *HTTPServer) ValidateJobRequest(resp http.ResponseWriter, req *http.Requ var out structs.JobValidateResponse if err := s.agent.RPC("Job.Validate", &args, &out); err != nil { - - // Fall back to do local validation - args.Job.Canonicalize() - if vErr := args.Job.Validate(); vErr != nil { - if merr, ok := vErr.(*multierror.Error); ok { - for _, e := range merr.Errors { - out.ValidationErrors = append(out.ValidationErrors, e.Error()) - } - } else { - out.ValidationErrors = append(out.ValidationErrors, vErr.Error()) - } - } + return nil, err } return out, nil @@ -301,7 +289,7 @@ func (s *HTTPServer) jobUpdate(resp http.ResponseWriter, req *http.Request, } s.parseRegion(req, &args.Region) - sJob := apiJobToStructJob(args.Job) + sJob := ApiJobToStructJob(args.Job) regReq := structs.JobRegisterRequest{ Job: sJob, @@ -380,7 +368,7 @@ func (s *HTTPServer) jobDispatchRequest(resp http.ResponseWriter, req *http.Requ return out, nil } -func apiJobToStructJob(job *api.Job) *structs.Job { +func ApiJobToStructJob(job *api.Job) *structs.Job { job.Canonicalize() j := &structs.Job{ @@ -405,7 +393,7 @@ func apiJobToStructJob(job *api.Job) *structs.Job { j.Constraints = make([]*structs.Constraint, len(job.Constraints)) for i, c := range job.Constraints { con := &structs.Constraint{} - apiConstraintToStructs(c, con) + ApiConstraintToStructs(c, con) j.Constraints[i] = con } if job.Update != nil { @@ -436,21 +424,21 @@ func apiJobToStructJob(job *api.Job) *structs.Job { j.TaskGroups = make([]*structs.TaskGroup, len(job.TaskGroups)) for i, taskGroup := range job.TaskGroups { tg := &structs.TaskGroup{} - apiTgToStructsTG(taskGroup, tg) + ApiTgToStructsTG(taskGroup, tg) j.TaskGroups[i] = tg } return j } -func apiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) { +func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) { tg.Name = *taskGroup.Name tg.Count = *taskGroup.Count tg.Meta = taskGroup.Meta tg.Constraints = make([]*structs.Constraint, len(taskGroup.Constraints)) for k, constraint := range taskGroup.Constraints { c := &structs.Constraint{} - apiConstraintToStructs(constraint, c) + ApiConstraintToStructs(constraint, c) tg.Constraints[k] = c } tg.RestartPolicy = &structs.RestartPolicy{ @@ -468,12 +456,12 @@ func apiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) { tg.Tasks = make([]*structs.Task, len(taskGroup.Tasks)) for l, task := range taskGroup.Tasks { t := &structs.Task{} - apiTaskToStructsTask(task, t) + ApiTaskToStructsTask(task, t) tg.Tasks[l] = t } } -func apiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) { +func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) { structsTask.Name = apiTask.Name structsTask.Driver = apiTask.Driver structsTask.User = apiTask.User @@ -482,7 +470,7 @@ func apiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) { structsTask.Constraints = make([]*structs.Constraint, len(apiTask.Constraints)) for i, constraint := range apiTask.Constraints { c := &structs.Constraint{} - apiConstraintToStructs(constraint, c) + ApiConstraintToStructs(constraint, c) structsTask.Constraints[i] = c } structsTask.Env = apiTask.Env @@ -579,7 +567,7 @@ func apiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) { } } -func apiConstraintToStructs(c1 *api.Constraint, c2 *structs.Constraint) { +func ApiConstraintToStructs(c1 *api.Constraint, c2 *structs.Constraint) { c2.LTarget = c1.LTarget c2.RTarget = c1.RTarget c2.Operand = c1.Operand diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index a1a4b6bbf..b2bcf2254 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -1036,7 +1036,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { JobModifyIndex: 5, } - structsJob := apiJobToStructJob(apiJob) + structsJob := ApiJobToStructJob(apiJob) if !reflect.DeepEqual(expected, structsJob) { t.Fatalf("bad %#v", structsJob) diff --git a/command/validate.go b/command/validate.go index fa59ff424..fcad337a0 100644 --- a/command/validate.go +++ b/command/validate.go @@ -4,13 +4,14 @@ import ( "fmt" "strings" - "github.com/mitchellh/colorstring" + multierror "github.com/hashicorp/go-multierror" + "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/command/agent" ) type ValidateCommand struct { Meta JobGetter - color *colorstring.Colorize } func (c *ValidateCommand) Help() string { @@ -66,23 +67,50 @@ func (c *ValidateCommand) Run(args []string) int { // Check that the job is valid jr, _, err := client.Jobs().Validate(job, nil) + if err != nil { + jr, err = c.validateLocal(job) + } if err != nil { c.Ui.Error(fmt.Sprintf("Error validating job: %s", err)) return 1 } + if jr != nil && !jr.DriverConfigValidated { - c.Ui.Output(c.Colorize().Color("[bold][orange]Driver configuration not validated.[reset]")) + c.Ui.Output( + c.Colorize().Color("[bold][yellow]Driver configuration not validated since connection to Nomad agent couldn't be established.[reset]\n")) } - if jr != nil && len(jr.ValidationErrors) > 0 { - c.Ui.Output("Job Validation errors:") - for _, err := range jr.ValidationErrors { - c.Ui.Output(err) - } + if jr != nil && jr.Error != "" { + c.Ui.Error( + c.Colorize().Color("[bold][red]Job validation errors:[reset]")) + c.Ui.Error(jr.Error) return 1 } // Done! - c.Ui.Output("Job validation successful") + c.Ui.Output( + c.Colorize().Color("[bold][green]Job validation successful[reset]")) return 0 } + +// validateLocal validates without talking to a Nomad agent +func (c *ValidateCommand) validateLocal(aj *api.Job) (*api.JobValidateResponse, error) { + var out api.JobValidateResponse + + job := agent.ApiJobToStructJob(aj) + job.Canonicalize() + + if vErr := job.Validate(); vErr != nil { + if merr, ok := vErr.(*multierror.Error); ok { + for _, err := range merr.Errors { + out.ValidationErrors = append(out.ValidationErrors, err.Error()) + } + out.Error = merr.Error() + } else { + out.ValidationErrors = append(out.ValidationErrors, vErr.Error()) + out.Error = vErr.Error() + } + } + + return &out, nil +} diff --git a/command/validate_test.go b/command/validate_test.go index c4f4069ca..2caae2a30 100644 --- a/command/validate_test.go +++ b/command/validate_test.go @@ -106,7 +106,7 @@ func TestValidateCommand_Fails(t *testing.T) { if code := cmd.Run([]string{fh2.Name()}); code != 1 { t.Fatalf("expect exit 1, got: %d", code) } - if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error validating") { + if out := ui.ErrorWriter.String(); !strings.Contains(out, "Job validation errors") { t.Fatalf("expect validation error, got: %s", out) } ui.ErrorWriter.Reset() diff --git a/jobspec/parse.go b/jobspec/parse.go index 48a657775..7c98c22aa 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -15,7 +15,6 @@ import ( "github.com/hashicorp/hcl" "github.com/hashicorp/hcl/hcl/ast" "github.com/hashicorp/nomad/api" - "github.com/hashicorp/nomad/client/driver" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs" "github.com/mitchellh/mapstructure" @@ -645,22 +644,6 @@ func parseTasks(jobName string, taskGroupName string, result *[]*api.Task, list return err } } - - // Instantiate a driver to validate the configuration - d, err := driver.NewDriver( - t.Driver, - driver.NewEmptyDriverContext(), - ) - - if err != nil { - return multierror.Prefix(err, - fmt.Sprintf("'%s', config ->", n)) - } - - if err := d.Validate(t.Config); err != nil { - return multierror.Prefix(err, - fmt.Sprintf("'%s', config ->", n)) - } } // Parse constraints diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 617ab29f6..58d9225c1 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -532,61 +532,6 @@ func TestParse(t *testing.T) { } } -func TestBadConfigEmpty(t *testing.T) { - path, err := filepath.Abs(filepath.Join("./test-fixtures", "bad-config-empty.hcl")) - if err != nil { - t.Fatalf("Can't get absolute path for file: %s", err) - } - - _, err = ParseFile(path) - - if !strings.Contains(err.Error(), "field \"image\" is required, but no value was found") { - t.Fatalf("\nExpected error\n %s\ngot\n %v", - "field \"image\" is required, but no value was found", - err, - ) - } -} - -func TestBadConfigMissing(t *testing.T) { - path, err := filepath.Abs(filepath.Join("./test-fixtures", "bad-config-missing.hcl")) - if err != nil { - t.Fatalf("Can't get absolute path for file: %s", err) - } - - _, err = ParseFile(path) - - if !strings.Contains(err.Error(), "field \"image\" is required") { - t.Fatalf("\nExpected error\n %s\ngot\n %v", - "field \"image\" is required", - err, - ) - } -} - -func TestBadConfig(t *testing.T) { - path, err := filepath.Abs(filepath.Join("./test-fixtures", "bad-config.hcl")) - if err != nil { - t.Fatalf("Can't get absolute path for file: %s", err) - } - - _, err = ParseFile(path) - - if !strings.Contains(err.Error(), "seem to be of type boolean") { - t.Fatalf("\nExpected error\n %s\ngot\n %v", - "seem to be of type boolean", - err, - ) - } - - if !strings.Contains(err.Error(), "\"foo\" is an invalid field") { - t.Fatalf("\nExpected error\n %s\ngot\n %v", - "\"foo\" is an invalid field", - err, - ) - } -} - func TestBadPorts(t *testing.T) { path, err := filepath.Abs(filepath.Join("./test-fixtures", "bad-ports.hcl")) if err != nil { diff --git a/jobspec/test-fixtures/bad-config-empty.hcl b/jobspec/test-fixtures/bad-config-empty.hcl deleted file mode 100644 index 29fa178c6..000000000 --- a/jobspec/test-fixtures/bad-config-empty.hcl +++ /dev/null @@ -1,16 +0,0 @@ -job "binstore-storagelocker" { - group "binsl" { - count = 5 - - task "binstore" { - driver = "docker" - - config { - image = "" - } - - resources { - } - } - } -} diff --git a/jobspec/test-fixtures/bad-config-missing.hcl b/jobspec/test-fixtures/bad-config-missing.hcl deleted file mode 100644 index bab89c073..000000000 --- a/jobspec/test-fixtures/bad-config-missing.hcl +++ /dev/null @@ -1,15 +0,0 @@ -job "binstore-storagelocker" { - group "binsl" { - count = 5 - - task "binstore" { - driver = "docker" - - config { - } - - resources { - } - } - } -} diff --git a/jobspec/test-fixtures/bad-config.hcl b/jobspec/test-fixtures/bad-config.hcl deleted file mode 100644 index 84f9ce743..000000000 --- a/jobspec/test-fixtures/bad-config.hcl +++ /dev/null @@ -1,18 +0,0 @@ -job "binstore-storagelocker" { - group "binsl" { - count = 5 - - task "binstore" { - driver = "docker" - - config { - image = "hashicorp/image" - privileged = "false" - foo = "bar" - } - - resources { - } - } - } -} diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index bbed6b745..780fd0776 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -294,13 +294,13 @@ func (j *Job) Validate(args *structs.JobValidateRequest, for _, err := range merr.Errors { reply.ValidationErrors = append(reply.ValidationErrors, err.Error()) } + reply.Error = merr.Error() } else { reply.ValidationErrors = append(reply.ValidationErrors, err.Error()) + reply.Error = err.Error() } - } reply.DriverConfigValidated = true - return nil } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 853aa0fde..560090510 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -296,6 +296,9 @@ type JobValidateResponse struct { // ValidationErrors is a list of validation errors ValidationErrors []string + + // Error is a string version of any error that may have occured + Error string } // NodeListRequest is used to parameterize a list request @@ -1277,7 +1280,7 @@ func (j *Job) Validate() error { // Validate the task group for _, tg := range j.TaskGroups { if err := tg.Validate(); err != nil { - outer := fmt.Errorf("Task group %s validation failed: %s", tg.Name, err) + outer := fmt.Errorf("Task group %s validation failed: %v", tg.Name, err) mErr.Errors = append(mErr.Errors, outer) } } @@ -2021,7 +2024,7 @@ func (tg *TaskGroup) Validate() error { // Validate the tasks for _, task := range tg.Tasks { if err := task.Validate(tg.EphemeralDisk); err != nil { - outer := fmt.Errorf("Task %s validation failed: %s", task.Name, err) + outer := fmt.Errorf("Task %s validation failed: %v", task.Name, err) mErr.Errors = append(mErr.Errors, outer) } }