Merge pull request #3139 from hashicorp/b-copied-update

Don't merge empty update from job into task groups

CHANGELOG.md

@@ -3,6 +3,7 @@
 BUG FIXES:
  * api: Search handles prefix longer than allowed UUIDs [GH-3138]
  * api: Search endpoint handles even UUID prefixes with hyphens [GH-3120]
+ * api: Don't merge empty update stanza from job into task groups [GH-3139]
  * cli: All status commands handle even UUID prefixes with hyphens [GH-3122]
  * cli: Fix autocompletion of paths that include directories on zsh [GH-3129] 
  * cli: Status command honors exact job match even when it is the prefix of

api/jobs.go

@@ -403,6 +403,43 @@ func (u *UpdateStrategy) Canonicalize() {
 	}
 }
 
+// Empty returns whether the UpdateStrategy is empty or has user defined values.
+func (u *UpdateStrategy) Empty() bool {
+	if u == nil {
+		return true
+	}
+
+	if u.Stagger != nil && *u.Stagger != 0 {
+		return false
+	}
+
+	if u.MaxParallel != nil && *u.MaxParallel != 0 {
+		return false
+	}
+
+	if u.HealthCheck != nil && *u.HealthCheck != "" {
+		return false
+	}
+
+	if u.MinHealthyTime != nil && *u.MinHealthyTime != 0 {
+		return false
+	}
+
+	if u.HealthyDeadline != nil && *u.HealthyDeadline != 0 {
+		return false
+	}
+
+	if u.AutoRevert != nil && *u.AutoRevert {
+		return false
+	}
+
+	if u.Canary != nil && *u.Canary != 0 {
+		return false
+	}
+
+	return true
+}
+
 // PeriodicConfig is for serializing periodic config for a job.
 type PeriodicConfig struct {
 	Enabled         *bool

api/tasks.go

@@ -188,8 +188,8 @@ func (g *TaskGroup) Canonicalize(job *Job) {
 		jc := job.Update.Copy()
 		jc.Merge(g.Update)
 		g.Update = jc
-	} else if ju {
-		// Inherit the jobs
+	} else if ju && !job.Update.Empty() {
+		// Inherit the jobs as long as it is non-empty.
 		jc := job.Update.Copy()
 		g.Update = jc
 	}

api/tasks_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/nomad/helper"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestTaskGroup_NewTaskGroup(t *testing.T) {
@@ -243,3 +244,25 @@ func TestTask_Artifact(t *testing.T) {
 		t.Errorf("expected local/foo.txt but found %q", *a.RelativeDest)
 	}
 }
+
+// Ensures no regression on https://github.com/hashicorp/nomad/issues/3132
+func TestTaskGroup_Canonicalize_Update(t *testing.T) {
+	job := &Job{
+		ID: helper.StringToPtr("test"),
+		Update: &UpdateStrategy{
+			AutoRevert:      helper.BoolToPtr(false),
+			Canary:          helper.IntToPtr(0),
+			HealthCheck:     helper.StringToPtr(""),
+			HealthyDeadline: helper.TimeToPtr(0),
+			MaxParallel:     helper.IntToPtr(0),
+			MinHealthyTime:  helper.TimeToPtr(0),
+			Stagger:         helper.TimeToPtr(0),
+		},
+	}
+	job.Canonicalize()
+	tg := &TaskGroup{
+		Name: helper.StringToPtr("foo"),
+	}
+	tg.Canonicalize(job)
+	assert.Nil(t, tg.Update)
+}