Merge pull request #5953 from hashicorp/b-job-update-auto_revert-merge

jobs update stanza merging cleanup
hashicorp · Jul 19, 2019 · 8833fb8 · 8833fb8
2 parents fdb14bd + 9b077bf
commit 8833fb8
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 6 deletions.
diff --git a/api/jobs.go b/api/jobs.go
@@ -383,7 +383,6 @@ type UpdateStrategy struct {
 // 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),
@@ -393,6 +392,7 @@ func DefaultUpdateStrategy() *UpdateStrategy {
 		ProgressDeadline: timeToPtr(10 * time.Minute),
 		AutoRevert:       boolToPtr(false),
 		Canary:           intToPtr(0),
+		AutoPromote:      boolToPtr(false),
 	}
 }
 
@@ -487,8 +487,6 @@ 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
 	}
@@ -520,6 +518,10 @@ 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.
@@ -556,6 +558,10 @@ func (u *UpdateStrategy) Empty() bool {
 		return false
 	}
 
+	if u.AutoPromote != nil && *u.AutoPromote {
+		return false
+	}
+
 	if u.Canary != nil && *u.Canary != 0 {
 		return false
 	}

diff --git a/api/jobs_test.go b/api/jobs_test.go
@@ -224,6 +224,7 @@ func TestJobs_Canonicalize(t *testing.T) {
 				Type:        stringToPtr("service"),
 				Update: &UpdateStrategy{
 					MaxParallel: intToPtr(1),
+					AutoPromote: boolToPtr(true),
 				},
 				TaskGroups: []*TaskGroup{
 					{
@@ -235,6 +236,9 @@ func TestJobs_Canonicalize(t *testing.T) {
 							Delay:    timeToPtr(25 * time.Second),
 							Mode:     stringToPtr("delay"),
 						},
+						Update: &UpdateStrategy{
+							AutoRevert: boolToPtr(true),
+						},
 						EphemeralDisk: &EphemeralDisk{
 							SizeMB: intToPtr(300),
 						},
@@ -323,7 +327,7 @@ func TestJobs_Canonicalize(t *testing.T) {
 					ProgressDeadline: timeToPtr(10 * time.Minute),
 					AutoRevert:       boolToPtr(false),
 					Canary:           intToPtr(0),
-					AutoPromote:      nil,
+					AutoPromote:      boolToPtr(true),
 				},
 				TaskGroups: []*TaskGroup{
 					{
@@ -356,9 +360,9 @@ func TestJobs_Canonicalize(t *testing.T) {
 							MinHealthyTime:   timeToPtr(10 * time.Second),
 							HealthyDeadline:  timeToPtr(5 * time.Minute),
 							ProgressDeadline: timeToPtr(10 * time.Minute),
-							AutoRevert:       boolToPtr(false),
+							AutoRevert:       boolToPtr(true),
 							Canary:           intToPtr(0),
-							AutoPromote:      nil,
+							AutoPromote:      boolToPtr(true),
 						},
 						Migrate: DefaultMigrateStrategy(),
 						Tasks: []*Task{

diff --git a/api/tasks_test.go b/api/tasks_test.go
@@ -370,6 +370,7 @@ func TestTask_Artifact(t *testing.T) {
 
 // Ensures no regression on https://github.com/hashicorp/nomad/issues/3132
 func TestTaskGroup_Canonicalize_Update(t *testing.T) {
+	// Job with an Empty() Update
 	job := &Job{
 		ID: stringToPtr("test"),
 		Update: &UpdateStrategy{
@@ -389,9 +390,41 @@ func TestTaskGroup_Canonicalize_Update(t *testing.T) {
 		Name: stringToPtr("foo"),
 	}
 	tg.Canonicalize(job)
+	assert.NotNil(t, job.Update)
 	assert.Nil(t, tg.Update)
 }
 
+func TestTaskGroup_Merge_Update(t *testing.T) {
+	job := &Job{
+		ID:     stringToPtr("test"),
+		Update: &UpdateStrategy{},
+	}
+	job.Canonicalize()
+
+	// Merge and canonicalize part of an update stanza
+	tg := &TaskGroup{
+		Name: stringToPtr("foo"),
+		Update: &UpdateStrategy{
+			AutoRevert:  boolToPtr(true),
+			Canary:      intToPtr(5),
+			HealthCheck: stringToPtr("foo"),
+		},
+	}
+
+	tg.Canonicalize(job)
+	require.Equal(t, &UpdateStrategy{
+		AutoRevert:       boolToPtr(true),
+		AutoPromote:      boolToPtr(false),
+		Canary:           intToPtr(5),
+		HealthCheck:      stringToPtr("foo"),
+		HealthyDeadline:  timeToPtr(5 * time.Minute),
+		ProgressDeadline: timeToPtr(10 * time.Minute),
+		MaxParallel:      intToPtr(1),
+		MinHealthyTime:   timeToPtr(10 * time.Second),
+		Stagger:          timeToPtr(30 * time.Second),
+	}, tg.Update)
+}
+
 // Verifies that migrate strategy is merged correctly
 func TestTaskGroup_Canonicalize_MigrateStrategy(t *testing.T) {
 	type testCase struct {