fix(doctor): surface container restart history, not just crash-loop-now (#1028 item 4)

internal/doctor/doctor.go

@@ -78,6 +78,18 @@ func Worst(results []Result) Status {
 const (
 	pendingGrace     = 5 * time.Minute // a pod Pending longer than this is flagged
 	httpProbeTimeout = 8 * time.Second
+
+	// restartWarnThreshold is the container RestartCount at/above which the
+	// doctor surfaces a pod's restart *history* as a ⚠. checkPods reads only the
+	// current waiting reason, so a container that crashed several times and then
+	// recovered — or a job that flapped before Succeeding — leaves no live trace
+	// there and reads as OK (the gap backend#1028 flagged). We pick 3, not 1: a
+	// single restart is routine (a node drain, an image warm-up, a dependency not
+	// ready on first boot, a one-off liveness-probe miss), so 1–2 would be noisy.
+	// 3+ distinct restarts is a genuine flap worth a log look, while staying a
+	// history hint that never escalates past ⚠ or overrides checkPods'
+	// crash-loop-now failure.
+	restartWarnThreshold = 3
 )
 
 // Options configures a diagnosis run. The zero value is usable: Namespace
@@ -109,6 +121,7 @@ func Run(ctx context.Context, cs kubernetes.Interface, opts Options) []Result {
 	return []Result{
 		checkReachable(release, relErr, ns),
 		checkPods(ctx, cs, ns),
+		checkRestartHistory(ctx, cs, ns),
 		checkPVC(ctx, cs, ns),
 		checkNodeFit(ctx, cs, jmEnv),
 		checkImagePull(ctx, cs, ns, release),
@@ -217,6 +230,49 @@ func podCrashLooping(p corev1.Pod) bool {
 	return false
 }
 
+// checkRestartHistory surfaces containers that have restarted repeatedly even
+// though they are not crash-looping right now — the restart-*history* signal
+// backend#1028 asked for. checkPods reads only the current waiting reason, so a
+// container that crashed several times and then came up healthy (or a job that
+// flapped before Succeeding) reads as OK there, hiding a real problem. This is
+// deliberately an ADDITIONAL check and never touches podCrashLooping: it caps
+// at ⚠ (history, not liveness) and both init AND app container statuses are
+// scanned, since an init container that keeps dying blocks startup just as much.
+func checkRestartHistory(ctx context.Context, cs kubernetes.Interface, ns string) Result {
+	const name = "Restart history"
+	pods, err := cs.CoreV1().Pods(ns).List(ctx, metav1.ListOptions{})
+	if err != nil {
+		return Result{
+			Name:   name,
+			Status: StatusWarn,
+			Detail: "could not list pods: " + err.Error(),
+			Remedy: "Ensure your kubeconfig user can list pods in " + ns + ".",
+		}
+	}
+
+	var flapped []string
+	for _, p := range pods.Items {
+		for _, group := range [][]corev1.ContainerStatus{p.Status.InitContainerStatuses, p.Status.ContainerStatuses} {
+			for _, c := range group {
+				if c.RestartCount >= restartWarnThreshold {
+					flapped = append(flapped, fmt.Sprintf("pod %s container %s restarted %d times", p.Name, c.Name, c.RestartCount))
+				}
+			}
+		}
+	}
+	sort.Strings(flapped)
+
+	if len(flapped) > 0 {
+		return Result{
+			Name:   name,
+			Status: StatusWarn,
+			Detail: fmt.Sprintf("restarted ≥%d times — check logs: %v", restartWarnThreshold, flapped),
+			Remedy: "A container that restarted repeatedly may be flapping even if it's up now. Check its logs: kubectl logs -n " + ns + " <pod> -c <container> --previous",
+		}
+	}
+	return Result{Name: name, Status: StatusOK, Detail: fmt.Sprintf("%d pod(s), none restarted ≥%d times", len(pods.Items), restartWarnThreshold)}
+}
+
 // checkPVC reuses cluster.DiscoverSharedPVC — which already verifies the
 // shared-data PVC exists and is Bound, with actionable errors.
 func checkPVC(ctx context.Context, cs kubernetes.Interface, ns string) Result {

internal/doctor/doctor_test.go

@@ -111,6 +111,23 @@ func recoveredPod(name string, restarts int32) *corev1.Pod {
 	}
 }
 
+// initRestartPod has an init container that restarted repeatedly but is not
+// crash-looping now (it terminated Completed). Exercises the
+// InitContainerStatuses arm of the restart-history scan.
+func initRestartPod(name string, restarts int32) *corev1.Pod {
+	return &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns},
+		Status: corev1.PodStatus{
+			Phase: corev1.PodRunning,
+			InitContainerStatuses: []corev1.ContainerStatus{{
+				Name:         "init",
+				RestartCount: restarts,
+				State:        corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Completed"}},
+			}},
+		},
+	}
+}
+
 func pendingPod(name string, age time.Duration) *corev1.Pod {
 	return &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
@@ -183,6 +200,41 @@ func TestCheckPods(t *testing.T) {
 	}
 }
 
+// checkRestartHistory is the backend#1028 restart-*history* signal: it must warn
+// on a container whose RestartCount crossed the threshold even though it is not
+// crash-looping now (checkPods reads only the current waiting reason and would
+// call these OK). It scans both init and app container statuses and caps at ⚠.
+func TestCheckRestartHistory(t *testing.T) {
+	tests := []struct {
+		name string
+		pod  *corev1.Pod
+		want Status
+	}{
+		{"no restarts", runningPod("ok"), StatusOK},
+		{"below threshold", recoveredPod("blip", restartWarnThreshold-1), StatusOK},
+		{"at threshold", recoveredPod("flap", restartWarnThreshold), StatusWarn},
+		{"above threshold", recoveredPod("flap", restartWarnThreshold+2), StatusWarn},
+		{"init container restarts", initRestartPod("initflap", restartWarnThreshold), StatusWarn},
+		// A recovered pod that flapped is OK to checkPods but ⚠ here — that's the gap.
+		{"crash-loop-now stays OK here (not this check's job)", crashPod("bad"), StatusOK},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			cs := fake.NewClientset(tc.pod)
+			r := checkRestartHistory(bg(), cs, ns)
+			if r.Status != tc.want {
+				t.Fatalf("checkRestartHistory = %v (%q), want %v", r.Status, r.Detail, tc.want)
+			}
+			// A warn must name the offending pod and container so the operator
+			// knows where to look.
+			if tc.want == StatusWarn &&
+				(!strings.Contains(r.Detail, tc.pod.Name) || !strings.Contains(r.Detail, "restarted")) {
+				t.Fatalf("warn detail %q should name pod %q and its restart count", r.Detail, tc.pod.Name)
+			}
+		})
+	}
+}
+
 func TestCheckPVC(t *testing.T) {
 	if r := checkPVC(bg(), fake.NewClientset(boundPVC()), ns); r.Status != StatusOK {
 		t.Fatalf("bound PVC => %v, want ok", r.Status)
@@ -337,8 +389,8 @@ func TestRun_HealthyCluster(t *testing.T) {
 		HTTPProbe: func(context.Context, string) error { return nil },
 	})
 
-	if len(results) != 8 {
-		t.Fatalf("want 8 checks, got %d", len(results))
+	if len(results) != 9 {
+		t.Fatalf("want 9 checks, got %d", len(results))
 	}
 	if w := Worst(results); w != StatusOK {
 		for _, r := range results {