api use job.update as the default for taskgroup.update

api/jobs.go

@@ -375,14 +375,15 @@ type UpdateStrategy struct {
 	MinHealthyTime   *time.Duration `mapstructure:"min_healthy_time"`
 	HealthyDeadline  *time.Duration `mapstructure:"healthy_deadline"`
 	ProgressDeadline *time.Duration `mapstructure:"progress_deadline"`
+	Canary           *int           `mapstructure:"canary"`
 	AutoRevert       *bool          `mapstructure:"auto_revert"`
 	AutoPromote      *bool          `mapstructure:"auto_promote"`
-	Canary           *int           `mapstructure:"canary"`
 }
 
 // DefaultUpdateStrategy provides a baseline that can be used to upgrade
 // jobs with the old policy or for populating field defaults.
 func DefaultUpdateStrategy() *UpdateStrategy {
+	// boolPtr fields are omitted to avoid masking an unconfigured nil
 	return &UpdateStrategy{
 		Stagger:          timeToPtr(30 * time.Second),
 		MaxParallel:      intToPtr(1),
@@ -392,7 +393,6 @@ func DefaultUpdateStrategy() *UpdateStrategy {
 		ProgressDeadline: timeToPtr(10 * time.Minute),
 		AutoRevert:       boolToPtr(false),
 		Canary:           intToPtr(0),
-		AutoPromote:      boolToPtr(false),
 	}
 }
 
@@ -487,6 +487,8 @@ func (u *UpdateStrategy) Merge(o *UpdateStrategy) {
 func (u *UpdateStrategy) Canonicalize() {
 	d := DefaultUpdateStrategy()
 
+	// boolPtr fields are omitted to avoid masking an unconfigured nil
+
 	if u.MaxParallel == nil {
 		u.MaxParallel = d.MaxParallel
 	}
@@ -518,10 +520,6 @@ func (u *UpdateStrategy) Canonicalize() {
 	if u.Canary == nil {
 		u.Canary = d.Canary
 	}
-
-	if u.AutoPromote == nil {
-		u.AutoPromote = d.AutoPromote
-	}
 }
 
 // Empty returns whether the UpdateStrategy is empty or has user defined values.

api/jobs_test.go

@@ -323,7 +323,7 @@ func TestJobs_Canonicalize(t *testing.T) {
 					ProgressDeadline: timeToPtr(10 * time.Minute),
 					AutoRevert:       boolToPtr(false),
 					Canary:           intToPtr(0),
-					AutoPromote:      boolToPtr(false),
+					AutoPromote:      nil,
 				},
 				TaskGroups: []*TaskGroup{
 					{
@@ -358,7 +358,7 @@ func TestJobs_Canonicalize(t *testing.T) {
 							ProgressDeadline: timeToPtr(10 * time.Minute),
 							AutoRevert:       boolToPtr(false),
 							Canary:           intToPtr(0),
-							AutoPromote:      boolToPtr(false),
+							AutoPromote:      nil,
 						},
 						Migrate: DefaultMigrateStrategy(),
 						Tasks: []*Task{

command/agent/job_endpoint.go

@@ -655,15 +655,15 @@ func ApiJobToStructJob(job *api.Job) *structs.Job {
 		j.TaskGroups = make([]*structs.TaskGroup, l)
 		for i, taskGroup := range job.TaskGroups {
 			tg := &structs.TaskGroup{}
-			ApiTgToStructsTG(taskGroup, tg)
+			apiTgToStructsTG(taskGroup, tg, job.Update)
 			j.TaskGroups[i] = tg
 		}
 	}
 
 	return j
 }
 
-func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) {
+func apiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup, jobUpdate *api.UpdateStrategy) {
 	tg.Name = *taskGroup.Name
 	tg.Count = *taskGroup.Count
 	tg.Meta = taskGroup.Meta
@@ -710,17 +710,32 @@ func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) {
 		}
 	}
 
-	if taskGroup.Update != nil {
+	// Convert the UpdateStrategy, using defaults from the job
+	if taskGroup.Update != nil || jobUpdate != nil {
+		var au *api.UpdateStrategy
+		if jobUpdate != nil {
+			au = jobUpdate.Copy()
+			au.Merge(taskGroup.Update)
+		} else {
+			au = taskGroup.Update
+		}
+
 		tg.Update = &structs.UpdateStrategy{
-			Stagger:          *taskGroup.Update.Stagger,
-			MaxParallel:      *taskGroup.Update.MaxParallel,
-			HealthCheck:      *taskGroup.Update.HealthCheck,
-			MinHealthyTime:   *taskGroup.Update.MinHealthyTime,
-			HealthyDeadline:  *taskGroup.Update.HealthyDeadline,
-			ProgressDeadline: *taskGroup.Update.ProgressDeadline,
-			AutoRevert:       *taskGroup.Update.AutoRevert,
-			AutoPromote:      *taskGroup.Update.AutoPromote,
-			Canary:           *taskGroup.Update.Canary,
+			Stagger:          *au.Stagger,
+			MaxParallel:      *au.MaxParallel,
+			HealthCheck:      *au.HealthCheck,
+			MinHealthyTime:   *au.MinHealthyTime,
+			HealthyDeadline:  *au.HealthyDeadline,
+			ProgressDeadline: *au.ProgressDeadline,
+			Canary:           *au.Canary,
+		}
+
+		if au.AutoRevert != nil {
+			tg.Update.AutoRevert = *au.AutoRevert
+		}
+
+		if au.AutoPromote != nil {
+			tg.Update.AutoPromote = *au.AutoPromote
 		}
 	}
 
@@ -734,6 +749,10 @@ func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) {
 	}
 }
 
+func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) {
+	apiTgToStructsTG(taskGroup, tg, nil)
+}
+
 // ApiTaskToStructsTask is a copy and type conversion between the API
 // representation of a task from a struct representation of a task.
 func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {

command/agent/job_endpoint_test.go

@@ -2040,6 +2040,79 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 	}
 }
 
+func TestJobs_ApiJobToStructsJobUpdate(t *testing.T) {
+	apiJob := &api.Job{
+		Update: &api.UpdateStrategy{
+			Stagger:          helper.TimeToPtr(1 * time.Second),
+			MaxParallel:      helper.IntToPtr(5),
+			HealthCheck:      helper.StringToPtr(structs.UpdateStrategyHealthCheck_Manual),
+			MinHealthyTime:   helper.TimeToPtr(1 * time.Minute),
+			HealthyDeadline:  helper.TimeToPtr(3 * time.Minute),
+			ProgressDeadline: helper.TimeToPtr(3 * time.Minute),
+			AutoRevert:       helper.BoolToPtr(false),
+			AutoPromote:      nil,
+			Canary:           helper.IntToPtr(1),
+		},
+		TaskGroups: []*api.TaskGroup{
+			{
+				Update: &api.UpdateStrategy{
+					Canary:     helper.IntToPtr(2),
+					AutoRevert: helper.BoolToPtr(true),
+				},
+			}, {
+				Update: &api.UpdateStrategy{
+					Canary:      helper.IntToPtr(3),
+					AutoPromote: helper.BoolToPtr(true),
+				},
+			},
+		},
+	}
+
+	structsJob := ApiJobToStructJob(apiJob)
+
+	// Update has been moved from job down to the groups
+	jobUpdate := structs.UpdateStrategy{
+		Stagger:          1000000000,
+		MaxParallel:      5,
+		HealthCheck:      "",
+		MinHealthyTime:   0,
+		HealthyDeadline:  0,
+		ProgressDeadline: 0,
+		AutoRevert:       false,
+		AutoPromote:      false,
+		Canary:           0,
+	}
+
+	// But the groups inherit settings from the job update
+	group1 := structs.UpdateStrategy{
+		Stagger:          1000000000,
+		MaxParallel:      5,
+		HealthCheck:      "manual",
+		MinHealthyTime:   60000000000,
+		HealthyDeadline:  180000000000,
+		ProgressDeadline: 180000000000,
+		AutoRevert:       true,
+		AutoPromote:      false,
+		Canary:           2,
+	}
+
+	group2 := structs.UpdateStrategy{
+		Stagger:          1000000000,
+		MaxParallel:      5,
+		HealthCheck:      "manual",
+		MinHealthyTime:   60000000000,
+		HealthyDeadline:  180000000000,
+		ProgressDeadline: 180000000000,
+		AutoRevert:       false,
+		AutoPromote:      true,
+		Canary:           3,
+	}
+
+	require.Equal(t, jobUpdate, structsJob.Update)
+	require.Equal(t, group1, *structsJob.TaskGroups[0].Update)
+	require.Equal(t, group2, *structsJob.TaskGroups[1].Update)
+}
+
 // TestHTTP_JobValidate_SystemMigrate asserts that a system job with a migrate
 // stanza fails to validate but does not panic (see #5477).
 func TestHTTP_JobValidate_SystemMigrate(t *testing.T) {