From 698e9d4940aa87efb4a6678bca8765a6a792bfb9 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Thu, 18 Jul 2019 13:04:20 -0400 Subject: [PATCH 1/3] tasks_test assert merging behavior around Canonicalize --- api/tasks_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/api/tasks_test.go b/api/tasks_test.go index 3ee016073..0d5b6dc79 100644 --- 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 { From e3b34c35a80c35dfe04fc5b106209deb92e1e2d2 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Thu, 11 Jul 2019 17:01:32 -0400 Subject: [PATCH 2/3] jobs update stanza canonicalize and default AutoPromote --- api/jobs.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/api/jobs.go b/api/jobs.go index 4f73da10c..901b41e7f 100644 --- 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 } From a6817359d8c58f0662b9a1ac8742bdb57c607acc Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Wed, 17 Jul 2019 17:55:26 -0400 Subject: [PATCH 3/3] jobs_test AutoRevert and AutoPromote merged differently --- api/jobs_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/api/jobs_test.go b/api/jobs_test.go index 078f1f79e..d88cd93fb 100644 --- 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{