diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index d789e6d0d..154e2b92c 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2897,6 +2897,15 @@ func (r *ReschedulePolicy) Validate() error { } var mErr multierror.Error // Check for ambiguous/confusing settings + if r.Attempts > 0 { + if r.Interval <= 0 { + multierror.Append(&mErr, fmt.Errorf("Interval must be a non zero value if Attempts > 0")) + } + if r.Unlimited { + multierror.Append(&mErr, fmt.Errorf("Reschedule Policy with Attempts = %v, Interval = %v, "+ + "and Unlimited = %v is ambiguous", r.Attempts, r.Interval, r.Unlimited)) + } + } delayPreCheck := true // Delay should be bigger than the default @@ -2914,11 +2923,11 @@ func (r *ReschedulePolicy) Validate() error { // Validate MaxDelay if not using linear delay progression if r.DelayFunction != "linear" { if r.MaxDelay.Nanoseconds() < ReschedulePolicyMinDelay.Nanoseconds() { - multierror.Append(&mErr, fmt.Errorf("Delay Ceiling cannot be less than %v (got %v)", ReschedulePolicyMinDelay, r.Delay)) + multierror.Append(&mErr, fmt.Errorf("Max Delay cannot be less than %v (got %v)", ReschedulePolicyMinDelay, r.Delay)) delayPreCheck = false } if r.MaxDelay < r.Delay { - multierror.Append(&mErr, fmt.Errorf("Delay Ceiling cannot be less than Delay %v (got %v)", r.Delay, r.MaxDelay)) + multierror.Append(&mErr, fmt.Errorf("Max Delay cannot be less than Delay %v (got %v)", r.Delay, r.MaxDelay)) delayPreCheck = false } @@ -5674,7 +5683,10 @@ func (a *Allocation) RescheduleEligible(reschedulePolicy *ReschedulePolicy, fail if !enabled { return false } - if (a.RescheduleTracker == nil || len(a.RescheduleTracker.Events) == 0) && attempts > 0 || reschedulePolicy.Unlimited { + if reschedulePolicy.Unlimited { + return true + } + if (a.RescheduleTracker == nil || len(a.RescheduleTracker.Events) == 0) && attempts > 0 { return true } attempted := 0 diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 9df3d9e4f..9cb6b8520 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -2157,7 +2157,7 @@ func TestReschedulePolicy_Validate(t *testing.T) { Delay: 15 * time.Second, MaxDelay: 5 * time.Second}, errors: []error{ - fmt.Errorf("Delay Ceiling cannot be less than Delay %v (got %v)", + fmt.Errorf("Max Delay cannot be less than Delay %v (got %v)", 15*time.Second, 5*time.Second), }, }, @@ -2226,7 +2226,7 @@ func TestReschedulePolicy_Validate(t *testing.T) { }, }, { - desc: "Valid Unlimited config", + desc: "Ambiguous Unlimited config", ReschedulePolicy: &ReschedulePolicy{ Attempts: 1, Unlimited: true, @@ -2234,6 +2234,10 @@ func TestReschedulePolicy_Validate(t *testing.T) { Delay: 5 * time.Minute, MaxDelay: 1 * time.Hour, }, + errors: []error{ + fmt.Errorf("Interval must be a non zero value if Attempts > 0"), + fmt.Errorf("Reschedule Policy with Attempts = %v, Interval = %v, and Unlimited = %v is ambiguous", 1, time.Duration(0), true), + }, }, { desc: "Invalid Unlimited config", @@ -2245,7 +2249,7 @@ func TestReschedulePolicy_Validate(t *testing.T) { }, errors: []error{ fmt.Errorf("Delay cannot be less than %v (got %v)", ReschedulePolicyMinDelay, 0*time.Second), - fmt.Errorf("Delay Ceiling cannot be less than %v (got %v)", ReschedulePolicyMinDelay, 0*time.Second), + fmt.Errorf("Max Delay cannot be less than %v (got %v)", ReschedulePolicyMinDelay, 0*time.Second), }, }, } @@ -2538,6 +2542,14 @@ func TestAllocation_ShouldReschedule(t *testing.T) { ReschedulePolicy: nil, ShouldReschedule: false, }, + { + Desc: "Reschedule with unlimited and attempts >0", + ClientStatus: AllocClientStatusFailed, + DesiredStatus: AllocDesiredStatusRun, + FailTime: fail, + ReschedulePolicy: &ReschedulePolicy{Attempts: 1, Unlimited: true}, + ShouldReschedule: true, + }, { Desc: "Reschedule when client status is complete", ClientStatus: AllocClientStatusComplete,