Merge pull request #2311 from hashicorp/f-no-interval

Validate the interval within a restart policy

nomad/structs/structs.go

@@ -1744,6 +1744,10 @@ const (
 	// RestartPolicyModeFail causes a job to fail if the specified number of
 	// attempts are reached within an interval.
 	RestartPolicyModeFail = "fail"
+
+	// RestartPolicyMinInterval is the minimum interval that is accepted for a
+	// restart policy.
+	RestartPolicyMinInterval = 5 * time.Second
 )
 
 // RestartPolicy configures how Tasks are restarted when they crash or fail.
@@ -1773,24 +1777,26 @@ func (r *RestartPolicy) Copy() *RestartPolicy {
 }
 
 func (r *RestartPolicy) Validate() error {
+	var mErr multierror.Error
 	switch r.Mode {
 	case RestartPolicyModeDelay, RestartPolicyModeFail:
 	default:
-		return fmt.Errorf("Unsupported restart mode: %q", r.Mode)
+		multierror.Append(&mErr, fmt.Errorf("Unsupported restart mode: %q", r.Mode))
 	}
 
 	// Check for ambiguous/confusing settings
 	if r.Attempts == 0 && r.Mode != RestartPolicyModeFail {
-		return fmt.Errorf("Restart policy %q with %d attempts is ambiguous", r.Mode, r.Attempts)
+		multierror.Append(&mErr, fmt.Errorf("Restart policy %q with %d attempts is ambiguous", r.Mode, r.Attempts))
 	}
 
-	if r.Interval == 0 {
-		return nil
+	if r.Interval.Nanoseconds() < RestartPolicyMinInterval.Nanoseconds() {
+		multierror.Append(&mErr, fmt.Errorf("Interval can not be less than %v (got %v)", RestartPolicyMinInterval, r.Interval))
 	}
 	if time.Duration(r.Attempts)*r.Delay > r.Interval {
-		return fmt.Errorf("Nomad can't restart the TaskGroup %v times in an interval of %v with a delay of %v", r.Attempts, r.Interval, r.Delay)
+		multierror.Append(&mErr,
+			fmt.Errorf("Nomad can't restart the TaskGroup %v times in an interval of %v with a delay of %v", r.Attempts, r.Interval, r.Delay))
 	}
-	return nil
+	return mErr.ErrorOrNil()
 }
 
 func NewRestartPolicy(jobType string) *RestartPolicy {

nomad/structs/structs_test.go

@@ -1254,6 +1254,7 @@ func TestRestartPolicy_Validate(t *testing.T) {
 	p := &RestartPolicy{
 		Mode:     RestartPolicyModeFail,
 		Attempts: 0,
+		Interval: 5 * time.Second,
 	}
 	if err := p.Validate(); err != nil {
 		t.Fatalf("err: %v", err)
@@ -1263,6 +1264,7 @@ func TestRestartPolicy_Validate(t *testing.T) {
 	p = &RestartPolicy{
 		Mode:     RestartPolicyModeDelay,
 		Attempts: 0,
+		Interval: 5 * time.Second,
 	}
 	if err := p.Validate(); err == nil || !strings.Contains(err.Error(), "ambiguous") {
 		t.Fatalf("expect ambiguity error, got: %v", err)
@@ -1272,6 +1274,7 @@ func TestRestartPolicy_Validate(t *testing.T) {
 	p = &RestartPolicy{
 		Mode:     "nope",
 		Attempts: 1,
+		Interval: 5 * time.Second,
 	}
 	if err := p.Validate(); err == nil || !strings.Contains(err.Error(), "mode") {
 		t.Fatalf("expect mode error, got: %v", err)
@@ -1282,11 +1285,22 @@ func TestRestartPolicy_Validate(t *testing.T) {
 		Mode:     RestartPolicyModeDelay,
 		Attempts: 3,
 		Delay:    5 * time.Second,
-		Interval: time.Second,
+		Interval: 5 * time.Second,
 	}
 	if err := p.Validate(); err == nil || !strings.Contains(err.Error(), "can't restart") {
 		t.Fatalf("expect restart interval error, got: %v", err)
 	}
+
+	// Fails when interval is to small
+	p = &RestartPolicy{
+		Mode:     RestartPolicyModeDelay,
+		Attempts: 3,
+		Delay:    5 * time.Second,
+		Interval: 2 * time.Second,
+	}
+	if err := p.Validate(); err == nil || !strings.Contains(err.Error(), "Interval can not be less than") {
+		t.Fatalf("expect interval too small error, got: %v", err)
+	}
 }
 
 func TestAllocation_Index(t *testing.T) {