From 917302ca3f0c4e09f4618afbd53dd1518228cdac Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 01:45:19 +0300 Subject: [PATCH 1/8] fix(issues): keep one restart-loop issue across a crashloop cycle A container stuck restarting cycles through states that each read as a different problem on a single poll: CrashLoopBackOff, Running-not-ready, readiness and liveness Unhealthy events, Terminated exit 0, and Running+Ready between crashes. The pod row changed category (and so issue id) every cycle and vanished on the healthy-looking ticks, and the owning Deployment's workload_degraded row came and went with it. In production one looping gateway produced thousands of open/resolve generations. The detector now classifies an active restart loop from restart history: 3+ restarts and a termination (current or last, any exit code, not OOM) within the past 30 minutes, for containers the kubelet restarts by design (restartPolicy Always containers and native sidecars). While it holds, the pod keeps one critical crashloop row through every tick, folding the per-tick faces of the loop into it. Probe failures, last exit and the container are attached as RestartLoop evidence and a restart_cause fact; the diagnosis depends only on container and exit code so it stays stable across polls and agrees across replicas. Pod health levels are unchanged. More specific problems keep their own rows: invalid probe targets, image pulls, create errors, active OOM (now including sidecars), init stalls. Also: a started native sidecar is no longer reported as a stalled init container, and ReplicaFailure is folded only into scheduling-source admission rejections, never into a runtime symptom on an existing pod. --- internal/issues/dedupe.go | 24 +- internal/issues/dedupe_test.go | 22 ++ internal/issues/diagnostic_context.go | 35 +++ internal/issues/grouping.go | 1 + internal/issues/normalize.go | 1 + .../issues/restart_loop_integration_test.go | 252 +++++++++++++++++ internal/k8s/detect.go | 78 +++++- .../k8s/detect_crashloop_severity_test.go | 4 +- internal/k8s/detect_test.go | 17 +- internal/k8s/restart_loop.go | 257 ++++++++++++++++++ internal/k8s/restart_loop_test.go | 218 +++++++++++++++ .../src/components/issues/IssuesView.tsx | 7 +- .../k8s-ui/src/components/issues/types.ts | 19 ++ pkg/health/reasons.go | 7 + pkg/issuesapi/types.go | 67 +++-- 15 files changed, 977 insertions(+), 32 deletions(-) create mode 100644 internal/issues/restart_loop_integration_test.go create mode 100644 internal/k8s/restart_loop.go create mode 100644 internal/k8s/restart_loop_test.go diff --git a/internal/issues/dedupe.go b/internal/issues/dedupe.go index c8bafe6f12..d5abf457ba 100644 --- a/internal/issues/dedupe.go +++ b/internal/issues/dedupe.go @@ -75,6 +75,17 @@ var childCategories = map[issuesapi.Category]bool{ issuesapi.CategoryPVCPending: true, } +// podCreationChildCategories are the child symptoms that explain a +// ReplicaFailure parent: the controller could not create pods at all, so only a +// rejection of pod creation names the cause. A pod runtime symptom such as a +// crashloop on an existing pod does not, and must not fold it. +var podCreationChildCategories = map[issuesapi.Category]bool{ + issuesapi.CategoryQuotaExceeded: true, + issuesapi.CategoryAdmissionWebhookBlocking: true, + issuesapi.CategoryPodSecurityViolation: true, + issuesapi.CategoryRBACForbidden: true, +} + // parentRollupCategories are the workload-level summaries that should be // suppressed when a more-specific child symptom exists for the same subject. // @@ -139,12 +150,19 @@ func dedupeRepeatedCronJobFailureOverChild(in []Issue) []Issue { func dedupeWorkloadDegradedOverChild(in []Issue) []Issue { // Per subject, the worst severity among its specific child-symptom rows. maxChildSev := map[string]int{} + maxCreationChildSev := map[string]int{} for _, i := range in { if childCategories[i.Category] { k := subjectKeyOf(subjectRef(i)) if r := SeverityRank(i.Severity); r > maxChildSev[k] { maxChildSev[k] = r } + // Only the scheduling source's admission rejections are about pod + // creation; the same categories from a running pod (an RBAC + // denial at runtime) are not. + if r := SeverityRank(i.Severity); i.Source == SourceScheduling && podCreationChildCategories[i.Category] && r > maxCreationChildSev[k] { + maxCreationChildSev[k] = r + } } } if len(maxChildSev) == 0 { @@ -164,7 +182,11 @@ func dedupeWorkloadDegradedOverChild(in []Issue) []Issue { // Suppress only when a child at least as severe exists — never // downgrade a critical rollup to a warning child. k := subjectKeyOf(subjectRef(i)) - if r, ok := maxChildSev[k]; ok && r >= SeverityRank(i.Severity) { + sev := maxChildSev + if i.Reason == "ReplicaFailure" { + sev = maxCreationChildSev + } + if r, ok := sev[k]; ok && r >= SeverityRank(i.Severity) { if i.IssueTiming != "" { if prev, seen := suppressedIssueTiming[k]; seen && prev != i.IssueTiming { suppressedIssueTiming[k] = "" diff --git a/internal/issues/dedupe_test.go b/internal/issues/dedupe_test.go index 35398300c8..3358a8bd8a 100644 --- a/internal/issues/dedupe_test.go +++ b/internal/issues/dedupe_test.go @@ -104,6 +104,28 @@ func TestDedupeWorkloadDegradedOverChild_Phase0(t *testing.T) { } }) + t.Run("ReplicaFailure is not folded into a crashloop on an existing pod", func(t *testing.T) { + rollout := Issue{Source: SourceProblem, Group: "apps", Kind: "Deployment", Namespace: "ns", Name: "web", + Category: issuesapi.CategoryRolloutStalled, Severity: SeverityCritical, Reason: "ReplicaFailure"} + crash := Issue{Source: SourceProblem, Kind: "Pod", Namespace: "ns", Name: "web-abc", + Owner: dep, Category: issuesapi.CategoryCrashLoop, Severity: SeverityCritical} + out := dedupeWorkloadDegradedOverChild([]Issue{rollout, crash}) + if !hasCategory(out, issuesapi.CategoryRolloutStalled) { + t.Fatalf("pod creation failure is independent of a crashlooping pod and must survive, got %+v", out) + } + }) + + t.Run("ReplicaFailure is not folded into a runtime RBAC denial on a running pod", func(t *testing.T) { + rollout := Issue{Source: SourceProblem, Group: "apps", Kind: "Deployment", Namespace: "ns", Name: "web", + Category: issuesapi.CategoryRolloutStalled, Severity: SeverityCritical, Reason: "ReplicaFailure"} + runtimeDenial := Issue{Source: SourceProblem, Kind: "Pod", Namespace: "ns", Name: "web-old", + Owner: dep, Category: issuesapi.CategoryRBACForbidden, Severity: SeverityCritical} + out := dedupeWorkloadDegradedOverChild([]Issue{rollout, runtimeDenial}) + if !hasCategory(out, issuesapi.CategoryRolloutStalled) { + t.Fatalf("a running pod's RBAC denial does not explain a pod creation failure, got %+v", out) + } + }) + t.Run("cronjob_failed is not a rollup and survives alongside an unrelated job_failed", func(t *testing.T) { cron := Issue{Source: SourceProblem, Group: "batch", Kind: "CronJob", Namespace: "ns", Name: "nightly", Category: issuesapi.CategoryCronJobFailed, Severity: SeverityWarning, Reason: "stale"} diff --git a/internal/issues/diagnostic_context.go b/internal/issues/diagnostic_context.go index 0686dac02a..0485b28d21 100644 --- a/internal/issues/diagnostic_context.go +++ b/internal/issues/diagnostic_context.go @@ -4,6 +4,7 @@ import ( "fmt" "sort" "strings" + "time" "github.com/skyhook-io/radar/internal/k8s" "github.com/skyhook-io/radar/pkg/issuesapi" @@ -973,6 +974,9 @@ func isBlockedInitContainer(i Issue) bool { } func restartCauseFact(i Issue) (issuesapi.DiagnosticFact, bool) { + if l := i.RestartLoop; l != nil { + return issuesapi.DiagnosticFact{Type: factRestartCause, Message: restartLoopEvidenceMessage(l)}, true + } if i.RestartCount <= 0 && i.LastTerminatedReason == "" { return issuesapi.DiagnosticFact{}, false } @@ -992,6 +996,37 @@ func restartCauseFact(i Issue) (issuesapi.DiagnosticFact, bool) { }, true } +// restartLoopEvidenceMessage states what was observed for a looping container. +// Probe failures are listed beside the restarts, never as their cause. +func restartLoopEvidenceMessage(l *issuesapi.RestartLoop) string { + parts := []string{ + fmt.Sprintf("container=%s", l.Container), + fmt.Sprintf("restartCount=%d", l.RestartCount), + } + last := fmt.Sprintf("lastExitCode=%d", l.LastExitCode) + if l.LastReason != "" { + last += fmt.Sprintf(" (%s)", l.LastReason) + } + if !l.LastFinishedAt.IsZero() { + last += " at " + l.LastFinishedAt.UTC().Format(time.RFC3339) + } + parts = append(parts, last) + for _, p := range []struct { + name string + pf *issuesapi.ProbeFailure + }{{"liveness", l.LivenessProbeFailure}, {"readiness", l.ReadinessProbeFailure}} { + if p.pf == nil { + continue + } + obs := fmt.Sprintf("%s probe failure last seen %s", p.name, p.pf.LastSeen.UTC().Format(time.RFC3339)) + if p.pf.Message != "" { + obs += fmt.Sprintf(" (%q)", p.pf.Message) + } + parts = append(parts, obs) + } + return "Restart loop evidence: " + strings.Join(parts, ", ") + "." +} + func diagnosticMessage(i Issue) string { if i.Message != "" { return i.Message diff --git a/internal/issues/grouping.go b/internal/issues/grouping.go index b1c50e416a..011bf0488e 100644 --- a/internal/issues/grouping.go +++ b/internal/issues/grouping.go @@ -157,6 +157,7 @@ func foldGroup(members []Issue) Issue { Fingerprint: rep.Fingerprint, RestartCount: rep.RestartCount, LastTerminatedReason: rep.LastTerminatedReason, + RestartLoop: rep.RestartLoop, FirstSeen: rep.FirstSeen, OnsetUnknown: rep.OnsetUnknown, ResourceCreatedAt: rep.ResourceCreatedAt, diff --git a/internal/issues/normalize.go b/internal/issues/normalize.go index 7bb64969e7..86b43e5448 100644 --- a/internal/issues/normalize.go +++ b/internal/issues/normalize.go @@ -180,6 +180,7 @@ func fromProblem(p k8s.Detection, now time.Time, source Source) Issue { Count: 1, RestartCount: p.RestartCount, LastTerminatedReason: p.LastTerminatedReason, + RestartLoop: p.RestartLoop, IssueTiming: issueTiming, IssueTimingBasis: issueTimingBasis, } diff --git a/internal/issues/restart_loop_integration_test.go b/internal/issues/restart_loop_integration_test.go new file mode 100644 index 0000000000..350d466214 --- /dev/null +++ b/internal/issues/restart_loop_integration_test.go @@ -0,0 +1,252 @@ +package issues + +import ( + "strings" + "testing" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + + "github.com/skyhook-io/radar/internal/k8s" + "github.com/skyhook-io/radar/pkg/issuesapi" +) + +// restartLoopTick is one poll of a Deployment whose single pod is in a restart +// loop: the pod's container state, the Deployment's availability, and the +// kubelet events visible at that moment. +type restartLoopTick struct { + name string + status corev1.ContainerStatus + ready bool + events []*corev1.Event +} + +// composeRestartLoopTick runs one tick through the real detector and composer +// and returns the grouped issues plus the raw detections. +func composeRestartLoopTick(t *testing.T, now time.Time, tick restartLoopTick) ([]Issue, []k8s.Detection) { + t.Helper() + k8s.ResetTestState() + controller := true + replicas := int32(1) + available, unavailable := int32(0), int32(1) + podReady := corev1.ConditionFalse + if tick.ready { + available, unavailable = 1, 0 + podReady = corev1.ConditionTrue + } + created := metav1.NewTime(now.Add(-48 * time.Hour)) + objs := []runtime.Object{ + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "gateway", Namespace: "knative", CreationTimestamp: created}, + Spec: appsv1.DeploymentSpec{Replicas: &replicas}, + Status: appsv1.DeploymentStatus{ + Replicas: 1, + UpdatedReplicas: 1, + ReadyReplicas: available, + AvailableReplicas: available, + UnavailableReplicas: unavailable, + }, + }, + &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-rs", + Namespace: "knative", + CreationTimestamp: created, + OwnerReferences: []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: "Deployment", Name: "gateway", Controller: &controller}}, + }}, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-abc", + Namespace: "knative", + CreationTimestamp: created, + OwnerReferences: []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: "ReplicaSet", Name: "gateway-rs", Controller: &controller}}, + }, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "gateway", + LivenessProbe: &corev1.Probe{}, + ReadinessProbe: &corev1.Probe{}, + }}}, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: podReady, LastTransitionTime: metav1.NewTime(now.Add(-6 * time.Minute))}, + {Type: corev1.ContainersReady, Status: podReady, LastTransitionTime: metav1.NewTime(now.Add(-6 * time.Minute))}, + }, + ContainerStatuses: []corev1.ContainerStatus{tick.status}, + }, + }, + } + for _, e := range tick.events { + objs = append(objs, e) + } + if err := k8s.InitTestResourceCache(fake.NewClientset(objs...)); err != nil { + t.Fatalf("%s: InitTestResourceCache: %v", tick.name, err) + } + provider := &CacheProvider{cache: k8s.GetResourceCache()} + var detections []k8s.Detection + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + detections = provider.DetectProblems([]string{"knative"}) + if hasDetection(detections, "Pod", "gateway-abc") || tick.ready { + break + } + time.Sleep(20 * time.Millisecond) + } + return Compose(provider, Filters{Namespaces: []string{"knative"}, Grouped: true, Limit: NoLimit}), detections +} + +func hasDetection(ds []k8s.Detection, kind, name string) bool { + for _, d := range ds { + if d.Kind == kind && d.Name == name { + return true + } + } + return false +} + +func probeEvent(name, probe string, at time.Time) *corev1.Event { + return &corev1.Event{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "knative"}, + InvolvedObject: corev1.ObjectReference{ + Kind: "Pod", Namespace: "knative", Name: "gateway-abc", + FieldPath: "spec.containers{gateway}", + }, + Type: corev1.EventTypeWarning, + Reason: "Unhealthy", + Message: probe + " probe failed: HTTP probe failed with statuscode: 503", + LastTimestamp: metav1.NewTime(at), + } +} + +// TestCompose_RestartLoopKeepsOneIssueAcrossTheCycle walks a liveness-driven +// restart loop (graceful exit 0, the kourier-gateway pattern seen in +// production) through every state the kubelet reports during one cycle. Read +// tick by tick, these used to be crashloop, readiness_failed, +// liveness_probe_failed, workload_degraded, and nothing at all — five issue +// ids for one problem. They must now be one critical crashloop issue. +func TestCompose_RestartLoopKeepsOneIssueAcrossTheCycle(t *testing.T) { + defer k8s.ResetTestState() + now := time.Now() + at := func(ago time.Duration) metav1.Time { return metav1.NewTime(now.Add(-ago)) } + completed := func(ago time.Duration) *corev1.ContainerStateTerminated { + return &corev1.ContainerStateTerminated{Reason: "Completed", ExitCode: 0, StartedAt: at(ago + 3*time.Minute), FinishedAt: at(ago)} + } + base := corev1.ContainerStatus{Name: "gateway", RestartCount: 4592} + with := func(mut func(*corev1.ContainerStatus)) corev1.ContainerStatus { + cs := base + mut(&cs) + return cs + } + + ticks := []restartLoopTick{ + {name: "backoff", status: with(func(cs *corev1.ContainerStatus) { + cs.State.Waiting = &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff", Message: "back-off 5m0s restarting failed container=gateway"} + cs.LastTerminationState.Terminated = completed(2 * time.Minute) + })}, + {name: "running-unready-no-events", status: with(func(cs *corev1.ContainerStatus) { + cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(30 * time.Second)} + cs.LastTerminationState.Terminated = completed(3 * time.Minute) + })}, + {name: "readiness-failing", status: with(func(cs *corev1.ContainerStatus) { + cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(time.Minute)} + cs.LastTerminationState.Terminated = completed(4 * time.Minute) + }), events: []*corev1.Event{probeEvent("r", "Readiness", now.Add(-10*time.Second))}}, + {name: "liveness-failing", status: with(func(cs *corev1.ContainerStatus) { + cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(2 * time.Minute)} + cs.LastTerminationState.Terminated = completed(5 * time.Minute) + }), events: []*corev1.Event{probeEvent("l", "Liveness", now.Add(-5*time.Second))}}, + {name: "just-terminated-exit-0", status: with(func(cs *corev1.ContainerStatus) { + cs.State.Terminated = completed(5 * time.Second) + cs.LastTerminationState.Terminated = completed(6 * time.Minute) + }), events: []*corev1.Event{probeEvent("l", "Liveness", now.Add(-20*time.Second))}}, + {name: "serving-between-crashes", ready: true, status: with(func(cs *corev1.ContainerStatus) { + cs.Ready = true + cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(8 * time.Minute)} + cs.LastTerminationState.Terminated = completed(8*time.Minute + 10*time.Second) + })}, + {name: "serving-29m-after-last-crash", ready: true, status: with(func(cs *corev1.ContainerStatus) { + cs.Ready = true + cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(28 * time.Minute)} + cs.LastTerminationState.Terminated = completed(29 * time.Minute) + })}, + } + + var id, cause string + for _, tick := range ticks { + issues, detections := composeRestartLoopTick(t, now, tick) + var mine []Issue + for _, iss := range issues { + if iss.Namespace == "knative" && iss.Name == "gateway" { + mine = append(mine, iss) + } + } + if len(mine) != 1 { + t.Fatalf("%s: gateway issues = %+v, want exactly one", tick.name, mine) + } + got := mine[0] + if got.Category != issuesapi.CategoryCrashLoop || got.Severity != SeverityCritical { + t.Fatalf("%s: issue = %s/%s, want critical crashloop", tick.name, got.Severity, got.Category) + } + if id == "" { + id = got.ID + } else if got.ID != id { + t.Fatalf("%s: issue id = %s, want %s for the whole loop", tick.name, got.ID, id) + } + if got.RestartLoop == nil || got.RestartLoop.Container != "gateway" || got.RestartLoop.RestartCount != 4592 || got.RestartLoop.LastExitCode != 0 { + t.Fatalf("%s: restart loop evidence = %+v", tick.name, got.RestartLoop) + } + if got.Cause == "" || got.Action == "" { + t.Fatalf("%s: issue has no diagnosis: %+v", tick.name, got) + } + if cause == "" { + cause = got.Cause + } else if got.Cause != cause { + t.Fatalf("%s: cause changed mid-loop:\n%q\n%q", tick.name, got.Cause, cause) + } + // The Deployment's own degraded row is folded into the loop, not + // dropped because it never fired: assert it was detected when down. + if !tick.ready && !hasDetection(detections, "Deployment", "gateway") { + t.Fatalf("%s: fixture should emit the Deployment degraded detection", tick.name) + } + switch tick.name { + case "liveness-failing", "just-terminated-exit-0": + if got.RestartLoop.LivenessProbeFailure == nil { + t.Fatalf("%s: want liveness evidence, got %+v", tick.name, got.RestartLoop) + } + if fact := restartCauseFactMessage(got); !strings.Contains(fact, "liveness probe failure last seen") { + t.Fatalf("%s: restart_cause fact = %q, want the liveness observation", tick.name, fact) + } + case "readiness-failing": + if got.RestartLoop.ReadinessProbeFailure == nil { + t.Fatalf("%s: want readiness evidence, got %+v", tick.name, got.RestartLoop) + } + } + } + + // 31 minutes after the last crash, the loop is over: no issue. + issues, _ := composeRestartLoopTick(t, now, restartLoopTick{name: "recovered", ready: true, status: with(func(cs *corev1.ContainerStatus) { + cs.Ready = true + cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(30 * time.Minute)} + cs.LastTerminationState.Terminated = completed(31 * time.Minute) + })}) + for _, iss := range issues { + if iss.Name == "gateway" || iss.Name == "gateway-abc" { + t.Fatalf("recovered: issue %+v still open 31m after the last crash", iss) + } + } +} + +func restartCauseFactMessage(i Issue) string { + if i.DiagnosticContext == nil { + return "" + } + for _, f := range i.DiagnosticContext.Facts { + if f.Type == factRestartCause { + return f.Message + } + } + return "" +} diff --git a/internal/k8s/detect.go b/internal/k8s/detect.go index 3e0eb44d64..f575d7a165 100644 --- a/internal/k8s/detect.go +++ b/internal/k8s/detect.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "github.com/skyhook-io/radar/pkg/health" + "github.com/skyhook-io/radar/pkg/issuesapi" "github.com/skyhook-io/radar/pkg/k8score" ) @@ -103,6 +104,9 @@ type Detection struct { // mean either non-Pod problem or no crash data on this Pod yet. RestartCount int32 LastTerminatedReason string + // RestartLoop is set on a Pod crashloop row emitted because a container is + // in an active restart loop (see activeRestartLoop). Nil otherwise. + RestartLoop *issuesapi.RestartLoop // OwnerKind + OwnerName name the topmost stable controller of a Pod // problem (Pod→Deployment, not the intermediate ReplicaSet), resolved // via topOwnerForPod when the Pod is detected. Empty for non-Pod and @@ -447,7 +451,7 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { } } - probeFailures := latestProbeFailures(cache, namespace, now) + probeFailures, containerProbeFailures := latestProbeFailures(cache, namespace, now) pvcPendingFailures := latestPVCPendingFailures(cache, namespace) // Pod problems: high-signal container waiting/terminated states, old @@ -461,7 +465,10 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { } healthStr := health.Pod(pod, now).LegacyString() earlyProbeTargetProblem, hasEarlyProbeTargetProblem := activeProbeTargetProblem(pod, "") - if healthStr == "healthy" && !hasEarlyProbeTargetProblem { + // A restart loop keeps its row through the ticks where the pod + // looks healthy between crashes; see restart_loop.go. + loop, looping := activeRestartLoop(pod, containerProbeFailures, now) + if healthStr == "healthy" && !hasEarlyProbeTargetProblem && !looping { continue } // Unschedulable pods are owned by the scheduling source, which @@ -482,6 +489,18 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { reason = pf.reason message = pf.message } + // Every per-tick face of a restart loop (crash backoff, a bare + // phase, probe failures, high restarts) becomes the one crashloop + // row, so the issue id holds for the whole loop. Specific reasons + // (image pulls, create errors) and an active OOM keep their own + // paths. Applied before the structural checks below so an invalid + // probe target still wins on every tick, not only on some. + if looping && restartLoopMayReplace(reason) && !health.PodHasActiveOOMKilled(pod, now) { + reason = crashLoopReason + message = loop.message() + } else { + looping = false + } fingerprint := "" if inv, ok := activeProbeTargetProblem(pod, reason); ok { reason = inv.reason @@ -496,9 +515,24 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { message = init.message fingerprint = init.fingerprint } + var restartLoopEvidence *issuesapi.RestartLoop + if looping && reason == crashLoopReason { + // Severity is pinned for the loop: serving or down at this + // instant is a per-tick fact, and letting it move the severity + // would drop the issue out of severity-filtered alerts and + // bring it back on every cycle. The loop container's own + // restart count and last termination replace the pod-wide + // ones so every field on the row describes one container. + severity = "critical" + restartCount = loop.restartCount + lastTermReason = loop.lastReason + restartLoopEvidence = loop.evidence() + } cause, action, diagnosisSource := oomLimitDiagnosis(cache, pod, reason, lastTermReason, now) if cause == "" { - if reason == crashLoopReason { + if restartLoopEvidence != nil { + cause, action = loop.diagnosis() + } else if reason == crashLoopReason { cause, action = health.PodCrashLoopDiagnosis(pod, now) } else { cause, action = imagePullDiagnosis(reason, message) @@ -607,6 +641,7 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { ResourceCreatedAt: pod.CreationTimestamp.Time, RestartCount: restartCount, LastTerminatedReason: lastTermReason, + RestartLoop: restartLoopEvidence, OwnerGroup: ownerGroup, OwnerKind: ownerKind, OwnerName: ownerName, @@ -1332,6 +1367,12 @@ func stalledInitContainerProblem(pod *corev1.Pod, now time.Time) (podSpecificPro if cs.State.Running == nil || cs.State.Running.StartedAt.IsZero() { continue } + // A started native sidecar is meant to keep running and no longer + // blocks the pod. One whose startup probe has not passed + // (Started=false) still holds back the containers after it. + if isNativeSidecarName(pod, cs.Name) && cs.Started != nil && *cs.Started { + continue + } if now.Sub(cs.State.Running.StartedAt.Time) < 5*time.Minute { continue } @@ -1506,10 +1547,15 @@ func pvcPendingFailureFromEvent(e *corev1.Event) (pvcPendingFailure, bool) { return f, true } -func latestProbeFailures(cache *ResourceCache, namespace string, now time.Time) map[string]probeFailure { +// latestProbeFailures returns, per pod ("ns/name"), the newest liveness or +// readiness probe failure inside probeFailureWindow, and the same newest +// failure per container and probe type ("ns/name/container/reason"; container +// is empty when the event carries no container fieldPath). +func latestProbeFailures(cache *ResourceCache, namespace string, now time.Time) (map[string]probeFailure, map[string]probeFailure) { out := map[string]probeFailure{} + byContainer := map[string]probeFailure{} if cache == nil || cache.Events() == nil { - return out + return out, byContainer } var events []*corev1.Event if namespace != "" { @@ -1530,12 +1576,30 @@ func latestProbeFailures(cache *ResourceCache, namespace string, now time.Time) continue } key := e.InvolvedObject.Namespace + "/" + e.InvolvedObject.Name + pf := probeFailure{reason: reason, message: strings.TrimSpace(e.Message), at: t} + containerKey := key + "/" + eventContainerName(e.InvolvedObject.FieldPath) + "/" + reason + if cur, exists := byContainer[containerKey]; !exists || t.After(cur.at) { + byContainer[containerKey] = pf + } if cur, exists := out[key]; exists && !t.After(cur.at) { continue } - out[key] = probeFailure{reason: reason, message: strings.TrimSpace(e.Message), at: t} + out[key] = pf } - return out + return out, byContainer +} + +// eventContainerName extracts the container from a pod event's fieldPath +// ("spec.containers{app}" or "spec.initContainers{proxy}"); empty otherwise. +func eventContainerName(fieldPath string) string { + for _, prefix := range []string{"spec.containers{", "spec.initContainers{"} { + if rest, ok := strings.CutPrefix(fieldPath, prefix); ok { + if name, ok := strings.CutSuffix(rest, "}"); ok { + return name + } + } + } + return "" } func classifyProbeFailureEvent(reason, msg string) (string, bool) { diff --git a/internal/k8s/detect_crashloop_severity_test.go b/internal/k8s/detect_crashloop_severity_test.go index 728fe22ad8..657cb1f98e 100644 --- a/internal/k8s/detect_crashloop_severity_test.go +++ b/internal/k8s/detect_crashloop_severity_test.go @@ -152,7 +152,9 @@ func TestDetectProblems_CrashLoopSeverityTracksCurrentState(t *testing.T) { if !strings.Contains(recovered.Action, "Watch for another restart") { t.Fatalf("recovered startup action = %q, want repeat-crash guidance", recovered.Action) } - assertProblem(t, problems, "Pod", "high-count-serving", "CrashLoopBackOff", "high") + // Past the restart-loop threshold, serving at this instant no longer + // lowers severity: the loop keeps one critical row through its ready ticks. + assertProblem(t, problems, "Pod", "high-count-serving", "CrashLoopBackOff", "critical") assertProblem(t, problems, "Pod", "down-at-creation", "CrashLoopBackOff", "critical") assertProblem(t, problems, "Pod", "runtime-down", "CrashLoopBackOff", "critical") assertProblem(t, problems, "Pod", "image-pull-sibling", "ImagePullBackOff", "critical") diff --git a/internal/k8s/detect_test.go b/internal/k8s/detect_test.go index 64a53c4f46..d66144b67d 100644 --- a/internal/k8s/detect_test.go +++ b/internal/k8s/detect_test.go @@ -1233,7 +1233,7 @@ func TestDetectProblems_ProbeFailures(t *testing.T) { }, &corev1.Event{ ObjectMeta: metav1.ObjectMeta{Name: "thrash.1", Namespace: "prod"}, - InvolvedObject: corev1.ObjectReference{Kind: "Pod", Namespace: "prod", Name: "thrash"}, + InvolvedObject: corev1.ObjectReference{Kind: "Pod", Namespace: "prod", Name: "thrash", FieldPath: "spec.containers{app}"}, Type: corev1.EventTypeWarning, Reason: "Unhealthy", Message: "Liveness probe failed: HTTP probe failed with statuscode: 500", @@ -1263,7 +1263,7 @@ func TestDetectProblems_ProbeFailures(t *testing.T) { problems = DetectProblems(cache, "prod") if hasProblem(problems, "Pod", "readiness", "ReadinessProbeFailed") && hasProblem(problems, "Pod", "liveness", "LivenessProbeFailed") && - hasProblem(problems, "Pod", "thrash", "HighRestartCount") && + hasProblem(problems, "Pod", "thrash", "CrashLoopBackOff") && hasProblem(problems, "Pod", "stale-probe", "CrashLoopBackOff") { break } @@ -1272,11 +1272,16 @@ func TestDetectProblems_ProbeFailures(t *testing.T) { assertProblem(t, problems, "Pod", "readiness", "ReadinessProbeFailed", "high") assertProblem(t, problems, "Pod", "liveness", "LivenessProbeFailed", "critical") - assertProblem(t, problems, "Pod", "thrash", "HighRestartCount", "high") - assertProblem(t, problems, "Pod", "stale-probe", "CrashLoopBackOff", "critical") - if hasProblem(problems, "Pod", "thrash", "LivenessProbeFailed") { - t.Fatalf("liveness event should not mask high restart thrash: %+v", problems) + // Clean exits with liveness failures alongside are a restart loop: one + // crashloop row carrying the probe failure as evidence, not a liveness row. + assertProblem(t, problems, "Pod", "thrash", "CrashLoopBackOff", "critical") + if hasProblem(problems, "Pod", "thrash", "LivenessProbeFailed") || hasProblem(problems, "Pod", "thrash", "HighRestartCount") { + t.Fatalf("restart loop should be one crashloop row: %+v", problems) + } + if got, _ := lookupProblem(problems, "Pod", "thrash", "CrashLoopBackOff"); got.RestartLoop == nil || got.RestartLoop.LivenessProbeFailure == nil { + t.Fatalf("thrash restart loop = %+v, want liveness probe evidence", got.RestartLoop) } + assertProblem(t, problems, "Pod", "stale-probe", "CrashLoopBackOff", "critical") if hasProblem(problems, "Pod", "stale-probe", "LivenessProbeFailed") { t.Fatalf("timeless probe event should not override the current pod reason: %+v", problems) } diff --git a/internal/k8s/restart_loop.go b/internal/k8s/restart_loop.go new file mode 100644 index 0000000000..28849bc00c --- /dev/null +++ b/internal/k8s/restart_loop.go @@ -0,0 +1,257 @@ +package k8s + +import ( + "fmt" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + + "github.com/skyhook-io/radar/pkg/issuesapi" +) + +// Restart-loop issue continuity. A container stuck restarting cycles through +// states that each look like a different problem on a single poll: Waiting +// (CrashLoopBackOff), Running but not ready (readiness), Unhealthy events +// (liveness), Terminated with exit 0, and Running+Ready between crashes. Read +// tick by tick, the pod row changed category (and therefore issue id) on every +// cycle and dropped out entirely on the healthy-looking ticks, so downstream +// alerting saw thousands of open/resolve generations for one ongoing problem. +// +// This classifier is issue-layer only. It decides that one crashloop row stays +// present, with stable severity, for the whole loop; it does not change the +// pod's health level (pkg/health), which still reports what the pod is doing +// right now. +const ( + // restartLoopWindow is how long after its last termination a restarting + // container is still treated as looping. It is the hysteresis: the issue + // clears this long after the last crash. Long enough to bridge kubelet + // backoff (max 5m) and loops whose Running phase lasts many minutes before + // the next liveness kill. + restartLoopWindow = 30 * time.Minute + // restartLoopStaleGap separates one continuous loop from an old termination + // followed by a fresh start, e.g. a node that was down for a while. Twice + // the kubelet's maximum backoff, matching pkg/health's stale-crash guard. + restartLoopStaleGap = 10 * time.Minute + // restartLoopMinRestarts is the restart count from which a recent + // termination reads as a loop rather than a one-off crash. RestartCount is + // cumulative over the container's life, so this approximates a rate: a + // container that restarted three times last month and once a minute ago + // also qualifies, for at most restartLoopWindow. + restartLoopMinRestarts = 3 + // restartLoopProbeMessageMax caps the probe output copied into evidence; + // exec probes can print arbitrary amounts. + restartLoopProbeMessageMax = 300 +) + +// restartLoop is the evidence behind an active restart loop, taken from one +// container so every field describes the same termination. +type restartLoop struct { + container string + sidecar bool + restartCount int32 + lastExitCode int32 + lastReason string + lastFinishedAt time.Time + liveness *probeFailure + readiness *probeFailure +} + +// activeRestartLoop reports the container keeping this pod in a restart loop. +// +// Eligible containers are those the kubelet restarts by design: regular +// containers of a restartPolicy=Always pod and native sidecars (init +// containers with restartPolicy=Always). Ordinary init containers retrying +// until they succeed, and OnFailure/Never pods such as Job workers, are +// excluded; their retries are handled by the init and Job paths. +// +// The exit code is deliberately ignored. The kubelet backs off restarts after +// any exit, and a liveness kill with a graceful shutdown ends Completed/0, so +// requiring a non-zero exit misses the loops that flap the most. An OOMKilled +// last termination is left to the OOM path, which has its own category. +// +// When several containers loop, the first in status order wins (regular +// containers before sidecars) so the evidence does not hop between containers +// from one poll to the next. +func activeRestartLoop(pod *corev1.Pod, probes map[string]probeFailure, now time.Time) (restartLoop, bool) { + if pod == nil || (pod.Status.Phase != corev1.PodRunning && pod.Status.Phase != corev1.PodPending) { + return restartLoop{}, false + } + if pod.Spec.RestartPolicy == "" || pod.Spec.RestartPolicy == corev1.RestartPolicyAlways { + for i := range pod.Status.ContainerStatuses { + if loop, ok := containerRestartLoop(&pod.Status.ContainerStatuses[i], now); ok { + return withProbeEvidence(loop, pod, probes), true + } + } + } + for i := range pod.Status.InitContainerStatuses { + cs := &pod.Status.InitContainerStatuses[i] + if !isNativeSidecarName(pod, cs.Name) { + continue + } + if loop, ok := containerRestartLoop(cs, now); ok { + loop.sidecar = true + return withProbeEvidence(loop, pod, probes), true + } + } + return restartLoop{}, false +} + +func isNativeSidecarName(pod *corev1.Pod, name string) bool { + for _, c := range pod.Spec.InitContainers { + if c.Name == name { + return isRestartableInitContainer(c) + } + } + return false +} + +func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoop, bool) { + if cs.RestartCount < restartLoopMinRestarts { + return restartLoop{}, false + } + // The current termination is the newest one when present; the kubelet + // only moves it to LastTerminationState once the next run starts. + term := cs.State.Terminated + if term == nil { + term = cs.LastTerminationState.Terminated + } + if term == nil || term.FinishedAt.IsZero() || term.Reason == "OOMKilled" { + return restartLoop{}, false + } + finished := term.FinishedAt.Time + if now.Sub(finished) > restartLoopWindow { + return restartLoop{}, false + } + if r := cs.State.Running; r != nil && !r.StartedAt.IsZero() && r.StartedAt.Sub(finished) > restartLoopStaleGap { + return restartLoop{}, false + } + return restartLoop{ + container: cs.Name, + restartCount: cs.RestartCount, + lastExitCode: term.ExitCode, + lastReason: term.Reason, + lastFinishedAt: finished, + }, true +} + +func withProbeEvidence(loop restartLoop, pod *corev1.Pod, probes map[string]probeFailure) restartLoop { + if pf, ok := probeFailureFor(probes, pod, loop.container, livenessProbeFailedReason); ok { + loop.liveness = &pf + } + if pf, ok := probeFailureFor(probes, pod, loop.container, readinessProbeFailedReason); ok { + loop.readiness = &pf + } + return loop +} + +// probeFailureFor finds the newest probe failure of one type for a container. +// The kubelet names the container in the event's fieldPath; an event without +// one is attributed only when the pod has a single container, so it can never +// pin another container's probe on the looping one. +func probeFailureFor(probes map[string]probeFailure, pod *corev1.Pod, container, reason string) (probeFailure, bool) { + prefix := pod.Namespace + "/" + pod.Name + "/" + if pf, ok := probes[prefix+container+"/"+reason]; ok { + return pf, true + } + if len(pod.Spec.Containers)+len(pod.Spec.InitContainers) == 1 { + pf, ok := probes[prefix+"/"+reason] + return pf, ok + } + return probeFailure{}, false +} + +// restartLoopMayReplace reports whether the loop's crashloop reason may stand +// in for the reason the pod walk produced. Phase strings, crash-class reasons, +// and probe/thrash reasons are exactly the per-tick faces of the loop. Anything +// more specific (image pulls, container-create errors, OOM, init stalls) names a +// different problem and keeps its own row. +func restartLoopMayReplace(reason string) bool { + switch reason { + case "Running", "Pending", "Unknown", "", "PodInitializing", "ContainerCreating", + crashLoopReason, "Error", "Completed", + highRestartReason, livenessProbeFailedReason, readinessProbeFailedReason: + return true + } + return false +} + +func (l restartLoop) ref() string { + if l.sidecar { + return fmt.Sprintf("sidecar container %q", l.container) + } + return fmt.Sprintf("container %q", l.container) +} + +func (l restartLoop) lastExit() string { + if l.lastReason != "" { + return fmt.Sprintf("exit code %d (%s)", l.lastExitCode, l.lastReason) + } + return fmt.Sprintf("exit code %d", l.lastExitCode) +} + +// message is the row's one-line summary. It names the probe failures seen +// alongside the restarts without claiming they caused them. +func (l restartLoop) message() string { + msg := fmt.Sprintf("%s is in a restart loop: %d restarts, last %s", l.ref(), l.restartCount, l.lastExit()) + var seen []string + if l.liveness != nil { + seen = append(seen, "liveness") + } + if l.readiness != nil { + seen = append(seen, "readiness") + } + if len(seen) > 0 { + msg += "; " + strings.Join(seen, " and ") + " probe failures observed" + } + return msg +} + +// diagnosis returns the loop's cause and next step. The text depends only on +// the container and its last exit code — not on serving state, restart counts, +// or whether a probe event happens to be inside its window — so it holds +// between polls of one loop and agrees across replicas, which a grouped +// workload row requires before it carries a diagnosis (agreedDiagnosis). Probe +// observations live in the evidence and the message instead. +func (l restartLoop) diagnosis() (cause, action string) { + ref := l.ref() + switch code := l.lastExitCode; code { + case 0: + return fmt.Sprintf("%s keeps restarting, and its last run ended with exit code 0. Under restartPolicy Always the kubelet restarts a container whenever it exits; a failed liveness probe also ends this way, because the kubelet lets the process shut down gracefully.", ref), + "Check the pod's Unhealthy events for liveness probe failures; if present, check the probe against the app (path and port, timeoutSeconds, failureThreshold, startup time or a missing startupProbe). If not, check why the main process exits: command and args, a process that daemonizes, or one-shot work that belongs in a Job." + case 127: + return fmt.Sprintf("%s keeps restarting after exit code 127: command not found.", ref), + "Check the image entrypoint and pod command/args; verify the binary exists in the image." + case 126: + return fmt.Sprintf("%s keeps restarting after exit code 126: command found but not executable.", ref), + "Check executable permissions, the shebang/interpreter, and the pod command/args." + case 139: + return fmt.Sprintf("%s keeps restarting after exit code 139: segmentation fault.", ref), + "Inspect previous container logs and recent image/code changes; check native libraries or unsafe code for a segfault." + case 137: + return fmt.Sprintf("%s keeps restarting after exit code 137 (SIGKILL), but Kubernetes did not report OOMKilled.", ref), + "Check for a liveness probe kill that outlived its grace period, node pressure, process-level SIGKILLs, and memory limits; inspect previous container logs for shutdown context." + default: + return fmt.Sprintf("%s keeps restarting after exit code %d.", ref, code), + "Inspect previous container logs for this container and verify the pod command, args, config, and dependencies." + } +} + +// evidence is the wire form attached to the issue. +func (l restartLoop) evidence() *issuesapi.RestartLoop { + out := &issuesapi.RestartLoop{ + Container: l.container, + Sidecar: l.sidecar, + RestartCount: l.restartCount, + LastExitCode: l.lastExitCode, + LastReason: l.lastReason, + LastFinishedAt: l.lastFinishedAt.UTC(), + } + if l.liveness != nil { + out.LivenessProbeFailure = &issuesapi.ProbeFailure{LastSeen: l.liveness.at.UTC(), Message: Truncate(l.liveness.message, restartLoopProbeMessageMax)} + } + if l.readiness != nil { + out.ReadinessProbeFailure = &issuesapi.ProbeFailure{LastSeen: l.readiness.at.UTC(), Message: Truncate(l.readiness.message, restartLoopProbeMessageMax)} + } + return out +} diff --git a/internal/k8s/restart_loop_test.go b/internal/k8s/restart_loop_test.go new file mode 100644 index 0000000000..f1fb920c44 --- /dev/null +++ b/internal/k8s/restart_loop_test.go @@ -0,0 +1,218 @@ +package k8s + +import ( + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/fake" +) + +func restartLoopPod(now time.Time, cs corev1.ContainerStatus) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "ns"}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: cs.Name}}}, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{cs}, + }, + } +} + +func terminatedAgo(now time.Time, ago time.Duration, reason string, code int32) *corev1.ContainerStateTerminated { + return &corev1.ContainerStateTerminated{Reason: reason, ExitCode: code, FinishedAt: metav1.NewTime(now.Add(-ago))} +} + +func TestActiveRestartLoop(t *testing.T) { + now := time.Now() + running := func(ago time.Duration) corev1.ContainerState { + return corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(now.Add(-ago))}} + } + status := func(restarts int32, state corev1.ContainerState, last *corev1.ContainerStateTerminated) corev1.ContainerStatus { + return corev1.ContainerStatus{Name: "app", RestartCount: restarts, State: state, LastTerminationState: corev1.ContainerState{Terminated: last}} + } + sidecarPolicy := corev1.ContainerRestartPolicyAlways + + cases := []struct { + name string + pod *corev1.Pod + want bool + sidecar bool + }{ + {"clean exit inside window", restartLoopPod(now, status(3, running(time.Minute), terminatedAgo(now, 2*time.Minute, "Completed", 0))), true, false}, + {"last crash 29m ago", restartLoopPod(now, status(5, running(28*time.Minute), terminatedAgo(now, 29*time.Minute, "Error", 1))), true, false}, + {"last crash 31m ago", restartLoopPod(now, status(5, running(30*time.Minute), terminatedAgo(now, 31*time.Minute, "Error", 1))), false, false}, + {"two restarts is not a loop", restartLoopPod(now, status(2, running(time.Minute), terminatedAgo(now, 2*time.Minute, "Error", 1))), false, false}, + {"run started 9m after the termination", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 10*time.Minute, "Error", 1))), true, false}, + {"run started 11m after the termination", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 12*time.Minute, "Error", 1))), false, false}, + {"OOM is left to the OOM path", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 2*time.Minute, "OOMKilled", 137))), false, false}, + {"current termination counts", restartLoopPod(now, status(5, corev1.ContainerState{Terminated: terminatedAgo(now, 5*time.Second, "Completed", 0)}, terminatedAgo(now, 40*time.Minute, "Error", 1))), true, false}, + {"no termination recorded", restartLoopPod(now, status(5, running(time.Minute), nil)), false, false}, + {"OnFailure pod (Job worker)", func() *corev1.Pod { + p := restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 2*time.Minute, "Error", 1))) + p.Spec.RestartPolicy = corev1.RestartPolicyOnFailure + return p + }(), false, false}, + {"succeeded pod", func() *corev1.Pod { + p := restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 2*time.Minute, "Error", 1))) + p.Status.Phase = corev1.PodSucceeded + return p + }(), false, false}, + {"ordinary init container retrying", func() *corev1.Pod { + p := restartLoopPod(now, corev1.ContainerStatus{Name: "app"}) + p.Spec.InitContainers = []corev1.Container{{Name: "init"}} + p.Status.Phase = corev1.PodPending + p.Status.InitContainerStatuses = []corev1.ContainerStatus{{Name: "init", RestartCount: 5, State: running(time.Minute), + LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, 2*time.Minute, "Error", 1)}}} + return p + }(), false, false}, + {"native sidecar looping", func() *corev1.Pod { + p := restartLoopPod(now, corev1.ContainerStatus{Name: "app", State: running(time.Hour)}) + p.Spec.InitContainers = []corev1.Container{{Name: "proxy", RestartPolicy: &sidecarPolicy}} + p.Status.InitContainerStatuses = []corev1.ContainerStatus{{Name: "proxy", RestartCount: 5, State: running(time.Minute), + LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, 2*time.Minute, "Completed", 0)}}} + return p + }(), true, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + loop, ok := activeRestartLoop(tc.pod, nil, now) + if ok != tc.want { + t.Fatalf("activeRestartLoop = %v, want %v (loop %+v)", ok, tc.want, loop) + } + if ok && loop.sidecar != tc.sidecar { + t.Fatalf("sidecar = %v, want %v", loop.sidecar, tc.sidecar) + } + }) + } +} + +func TestActiveRestartLoop_ProbeEvidenceIsPerContainer(t *testing.T) { + now := time.Now() + pod := restartLoopPod(now, corev1.ContainerStatus{Name: "app", RestartCount: 4, LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, time.Minute, "Completed", 0)}}) + probes := map[string]probeFailure{ + "ns/p/other/" + livenessProbeFailedReason: {reason: livenessProbeFailedReason, at: now}, + "ns/p//" + readinessProbeFailedReason: {reason: readinessProbeFailedReason, at: now, message: "Readiness probe failed"}, + } + loop, ok := activeRestartLoop(pod, probes, now) + if !ok { + t.Fatal("want a loop") + } + if loop.liveness != nil { + t.Fatalf("liveness failure of another container attributed to the loop: %+v", loop.liveness) + } + if loop.readiness == nil { + t.Fatal("pod-level readiness failure (no fieldPath) should be attributed") + } +} + +func TestRestartLoopDiagnosis_StableAcrossPollsAndReplicas(t *testing.T) { + a := restartLoop{container: "app", restartCount: 4, lastExitCode: 0, lastReason: "Completed", liveness: &probeFailure{}} + b := restartLoop{container: "app", restartCount: 4592, lastExitCode: 0, lastReason: "Completed", lastFinishedAt: time.Now()} + causeA, actionA := a.diagnosis() + causeB, actionB := b.diagnosis() + if causeA != causeB || actionA != actionB { + t.Fatalf("diagnosis differs by restart count, time, or probe evidence:\n%q\n%q", causeA, causeB) + } + if strings.Contains(causeA, "caused") { + t.Fatalf("cause = %q, must not assert causation", causeA) + } + if !strings.Contains(a.message(), "liveness probe failures observed") || strings.Contains(b.message(), "liveness") { + t.Fatalf("message must name probe observations only when seen: %q / %q", a.message(), b.message()) + } +} + +func TestActiveRestartLoop_UnattributedProbeEventNeedsSingleContainer(t *testing.T) { + now := time.Now() + cs := corev1.ContainerStatus{Name: "app", RestartCount: 4, LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, time.Minute, "Completed", 0)}} + probes := map[string]probeFailure{"ns/p//" + livenessProbeFailedReason: {reason: livenessProbeFailedReason, at: now}} + single := restartLoopPod(now, cs) + if loop, _ := activeRestartLoop(single, probes, now); loop.liveness == nil { + t.Fatal("single-container pod: unattributed liveness event should count") + } + multi := restartLoopPod(now, cs) + multi.Spec.Containers = append(multi.Spec.Containers, corev1.Container{Name: "other"}) + if loop, _ := activeRestartLoop(multi, probes, now); loop.liveness != nil { + t.Fatal("multi-container pod: unattributed liveness event must not be pinned on the looping container") + } +} + +func TestStalledInitContainerProblem_IgnoresStartedNativeSidecar(t *testing.T) { + now := time.Now() + always := corev1.ContainerRestartPolicyAlways + pod := &corev1.Pod{ + Spec: corev1.PodSpec{InitContainers: []corev1.Container{{Name: "proxy", RestartPolicy: &always}}}, + Status: corev1.PodStatus{Phase: corev1.PodPending, InitContainerStatuses: []corev1.ContainerStatus{{ + Name: "proxy", State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(now.Add(-20 * time.Minute))}}, + }}}, + } + if _, ok := stalledInitContainerProblem(pod, now); !ok { + t.Fatal("a sidecar that has not passed its startup probe still blocks the pod") + } + started := true + pod.Status.InitContainerStatuses[0].Started = &started + if got, ok := stalledInitContainerProblem(pod, now); ok { + t.Fatalf("started native sidecar reported as a stalled init container: %+v", got) + } +} + +// A restart loop must not hide a more specific problem: an invalid liveness +// target keeps its fingerprinted row on every tick, including a ready one, and +// an image pull failure on a sibling container keeps its own reason. +func TestDetectProblems_RestartLoopPrecedence(t *testing.T) { + defer ResetTestState() + now := time.Now() + old := metav1.NewTime(now.Add(-time.Hour)) + loopStatus := corev1.ContainerStatus{ + Name: "app", Ready: true, RestartCount: 9, + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(now.Add(-time.Minute))}}, + LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, 2*time.Minute, "Completed", 0)}, + } + badProbe := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "bad-liveness", Namespace: "prod", CreationTimestamp: old}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "app", + LivenessProbe: &corev1.Probe{ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{Path: "/healthz", Port: intstr.FromString("admin")}, + }}, + }}}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, ContainerStatuses: []corev1.ContainerStatus{loopStatus}}, + } + imageSibling := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "image-sibling", Namespace: "prod", CreationTimestamp: old}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}, {Name: "img"}}}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, ContainerStatuses: []corev1.ContainerStatus{ + loopStatus, + {Name: "img", State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ImagePullBackOff"}}}, + }}, + } + plainLoop := badProbe.DeepCopy() + plainLoop.Name = "plain-loop" + plainLoop.Spec.Containers[0].LivenessProbe = nil + + if err := InitTestResourceCache(fake.NewClientset(badProbe, imageSibling, plainLoop)); err != nil { + t.Fatalf("InitTestResourceCache: %v", err) + } + var problems []Detection + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + problems = DetectProblems(GetResourceCache(), "prod") + if hasProblem(problems, "Pod", "bad-liveness", livenessProbeInvalidReason) && hasProblem(problems, "Pod", "image-sibling", "ImagePullBackOff") && + hasProblem(problems, "Pod", "plain-loop", crashLoopReason) { + break + } + time.Sleep(20 * time.Millisecond) + } + if got, ok := lookupProblem(problems, "Pod", "bad-liveness", livenessProbeInvalidReason); !ok || got.Fingerprint == "" || got.RestartLoop != nil { + t.Fatalf("bad-liveness = %+v (found %v), want the fingerprinted invalid-probe row", got, ok) + } + if got, ok := lookupProblem(problems, "Pod", "image-sibling", "ImagePullBackOff"); !ok || got.RestartLoop != nil { + t.Fatalf("image-sibling = %+v (found %v), want the image pull row", got, ok) + } + if got, ok := lookupProblem(problems, "Pod", "plain-loop", crashLoopReason); !ok || got.Severity != "critical" || got.RestartLoop == nil { + t.Fatalf("plain-loop = %+v (found %v), want a critical crashloop row on a ready tick", got, ok) + } +} diff --git a/packages/k8s-ui/src/components/issues/IssuesView.tsx b/packages/k8s-ui/src/components/issues/IssuesView.tsx index 780c7f56b6..40f377ea0b 100644 --- a/packages/k8s-ui/src/components/issues/IssuesView.tsx +++ b/packages/k8s-ui/src/components/issues/IssuesView.tsx @@ -403,7 +403,12 @@ export function IssueRow({ function Diagnosis({ issue, source }: { issue: Issue; source?: IssueDiagnosisSource }) { const crash = issue.restart_count || issue.last_terminated_reason - ? [issue.restart_count ? `${issue.restart_count} restart${issue.restart_count === 1 ? '' : 's'}` : null, issue.last_terminated_reason ? `last exit: ${issue.last_terminated_reason}` : null] + ? [ + issue.restart_count ? `${issue.restart_count} restart${issue.restart_count === 1 ? '' : 's'}` : null, + issue.last_terminated_reason ? `last exit: ${issue.last_terminated_reason}` : null, + issue.restart_loop?.liveness_probe_failure ? 'liveness probe failing' : null, + issue.restart_loop?.readiness_probe_failure ? 'readiness probe failing' : null, + ] .filter(Boolean) .join(' · ') : null; diff --git a/packages/k8s-ui/src/components/issues/types.ts b/packages/k8s-ui/src/components/issues/types.ts index 3f6e61b80a..1333f1217c 100644 --- a/packages/k8s-ui/src/components/issues/types.ts +++ b/packages/k8s-ui/src/components/issues/types.ts @@ -230,6 +230,9 @@ export interface Issue { // Pod crash context carried from the representative member. restart_count?: number; last_terminated_reason?: string; + /** Evidence for a container in an active restart loop (crashloop issues). + * Probe failures are observations alongside the restarts, not a cause. */ + restart_loop?: IssueRestartLoop; /** * Best-effort timing evidence from K8s-native signals. Absent when Radar has @@ -360,3 +363,19 @@ export function issueMessageParts(issue: Issue): { headline: string; detail: str if (normalized && normalized !== raw) return { headline: normalized, detail: raw }; return { headline: raw, detail: '' }; } + +export interface IssueProbeFailure { + last_seen: string; + message?: string; +} + +export interface IssueRestartLoop { + container: string; + sidecar?: boolean; + restart_count: number; + last_exit_code: number; + last_reason?: string; + last_finished_at?: string; + liveness_probe_failure?: IssueProbeFailure; + readiness_probe_failure?: IssueProbeFailure; +} diff --git a/pkg/health/reasons.go b/pkg/health/reasons.go index c4d34a3994..dd68a15928 100644 --- a/pkg/health/reasons.go +++ b/pkg/health/reasons.go @@ -249,6 +249,13 @@ func ActiveOOMKilledContainers(pod *corev1.Pod, now time.Time) []corev1.Containe return active } +// PodHasActiveOOMKilled reports whether any container, init containers and +// native sidecars included, is currently affected by an OOM kill (one that has +// not since recovered). Exported for the problem detector. +func PodHasActiveOOMKilled(pod *corev1.Pod, now time.Time) bool { + return podHasActiveOOMKilled(pod, now) +} + func podHasActiveOOMKilled(pod *corev1.Pod, now time.Time) bool { if len(ActiveOOMKilledContainers(pod, now)) > 0 { return true diff --git a/pkg/issuesapi/types.go b/pkg/issuesapi/types.go index 51b41c49b3..8bf88fa7cc 100644 --- a/pkg/issuesapi/types.go +++ b/pkg/issuesapi/types.go @@ -352,6 +352,37 @@ type ClusterDNSFinding struct { Evidence string `json:"evidence,omitempty"` } +// RestartLoop describes a container that keeps restarting. A crashloop issue +// carries it while the container has restarted at least three times and last +// terminated within the past 30 minutes, whatever the exit code, so the issue +// stays open across the loop's healthy-looking moments instead of resolving and +// reopening on every cycle. +// +// Probe failures are observations from the same window (kubelet Unhealthy +// events), not a causal verdict: a liveness failure seen next to the restarts +// is strong evidence of a probe-driven restart, but the consumer decides. +type RestartLoop struct { + Container string `json:"container"` + // Sidecar is true for a native sidecar (an init container with + // restartPolicy Always). + Sidecar bool `json:"sidecar,omitempty"` + RestartCount int32 `json:"restart_count"` + LastExitCode int32 `json:"last_exit_code"` + LastReason string `json:"last_reason,omitempty"` + LastFinishedAt time.Time `json:"last_finished_at,omitzero"` + // LivenessProbeFailure / ReadinessProbeFailure are the newest failure of + // each probe type seen for this container in the last 10 minutes. + LivenessProbeFailure *ProbeFailure `json:"liveness_probe_failure,omitempty"` + ReadinessProbeFailure *ProbeFailure `json:"readiness_probe_failure,omitempty"` +} + +// ProbeFailure is one observed probe failure: when it was last reported and +// the kubelet's message (truncated). +type ProbeFailure struct { + LastSeen time.Time `json:"last_seen"` + Message string `json:"message,omitempty"` +} + // OnsetCoverage counts contributing failure signals when at least one signal // has no evidence-backed active-time anchor. A signal is usually one affected // resource, but controllers may collapse multiple status entries for one @@ -400,22 +431,26 @@ type Issue struct { // FirstSeen is the earliest evidence-backed time the issue was active. It // may be the time Radar first observed a state rather than its exact onset, // so consumers should read it as "active at least since". - FirstSeen time.Time `json:"first_seen,omitzero"` - OnsetUnknown bool `json:"onset_unknown,omitempty"` - OnsetCoverage *OnsetCoverage `json:"onset_coverage,omitempty"` - ResourceCreatedAt time.Time `json:"resource_created_at,omitzero"` - LastSeen time.Time `json:"last_seen,omitzero"` - Count int `json:"count,omitempty"` - Owner Ref `json:"owner,omitzero"` - Fingerprint string `json:"-"` - RestartCount int32 `json:"restart_count,omitempty"` - LastTerminatedReason string `json:"last_terminated_reason,omitempty"` - Affected Affected `json:"affected,omitzero"` - Members []Ref `json:"members,omitempty"` - MembersTruncated bool `json:"members_truncated,omitempty"` - DiagnosticContext *DiagnosticContext `json:"diagnostic_context,omitempty"` - IncidentParent *IncidentParent `json:"incident_parent,omitempty"` - ChangeContext *ChangeContext `json:"change_context,omitempty"` + FirstSeen time.Time `json:"first_seen,omitzero"` + OnsetUnknown bool `json:"onset_unknown,omitempty"` + OnsetCoverage *OnsetCoverage `json:"onset_coverage,omitempty"` + ResourceCreatedAt time.Time `json:"resource_created_at,omitzero"` + LastSeen time.Time `json:"last_seen,omitzero"` + Count int `json:"count,omitempty"` + Owner Ref `json:"owner,omitzero"` + Fingerprint string `json:"-"` + RestartCount int32 `json:"restart_count,omitempty"` + LastTerminatedReason string `json:"last_terminated_reason,omitempty"` + // RestartLoop is set on a crashloop issue whose container Radar classifies + // as in an active restart loop. It is the evidence for the loop, taken + // from one container so every field describes the same termination. + RestartLoop *RestartLoop `json:"restart_loop,omitempty"` + Affected Affected `json:"affected,omitzero"` + Members []Ref `json:"members,omitempty"` + MembersTruncated bool `json:"members_truncated,omitempty"` + DiagnosticContext *DiagnosticContext `json:"diagnostic_context,omitempty"` + IncidentParent *IncidentParent `json:"incident_parent,omitempty"` + ChangeContext *ChangeContext `json:"change_context,omitempty"` // IssueTiming is best-effort timing evidence for when this issue entered // the failing state, derived from K8s-native signals (condition // lastTransitionTime, resource phase, deletion timestamp) at detection From d6b40017ae9d9c3d7ac62aec787f9ac60cfbec9c Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 01:55:39 +0300 Subject: [PATCH 2/8] fix(issues): restart-loop review follow-ups Skip pods being deleted (a rollout SIGTERM is not a crash), require the terminated run itself to be shorter than the loop window so a long-lived container's one restart after a node bounce is not a loop, treat StartError/ContainerCannotRun as faces of the loop, and ignore probe events from a same-named predecessor pod. --- .../issues/restart_loop_integration_test.go | 8 ++--- internal/k8s/detect.go | 3 +- internal/k8s/restart_loop.go | 29 +++++++++++++------ internal/k8s/restart_loop_test.go | 25 ++++++++++++++++ 4 files changed, 51 insertions(+), 14 deletions(-) diff --git a/internal/issues/restart_loop_integration_test.go b/internal/issues/restart_loop_integration_test.go index 350d466214..7f32dfcd18 100644 --- a/internal/issues/restart_loop_integration_test.go +++ b/internal/issues/restart_loop_integration_test.go @@ -123,10 +123,10 @@ func probeEvent(name, probe string, at time.Time) *corev1.Event { // TestCompose_RestartLoopKeepsOneIssueAcrossTheCycle walks a liveness-driven // restart loop (graceful exit 0, the kourier-gateway pattern seen in -// production) through every state the kubelet reports during one cycle. Read -// tick by tick, these used to be crashloop, readiness_failed, -// liveness_probe_failed, workload_degraded, and nothing at all — five issue -// ids for one problem. They must now be one critical crashloop issue. +// production) through every state the kubelet reports during one cycle. Each +// state alone looks like a different problem (crash backoff, readiness, +// liveness, a degraded Deployment, or a healthy pod); across all of them the +// loop must stay one critical crashloop issue with one id. func TestCompose_RestartLoopKeepsOneIssueAcrossTheCycle(t *testing.T) { defer k8s.ResetTestState() now := time.Now() diff --git a/internal/k8s/detect.go b/internal/k8s/detect.go index f575d7a165..ff93f0fce5 100644 --- a/internal/k8s/detect.go +++ b/internal/k8s/detect.go @@ -1284,6 +1284,7 @@ type probeFailure struct { reason string message string at time.Time + podUID types.UID } type pvcPendingFailure struct { @@ -1576,7 +1577,7 @@ func latestProbeFailures(cache *ResourceCache, namespace string, now time.Time) continue } key := e.InvolvedObject.Namespace + "/" + e.InvolvedObject.Name - pf := probeFailure{reason: reason, message: strings.TrimSpace(e.Message), at: t} + pf := probeFailure{reason: reason, message: strings.TrimSpace(e.Message), at: t, podUID: e.InvolvedObject.UID} containerKey := key + "/" + eventContainerName(e.InvolvedObject.FieldPath) + "/" + reason if cur, exists := byContainer[containerKey]; !exists || t.After(cur.at) { byContainer[containerKey] = pf diff --git a/internal/k8s/restart_loop.go b/internal/k8s/restart_loop.go index 28849bc00c..9cacca72c5 100644 --- a/internal/k8s/restart_loop.go +++ b/internal/k8s/restart_loop.go @@ -74,7 +74,9 @@ type restartLoop struct { // containers before sidecars) so the evidence does not hop between containers // from one poll to the next. func activeRestartLoop(pod *corev1.Pod, probes map[string]probeFailure, now time.Time) (restartLoop, bool) { - if pod == nil || (pod.Status.Phase != corev1.PodRunning && pod.Status.Phase != corev1.PodPending) { + // A pod being deleted stops its containers on purpose: a rollout or + // drain SIGTERM ends Completed/0 and is not a crash. + if pod == nil || pod.DeletionTimestamp != nil || (pod.Status.Phase != corev1.PodRunning && pod.Status.Phase != corev1.PodPending) { return restartLoop{}, false } if pod.Spec.RestartPolicy == "" || pod.Spec.RestartPolicy == corev1.RestartPolicyAlways { @@ -123,6 +125,14 @@ func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoo if now.Sub(finished) > restartLoopWindow { return restartLoop{}, false } + // A run that lasted longer than the window was not part of a loop: a + // container that served for days and restarted once (a node or kubelet + // bounce) keeps its old restarts in RestartCount, and would otherwise + // read as looping on its first restart. A real loop is caught from its + // next, short run. + if !term.StartedAt.IsZero() && finished.Sub(term.StartedAt.Time) > restartLoopWindow { + return restartLoop{}, false + } if r := cs.State.Running; r != nil && !r.StartedAt.IsZero() && r.StartedAt.Sub(finished) > restartLoopStaleGap { return restartLoop{}, false } @@ -151,14 +161,16 @@ func withProbeEvidence(loop restartLoop, pod *corev1.Pod, probes map[string]prob // pin another container's probe on the looping one. func probeFailureFor(probes map[string]probeFailure, pod *corev1.Pod, container, reason string) (probeFailure, bool) { prefix := pod.Namespace + "/" + pod.Name + "/" - if pf, ok := probes[prefix+container+"/"+reason]; ok { - return pf, true + pf, ok := probes[prefix+container+"/"+reason] + if !ok && len(pod.Spec.Containers)+len(pod.Spec.InitContainers) == 1 { + pf, ok = probes[prefix+"/"+reason] } - if len(pod.Spec.Containers)+len(pod.Spec.InitContainers) == 1 { - pf, ok := probes[prefix+"/"+reason] - return pf, ok + // Events outlive a pod; a recreated pod with the same name (a + // StatefulSet replica) must not inherit its predecessor's failures. + if ok && pf.podUID != "" && pod.UID != "" && pf.podUID != pod.UID { + return probeFailure{}, false } - return probeFailure{}, false + return pf, ok } // restartLoopMayReplace reports whether the loop's crashloop reason may stand @@ -169,7 +181,7 @@ func probeFailureFor(probes map[string]probeFailure, pod *corev1.Pod, container, func restartLoopMayReplace(reason string) bool { switch reason { case "Running", "Pending", "Unknown", "", "PodInitializing", "ContainerCreating", - crashLoopReason, "Error", "Completed", + crashLoopReason, "Error", "Completed", "StartError", "ContainerCannotRun", highRestartReason, livenessProbeFailedReason, readinessProbeFailedReason: return true } @@ -237,7 +249,6 @@ func (l restartLoop) diagnosis() (cause, action string) { } } -// evidence is the wire form attached to the issue. func (l restartLoop) evidence() *issuesapi.RestartLoop { out := &issuesapi.RestartLoop{ Container: l.container, diff --git a/internal/k8s/restart_loop_test.go b/internal/k8s/restart_loop_test.go index f1fb920c44..8e0642913b 100644 --- a/internal/k8s/restart_loop_test.go +++ b/internal/k8s/restart_loop_test.go @@ -50,6 +50,18 @@ func TestActiveRestartLoop(t *testing.T) { {"run started 11m after the termination", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 12*time.Minute, "Error", 1))), false, false}, {"OOM is left to the OOM path", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 2*time.Minute, "OOMKilled", 137))), false, false}, {"current termination counts", restartLoopPod(now, status(5, corev1.ContainerState{Terminated: terminatedAgo(now, 5*time.Second, "Completed", 0)}, terminatedAgo(now, 40*time.Minute, "Error", 1))), true, false}, + {"first restart after a long run (node bounce)", restartLoopPod(now, status(5, running(time.Minute), &corev1.ContainerStateTerminated{ + Reason: "Error", ExitCode: 255, StartedAt: metav1.NewTime(now.Add(-72 * time.Hour)), FinishedAt: metav1.NewTime(now.Add(-2 * time.Minute)), + })), false, false}, + {"short previous run", restartLoopPod(now, status(5, running(time.Minute), &corev1.ContainerStateTerminated{ + Reason: "Error", ExitCode: 1, StartedAt: metav1.NewTime(now.Add(-5 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-2 * time.Minute)), + })), true, false}, + {"pod being deleted", func() *corev1.Pod { + p := restartLoopPod(now, status(5, corev1.ContainerState{Terminated: terminatedAgo(now, 5*time.Second, "Completed", 0)}, terminatedAgo(now, 3*time.Minute, "Error", 1))) + deleted := metav1.NewTime(now.Add(-10 * time.Second)) + p.DeletionTimestamp = &deleted + return p + }(), false, false}, {"no termination recorded", restartLoopPod(now, status(5, running(time.Minute), nil)), false, false}, {"OnFailure pod (Job worker)", func() *corev1.Pod { p := restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 2*time.Minute, "Error", 1))) @@ -140,6 +152,19 @@ func TestActiveRestartLoop_UnattributedProbeEventNeedsSingleContainer(t *testing } } +func TestActiveRestartLoop_IgnoresProbeEventsOfAPredecessorPod(t *testing.T) { + now := time.Now() + pod := restartLoopPod(now, corev1.ContainerStatus{Name: "app", RestartCount: 4, LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, time.Minute, "Completed", 0)}}) + pod.UID = "new" + key := "ns/p/app/" + livenessProbeFailedReason + if loop, _ := activeRestartLoop(pod, map[string]probeFailure{key: {at: now, podUID: "old"}}, now); loop.liveness != nil { + t.Fatal("probe event from a previous pod with the same name was attributed") + } + if loop, _ := activeRestartLoop(pod, map[string]probeFailure{key: {at: now, podUID: "new"}}, now); loop.liveness == nil { + t.Fatal("probe event of this pod should be attributed") + } +} + func TestStalledInitContainerProblem_IgnoresStartedNativeSidecar(t *testing.T) { now := time.Now() always := corev1.ContainerRestartPolicyAlways From 8c7d8a06390f4ceb95a75871bdac04cd6fd9ddaa Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 10:17:53 +0300 Subject: [PATCH 3/8] fix(issues): an epoch StartedAt is not a run start StartError / ContainerCannotRun terminations are stamped with the Unix epoch, which the run-length guard read as a decades-long run and so rejected the loop. --- internal/k8s/restart_loop.go | 5 +++-- internal/k8s/restart_loop_test.go | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/k8s/restart_loop.go b/internal/k8s/restart_loop.go index 9cacca72c5..8039e287dd 100644 --- a/internal/k8s/restart_loop.go +++ b/internal/k8s/restart_loop.go @@ -129,8 +129,9 @@ func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoo // container that served for days and restarted once (a node or kubelet // bounce) keeps its old restarts in RestartCount, and would otherwise // read as looping on its first restart. A real loop is caught from its - // next, short run. - if !term.StartedAt.IsZero() && finished.Sub(term.StartedAt.Time) > restartLoopWindow { + // next, short run. A container that never started (StartError, + // ContainerCannotRun) is stamped with the Unix epoch, which is no start. + if started := term.StartedAt.Time; started.Unix() > 0 && finished.Sub(started) > restartLoopWindow { return restartLoop{}, false } if r := cs.State.Running; r != nil && !r.StartedAt.IsZero() && r.StartedAt.Sub(finished) > restartLoopStaleGap { diff --git a/internal/k8s/restart_loop_test.go b/internal/k8s/restart_loop_test.go index 8e0642913b..3f2301020a 100644 --- a/internal/k8s/restart_loop_test.go +++ b/internal/k8s/restart_loop_test.go @@ -56,6 +56,9 @@ func TestActiveRestartLoop(t *testing.T) { {"short previous run", restartLoopPod(now, status(5, running(time.Minute), &corev1.ContainerStateTerminated{ Reason: "Error", ExitCode: 1, StartedAt: metav1.NewTime(now.Add(-5 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-2 * time.Minute)), })), true, false}, + {"start failure stamped with the Unix epoch", restartLoopPod(now, status(5, corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}}, &corev1.ContainerStateTerminated{ + Reason: "StartError", ExitCode: 128, StartedAt: metav1.NewTime(time.Unix(0, 0)), FinishedAt: metav1.NewTime(now.Add(-time.Minute)), + })), true, false}, {"pod being deleted", func() *corev1.Pod { p := restartLoopPod(now, status(5, corev1.ContainerState{Terminated: terminatedAgo(now, 5*time.Second, "Completed", 0)}, terminatedAgo(now, 3*time.Minute, "Error", 1))) deleted := metav1.NewTime(now.Add(-10 * time.Second)) From 72ecbfe188ce49cfc3b0d4a5483e9d8846e04fac Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 11:24:18 +0300 Subject: [PATCH 4/8] fix(issues): start-failure loops keep one issue and their runtime error Live testing showed a container that cannot start alternating between crashloop and container_waiting: between attempts it waits in RunContainerError. Once the loop rule holds the container has been started and terminated repeatedly, so RunContainerError is another face of the loop. The loop row now also carries the runtime's termination message as raw_message and names start failures in its cause. --- internal/k8s/detect.go | 3 +++ internal/k8s/restart_loop.go | 21 ++++++++++++++++----- internal/k8s/restart_loop_test.go | 21 +++++++++++++++++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/internal/k8s/detect.go b/internal/k8s/detect.go index ff93f0fce5..78a22a923a 100644 --- a/internal/k8s/detect.go +++ b/internal/k8s/detect.go @@ -501,6 +501,7 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { } else { looping = false } + rawMessage := "" fingerprint := "" if inv, ok := activeProbeTargetProblem(pod, reason); ok { reason = inv.reason @@ -527,6 +528,7 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { restartCount = loop.restartCount lastTermReason = loop.lastReason restartLoopEvidence = loop.evidence() + rawMessage = loop.lastMessage } cause, action, diagnosisSource := oomLimitDiagnosis(cache, pod, reason, lastTermReason, now) if cause == "" { @@ -635,6 +637,7 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { Severity: severity, Reason: reason, Message: message, + RawMessage: rawMessage, Fingerprint: fingerprint, Age: FormatAge(ageDur), AgeSeconds: int64(ageDur.Seconds()), diff --git a/internal/k8s/restart_loop.go b/internal/k8s/restart_loop.go index 8039e287dd..dc478b894a 100644 --- a/internal/k8s/restart_loop.go +++ b/internal/k8s/restart_loop.go @@ -53,8 +53,11 @@ type restartLoop struct { lastExitCode int32 lastReason string lastFinishedAt time.Time - liveness *probeFailure - readiness *probeFailure + // lastMessage is the runtime's message for the last termination; for a + // start failure it is the actual error (e.g. exec: no such file). + lastMessage string + liveness *probeFailure + readiness *probeFailure } // activeRestartLoop reports the container keeping this pod in a restart loop. @@ -143,6 +146,7 @@ func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoo lastExitCode: term.ExitCode, lastReason: term.Reason, lastFinishedAt: finished, + lastMessage: strings.TrimSpace(term.Message), }, true } @@ -177,12 +181,15 @@ func probeFailureFor(probes map[string]probeFailure, pod *corev1.Pod, container, // restartLoopMayReplace reports whether the loop's crashloop reason may stand // in for the reason the pod walk produced. Phase strings, crash-class reasons, // and probe/thrash reasons are exactly the per-tick faces of the loop. Anything -// more specific (image pulls, container-create errors, OOM, init stalls) names a -// different problem and keeps its own row. +// more specific (image pulls, config errors, OOM, init stalls) names a +// different problem and keeps its own row. RunContainerError is the Waiting +// state between attempts of a container that fails to start; once the loop +// rule holds, the container has been started and terminated repeatedly, so it +// is another face of the same loop. func restartLoopMayReplace(reason string) bool { switch reason { case "Running", "Pending", "Unknown", "", "PodInitializing", "ContainerCreating", - crashLoopReason, "Error", "Completed", "StartError", "ContainerCannotRun", + crashLoopReason, "Error", "Completed", "StartError", "ContainerCannotRun", "RunContainerError", highRestartReason, livenessProbeFailedReason, readinessProbeFailedReason: return true } @@ -228,6 +235,10 @@ func (l restartLoop) message() string { // observations live in the evidence and the message instead. func (l restartLoop) diagnosis() (cause, action string) { ref := l.ref() + if l.lastReason == "StartError" || l.lastReason == "ContainerCannotRun" { + return fmt.Sprintf("%s keeps failing to start (%s): the runtime could not start its process.", ref, l.lastReason), + "Read the runtime error on this issue: usually a missing binary or entrypoint, a bad working directory, or a permission or mount problem in the container spec." + } switch code := l.lastExitCode; code { case 0: return fmt.Sprintf("%s keeps restarting, and its last run ended with exit code 0. Under restartPolicy Always the kubelet restarts a container whenever it exits; a failed liveness probe also ends this way, because the kubelet lets the process shut down gracefully.", ref), diff --git a/internal/k8s/restart_loop_test.go b/internal/k8s/restart_loop_test.go index 3f2301020a..12074aab93 100644 --- a/internal/k8s/restart_loop_test.go +++ b/internal/k8s/restart_loop_test.go @@ -220,8 +220,22 @@ func TestDetectProblems_RestartLoopPrecedence(t *testing.T) { plainLoop := badProbe.DeepCopy() plainLoop.Name = "plain-loop" plainLoop.Spec.Containers[0].LivenessProbe = nil + // Between attempts a container that cannot start waits in + // RunContainerError; its terminations are StartError with an epoch start. + startFailure := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "start-failure", Namespace: "prod", CreationTimestamp: old}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, ContainerStatuses: []corev1.ContainerStatus{{ + Name: "app", RestartCount: 5, + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "RunContainerError"}}, + LastTerminationState: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ + Reason: "StartError", ExitCode: 128, Message: `exec: "/nonexistent": no such file or directory`, + StartedAt: metav1.NewTime(time.Unix(0, 0)), FinishedAt: metav1.NewTime(now.Add(-time.Minute)), + }}, + }}}, + } - if err := InitTestResourceCache(fake.NewClientset(badProbe, imageSibling, plainLoop)); err != nil { + if err := InitTestResourceCache(fake.NewClientset(badProbe, imageSibling, plainLoop, startFailure)); err != nil { t.Fatalf("InitTestResourceCache: %v", err) } var problems []Detection @@ -229,7 +243,7 @@ func TestDetectProblems_RestartLoopPrecedence(t *testing.T) { for time.Now().Before(deadline) { problems = DetectProblems(GetResourceCache(), "prod") if hasProblem(problems, "Pod", "bad-liveness", livenessProbeInvalidReason) && hasProblem(problems, "Pod", "image-sibling", "ImagePullBackOff") && - hasProblem(problems, "Pod", "plain-loop", crashLoopReason) { + hasProblem(problems, "Pod", "plain-loop", crashLoopReason) && hasProblem(problems, "Pod", "start-failure", crashLoopReason) { break } time.Sleep(20 * time.Millisecond) @@ -240,6 +254,9 @@ func TestDetectProblems_RestartLoopPrecedence(t *testing.T) { if got, ok := lookupProblem(problems, "Pod", "image-sibling", "ImagePullBackOff"); !ok || got.RestartLoop != nil { t.Fatalf("image-sibling = %+v (found %v), want the image pull row", got, ok) } + if got, ok := lookupProblem(problems, "Pod", "start-failure", crashLoopReason); !ok || got.RestartLoop == nil || !strings.Contains(got.RawMessage, "no such file") || !strings.Contains(got.Cause, "failing to start") { + t.Fatalf("start-failure = %+v (found %v), want a crashloop row carrying the runtime error", got, ok) + } if got, ok := lookupProblem(problems, "Pod", "plain-loop", crashLoopReason); !ok || got.Severity != "critical" || got.RestartLoop == nil { t.Fatalf("plain-loop = %+v (found %v), want a critical crashloop row on a ready tick", got, ok) } From 064d93fdd621c06350f814e62b2fcba7a2d99364 Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 12:41:39 +0300 Subject: [PATCH 5/8] fix(issues): restart-loop stress-test follow-ups - Name the container that terminated most recently, not the first that qualifies, so a container that recovered earlier in the window is not blamed for a sibling's loop. - Honor per-container restartPolicy (Kubernetes 1.34+): a container's own policy overrides the pod's when deciding whether it restarts by design. - Record startup probe failures as loop evidence and explain exit 143. - Keep critical severity when an invalid-probe-target row wins during a loop, so it does not flap with the crash cycle. - Read Series.LastObservedTime for events recorded through events.k8s.io. - Describe probe evidence as failures seen, not as failing now, and widen the crashloop catalog definition to repeated restarts of any exit code. --- internal/issues/diagnostic_context.go | 2 +- internal/k8s/detect.go | 16 +++++ internal/k8s/detect_scheduling.go | 7 ++ internal/k8s/restart_loop.go | 66 ++++++++++++++----- internal/k8s/restart_loop_test.go | 59 ++++++++++++++++- .../src/components/issues/IssuesView.tsx | 5 +- .../k8s-ui/src/components/issues/types.ts | 1 + pkg/issuesapi/catalog.go | 2 +- pkg/issuesapi/types.go | 6 +- 9 files changed, 142 insertions(+), 22 deletions(-) diff --git a/internal/issues/diagnostic_context.go b/internal/issues/diagnostic_context.go index 0485b28d21..a94a505f03 100644 --- a/internal/issues/diagnostic_context.go +++ b/internal/issues/diagnostic_context.go @@ -1014,7 +1014,7 @@ func restartLoopEvidenceMessage(l *issuesapi.RestartLoop) string { for _, p := range []struct { name string pf *issuesapi.ProbeFailure - }{{"liveness", l.LivenessProbeFailure}, {"readiness", l.ReadinessProbeFailure}} { + }{{"startup", l.StartupProbeFailure}, {"liveness", l.LivenessProbeFailure}, {"readiness", l.ReadinessProbeFailure}} { if p.pf == nil { continue } diff --git a/internal/k8s/detect.go b/internal/k8s/detect.go index 78a22a923a..62e05045c7 100644 --- a/internal/k8s/detect.go +++ b/internal/k8s/detect.go @@ -44,6 +44,10 @@ const ScaledToZeroReason = "Backing workload scaled to 0" const livenessProbeFailedReason = "LivenessProbeFailed" +// startupProbeFailedReason is evidence only: a failed startup probe restarts +// the container like a liveness failure, but it never becomes a row reason. +const startupProbeFailedReason = "StartupProbeFailed" + // Core ConfigMaps and Secrets have no kind-specific graceful termination phase. // Once deletion starts, a remaining finalizer is the only thing keeping the // object present, so delayed cleanup is actionable sooner than workload drain. @@ -495,6 +499,7 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { // (image pulls, create errors) and an active OOM keep their own // paths. Applied before the structural checks below so an invalid // probe target still wins on every tick, not only on some. + loopActive := looping if looping && restartLoopMayReplace(reason) && !health.PodHasActiveOOMKilled(pod, now) { reason = crashLoopReason message = loop.message() @@ -516,6 +521,12 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { message = init.message fingerprint = init.fingerprint } + // A structural root (an invalid probe target) that wins the row + // during a loop keeps the loop's pinned severity, or its alert + // would come and go with the crash cycle like the loop's used to. + if loopActive && fingerprint != "" { + severity = "critical" + } var restartLoopEvidence *issuesapi.RestartLoop if looping && reason == crashLoopReason { // Severity is pinned for the loop: serving or down at this @@ -1585,6 +1596,9 @@ func latestProbeFailures(cache *ResourceCache, namespace string, now time.Time) if cur, exists := byContainer[containerKey]; !exists || t.After(cur.at) { byContainer[containerKey] = pf } + if reason == startupProbeFailedReason { + continue + } if cur, exists := out[key]; exists && !t.After(cur.at) { continue } @@ -1616,6 +1630,8 @@ func classifyProbeFailureEvent(reason, msg string) (string, bool) { return livenessProbeFailedReason, true case strings.Contains(lower, "readiness probe failed"): return readinessProbeFailedReason, true + case strings.Contains(lower, "startup probe failed"): + return startupProbeFailedReason, true default: return "", false } diff --git a/internal/k8s/detect_scheduling.go b/internal/k8s/detect_scheduling.go index 7e1cdf8994..0deb11b38d 100644 --- a/internal/k8s/detect_scheduling.go +++ b/internal/k8s/detect_scheduling.go @@ -931,6 +931,13 @@ func detectAdmissionFailures(cache *ResourceCache, namespace string) []Detection // an Event, falling back to EventTime (events API v1) when the legacy // First/LastTimestamp fields are unset. func eventLastTime(e *corev1.Event) time.Time { + // An event recorded through the events.k8s.io API carries its latest + // occurrence in Series.LastObservedTime; EventTime is the first one. + if e.Series != nil && !e.Series.LastObservedTime.Time.IsZero() { + if t := e.Series.LastObservedTime.Time; t.After(e.LastTimestamp.Time) { + return t + } + } if !e.LastTimestamp.Time.IsZero() { return e.LastTimestamp.Time } diff --git a/internal/k8s/restart_loop.go b/internal/k8s/restart_loop.go index dc478b894a..9deb33d37f 100644 --- a/internal/k8s/restart_loop.go +++ b/internal/k8s/restart_loop.go @@ -58,6 +58,7 @@ type restartLoop struct { lastMessage string liveness *probeFailure readiness *probeFailure + startup *probeFailure } // activeRestartLoop reports the container keeping this pod in a restart loop. @@ -73,20 +74,27 @@ type restartLoop struct { // requiring a non-zero exit misses the loops that flap the most. An OOMKilled // last termination is left to the OOM path, which has its own category. // -// When several containers loop, the first in status order wins (regular -// containers before sidecars) so the evidence does not hop between containers -// from one poll to the next. +// When several containers qualify, the one that terminated most recently is +// the one actively failing; a container that looped earlier and has since +// recovered can still qualify for the rest of the window and must not be the +// one named. Ties keep status order (regular containers before sidecars). func activeRestartLoop(pod *corev1.Pod, probes map[string]probeFailure, now time.Time) (restartLoop, bool) { // A pod being deleted stops its containers on purpose: a rollout or // drain SIGTERM ends Completed/0 and is not a crash. if pod == nil || pod.DeletionTimestamp != nil || (pod.Status.Phase != corev1.PodRunning && pod.Status.Phase != corev1.PodPending) { return restartLoop{}, false } - if pod.Spec.RestartPolicy == "" || pod.Spec.RestartPolicy == corev1.RestartPolicyAlways { - for i := range pod.Status.ContainerStatuses { - if loop, ok := containerRestartLoop(&pod.Status.ContainerStatuses[i], now); ok { - return withProbeEvidence(loop, pod, probes), true - } + var best restartLoop + found := false + consider := func(loop restartLoop, ok bool) { + if ok && (!found || loop.lastFinishedAt.After(best.lastFinishedAt)) { + best, found = loop, true + } + } + for i := range pod.Status.ContainerStatuses { + cs := &pod.Status.ContainerStatuses[i] + if containerRestartsAlways(pod, cs.Name) { + consider(containerRestartLoop(cs, now)) } } for i := range pod.Status.InitContainerStatuses { @@ -94,12 +102,28 @@ func activeRestartLoop(pod *corev1.Pod, probes map[string]probeFailure, now time if !isNativeSidecarName(pod, cs.Name) { continue } - if loop, ok := containerRestartLoop(cs, now); ok { - loop.sidecar = true - return withProbeEvidence(loop, pod, probes), true + loop, ok := containerRestartLoop(cs, now) + loop.sidecar = true + consider(loop, ok) + } + if !found { + return restartLoop{}, false + } + return withProbeEvidence(best, pod, probes), true +} + +// containerRestartsAlways reports whether the kubelet restarts this regular +// container after every exit: its own restartPolicy when set (per-container +// policies, Kubernetes 1.34+), else the pod's. OnFailure and Never containers +// (Job workers) retry or stop by design and are not restart loops. +func containerRestartsAlways(pod *corev1.Pod, name string) bool { + policy := corev1.ContainerRestartPolicy(pod.Spec.RestartPolicy) + for i := range pod.Spec.Containers { + if c := &pod.Spec.Containers[i]; c.Name == name && c.RestartPolicy != nil { + policy = *c.RestartPolicy } } - return restartLoop{}, false + return policy == "" || policy == corev1.ContainerRestartPolicyAlways } func isNativeSidecarName(pod *corev1.Pod, name string) bool { @@ -157,6 +181,9 @@ func withProbeEvidence(loop restartLoop, pod *corev1.Pod, probes map[string]prob if pf, ok := probeFailureFor(probes, pod, loop.container, readinessProbeFailedReason); ok { loop.readiness = &pf } + if pf, ok := probeFailureFor(probes, pod, loop.container, startupProbeFailedReason); ok { + loop.startup = &pf + } return loop } @@ -215,6 +242,9 @@ func (l restartLoop) lastExit() string { func (l restartLoop) message() string { msg := fmt.Sprintf("%s is in a restart loop: %d restarts, last %s", l.ref(), l.restartCount, l.lastExit()) var seen []string + if l.startup != nil { + seen = append(seen, "startup") + } if l.liveness != nil { seen = append(seen, "liveness") } @@ -222,7 +252,7 @@ func (l restartLoop) message() string { seen = append(seen, "readiness") } if len(seen) > 0 { - msg += "; " + strings.Join(seen, " and ") + " probe failures observed" + msg += "; " + strings.Join(seen, ", ") + " probe failures observed" } return msg } @@ -241,8 +271,8 @@ func (l restartLoop) diagnosis() (cause, action string) { } switch code := l.lastExitCode; code { case 0: - return fmt.Sprintf("%s keeps restarting, and its last run ended with exit code 0. Under restartPolicy Always the kubelet restarts a container whenever it exits; a failed liveness probe also ends this way, because the kubelet lets the process shut down gracefully.", ref), - "Check the pod's Unhealthy events for liveness probe failures; if present, check the probe against the app (path and port, timeoutSeconds, failureThreshold, startup time or a missing startupProbe). If not, check why the main process exits: command and args, a process that daemonizes, or one-shot work that belongs in a Job." + return fmt.Sprintf("%s keeps restarting, and its last run ended with exit code 0. Under restartPolicy Always the kubelet restarts a container whenever it exits; a failed liveness or startup probe also ends this way, because the kubelet lets the process shut down gracefully.", ref), + "Check the pod's Unhealthy events for liveness or startup probe failures; if present, check the probe against the app (path and port, timeoutSeconds, failureThreshold, startup time). If not, check why the main process exits: command and args, a process that daemonizes, or one-shot work that belongs in a Job." case 127: return fmt.Sprintf("%s keeps restarting after exit code 127: command not found.", ref), "Check the image entrypoint and pod command/args; verify the binary exists in the image." @@ -252,6 +282,9 @@ func (l restartLoop) diagnosis() (cause, action string) { case 139: return fmt.Sprintf("%s keeps restarting after exit code 139: segmentation fault.", ref), "Inspect previous container logs and recent image/code changes; check native libraries or unsafe code for a segfault." + case 143: + return fmt.Sprintf("%s keeps restarting after exit code 143 (SIGTERM).", ref), + "A process that exits 143 on SIGTERM (common for JVM and Node apps) was told to stop: check the pod's Unhealthy events for a liveness or startup probe restart, and the previous container logs for shutdown context." case 137: return fmt.Sprintf("%s keeps restarting after exit code 137 (SIGKILL), but Kubernetes did not report OOMKilled.", ref), "Check for a liveness probe kill that outlived its grace period, node pressure, process-level SIGKILLs, and memory limits; inspect previous container logs for shutdown context." @@ -276,5 +309,8 @@ func (l restartLoop) evidence() *issuesapi.RestartLoop { if l.readiness != nil { out.ReadinessProbeFailure = &issuesapi.ProbeFailure{LastSeen: l.readiness.at.UTC(), Message: Truncate(l.readiness.message, restartLoopProbeMessageMax)} } + if l.startup != nil { + out.StartupProbeFailure = &issuesapi.ProbeFailure{LastSeen: l.startup.at.UTC(), Message: Truncate(l.startup.message, restartLoopProbeMessageMax)} + } return out } diff --git a/internal/k8s/restart_loop_test.go b/internal/k8s/restart_loop_test.go index 12074aab93..65eafe823a 100644 --- a/internal/k8s/restart_loop_test.go +++ b/internal/k8s/restart_loop_test.go @@ -248,7 +248,7 @@ func TestDetectProblems_RestartLoopPrecedence(t *testing.T) { } time.Sleep(20 * time.Millisecond) } - if got, ok := lookupProblem(problems, "Pod", "bad-liveness", livenessProbeInvalidReason); !ok || got.Fingerprint == "" || got.RestartLoop != nil { + if got, ok := lookupProblem(problems, "Pod", "bad-liveness", livenessProbeInvalidReason); !ok || got.Fingerprint == "" || got.RestartLoop != nil || got.Severity != "critical" { t.Fatalf("bad-liveness = %+v (found %v), want the fingerprinted invalid-probe row", got, ok) } if got, ok := lookupProblem(problems, "Pod", "image-sibling", "ImagePullBackOff"); !ok || got.RestartLoop != nil { @@ -261,3 +261,60 @@ func TestDetectProblems_RestartLoopPrecedence(t *testing.T) { t.Fatalf("plain-loop = %+v (found %v), want a critical crashloop row on a ready tick", got, ok) } } + +func TestActiveRestartLoop_NamesTheContainerFailingNow(t *testing.T) { + now := time.Now() + recovered := corev1.ContainerStatus{Name: "api", Ready: true, RestartCount: 3, + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(now.Add(-19 * time.Minute))}}, + LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, 20*time.Minute, "Error", 1)}} + failing := corev1.ContainerStatus{Name: "sql-proxy", RestartCount: 4, + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}}, + LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, time.Minute, "Error", 2)}} + pod := restartLoopPod(now, recovered) + pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{Name: "sql-proxy"}) + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, failing) + loop, ok := activeRestartLoop(pod, nil, now) + if !ok || loop.container != "sql-proxy" || loop.lastExitCode != 2 { + t.Fatalf("loop = %+v, want the container that crashed a minute ago", loop) + } +} + +func TestActiveRestartLoop_UsesContainerRestartPolicy(t *testing.T) { + now := time.Now() + cs := corev1.ContainerStatus{Name: "app", RestartCount: 5, LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, time.Minute, "Error", 1)}} + always, onFailure := corev1.ContainerRestartPolicyAlways, corev1.ContainerRestartPolicyOnFailure + + jobPodAlwaysContainer := restartLoopPod(now, cs) + jobPodAlwaysContainer.Spec.RestartPolicy = corev1.RestartPolicyOnFailure + jobPodAlwaysContainer.Spec.Containers[0].RestartPolicy = &always + if _, ok := activeRestartLoop(jobPodAlwaysContainer, nil, now); !ok { + t.Fatal("a container with restartPolicy Always loops whatever the pod's policy") + } + alwaysPodOnFailureContainer := restartLoopPod(now, cs) + alwaysPodOnFailureContainer.Spec.Containers[0].RestartPolicy = &onFailure + if _, ok := activeRestartLoop(alwaysPodOnFailureContainer, nil, now); ok { + t.Fatal("a container with restartPolicy OnFailure retries by design and is not a restart loop") + } +} + +func TestActiveRestartLoop_StartupProbeEvidence(t *testing.T) { + now := time.Now() + pod := restartLoopPod(now, corev1.ContainerStatus{Name: "app", RestartCount: 4, LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, time.Minute, "Error", 143)}}) + probes := map[string]probeFailure{"ns/p/app/" + startupProbeFailedReason: {reason: startupProbeFailedReason, at: now, message: "Startup probe failed: connection refused"}} + loop, ok := activeRestartLoop(pod, probes, now) + if !ok || loop.startup == nil || loop.evidence().StartupProbeFailure == nil || !strings.Contains(loop.message(), "startup probe failures observed") { + t.Fatalf("loop = %+v, want startup probe evidence", loop) + } + if cause, _ := loop.diagnosis(); !strings.Contains(cause, "143 (SIGTERM)") { + t.Fatalf("cause = %q, want the SIGTERM exit explained", cause) + } +} + +func TestEventLastTime_UsesSeriesLastObservedTime(t *testing.T) { + first := metav1.NewMicroTime(time.Now().Add(-time.Hour)) + last := metav1.NewMicroTime(time.Now().Add(-time.Minute)) + e := &corev1.Event{EventTime: first, Series: &corev1.EventSeries{LastObservedTime: last}} + if got := eventLastTime(e); !got.Equal(last.Time) { + t.Fatalf("eventLastTime = %v, want the series' last observation %v", got, last.Time) + } +} diff --git a/packages/k8s-ui/src/components/issues/IssuesView.tsx b/packages/k8s-ui/src/components/issues/IssuesView.tsx index 40f377ea0b..743c355142 100644 --- a/packages/k8s-ui/src/components/issues/IssuesView.tsx +++ b/packages/k8s-ui/src/components/issues/IssuesView.tsx @@ -406,8 +406,9 @@ function Diagnosis({ issue, source }: { issue: Issue; source?: IssueDiagnosisSou ? [ issue.restart_count ? `${issue.restart_count} restart${issue.restart_count === 1 ? '' : 's'}` : null, issue.last_terminated_reason ? `last exit: ${issue.last_terminated_reason}` : null, - issue.restart_loop?.liveness_probe_failure ? 'liveness probe failing' : null, - issue.restart_loop?.readiness_probe_failure ? 'readiness probe failing' : null, + issue.restart_loop?.startup_probe_failure ? 'startup probe failures seen' : null, + issue.restart_loop?.liveness_probe_failure ? 'liveness probe failures seen' : null, + issue.restart_loop?.readiness_probe_failure ? 'readiness probe failures seen' : null, ] .filter(Boolean) .join(' · ') diff --git a/packages/k8s-ui/src/components/issues/types.ts b/packages/k8s-ui/src/components/issues/types.ts index 1333f1217c..8008ade8c2 100644 --- a/packages/k8s-ui/src/components/issues/types.ts +++ b/packages/k8s-ui/src/components/issues/types.ts @@ -378,4 +378,5 @@ export interface IssueRestartLoop { last_finished_at?: string; liveness_probe_failure?: IssueProbeFailure; readiness_probe_failure?: IssueProbeFailure; + startup_probe_failure?: IssueProbeFailure; } diff --git a/pkg/issuesapi/catalog.go b/pkg/issuesapi/catalog.go index 85b43cac68..3fc6e3f402 100644 --- a/pkg/issuesapi/catalog.go +++ b/pkg/issuesapi/catalog.go @@ -100,7 +100,7 @@ var categoryDescription = map[Category]string{ CategoryContainerWaiting: "A container is stuck Waiting and never reached Running — blocked on config, secrets, volumes, its image, or a pod sandbox / IP from the CNI.", CategoryInitContainerFailed: "An init container is failing or looping, so the main containers never start.", // Runtime - CategoryCrashLoop: "A container keeps crashing and restarting (CrashLoopBackOff) — it exits non-zero shortly after starting.", + CategoryCrashLoop: "A container keeps exiting and being restarted (CrashLoopBackOff or repeated restarts) — it crashes, fails a liveness or startup probe, or its process exits, whatever the exit code.", CategoryOOMKilled: "A container was OOMKilled — it hit its own memory limit, or the node ran out of memory. Check usage vs limits and node memory pressure before raising limits.", CategoryLivenessProbeFail: "The liveness probe keeps failing, so the kubelet repeatedly restarts the container.", CategoryReadinessFailed: "The readiness probe is failing, so the pod is kept out of Service endpoints and receives no traffic.", diff --git a/pkg/issuesapi/types.go b/pkg/issuesapi/types.go index 8bf88fa7cc..6ebcf32003 100644 --- a/pkg/issuesapi/types.go +++ b/pkg/issuesapi/types.go @@ -370,10 +370,12 @@ type RestartLoop struct { LastExitCode int32 `json:"last_exit_code"` LastReason string `json:"last_reason,omitempty"` LastFinishedAt time.Time `json:"last_finished_at,omitzero"` - // LivenessProbeFailure / ReadinessProbeFailure are the newest failure of - // each probe type seen for this container in the last 10 minutes. + // LivenessProbeFailure / ReadinessProbeFailure / StartupProbeFailure are + // the newest failure of each probe type seen for this container in the + // last 10 minutes. LivenessProbeFailure *ProbeFailure `json:"liveness_probe_failure,omitempty"` ReadinessProbeFailure *ProbeFailure `json:"readiness_probe_failure,omitempty"` + StartupProbeFailure *ProbeFailure `json:"startup_probe_failure,omitempty"` } // ProbeFailure is one observed probe failure: when it was last reported and From d483e0f53b1f2e90e6b4c1d23827ece38c4cbfaa Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 14:04:00 +0300 Subject: [PATCH 6/8] feat(issues): tier restart-loop severity, clear on recovery, cover init and OOM loops - Severity: a loop is critical only when it is fast (last run under 10 minutes, the kubelet's backoff-reset line) and covers at least half of its workload's live pods; a slow loop or one bad replica among many is a warning. Both inputs hold steady through a loop. - A loop row folds its workload's workload_degraded row at any severity: the loop explains the unavailability, and the "N/M available" row is critical whenever a replica is down and otherwise comes and goes with the crash cycle. Invalid-probe-target rows during a loop carry the loop evidence so they fold the same way. - Recovery: a loop ends once the container is Ready and has run longer than max(10m, 2x its last run), instead of holding 30 minutes after the last crash. - Init:CrashLoopBackOff: ordinary init containers that keep failing while the pod is Pending are loops; their long attempts are not also reported as a stalled init container. - OOM loops get the same continuity and classify as oom_killed; another container's active OOM still owns the row; the limit-discrepancy diagnosis follows the looping container between kills, and the plain OOM diagnosis names node pressure when there is no limit. --- internal/issues/dedupe.go | 11 +- internal/issues/dedupe_test.go | 17 ++ .../issues/restart_loop_integration_test.go | 15 +- internal/k8s/detect.go | 65 ++++-- internal/k8s/detect_oom.go | 14 +- internal/k8s/detect_oom_test.go | 4 +- internal/k8s/restart_loop.go | 203 ++++++++++++++++-- internal/k8s/restart_loop_test.go | 165 +++++++++++++- pkg/issuesapi/types.go | 4 +- 9 files changed, 438 insertions(+), 60 deletions(-) diff --git a/internal/issues/dedupe.go b/internal/issues/dedupe.go index d5abf457ba..4d4ee0f949 100644 --- a/internal/issues/dedupe.go +++ b/internal/issues/dedupe.go @@ -151,9 +151,17 @@ func dedupeWorkloadDegradedOverChild(in []Issue) []Issue { // Per subject, the worst severity among its specific child-symptom rows. maxChildSev := map[string]int{} maxCreationChildSev := map[string]int{} + // A restart loop explains its workload's unavailability at any severity: + // a slow loop or one bad replica is a warning, while the Deployment's + // "N/M available" row is critical whenever a replica is down and comes + // and goes with the crash cycle. + loopChild := map[string]bool{} for _, i := range in { if childCategories[i.Category] { k := subjectKeyOf(subjectRef(i)) + if i.RestartLoop != nil { + loopChild[k] = true + } if r := SeverityRank(i.Severity); r > maxChildSev[k] { maxChildSev[k] = r } @@ -186,7 +194,8 @@ func dedupeWorkloadDegradedOverChild(in []Issue) []Issue { if i.Reason == "ReplicaFailure" { sev = maxCreationChildSev } - if r, ok := sev[k]; ok && r >= SeverityRank(i.Severity) { + loopFolds := i.Category == issuesapi.CategoryWorkloadDegraded && loopChild[k] + if r, ok := sev[k]; loopFolds || (ok && r >= SeverityRank(i.Severity)) { if i.IssueTiming != "" { if prev, seen := suppressedIssueTiming[k]; seen && prev != i.IssueTiming { suppressedIssueTiming[k] = "" diff --git a/internal/issues/dedupe_test.go b/internal/issues/dedupe_test.go index 3358a8bd8a..8cc1e2beb1 100644 --- a/internal/issues/dedupe_test.go +++ b/internal/issues/dedupe_test.go @@ -126,6 +126,23 @@ func TestDedupeWorkloadDegradedOverChild_Phase0(t *testing.T) { } }) + t.Run("a warning restart loop folds its workload's critical availability row", func(t *testing.T) { + degraded := Issue{Source: SourceProblem, Group: "apps", Kind: "Deployment", Namespace: "ns", Name: "web", + Category: issuesapi.CategoryWorkloadDegraded, Severity: SeverityCritical, Reason: "9/10 available"} + loop := Issue{Source: SourceProblem, Kind: "Pod", Namespace: "ns", Name: "web-abc", Owner: dep, + Category: issuesapi.CategoryCrashLoop, Severity: SeverityWarning, RestartLoop: &issuesapi.RestartLoop{Container: "app"}} + out := dedupeWorkloadDegradedOverChild([]Issue{degraded, loop}) + if hasCategory(out, issuesapi.CategoryWorkloadDegraded) { + t.Fatalf("the loop explains the unavailability and must own it at any severity, got %+v", out) + } + stalled := Issue{Source: SourceProblem, Group: "apps", Kind: "Deployment", Namespace: "ns", Name: "web", + Category: issuesapi.CategoryRolloutStalled, Severity: SeverityCritical, Reason: "Rollout stuck"} + out = dedupeWorkloadDegradedOverChild([]Issue{stalled, loop}) + if !hasCategory(out, issuesapi.CategoryRolloutStalled) { + t.Fatalf("rollout_stalled keeps the severity gate; a warning loop must not hide it, got %+v", out) + } + }) + t.Run("cronjob_failed is not a rollup and survives alongside an unrelated job_failed", func(t *testing.T) { cron := Issue{Source: SourceProblem, Group: "batch", Kind: "CronJob", Namespace: "ns", Name: "nightly", Category: issuesapi.CategoryCronJobFailed, Severity: SeverityWarning, Reason: "stale"} diff --git a/internal/issues/restart_loop_integration_test.go b/internal/issues/restart_loop_integration_test.go index 7f32dfcd18..5dd61b247d 100644 --- a/internal/issues/restart_loop_integration_test.go +++ b/internal/issues/restart_loop_integration_test.go @@ -167,10 +167,10 @@ func TestCompose_RestartLoopKeepsOneIssueAcrossTheCycle(t *testing.T) { cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(8 * time.Minute)} cs.LastTerminationState.Terminated = completed(8*time.Minute + 10*time.Second) })}, - {name: "serving-29m-after-last-crash", ready: true, status: with(func(cs *corev1.ContainerStatus) { + {name: "serving-9m-after-restart", ready: true, status: with(func(cs *corev1.ContainerStatus) { cs.Ready = true - cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(28 * time.Minute)} - cs.LastTerminationState.Terminated = completed(29 * time.Minute) + cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(9 * time.Minute)} + cs.LastTerminationState.Terminated = completed(9*time.Minute + 10*time.Second) })}, } @@ -226,15 +226,16 @@ func TestCompose_RestartLoopKeepsOneIssueAcrossTheCycle(t *testing.T) { } } - // 31 minutes after the last crash, the loop is over: no issue. + // Serving for 11 minutes after 3-minute runs: recovered, the loop is + // over well before its 30-minute window would have ended it. issues, _ := composeRestartLoopTick(t, now, restartLoopTick{name: "recovered", ready: true, status: with(func(cs *corev1.ContainerStatus) { cs.Ready = true - cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(30 * time.Minute)} - cs.LastTerminationState.Terminated = completed(31 * time.Minute) + cs.State.Running = &corev1.ContainerStateRunning{StartedAt: at(11 * time.Minute)} + cs.LastTerminationState.Terminated = completed(11*time.Minute + 10*time.Second) })}) for _, iss := range issues { if iss.Name == "gateway" || iss.Name == "gateway-abc" { - t.Fatalf("recovered: issue %+v still open 31m after the last crash", iss) + t.Fatalf("recovered: issue %+v still open after 11 Ready minutes", iss) } } } diff --git a/internal/k8s/detect.go b/internal/k8s/detect.go index 62e05045c7..f3247dc385 100644 --- a/internal/k8s/detect.go +++ b/internal/k8s/detect.go @@ -456,6 +456,7 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { } probeFailures, containerProbeFailures := latestProbeFailures(cache, namespace, now) + var loopRows []loopRow pvcPendingFailures := latestPVCPendingFailures(cache, namespace) // Pod problems: high-signal container waiting/terminated states, old @@ -500,7 +501,16 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { // paths. Applied before the structural checks below so an invalid // probe target still wins on every tick, not only on some. loopActive := looping - if looping && restartLoopMayReplace(reason) && !health.PodHasActiveOOMKilled(pod, now) { + // An OOM loop replaces the OOM reason too: the row keeps the + // crashloop reason and classifies as oom_killed through the loop + // container's OOMKilled termination. Another container's active + // OOM still owns the row. + oomLoop := looping && loop.lastReason == "OOMKilled" + oomVeto := health.PodHasActiveOOMKilled(pod, now) + if oomLoop { + oomVeto = otherContainerActiveOOM(pod, loop.container, now) + } + if looping && (restartLoopMayReplace(reason) || (oomLoop && reason == "OOMKilled")) && !oomVeto { reason = crashLoopReason message = loop.message() } else { @@ -516,34 +526,39 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { reason = earlyProbeTargetProblem.reason message = earlyProbeTargetProblem.message fingerprint = earlyProbeTargetProblem.fingerprint - } else if init, ok := stalledInitContainerProblem(pod, now); ok { + } else if init, ok := stalledInitContainerProblem(pod, now, loopingInitContainer(loop, looping)); ok { reason = init.reason message = init.message fingerprint = init.fingerprint } - // A structural root (an invalid probe target) that wins the row - // during a loop keeps the loop's pinned severity, or its alert - // would come and go with the crash cycle like the loop's used to. - if loopActive && fingerprint != "" { - severity = "critical" - } + // Severity is pinned for the loop (set by setLoopSeverities once + // every pod has been seen): serving or down at this instant is a + // per-tick fact, and letting it move the severity would drop the + // issue out of severity-filtered alerts and bring it back on every + // cycle. A structural root (an invalid probe target) that wins the + // row during a loop is pinned the same way. + pinLoopSeverity := loopActive && (fingerprint != "" || (looping && reason == crashLoopReason)) + // Rows pinned for a loop carry its evidence, including a + // structural root's row: the evidence is what lets the issues + // layer fold the workload's availability row into either. var restartLoopEvidence *issuesapi.RestartLoop - if looping && reason == crashLoopReason { - // Severity is pinned for the loop: serving or down at this - // instant is a per-tick fact, and letting it move the severity - // would drop the issue out of severity-filtered alerts and - // bring it back on every cycle. The loop container's own - // restart count and last termination replace the pod-wide - // ones so every field on the row describes one container. - severity = "critical" + var oomStatuses []corev1.ContainerStatus + if pinLoopSeverity { + restartLoopEvidence = loop.evidence() + } + isLoopRow := looping && reason == crashLoopReason + if isLoopRow { + // The loop container's own restart count and last termination + // replace the pod-wide ones so every field on the row + // describes one container. restartCount = loop.restartCount lastTermReason = loop.lastReason - restartLoopEvidence = loop.evidence() rawMessage = loop.lastMessage + oomStatuses = loop.oomStatuses() } - cause, action, diagnosisSource := oomLimitDiagnosis(cache, pod, reason, lastTermReason, now) + cause, action, diagnosisSource := oomLimitDiagnosis(cache, pod, reason, lastTermReason, oomStatuses, now) if cause == "" { - if restartLoopEvidence != nil { + if isLoopRow { cause, action = loop.diagnosis() } else if reason == crashLoopReason { cause, action = health.PodCrashLoopDiagnosis(pod, now) @@ -668,9 +683,13 @@ func DetectProblems(cache *ResourceCache, namespace string) []Detection { // The evidence above classifies workload/rollout timing, not when this // Pod's specific waiting or termination reason began. setDetectionOnset(&detection, now, time.Time{}) + if pinLoopSeverity { + loopRows = append(loopRows, loopRow{index: len(problems), owner: loopOwnerKey(pod, ownerGroup, ownerKind, ownerName), fast: loop.fast()}) + } problems = append(problems, detection) } } + setLoopSeverities(cache, problems, loopRows, podsByNamespace) // Service problems: routing health that workload .status often misses. // EndpointSlice would be the strongest source for realized backend state, @@ -1374,12 +1393,16 @@ func missingNamedProbePort(c corev1.Container, probe *corev1.Probe) (string, boo return port.StrVal, true } -func stalledInitContainerProblem(pod *corev1.Pod, now time.Time) (podSpecificProblem, bool) { +// stalledInitContainerProblem reports an init container that has been running +// long enough to block the pod. skip names an init container already reported +// as a restart loop: each of its long attempts is part of that loop, not a +// separate stall. +func stalledInitContainerProblem(pod *corev1.Pod, now time.Time, skip string) (podSpecificProblem, bool) { if pod.Status.Phase != corev1.PodPending { return podSpecificProblem{}, false } for _, cs := range pod.Status.InitContainerStatuses { - if cs.State.Running == nil || cs.State.Running.StartedAt.IsZero() { + if cs.State.Running == nil || cs.State.Running.StartedAt.IsZero() || cs.Name == skip { continue } // A started native sidecar is meant to keep running and no longer diff --git a/internal/k8s/detect_oom.go b/internal/k8s/detect_oom.go index 11c78fbeb1..77d2bdb625 100644 --- a/internal/k8s/detect_oom.go +++ b/internal/k8s/detect_oom.go @@ -19,12 +19,16 @@ type oomLimitDiscrepancy struct { limitSource string } -func oomLimitDiagnosis(cache *ResourceCache, pod *corev1.Pod, reason, lastTerminatedReason string, now time.Time) (string, string, *corev1.ObjectReference) { +// oomLimitDiagnosis explains an OOM against the owning ReplicaSet's template +// limit. statuses are the OOMKilled containers to check; nil means the pod's +// currently active OOMs (an OOM restart loop passes its own container, so the +// diagnosis holds between kills instead of lapsing with the active window). +func oomLimitDiagnosis(cache *ResourceCache, pod *corev1.Pod, reason, lastTerminatedReason string, statuses []corev1.ContainerStatus, now time.Time) (string, string, *corev1.ObjectReference) { if !podReasonClassifiesAsOOM(reason, lastTerminatedReason) { return "", "", nil } - discrepancy, ok := activeOOMLimitDiscrepancy(cache, pod, now) + discrepancy, ok := activeOOMLimitDiscrepancy(cache, pod, statuses, now) if !ok { return "", "", nil } @@ -57,8 +61,10 @@ func oomLimitDiagnosis(cache *ResourceCache, pod *corev1.Pod, reason, lastTermin return cause, action, &corev1.ObjectReference{APIVersion: "apps/v1", Kind: "ReplicaSet", Namespace: pod.Namespace, Name: discrepancy.replicaSet} } -func activeOOMLimitDiscrepancy(cache *ResourceCache, pod *corev1.Pod, now time.Time) (oomLimitDiscrepancy, bool) { - statuses := health.ActiveOOMKilledContainers(pod, now) +func activeOOMLimitDiscrepancy(cache *ResourceCache, pod *corev1.Pod, statuses []corev1.ContainerStatus, now time.Time) (oomLimitDiscrepancy, bool) { + if statuses == nil { + statuses = health.ActiveOOMKilledContainers(pod, now) + } if len(statuses) == 0 || cache == nil { return oomLimitDiscrepancy{}, false } diff --git a/internal/k8s/detect_oom_test.go b/internal/k8s/detect_oom_test.go index d69f6d4b69..2cd55f2312 100644 --- a/internal/k8s/detect_oom_test.go +++ b/internal/k8s/detect_oom_test.go @@ -416,7 +416,9 @@ func TestDetectProblems_OOMLimitDiscrepancy(t *testing.T) { if !strings.Contains(detection.Action, "ReplicaSet template") || !strings.Contains(detection.Action, "changed after Pod creation") || !strings.Contains(detection.Action, "VPA") || !strings.Contains(detection.Action, "admission") { t.Errorf("action does not identify bounded mutation sources: %q", detection.Action) } - } else if detection.Cause != "" || detection.Action != "" { + } else if strings.Contains(detection.Cause, "ReplicaSet") || strings.Contains(detection.Action, "ReplicaSet template") { + // A restart loop still carries its generic OOM diagnosis; what + // must be absent is the limit-discrepancy one. t.Fatalf("unexpected OOM discrepancy diagnosis: cause=%q action=%q", detection.Cause, detection.Action) } }) diff --git a/internal/k8s/restart_loop.go b/internal/k8s/restart_loop.go index 9deb33d37f..458324ab6a 100644 --- a/internal/k8s/restart_loop.go +++ b/internal/k8s/restart_loop.go @@ -7,6 +7,7 @@ import ( corev1 "k8s.io/api/core/v1" + "github.com/skyhook-io/radar/pkg/health" "github.com/skyhook-io/radar/pkg/issuesapi" ) @@ -39,6 +40,14 @@ const ( // container that restarted three times last month and once a minute ago // also qualifies, for at most restartLoopWindow. restartLoopMinRestarts = 3 + // restartLoopFastRun is the run length below which a loop is "fast": the + // container spends most of its time down or in backoff. It is the + // kubelet's own line: a container that runs longer than 10 minutes has its + // crash backoff reset. + restartLoopFastRun = 10 * time.Minute + // restartLoopImpactShare is the share of a workload's pods that must be + // looping for a fast loop to be critical. + restartLoopImpactShare = 0.5 // restartLoopProbeMessageMax caps the probe output copied into evidence; // exec probes can print arbitrary amounts. restartLoopProbeMessageMax = 300 @@ -47,12 +56,20 @@ const ( // restartLoop is the evidence behind an active restart loop, taken from one // container so every field describes the same termination. type restartLoop struct { - container string - sidecar bool + container string + sidecar bool + init bool + // status is the looping container's status, for diagnoses that need it. + status corev1.ContainerStatus + // memoryLimit reports whether the container declares a memory limit. + memoryLimit bool restartCount int32 lastExitCode int32 lastReason string lastFinishedAt time.Time + // lastRun is how long the terminated run lasted; zero when the container + // never started (StartError) or the runtime did not record a start. + lastRun time.Duration // lastMessage is the runtime's message for the last termination; for a // start failure it is the actual error (e.g. exec: no such file). lastMessage string @@ -63,16 +80,18 @@ type restartLoop struct { // activeRestartLoop reports the container keeping this pod in a restart loop. // -// Eligible containers are those the kubelet restarts by design: regular -// containers of a restartPolicy=Always pod and native sidecars (init -// containers with restartPolicy=Always). Ordinary init containers retrying -// until they succeed, and OnFailure/Never pods such as Job workers, are -// excluded; their retries are handled by the init and Job paths. +// Eligible containers are those the kubelet keeps restarting: regular +// containers whose effective restartPolicy is Always, native sidecars (init +// containers with restartPolicy=Always), and ordinary init containers that +// keep failing while the pod is still Pending (Init:CrashLoopBackOff). Job +// workers (OnFailure/Never) retry by design and are excluded. // -// The exit code is deliberately ignored. The kubelet backs off restarts after -// any exit, and a liveness kill with a graceful shutdown ends Completed/0, so -// requiring a non-zero exit misses the loops that flap the most. An OOMKilled -// last termination is left to the OOM path, which has its own category. +// The exit code is deliberately ignored for regular containers and sidecars. +// The kubelet backs off restarts after any exit, and a liveness kill with a +// graceful shutdown ends Completed/0, so requiring a non-zero exit misses the +// loops that flap the most. An ordinary init container that exits 0 has done +// its job, so only its failures count. OOMKilled terminations count too; the +// row then classifies as oom_killed through its last termination reason. // // When several containers qualify, the one that terminated most recently is // the one actively failing; a container that looped earlier and has since @@ -99,19 +118,63 @@ func activeRestartLoop(pod *corev1.Pod, probes map[string]probeFailure, now time } for i := range pod.Status.InitContainerStatuses { cs := &pod.Status.InitContainerStatuses[i] - if !isNativeSidecarName(pod, cs.Name) { - continue - } loop, ok := containerRestartLoop(cs, now) - loop.sidecar = true + if isNativeSidecarName(pod, cs.Name) { + loop.sidecar = true + } else { + // An ordinary init container blocks the pod until it succeeds; + // it loops only while the pod is still initializing and its + // latest attempt failed. + loop.init = true + ok = ok && pod.Status.Phase == corev1.PodPending && loop.lastExitCode != 0 + } consider(loop, ok) } if !found { return restartLoop{}, false } + best.memoryLimit = containerHasMemoryLimit(pod, best.container) return withProbeEvidence(best, pod, probes), true } +func containerHasMemoryLimit(pod *corev1.Pod, name string) bool { + for _, list := range [][]corev1.Container{pod.Spec.Containers, pod.Spec.InitContainers} { + for i := range list { + if list[i].Name == name { + _, ok := list[i].Resources.Limits[corev1.ResourceMemory] + return ok + } + } + } + return false +} + +// otherContainerActiveOOM reports an active OOM on any container other than +// the named one. A sibling's OOM owns the pod's row over a different +// container's loop; the loop container's own OOM does not veto its loop. +func otherContainerActiveOOM(pod *corev1.Pod, name string, now time.Time) bool { + for _, cs := range health.ActiveOOMKilledContainers(pod, now) { + if cs.Name != name { + return true + } + } + for _, cs := range pod.Status.InitContainerStatuses { + if cs.Name != name && cs.State.Terminated != nil && cs.State.Terminated.Reason == "OOMKilled" { + return true + } + } + return false +} + +// oomStatuses is the container an OOM loop's limit diagnosis should examine: +// the looping regular container, or nil (the pod's active OOMs) otherwise. +func (l restartLoop) oomStatuses() []corev1.ContainerStatus { + if l.lastReason != "OOMKilled" || l.init || l.sidecar { + return nil + } + return []corev1.ContainerStatus{l.status} +} + // containerRestartsAlways reports whether the kubelet restarts this regular // container after every exit: its own restartPolicy when set (per-container // policies, Kubernetes 1.34+), else the pod's. OnFailure and Never containers @@ -145,7 +208,7 @@ func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoo if term == nil { term = cs.LastTerminationState.Terminated } - if term == nil || term.FinishedAt.IsZero() || term.Reason == "OOMKilled" { + if term == nil || term.FinishedAt.IsZero() { return restartLoop{}, false } finished := term.FinishedAt.Time @@ -158,18 +221,37 @@ func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoo // read as looping on its first restart. A real loop is caught from its // next, short run. A container that never started (StartError, // ContainerCannotRun) is stamped with the Unix epoch, which is no start. - if started := term.StartedAt.Time; started.Unix() > 0 && finished.Sub(started) > restartLoopWindow { - return restartLoop{}, false + var lastRun time.Duration + if started := term.StartedAt.Time; started.Unix() > 0 { + lastRun = finished.Sub(started) } - if r := cs.State.Running; r != nil && !r.StartedAt.IsZero() && r.StartedAt.Sub(finished) > restartLoopStaleGap { + if lastRun > restartLoopWindow { return restartLoop{}, false } + if r := cs.State.Running; r != nil && !r.StartedAt.IsZero() { + if r.StartedAt.Sub(finished) > restartLoopStaleGap { + return restartLoop{}, false + } + // Recovered: the container is serving and its current run has + // clearly outlived the loop's rhythm. A loop restarts within one run + // length plus backoff, so a Ready run past twice the last one (and + // past the kubelet's 10-minute stability line) means the cause went + // away; clear now instead of holding the full window after the last + // crash. An unready container stays held, so the row does not fall + // straight into a readiness issue. The window above still bounds the + // hold either way. + if cs.Ready && now.Sub(r.StartedAt.Time) > max(restartLoopFastRun, 2*lastRun) { + return restartLoop{}, false + } + } return restartLoop{ + status: *cs, container: cs.Name, restartCount: cs.RestartCount, lastExitCode: term.ExitCode, lastReason: term.Reason, lastFinishedAt: finished, + lastRun: lastRun, lastMessage: strings.TrimSpace(term.Message), }, true } @@ -223,10 +305,19 @@ func restartLoopMayReplace(reason string) bool { return false } +// fast reports whether the loop keeps the container mostly down: its last run +// was shorter than the kubelet's stability line, or it never started. +func (l restartLoop) fast() bool { + return l.lastRun < restartLoopFastRun +} + func (l restartLoop) ref() string { if l.sidecar { return fmt.Sprintf("sidecar container %q", l.container) } + if l.init { + return fmt.Sprintf("init container %q", l.container) + } return fmt.Sprintf("container %q", l.container) } @@ -265,6 +356,14 @@ func (l restartLoop) message() string { // observations live in the evidence and the message instead. func (l restartLoop) diagnosis() (cause, action string) { ref := l.ref() + if l.lastReason == "OOMKilled" { + if !l.memoryLimit { + return fmt.Sprintf("%s keeps being OOMKilled. It has no memory limit, so the kernel killed it under node memory pressure.", ref), + "Check the node's memory pressure and what else runs there, set a memory request that reflects real usage so the scheduler places it with room, and look for a leak or spike in this container." + } + return fmt.Sprintf("%s keeps being OOMKilled.", ref), + "Compare the container's memory use before each kill with its limit: raise the limit if the working set is legitimate, or find the leak or spike (heap settings, caches, request bursts). If it stayed under its limit, the node was under memory pressure: check the node." + } if l.lastReason == "StartError" || l.lastReason == "ContainerCannotRun" { return fmt.Sprintf("%s keeps failing to start (%s): the runtime could not start its process.", ref, l.lastReason), "Read the runtime error on this issue: usually a missing binary or entrypoint, a bad working directory, or a permission or mount problem in the container spec." @@ -314,3 +413,69 @@ func (l restartLoop) evidence() *issuesapi.RestartLoop { } return out } + +// loopingInitContainer names the ordinary init container a loop is on, or "" +// when the loop is elsewhere or absent. +func loopingInitContainer(loop restartLoop, looping bool) string { + if looping && loop.init { + return loop.container + } + return "" +} + +// loopRow marks a detection emitted for a pod in a restart loop, for +// setLoopSeverities. +type loopRow struct { + index int + owner string + fast bool +} + +func loopOwnerKey(pod *corev1.Pod, ownerGroup, ownerKind, ownerName string) string { + if ownerKind == "" { + return pod.Namespace + "//Pod/" + pod.Name + } + return pod.Namespace + "/" + ownerGroup + "/" + ownerKind + "/" + ownerName +} + +// setLoopSeverities sets each loop row's pinned severity. A loop is critical +// when it is fast (the container is mostly down) and it covers at least half +// of its workload's pods; a slow loop, or one bad replica among many, is a +// warning. Both inputs hold steady for the life of a loop, so the severity +// does too. The denominator is the workload's live pods as observed, which +// works for any owner kind; pods that were never created are not counted, +// which errs toward critical. +func setLoopSeverities(cache *ResourceCache, problems []Detection, rows []loopRow, podsByNamespace map[string][]*corev1.Pod) { + if len(rows) == 0 { + return + } + looping := map[string]int{} + namespaces := map[string]bool{} + for _, r := range rows { + looping[r.owner]++ + namespaces[problems[r.index].Namespace] = true + } + observed := map[string]int{} + for ns := range namespaces { + for _, pod := range podsByNamespace[ns] { + if pod.DeletionTimestamp != nil || (pod.Status.Phase != corev1.PodRunning && pod.Status.Phase != corev1.PodPending) { + continue + } + group, kind, name := podOwnerKindName(cache, pod) + if key := loopOwnerKey(pod, group, kind, name); looping[key] > 0 { + observed[key]++ + } + } + } + for _, r := range rows { + share := 1.0 + if n := observed[r.owner]; n > 0 { + share = float64(looping[r.owner]) / float64(n) + } + if r.fast && share >= restartLoopImpactShare { + problems[r.index].Severity = "critical" + } else { + problems[r.index].Severity = "high" + } + } +} diff --git a/internal/k8s/restart_loop_test.go b/internal/k8s/restart_loop_test.go index 65eafe823a..4d8c940996 100644 --- a/internal/k8s/restart_loop_test.go +++ b/internal/k8s/restart_loop_test.go @@ -1,12 +1,15 @@ package k8s import ( + "fmt" "strings" "testing" "time" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/fake" ) @@ -48,7 +51,18 @@ func TestActiveRestartLoop(t *testing.T) { {"two restarts is not a loop", restartLoopPod(now, status(2, running(time.Minute), terminatedAgo(now, 2*time.Minute, "Error", 1))), false, false}, {"run started 9m after the termination", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 10*time.Minute, "Error", 1))), true, false}, {"run started 11m after the termination", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 12*time.Minute, "Error", 1))), false, false}, - {"OOM is left to the OOM path", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 2*time.Minute, "OOMKilled", 137))), false, false}, + {"OOM loop counts (the row classifies as oom_killed)", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 2*time.Minute, "OOMKilled", 137))), true, false}, + {"recovered: Ready past max(10m, 2x last run)", func() *corev1.Pod { + cs := status(5, running(11*time.Minute), &corev1.ContainerStateTerminated{Reason: "Error", ExitCode: 1, StartedAt: metav1.NewTime(now.Add(-14 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-11*time.Minute - 10*time.Second))}) + cs.Ready = true + return restartLoopPod(now, cs) + }(), false, false}, + {"still looping: Ready 9m after a 3m run", func() *corev1.Pod { + cs := status(5, running(9*time.Minute), &corev1.ContainerStateTerminated{Reason: "Error", ExitCode: 1, StartedAt: metav1.NewTime(now.Add(-12 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-9*time.Minute - 10*time.Second))}) + cs.Ready = true + return restartLoopPod(now, cs) + }(), true, false}, + {"held while unready, even past the early-clear line", restartLoopPod(now, status(5, running(15*time.Minute), &corev1.ContainerStateTerminated{Reason: "Error", ExitCode: 1, StartedAt: metav1.NewTime(now.Add(-18 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-15*time.Minute - 10*time.Second))})), true, false}, {"current termination counts", restartLoopPod(now, status(5, corev1.ContainerState{Terminated: terminatedAgo(now, 5*time.Second, "Completed", 0)}, terminatedAgo(now, 40*time.Minute, "Error", 1))), true, false}, {"first restart after a long run (node bounce)", restartLoopPod(now, status(5, running(time.Minute), &corev1.ContainerStateTerminated{ Reason: "Error", ExitCode: 255, StartedAt: metav1.NewTime(now.Add(-72 * time.Hour)), FinishedAt: metav1.NewTime(now.Add(-2 * time.Minute)), @@ -76,13 +90,22 @@ func TestActiveRestartLoop(t *testing.T) { p.Status.Phase = corev1.PodSucceeded return p }(), false, false}, - {"ordinary init container retrying", func() *corev1.Pod { + {"ordinary init container failing (Init:CrashLoopBackOff)", func() *corev1.Pod { p := restartLoopPod(now, corev1.ContainerStatus{Name: "app"}) p.Spec.InitContainers = []corev1.Container{{Name: "init"}} p.Status.Phase = corev1.PodPending p.Status.InitContainerStatuses = []corev1.ContainerStatus{{Name: "init", RestartCount: 5, State: running(time.Minute), LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, 2*time.Minute, "Error", 1)}}} return p + }(), true, false}, + {"ordinary init container that finally succeeded", func() *corev1.Pod { + p := restartLoopPod(now, corev1.ContainerStatus{Name: "app"}) + p.Spec.InitContainers = []corev1.Container{{Name: "init"}} + p.Status.Phase = corev1.PodPending + p.Status.InitContainerStatuses = []corev1.ContainerStatus{{Name: "init", RestartCount: 5, + State: corev1.ContainerState{Terminated: terminatedAgo(now, time.Minute, "Completed", 0)}, + LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, 3*time.Minute, "Error", 1)}}} + return p }(), false, false}, {"native sidecar looping", func() *corev1.Pod { p := restartLoopPod(now, corev1.ContainerStatus{Name: "app", State: running(time.Hour)}) @@ -177,12 +200,12 @@ func TestStalledInitContainerProblem_IgnoresStartedNativeSidecar(t *testing.T) { Name: "proxy", State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(now.Add(-20 * time.Minute))}}, }}}, } - if _, ok := stalledInitContainerProblem(pod, now); !ok { + if _, ok := stalledInitContainerProblem(pod, now, ""); !ok { t.Fatal("a sidecar that has not passed its startup probe still blocks the pod") } started := true pod.Status.InitContainerStatuses[0].Started = &started - if got, ok := stalledInitContainerProblem(pod, now); ok { + if got, ok := stalledInitContainerProblem(pod, now, ""); ok { t.Fatalf("started native sidecar reported as a stalled init container: %+v", got) } } @@ -248,7 +271,7 @@ func TestDetectProblems_RestartLoopPrecedence(t *testing.T) { } time.Sleep(20 * time.Millisecond) } - if got, ok := lookupProblem(problems, "Pod", "bad-liveness", livenessProbeInvalidReason); !ok || got.Fingerprint == "" || got.RestartLoop != nil || got.Severity != "critical" { + if got, ok := lookupProblem(problems, "Pod", "bad-liveness", livenessProbeInvalidReason); !ok || got.Fingerprint == "" || got.RestartLoop == nil || got.Severity != "critical" { t.Fatalf("bad-liveness = %+v (found %v), want the fingerprinted invalid-probe row", got, ok) } if got, ok := lookupProblem(problems, "Pod", "image-sibling", "ImagePullBackOff"); !ok || got.RestartLoop != nil { @@ -318,3 +341,135 @@ func TestEventLastTime_UsesSeriesLastObservedTime(t *testing.T) { t.Fatalf("eventLastTime = %v, want the series' last observation %v", got, last.Time) } } + +func TestDetectProblems_RestartLoopSeverityTier(t *testing.T) { + defer ResetTestState() + now := time.Now() + controller := true + created := metav1.NewTime(now.Add(-24 * time.Hour)) + looping := func(name string, run time.Duration) corev1.ContainerStatus { + return corev1.ContainerStatus{Name: "app", RestartCount: 6, + State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}}, + LastTerminationState: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Error", ExitCode: 1, + StartedAt: metav1.NewTime(now.Add(-time.Minute - run)), FinishedAt: metav1.NewTime(now.Add(-time.Minute))}}} + } + healthy := corev1.ContainerStatus{Name: "app", Ready: true, State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: created}}} + var objs []runtime.Object + workload := func(dep string, statuses ...corev1.ContainerStatus) { + objs = append(objs, + &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: dep, Namespace: "prod", CreationTimestamp: created}}, + &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: dep + "-rs", Namespace: "prod", CreationTimestamp: created, + OwnerReferences: []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: "Deployment", Name: dep, Controller: &controller}}}}) + for i, cs := range statuses { + objs = append(objs, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("%s-%d", dep, i), Namespace: "prod", CreationTimestamp: created, + OwnerReferences: []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: "ReplicaSet", Name: dep + "-rs", Controller: &controller}}}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, ContainerStatuses: []corev1.ContainerStatus{cs}}, + }) + } + } + workload("one-of-four", looping("x", time.Minute), healthy, healthy, healthy) + workload("half", looping("x", time.Minute), looping("y", time.Minute), healthy, healthy) + workload("slow-single", looping("x", 11*time.Minute)) + workload("fast-single", looping("x", 30*time.Second)) + if err := InitTestResourceCache(fake.NewClientset(objs...)); err != nil { + t.Fatalf("InitTestResourceCache: %v", err) + } + want := map[string]string{"one-of-four-0": "high", "half-0": "critical", "half-1": "critical", "slow-single-0": "high", "fast-single-0": "critical"} + var problems []Detection + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + problems = DetectProblems(GetResourceCache(), "prod") + n := 0 + for name := range want { + if hasProblem(problems, "Pod", name, crashLoopReason) { + n++ + } + } + if n == len(want) { + break + } + time.Sleep(20 * time.Millisecond) + } + for name, sev := range want { + assertProblem(t, problems, "Pod", name, crashLoopReason, sev) + } +} + +func TestDetectProblems_InitAndOOMLoops(t *testing.T) { + defer ResetTestState() + now := time.Now() + old := metav1.NewTime(now.Add(-time.Hour)) + // A failing migration: each attempt runs 7 minutes (past the 5-minute + // stall line) and fails; the loop, not a stall, owns the row. + initLoop := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "migrate", Namespace: "prod", CreationTimestamp: old}, + Spec: corev1.PodSpec{InitContainers: []corev1.Container{{Name: "migrate"}}, Containers: []corev1.Container{{Name: "app"}}}, + Status: corev1.PodStatus{Phase: corev1.PodPending, + InitContainerStatuses: []corev1.ContainerStatus{{Name: "migrate", RestartCount: 4, + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(now.Add(-7 * time.Minute))}}, + LastTerminationState: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "Error", ExitCode: 1, + StartedAt: metav1.NewTime(now.Add(-15 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-8 * time.Minute))}}}}, + ContainerStatuses: []corev1.ContainerStatus{{Name: "app", State: corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "PodInitializing"}}}}, + }, + } + // An OOM loop between kills: Running and Ready again. + oomLoop := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "leaky", Namespace: "prod", CreationTimestamp: old}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, ContainerStatuses: []corev1.ContainerStatus{{Name: "app", Ready: true, RestartCount: 9, + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(now.Add(-6 * time.Minute))}}, + LastTerminationState: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "OOMKilled", ExitCode: 137, + StartedAt: metav1.NewTime(now.Add(-10 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-6*time.Minute - 5*time.Second))}}}}}, + } + if err := InitTestResourceCache(fake.NewClientset(initLoop, oomLoop)); err != nil { + t.Fatalf("InitTestResourceCache: %v", err) + } + var problems []Detection + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + problems = DetectProblems(GetResourceCache(), "prod") + if hasProblem(problems, "Pod", "migrate", crashLoopReason) && hasProblem(problems, "Pod", "leaky", crashLoopReason) { + break + } + time.Sleep(20 * time.Millisecond) + } + if got, ok := lookupProblem(problems, "Pod", "migrate", crashLoopReason); !ok || got.RestartLoop == nil || !strings.Contains(got.Cause, "init container") { + t.Fatalf("migrate = %+v (found %v), want an init-container restart loop, not a stall", got, ok) + } + if hasProblem(problems, "Pod", "migrate", initContainerStalledReason) { + t.Fatalf("a looping init container must not also read as stalled: %+v", problems) + } + got, ok := lookupProblem(problems, "Pod", "leaky", crashLoopReason) + if !ok || got.RestartLoop == nil || got.LastTerminatedReason != "OOMKilled" || !strings.Contains(got.Cause, "OOMKilled") { + t.Fatalf("leaky = %+v (found %v), want an OOM loop row held on a Ready tick", got, ok) + } +} + +func TestOtherContainerActiveOOM(t *testing.T) { + now := time.Now() + oom := func(name string) corev1.ContainerStatus { + return corev1.ContainerStatus{Name: name, State: corev1.ContainerState{Terminated: terminatedAgo(now, 5*time.Second, "OOMKilled", 137)}} + } + pod := restartLoopPod(now, oom("app")) + if otherContainerActiveOOM(pod, "app", now) { + t.Fatal("the loop container's own OOM must not veto its OOM loop") + } + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, oom("cache")) + if !otherContainerActiveOOM(pod, "app", now) { + t.Fatal("a sibling's active OOM owns the row") + } +} + +func TestRestartLoopDiagnosis_OOMWordingFollowsTheLimit(t *testing.T) { + limited := restartLoop{container: "app", lastReason: "OOMKilled", lastExitCode: 137, memoryLimit: true} + if cause, action := limited.diagnosis(); !strings.Contains(cause, "OOMKilled") || !strings.Contains(action, "limit") || strings.Contains(cause, "exceeds") { + t.Fatalf("limited OOM diagnosis = %q / %q", cause, action) + } + unlimited := limited + unlimited.memoryLimit = false + if cause, _ := unlimited.diagnosis(); !strings.Contains(cause, "no memory limit") || !strings.Contains(cause, "node memory pressure") { + t.Fatalf("unlimited OOM diagnosis = %q, want node pressure named", cause) + } +} diff --git a/pkg/issuesapi/types.go b/pkg/issuesapi/types.go index 6ebcf32003..187442aaad 100644 --- a/pkg/issuesapi/types.go +++ b/pkg/issuesapi/types.go @@ -443,8 +443,8 @@ type Issue struct { Fingerprint string `json:"-"` RestartCount int32 `json:"restart_count,omitempty"` LastTerminatedReason string `json:"last_terminated_reason,omitempty"` - // RestartLoop is set on a crashloop issue whose container Radar classifies - // as in an active restart loop. It is the evidence for the loop, taken + // RestartLoop is set on a crashloop or oom_killed issue whose container + // Radar classifies as in an active restart loop. It is the evidence for the loop, taken // from one container so every field describes the same termination. RestartLoop *RestartLoop `json:"restart_loop,omitempty"` Affected Affected `json:"affected,omitzero"` From ceb11620ef58b008a723406716e78b64d6e1e8f9 Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 14:12:17 +0300 Subject: [PATCH 7/8] fix(issues): early clear without a readiness gate; sidecar OOM veto; enacted memory limits A readiness blip after a loop recovered reopened it until 30 minutes after its last crash. The early clear now rests on run length alone; a container that stops crashing but stays unready is reported by its readiness row. An init container's or native sidecar's unrecovered last-state OOM now owns the row over another container's loop, as PodHasActiveOOMKilled judges it, and an enacted memory limit in status counts when choosing the OOM wording. --- internal/k8s/detect_test.go | 2 +- internal/k8s/restart_loop.go | 45 ++++++++++++++++++++++++------- internal/k8s/restart_loop_test.go | 27 +++++++++++++++++-- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/internal/k8s/detect_test.go b/internal/k8s/detect_test.go index d66144b67d..8c80d23956 100644 --- a/internal/k8s/detect_test.go +++ b/internal/k8s/detect_test.go @@ -1204,7 +1204,7 @@ func TestDetectProblems_ProbeFailures(t *testing.T) { Name: "app", Ready: false, RestartCount: highRestartThreshold + 1, - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: old}}, + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: recent}}, LastTerminationState: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{Reason: "Completed", FinishedAt: recent}, }, diff --git a/internal/k8s/restart_loop.go b/internal/k8s/restart_loop.go index 458324ab6a..ad62aa85d5 100644 --- a/internal/k8s/restart_loop.go +++ b/internal/k8s/restart_loop.go @@ -138,6 +138,17 @@ func activeRestartLoop(pod *corev1.Pod, probes map[string]probeFailure, now time } func containerHasMemoryLimit(pod *corev1.Pod, name string) bool { + // The enacted limit (status) wins: an in-place resize can set one the + // spec no longer shows. + for _, list := range [][]corev1.ContainerStatus{pod.Status.ContainerStatuses, pod.Status.InitContainerStatuses} { + for i := range list { + if list[i].Name == name && list[i].Resources != nil { + if _, ok := list[i].Resources.Limits[corev1.ResourceMemory]; ok { + return true + } + } + } + } for _, list := range [][]corev1.Container{pod.Spec.Containers, pod.Spec.InitContainers} { for i := range list { if list[i].Name == name { @@ -158,8 +169,22 @@ func otherContainerActiveOOM(pod *corev1.Pod, name string, now time.Time) bool { return true } } + // Init containers and native sidecars, as health.PodHasActiveOOMKilled + // judges them: a current OOM, or a last one not yet recovered from. for _, cs := range pod.Status.InitContainerStatuses { - if cs.Name != name && cs.State.Terminated != nil && cs.State.Terminated.Reason == "OOMKilled" { + if cs.Name == name { + continue + } + if cs.State.Terminated != nil && cs.State.Terminated.Reason == "OOMKilled" { + return true + } + if t := cs.LastTerminationState.Terminated; t != nil && t.Reason == "OOMKilled" { + if cs.State.Terminated != nil && cs.State.Terminated.ExitCode == 0 { + continue + } + if r := cs.State.Running; r != nil && !r.StartedAt.IsZero() && now.Sub(r.StartedAt.Time) > 5*time.Minute { + continue + } return true } } @@ -232,15 +257,15 @@ func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoo if r.StartedAt.Sub(finished) > restartLoopStaleGap { return restartLoop{}, false } - // Recovered: the container is serving and its current run has - // clearly outlived the loop's rhythm. A loop restarts within one run - // length plus backoff, so a Ready run past twice the last one (and - // past the kubelet's 10-minute stability line) means the cause went - // away; clear now instead of holding the full window after the last - // crash. An unready container stays held, so the row does not fall - // straight into a readiness issue. The window above still bounds the - // hold either way. - if cs.Ready && now.Sub(r.StartedAt.Time) > max(restartLoopFastRun, 2*lastRun) { + // Recovered: the current run has clearly outlived the loop's rhythm. + // A loop restarts within one run length plus backoff, so a run past + // twice the last one (and past the kubelet's 10-minute stability + // line) means it has stopped crashing; clear now instead of holding + // the full window after the last crash. Readiness is deliberately not + // required: a readiness blip after recovery must not reopen the loop, + // and a container that runs without crashing but stays unready has a + // readiness problem, which its own row then reports. + if now.Sub(r.StartedAt.Time) > max(restartLoopFastRun, 2*lastRun) { return restartLoop{}, false } } diff --git a/internal/k8s/restart_loop_test.go b/internal/k8s/restart_loop_test.go index 4d8c940996..8954709e9c 100644 --- a/internal/k8s/restart_loop_test.go +++ b/internal/k8s/restart_loop_test.go @@ -8,6 +8,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" @@ -46,7 +47,8 @@ func TestActiveRestartLoop(t *testing.T) { sidecar bool }{ {"clean exit inside window", restartLoopPod(now, status(3, running(time.Minute), terminatedAgo(now, 2*time.Minute, "Completed", 0))), true, false}, - {"last crash 29m ago", restartLoopPod(now, status(5, running(28*time.Minute), terminatedAgo(now, 29*time.Minute, "Error", 1))), true, false}, + {"last crash 29m ago, running 28m since: recovered", restartLoopPod(now, status(5, running(28*time.Minute), terminatedAgo(now, 29*time.Minute, "Error", 1))), false, false}, + {"last crash 29m ago, back in backoff", restartLoopPod(now, status(5, corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}}, terminatedAgo(now, 29*time.Minute, "Error", 1))), true, false}, {"last crash 31m ago", restartLoopPod(now, status(5, running(30*time.Minute), terminatedAgo(now, 31*time.Minute, "Error", 1))), false, false}, {"two restarts is not a loop", restartLoopPod(now, status(2, running(time.Minute), terminatedAgo(now, 2*time.Minute, "Error", 1))), false, false}, {"run started 9m after the termination", restartLoopPod(now, status(5, running(time.Minute), terminatedAgo(now, 10*time.Minute, "Error", 1))), true, false}, @@ -62,7 +64,7 @@ func TestActiveRestartLoop(t *testing.T) { cs.Ready = true return restartLoopPod(now, cs) }(), true, false}, - {"held while unready, even past the early-clear line", restartLoopPod(now, status(5, running(15*time.Minute), &corev1.ContainerStateTerminated{Reason: "Error", ExitCode: 1, StartedAt: metav1.NewTime(now.Add(-18 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-15*time.Minute - 10*time.Second))})), true, false}, + {"recovered even while unready (a readiness problem is its own row)", restartLoopPod(now, status(5, running(15*time.Minute), &corev1.ContainerStateTerminated{Reason: "Error", ExitCode: 1, StartedAt: metav1.NewTime(now.Add(-18 * time.Minute)), FinishedAt: metav1.NewTime(now.Add(-15*time.Minute - 10*time.Second))})), false, false}, {"current termination counts", restartLoopPod(now, status(5, corev1.ContainerState{Terminated: terminatedAgo(now, 5*time.Second, "Completed", 0)}, terminatedAgo(now, 40*time.Minute, "Error", 1))), true, false}, {"first restart after a long run (node bounce)", restartLoopPod(now, status(5, running(time.Minute), &corev1.ContainerStateTerminated{ Reason: "Error", ExitCode: 255, StartedAt: metav1.NewTime(now.Add(-72 * time.Hour)), FinishedAt: metav1.NewTime(now.Add(-2 * time.Minute)), @@ -473,3 +475,24 @@ func TestRestartLoopDiagnosis_OOMWordingFollowsTheLimit(t *testing.T) { t.Fatalf("unlimited OOM diagnosis = %q, want node pressure named", cause) } } + +func TestOtherContainerActiveOOM_SidecarLastState(t *testing.T) { + now := time.Now() + always := corev1.ContainerRestartPolicyAlways + pod := restartLoopPod(now, corev1.ContainerStatus{Name: "app"}) + pod.Spec.InitContainers = []corev1.Container{{Name: "proxy", RestartPolicy: &always}} + pod.Status.InitContainerStatuses = []corev1.ContainerStatus{{Name: "proxy", + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: metav1.NewTime(now.Add(-time.Minute))}}, + LastTerminationState: corev1.ContainerState{Terminated: terminatedAgo(now, time.Minute, "OOMKilled", 137)}}} + if !otherContainerActiveOOM(pod, "app", now) { + t.Fatal("a sidecar that was just OOMKilled owns the row over another container's loop") + } +} + +func TestContainerHasMemoryLimit_EnactedLimit(t *testing.T) { + pod := restartLoopPod(time.Now(), corev1.ContainerStatus{Name: "app", Resources: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("16Mi")}}}) + if !containerHasMemoryLimit(pod, "app") { + t.Fatal("an enacted limit in status counts even when the spec shows none") + } +} From cb4b1d6c2d98402172591a873dfd7ff66fa66ed4 Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 17:44:28 +0300 Subject: [PATCH 8/8] feat(issues): restart-loop evidence carries impact, run length and severity reason The loop folds its workload's "N/M available" row, which flips with every crash. Carry its stable equivalent instead: how many of the workload's live pods are looping (looping_pods / workload_pods), when the last run started (so its length shows whether the container dies on startup or serves between restarts), and why the loop is critical or warning. The restart_cause fact and the issue's meta line show them. Also keep EventTime and Series when the cache strips Events, so events recorded through events.k8s.io keep their times. --- internal/issues/diagnostic_context.go | 15 +++++++- .../issues/restart_loop_integration_test.go | 2 +- internal/k8s/restart_loop.go | 34 ++++++++++++++++--- internal/k8s/restart_loop_test.go | 10 ++++++ .../src/components/issues/IssuesView.tsx | 3 ++ .../k8s-ui/src/components/issues/types.ts | 4 +++ pkg/issuesapi/types.go | 13 +++++++ pkg/k8score/transform.go | 4 +++ pkg/k8score/transform_test.go | 16 +++++++++ 9 files changed, 94 insertions(+), 7 deletions(-) diff --git a/internal/issues/diagnostic_context.go b/internal/issues/diagnostic_context.go index a94a505f03..efe88e7071 100644 --- a/internal/issues/diagnostic_context.go +++ b/internal/issues/diagnostic_context.go @@ -1010,7 +1010,16 @@ func restartLoopEvidenceMessage(l *issuesapi.RestartLoop) string { if !l.LastFinishedAt.IsZero() { last += " at " + l.LastFinishedAt.UTC().Format(time.RFC3339) } + switch { + case !l.LastStartedAt.IsZero() && !l.LastFinishedAt.IsZero(): + last += fmt.Sprintf(" after running %s", l.LastFinishedAt.Sub(l.LastStartedAt).Round(time.Second)) + case !l.LastFinishedAt.IsZero(): + last += " without starting" + } parts = append(parts, last) + if l.WorkloadPods > 0 { + parts = append(parts, fmt.Sprintf("loopingPods=%d/%d", l.LoopingPods, l.WorkloadPods)) + } for _, p := range []struct { name string pf *issuesapi.ProbeFailure @@ -1024,7 +1033,11 @@ func restartLoopEvidenceMessage(l *issuesapi.RestartLoop) string { } parts = append(parts, obs) } - return "Restart loop evidence: " + strings.Join(parts, ", ") + "." + msg := "Restart loop evidence: " + strings.Join(parts, ", ") + "." + if l.SeverityReason != "" { + msg += " Severity " + l.SeverityReason + "." + } + return msg } func diagnosticMessage(i Issue) string { diff --git a/internal/issues/restart_loop_integration_test.go b/internal/issues/restart_loop_integration_test.go index 5dd61b247d..5d6b7af175 100644 --- a/internal/issues/restart_loop_integration_test.go +++ b/internal/issues/restart_loop_integration_test.go @@ -216,7 +216,7 @@ func TestCompose_RestartLoopKeepsOneIssueAcrossTheCycle(t *testing.T) { if got.RestartLoop.LivenessProbeFailure == nil { t.Fatalf("%s: want liveness evidence, got %+v", tick.name, got.RestartLoop) } - if fact := restartCauseFactMessage(got); !strings.Contains(fact, "liveness probe failure last seen") { + if fact := restartCauseFactMessage(got); !strings.Contains(fact, "liveness probe failure last seen") || !strings.Contains(fact, "after running 3m0s") || !strings.Contains(fact, "loopingPods=1/1") || !strings.Contains(fact, "Severity critical") { t.Fatalf("%s: restart_cause fact = %q, want the liveness observation", tick.name, fact) } case "readiness-failing": diff --git a/internal/k8s/restart_loop.go b/internal/k8s/restart_loop.go index ad62aa85d5..aae39bc4f3 100644 --- a/internal/k8s/restart_loop.go +++ b/internal/k8s/restart_loop.go @@ -70,6 +70,9 @@ type restartLoop struct { // lastRun is how long the terminated run lasted; zero when the container // never started (StartError) or the runtime did not record a start. lastRun time.Duration + // lastStartedAt is when the last terminated run started; zero when it + // never started. + lastStartedAt time.Time // lastMessage is the runtime's message for the last termination; for a // start failure it is the actual error (e.g. exec: no such file). lastMessage string @@ -247,8 +250,10 @@ func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoo // next, short run. A container that never started (StartError, // ContainerCannotRun) is stamped with the Unix epoch, which is no start. var lastRun time.Duration + var lastStarted time.Time if started := term.StartedAt.Time; started.Unix() > 0 { lastRun = finished.Sub(started) + lastStarted = started } if lastRun > restartLoopWindow { return restartLoop{}, false @@ -277,6 +282,7 @@ func containerRestartLoop(cs *corev1.ContainerStatus, now time.Time) (restartLoo lastReason: term.Reason, lastFinishedAt: finished, lastRun: lastRun, + lastStartedAt: lastStarted, lastMessage: strings.TrimSpace(term.Message), }, true } @@ -427,6 +433,9 @@ func (l restartLoop) evidence() *issuesapi.RestartLoop { LastReason: l.lastReason, LastFinishedAt: l.lastFinishedAt.UTC(), } + if !l.lastStartedAt.IsZero() { + out.LastStartedAt = l.lastStartedAt.UTC() + } if l.liveness != nil { out.LivenessProbeFailure = &issuesapi.ProbeFailure{LastSeen: l.liveness.at.UTC(), Message: Truncate(l.liveness.message, restartLoopProbeMessageMax)} } @@ -493,14 +502,29 @@ func setLoopSeverities(cache *ResourceCache, problems []Detection, rows []loopRo } } for _, r := range rows { - share := 1.0 - if n := observed[r.owner]; n > 0 { - share = float64(looping[r.owner]) / float64(n) - } - if r.fast && share >= restartLoopImpactShare { + n := max(observed[r.owner], looping[r.owner]) + share := float64(looping[r.owner]) / float64(n) + critical := r.fast && share >= restartLoopImpactShare + if critical { problems[r.index].Severity = "critical" } else { problems[r.index].Severity = "high" } + if ev := problems[r.index].RestartLoop; ev != nil { + ev.LoopingPods, ev.WorkloadPods = looping[r.owner], n + ev.SeverityReason = loopSeverityReason(critical, r.fast, looping[r.owner], n) + } + } +} + +func loopSeverityReason(critical, fast bool, looping, pods int) string { + share := fmt.Sprintf("%d of %d pods", looping, pods) + switch { + case critical: + return fmt.Sprintf("critical: the container runs under 10 minutes between restarts, so it is mostly down, and %s are looping", share) + case !fast: + return fmt.Sprintf("warning: the container serves 10 minutes or more between restarts (%s looping)", share) + default: + return fmt.Sprintf("warning: only %s are looping; the rest of the workload is serving", share) } } diff --git a/internal/k8s/restart_loop_test.go b/internal/k8s/restart_loop_test.go index 8954709e9c..0ec6cb58ed 100644 --- a/internal/k8s/restart_loop_test.go +++ b/internal/k8s/restart_loop_test.go @@ -397,6 +397,16 @@ func TestDetectProblems_RestartLoopSeverityTier(t *testing.T) { for name, sev := range want { assertProblem(t, problems, "Pod", name, crashLoopReason, sev) } + // The loop carries its impact and why it got its severity, replacing the + // folded "N/M available" row with numbers that hold steady. + got, _ := lookupProblem(problems, "Pod", "one-of-four-0", crashLoopReason) + if ev := got.RestartLoop; ev == nil || ev.LoopingPods != 1 || ev.WorkloadPods != 4 || !strings.Contains(ev.SeverityReason, "only 1 of 4 pods") || ev.LastStartedAt.IsZero() { + t.Fatalf("one-of-four evidence = %+v, want 1/4 pods, a severity reason and the last run's start", got.RestartLoop) + } + got, _ = lookupProblem(problems, "Pod", "slow-single-0", crashLoopReason) + if ev := got.RestartLoop; ev == nil || !strings.Contains(ev.SeverityReason, "serves 10 minutes or more") { + t.Fatalf("slow-single evidence = %+v, want the slow-loop reason", got.RestartLoop) + } } func TestDetectProblems_InitAndOOMLoops(t *testing.T) { diff --git a/packages/k8s-ui/src/components/issues/IssuesView.tsx b/packages/k8s-ui/src/components/issues/IssuesView.tsx index 743c355142..01384d67d4 100644 --- a/packages/k8s-ui/src/components/issues/IssuesView.tsx +++ b/packages/k8s-ui/src/components/issues/IssuesView.tsx @@ -406,6 +406,9 @@ function Diagnosis({ issue, source }: { issue: Issue; source?: IssueDiagnosisSou ? [ issue.restart_count ? `${issue.restart_count} restart${issue.restart_count === 1 ? '' : 's'}` : null, issue.last_terminated_reason ? `last exit: ${issue.last_terminated_reason}` : null, + issue.restart_loop?.workload_pods && issue.restart_loop.workload_pods > 1 + ? `${issue.restart_loop.looping_pods} of ${issue.restart_loop.workload_pods} pods looping` + : null, issue.restart_loop?.startup_probe_failure ? 'startup probe failures seen' : null, issue.restart_loop?.liveness_probe_failure ? 'liveness probe failures seen' : null, issue.restart_loop?.readiness_probe_failure ? 'readiness probe failures seen' : null, diff --git a/packages/k8s-ui/src/components/issues/types.ts b/packages/k8s-ui/src/components/issues/types.ts index 8008ade8c2..4484232483 100644 --- a/packages/k8s-ui/src/components/issues/types.ts +++ b/packages/k8s-ui/src/components/issues/types.ts @@ -376,6 +376,10 @@ export interface IssueRestartLoop { last_exit_code: number; last_reason?: string; last_finished_at?: string; + last_started_at?: string; + looping_pods?: number; + workload_pods?: number; + severity_reason?: string; liveness_probe_failure?: IssueProbeFailure; readiness_probe_failure?: IssueProbeFailure; startup_probe_failure?: IssueProbeFailure; diff --git a/pkg/issuesapi/types.go b/pkg/issuesapi/types.go index 187442aaad..6541481792 100644 --- a/pkg/issuesapi/types.go +++ b/pkg/issuesapi/types.go @@ -370,6 +370,19 @@ type RestartLoop struct { LastExitCode int32 `json:"last_exit_code"` LastReason string `json:"last_reason,omitempty"` LastFinishedAt time.Time `json:"last_finished_at,omitzero"` + // LastStartedAt is when the last terminated run started; absent when the + // container never started (a start failure). With LastFinishedAt it gives + // the run length: seconds means it dies on startup, minutes that it + // serves between restarts. + LastStartedAt time.Time `json:"last_started_at,omitzero"` + // LoopingPods / WorkloadPods are how many of the workload's live pods are + // in this restart loop. This is the loop's impact: the workload's own + // "N/M available" row is folded into the loop because it flips with + // every crash. + LoopingPods int `json:"looping_pods,omitempty"` + WorkloadPods int `json:"workload_pods,omitempty"` + // SeverityReason says why the loop is critical or warning. + SeverityReason string `json:"severity_reason,omitempty"` // LivenessProbeFailure / ReadinessProbeFailure / StartupProbeFailure are // the newest failure of each probe type seen for this container in the // last 10 minutes. diff --git a/pkg/k8score/transform.go b/pkg/k8score/transform.go index ba6b97518c..85f94bb731 100644 --- a/pkg/k8score/transform.go +++ b/pkg/k8score/transform.go @@ -39,6 +39,10 @@ func DropManagedFields(obj any) (any, error) { Count: event.Count, FirstTimestamp: event.FirstTimestamp, LastTimestamp: event.LastTimestamp, + // Events recorded through events.k8s.io carry their times here + // instead of in First/LastTimestamp. + EventTime: event.EventTime, + Series: event.Series, }, nil } diff --git a/pkg/k8score/transform_test.go b/pkg/k8score/transform_test.go index 7e6c04f7ce..52e00a15e6 100644 --- a/pkg/k8score/transform_test.go +++ b/pkg/k8score/transform_test.go @@ -323,3 +323,19 @@ func TestDropManagedFields_TypedStripsLastAppliedFromPod(t *testing.T) { t.Errorf("Pod should also have last-applied stripped") } } + +func TestDropManagedFields_EventKeepsEventsAPITimes(t *testing.T) { + first := metav1.NewMicroTime(metav1.Now().Add(-3600e9)) + last := metav1.NewMicroTime(metav1.Now().Time) + out, err := DropManagedFields(&corev1.Event{ + EventTime: first, + Series: &corev1.EventSeries{Count: 4, LastObservedTime: last}, + }) + if err != nil { + t.Fatal(err) + } + ev := out.(*corev1.Event) + if !ev.EventTime.Equal(&first) || ev.Series == nil || !ev.Series.LastObservedTime.Equal(&last) { + t.Fatalf("event times stripped: eventTime=%v series=%+v", ev.EventTime, ev.Series) + } +}