From 888d649a0f34927c5ae57c7d4598abb801aba149 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 6 Aug 2021 14:48:28 -0400 Subject: [PATCH 1/3] deployments: canary=0 is implicitly autorpomote In a multi-task-group job, treat 0 canary groups as auto-promote. This change fixes an edge case where Nomad requires a manual promotion, if the job had any group with canary=0 and rest of groups having auto_promote set. --- nomad/structs/structs.go | 13 ++++++++----- nomad/structs/structs_test.go | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 71793496c4c..51c1c408baf 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4302,19 +4302,22 @@ func (j *Job) Warnings() error { var mErr multierror.Error // Check the groups - ap := 0 + hasAutoPromote, allAutoPromote := false, true + 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) } - if tg.Update != nil && tg.Update.AutoPromote { - ap += 1 + + if u := tg.Update; u != nil { + hasAutoPromote = hasAutoPromote || u.AutoPromote + allAutoPromote = allAutoPromote && (u.Canary == 0 || u.AutoPromote) } } // Check AutoPromote, should be all or none - if ap > 0 && ap < len(j.TaskGroups) { + if hasAutoPromote && !allAutoPromote { err := fmt.Errorf("auto_promote must be true for all groups to enable automatic promotion") mErr.Errors = append(mErr.Errors, err) } @@ -8877,7 +8880,7 @@ func (d *Deployment) HasAutoPromote() bool { return false } for _, group := range d.TaskGroups { - if !group.AutoPromote { + if group.DesiredCanaries > 0 && !group.AutoPromote { return false } } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 5d19f8b01b6..2409f7021fb 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -200,6 +200,27 @@ func TestJob_Warnings(t *testing.T) { { Update: &UpdateStrategy{ AutoPromote: false, + Canary: 1, + }, + }, + }, + }, + }, + { + Name: "no error for mixed but implied AutoPromote", + Expected: []string{}, + Job: &Job{ + Type: JobTypeService, + TaskGroups: []*TaskGroup{ + { + Update: &UpdateStrategy{ + AutoPromote: true, + }, + }, + { + Update: &UpdateStrategy{ + AutoPromote: false, + Canary: 0, }, }, }, From 81b366323922a2640ffaadef829f3190f4d0a55d Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 10 Aug 2021 16:23:01 -0400 Subject: [PATCH 2/3] Update nomad/structs/structs.go Co-authored-by: Michael Schurter --- nomad/structs/structs.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 51c1c408baf..13f2125ee34 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4312,6 +4312,8 @@ func (j *Job) Warnings() error { if u := tg.Update; u != nil { hasAutoPromote = hasAutoPromote || u.AutoPromote + + // Having no canaries implies auto-promotion since there are no canaries to promote. allAutoPromote = allAutoPromote && (u.Canary == 0 || u.AutoPromote) } } From a54b464cf6580cdb54211edc700002fec6f58a83 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 10 Aug 2021 16:32:07 -0400 Subject: [PATCH 3/3] changelog --- .changelog/11013.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changelog/11013.txt diff --git a/.changelog/11013.txt b/.changelog/11013.txt new file mode 100644 index 00000000000..0dadfa14e9e --- /dev/null +++ b/.changelog/11013.txt @@ -0,0 +1,4 @@ + +```release-note:bug +deployments: Fixed a bug where multi-group deployments don't get auto-promoted when one group has no canaries. +```