Always fail script checks when deadline exceeded

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
+				// If no error was returned, set one to make sure the task goes critical
-				s.lastCheckOk = false
+				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)
 			}

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