Merge pull request #2830 from hashicorp/b-max-warning

Warn instead of error when max_parallel is greater than count.

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 simultaneous 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 {

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)
 	}