diff --git a/client/driver/rkt.go b/client/driver/rkt.go index abc1f9488..764e17fb1 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -24,8 +24,8 @@ import ( ) var ( - reRktVersion = regexp.MustCompile(`rkt version (\d[.\d]+)`) - reAppcVersion = regexp.MustCompile(`appc version (\d[.\d]+)`) + reRktVersion = regexp.MustCompile(`rkt [vV]ersion[:]? (\d[.\d]+)`) + reAppcVersion = regexp.MustCompile(`appc [vV]ersion[:]? (\d[.\d]+)`) ) const ( diff --git a/client/restarts.go b/client/restarts.go index 3f1b41acd..79aff54f3 100644 --- a/client/restarts.go +++ b/client/restarts.go @@ -26,6 +26,11 @@ type RestartTracker struct { } func (r *RestartTracker) NextRestart(exitCode int) (bool, time.Duration) { + // Hot path if no attempts are expected + if r.policy.Attempts == 0 { + return false, 0 + } + // Check if we have entered a new interval. end := r.startTime.Add(r.policy.Interval) now := time.Now() diff --git a/client/restarts_test.go b/client/restarts_test.go index 4c84193c5..719cbd088 100644 --- a/client/restarts_test.go +++ b/client/restarts_test.go @@ -77,5 +77,14 @@ func TestClient_RestartTracker_NoRestartOnSuccess(t *testing.T) { if shouldRestart, _ := rt.NextRestart(0); shouldRestart { t.Fatalf("NextRestart() returned %v, expected: %v", shouldRestart, false) } - +} + +func TestClient_RestartTracker_ZeroAttempts(t *testing.T) { + t.Parallel() + p := testPolicy(true, structs.RestartPolicyModeFail) + p.Attempts = 0 + rt := newRestartTracker(p) + if actual, when := rt.NextRestart(1); actual { + t.Fatalf("expect no restart, got restart/delay: %v", when) + } } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 477562d01..9742b47fc 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1199,6 +1199,11 @@ func (r *RestartPolicy) Validate() error { return 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) + } + if r.Interval == 0 { return nil } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 25b543118..58e288c57 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -654,3 +654,43 @@ func TestPeriodicConfig_NextCron(t *testing.T) { } } } + +func TestRestartPolicy_Validate(t *testing.T) { + // Policy with acceptable restart options passes + p := &RestartPolicy{ + Mode: RestartPolicyModeFail, + Attempts: 0, + } + if err := p.Validate(); err != nil { + t.Fatalf("err: %v", err) + } + + // Policy with ambiguous restart options fails + p = &RestartPolicy{ + Mode: RestartPolicyModeDelay, + Attempts: 0, + } + if err := p.Validate(); err == nil || !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("expect ambiguity error, got: %v", err) + } + + // Bad policy mode fails + p = &RestartPolicy{ + Mode: "nope", + Attempts: 1, + } + if err := p.Validate(); err == nil || !strings.Contains(err.Error(), "mode") { + t.Fatalf("expect mode error, got: %v", err) + } + + // Fails when attempts*delay does not fit inside interval + p = &RestartPolicy{ + Mode: RestartPolicyModeDelay, + Attempts: 3, + Delay: 5 * time.Second, + Interval: time.Second, + } + if err := p.Validate(); err == nil || !strings.Contains(err.Error(), "can't restart") { + t.Fatalf("expect restart interval error, got: %v", err) + } +}