Skip to content

Commit

Permalink
core: fix kill_timeout validation when progress_deadline is 0 (#17342)
Browse files Browse the repository at this point in the history
  • Loading branch information
lgfa29 committed Jun 1, 2023
1 parent 9ee68fc commit 0ede6ac
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
3 changes: 3 additions & 0 deletions .changelog/17342.txt
Original file line number Diff line number Diff line change
@@ -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
```
5 changes: 3 additions & 2 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
39 changes: 38 additions & 1 deletion nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}
})
}
}
Expand Down

0 comments on commit 0ede6ac

Please sign in to comment.