Unverified Commit e8dae69e authored by Preetha Appan's avatar Preetha Appan
Browse files

Adds additional validation for ambigous settings (having both unlimited and attempts set)

parent cc02e101
Showing with 30 additions and 6 deletions
+30 -6
......@@ -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
......
......@@ -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,
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment