diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index 43d8b0f..9b669e9 100644 --- a/internal/doctor/doctor.go +++ b/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 + " -c --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 { diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index 7eaaa22..dc96a1f 100644 --- a/internal/doctor/doctor_test.go +++ b/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 {