diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index cda59456b..e33201822 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1666,6 +1666,15 @@ func (j *Job) Validate() error { // deprecation warnings. func (j *Job) Warnings() error { var mErr multierror.Error + + // Check the groups + for _, tg := range j.TaskGroups { + if err := tg.Warnings(j); err != nil { + outer := fmt.Errorf("Group %q has warnings: %v", tg.Name, err) + mErr.Errors = append(mErr.Errors, outer) + } + } + return mErr.ErrorOrNil() } @@ -2519,16 +2528,6 @@ func (tg *TaskGroup) Validate(j *Job) error { if err := u.Validate(); err != nil { mErr.Errors = append(mErr.Errors, err) } - - // Validate the counts are appropriate - if u.MaxParallel > tg.Count { - mErr.Errors = append(mErr.Errors, - fmt.Errorf("Update max parallel count is greater than task group count: %d > %d", u.MaxParallel, tg.Count)) - } - if u.Canary > tg.Count { - mErr.Errors = append(mErr.Errors, - fmt.Errorf("Update canary count is greater than task group count: %d > %d", u.Canary, tg.Count)) - } } // Check for duplicate tasks, that there is only leader task if any, @@ -2579,6 +2578,24 @@ func (tg *TaskGroup) Validate(j *Job) error { return mErr.ErrorOrNil() } +// Warnings returns a list of warnings that may be from dubious settings or +// deprecation warnings. +func (tg *TaskGroup) Warnings(j *Job) error { + var mErr multierror.Error + + // Validate the update strategy + if u := tg.Update; u != nil { + // Check the counts are appropriate + if u.MaxParallel > tg.Count { + mErr.Errors = append(mErr.Errors, + fmt.Errorf("Update max parallel count is greater than task group count (%d > %d). "+ + "A destructive change would result in the replacement of all allocations.", u.MaxParallel, tg.Count)) + } + } + + return mErr.ErrorOrNil() +} + // LookupTask finds a task by name func (tg *TaskGroup) LookupTask(name string) *Task { for _, t := range tg.Tasks { diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 469951376..115447efc 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -109,7 +109,24 @@ func TestJob_Warnings(t *testing.T) { Name string Job *Job Expected []string - }{} + }{ + { + Name: "Higher counts for update stanza", + Expected: []string{"max parallel count is greater"}, + Job: &Job{ + Type: JobTypeService, + TaskGroups: []*TaskGroup{ + { + Name: "foo", + Count: 2, + Update: &UpdateStrategy{ + MaxParallel: 10, + }, + }, + }, + }, + }, + } for _, c := range cases { t.Run(c.Name, func(t *testing.T) { @@ -881,15 +898,6 @@ func TestTaskGroup_Validate(t *testing.T) { Attempts: 10, Mode: RestartPolicyModeDelay, }, - Update: &UpdateStrategy{ - Stagger: 10 * time.Second, - MaxParallel: 3, - HealthCheck: UpdateStrategyHealthCheck_Manual, - MinHealthyTime: 1 * time.Second, - HealthyDeadline: 1 * time.Second, - AutoRevert: false, - Canary: 3, - }, } err = tg.Validate(j) @@ -897,22 +905,16 @@ func TestTaskGroup_Validate(t *testing.T) { if !strings.Contains(mErr.Errors[0].Error(), "should have an ephemeral disk object") { t.Fatalf("err: %s", err) } - if !strings.Contains(mErr.Errors[1].Error(), "max parallel count is greater") { + if !strings.Contains(mErr.Errors[1].Error(), "2 redefines 'web' from task 1") { t.Fatalf("err: %s", err) } - if !strings.Contains(mErr.Errors[2].Error(), "canary count is greater") { + if !strings.Contains(mErr.Errors[2].Error(), "Task 3 missing name") { t.Fatalf("err: %s", err) } - if !strings.Contains(mErr.Errors[3].Error(), "2 redefines 'web' from task 1") { + if !strings.Contains(mErr.Errors[3].Error(), "Only one task may be marked as leader") { t.Fatalf("err: %s", err) } - if !strings.Contains(mErr.Errors[4].Error(), "Task 3 missing name") { - t.Fatalf("err: %s", err) - } - if !strings.Contains(mErr.Errors[5].Error(), "Only one task may be marked as leader") { - t.Fatalf("err: %s", err) - } - if !strings.Contains(mErr.Errors[6].Error(), "Task web validation failed") { + if !strings.Contains(mErr.Errors[4].Error(), "Task web validation failed") { t.Fatalf("err: %s", err) }