From 4be8d7c04938df63b691de2aa27400f07e62031f Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Thu, 1 Jun 2023 19:01:32 -0400 Subject: [PATCH] core: fix kill_timeout validation when progress_deadline is 0 (#17342) --- .changelog/17342.txt | 3 +++ nomad/structs/structs.go | 5 +++-- nomad/structs/structs_test.go | 39 ++++++++++++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 .changelog/17342.txt diff --git a/.changelog/17342.txt b/.changelog/17342.txt new file mode 100644 index 000000000..cac44881d --- /dev/null +++ b/.changelog/17342.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: fixed a bug that caused job validation to fail when a task with `kill_timeout` was placed inside a group with `update.progress_deadline` set to 0 +``` diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 54aeefc58..7bebb0247 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -6846,12 +6846,13 @@ func (tg *TaskGroup) Validate(j *Job) error { // Validate the group's Update Strategy does not conflict with the Task's kill_timeout for service type jobs if isTypeService && tg.Update != nil { - if task.KillTimeout > tg.Update.ProgressDeadline { + // progress_deadline = 0 has a special meaning so it should not be + // validated against the task's kill_timeout. + if tg.Update.ProgressDeadline > 0 && task.KillTimeout > tg.Update.ProgressDeadline { mErr.Errors = append(mErr.Errors, fmt.Errorf("Task %s has a kill timout (%s) longer than the group's progress deadline (%s)", task.Name, task.KillTimeout.String(), tg.Update.ProgressDeadline.String())) } } - } return mErr.ErrorOrNil() diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index a1e02d2c1..f703cf84f 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -1426,6 +1426,39 @@ func TestTaskGroup_Validate(t *testing.T) { }, jobType: JobTypeService, }, + { + name: "progress_deadline 0 does not conflict with kill_timeout", + tg: &TaskGroup{ + Name: "web", + Count: 1, + Tasks: []*Task{ + { + Name: "web", + Driver: "mock_driver", + Leader: true, + KillTimeout: DefaultUpdateStrategy.ProgressDeadline + 25*time.Minute, + Resources: DefaultResources(), + LogConfig: DefaultLogConfig(), + }, + }, + Update: &UpdateStrategy{ + Stagger: 30 * time.Second, + MaxParallel: 1, + HealthCheck: UpdateStrategyHealthCheck_Checks, + MinHealthyTime: 10 * time.Second, + HealthyDeadline: 5 * time.Minute, + ProgressDeadline: 0, + AutoRevert: false, + AutoPromote: false, + Canary: 0, + }, + RestartPolicy: NewRestartPolicy(JobTypeService), + ReschedulePolicy: NewReschedulePolicy(JobTypeService), + Migrate: DefaultMigrateStrategy(), + EphemeralDisk: DefaultEphemeralDisk(), + }, + jobType: JobTypeService, + }, { name: "service and task using different provider", tg: &TaskGroup{ @@ -1461,7 +1494,11 @@ func TestTaskGroup_Validate(t *testing.T) { j.Type = tc.jobType err := tc.tg.Validate(j) - requireErrors(t, err, tc.expErr...) + if len(tc.expErr) > 0 { + requireErrors(t, err, tc.expErr...) + } else { + must.NoError(t, err) + } }) } }