diff --git a/command/agent/consul/script.go b/command/agent/consul/script.go index 3e96df00d..26228b42b 100644 --- a/command/agent/consul/script.go +++ b/command/agent/consul/script.go @@ -96,8 +96,14 @@ func (s *scriptCheck) run() *scriptHandle { // check removed during execution; exit return case context.DeadlineExceeded: - // Log deadline exceeded every time, but flip last check to false - s.lastCheckOk = false + // If no error was returned, set one to make sure the task goes critical + if err == nil { + err = context.DeadlineExceeded + } + + // Log deadline exceeded every time as it's a + // distinct issue from checks returning + // failures s.logger.Printf("[WARN] consul.checks: check %q for task %q alloc %q timed out (%s)", s.check.Name, s.taskName, s.allocID, s.check.Timeout) } diff --git a/command/agent/consul/script_test.go b/command/agent/consul/script_test.go index 76192692a..1fa565d16 100644 --- a/command/agent/consul/script_test.go +++ b/command/agent/consul/script_test.go @@ -140,6 +140,43 @@ func TestConsulScript_Exec_Timeout(t *testing.T) { } } +// sleeperExec sleeps for 100ms but returns successfully to allow testing timeout conditions +type sleeperExec struct{} + +func (sleeperExec) Exec(context.Context, string, []string) ([]byte, int, error) { + time.Sleep(100 * time.Millisecond) + return []byte{}, 0, nil +} + +// TestConsulScript_Exec_TimeoutCritical asserts a script will be killed when +// the timeout is reached and always set a critical status regardless of what +// Exec returns. +func TestConsulScript_Exec_TimeoutCritical(t *testing.T) { + t.Parallel() // run the slow tests in parallel + serviceCheck := structs.ServiceCheck{ + Name: "sleeper", + Interval: time.Hour, + Timeout: time.Nanosecond, + } + hb := newFakeHeartbeater() + check := newScriptCheck("allocid", "testtask", "checkid", &serviceCheck, sleeperExec{}, hb, testLogger(), nil) + handle := check.run() + defer handle.cancel() // just-in-case cleanup + + // Check for UpdateTTL call + select { + case update := <-hb.updates: + if update.status != api.HealthCritical { + t.Error("expected %q due to timeout but received %q", api.HealthCritical, update) + } + if update.output != context.DeadlineExceeded.Error() { + t.Errorf("expected output=%q but found: %q", context.DeadlineExceeded.Error(), update.output) + } + case <-time.After(3 * time.Second): + t.Fatalf("timed out waiting for script check to timeout") + } +} + // simpleExec is a fake ScriptExecutor that returns whatever is specified. type simpleExec struct { code int