Skip to content

Commit fbfe986

Browse files
authored
Detect custom resources stuck deleting on an operator's finalizer (#2053)
VictoriaMetrics' operator was uninstalled while its VMAgent and VMSingle objects still carried `apps.victoriametrics.com/finalizer`. They stayed deleting for weeks, kept workloads running, and left the Helm release stuck uninstalling. Radar's terminating detector covered built-in resources but missed these custom resources. This adds stuck-deletion findings for watched custom resources and explains their finalizers. The primary advice is to repair or restore the controller. Manual removal appears separately, only for reviewed catalog entries and only under an explicit intentional-uninstall or permanent-scale-down condition. ### Behavior and read rules - Scan existing watched CRD stores; start no new watches. After 10 minutes deleting, emit a warning. After 30 minutes, controller-backed findings become critical only with evidence of an installed controller that is down or not fully healthy. Healthy, unidentified, unrecognized or unreadable controllers stay warning. Protection and garbage-collection finalizers retain their documented severity rules. - Use #2071's subject/evidence authorization: ordinary namespaced subjects require namespace access; Secrets require the additional Secret read; cluster-scoped subjects require exact-kind list. Controller inventory follows the same evidence granularity: namespace access for namespaced reads and exact-kind list for cluster-wide reads. Incomplete inventory cannot establish a missing/down controller or enable cleanup. A synchronized namespace still produces findings when another namespace's informer has not synchronized. - Keep #2071's per-resource coverage envelope and withheld count. A user allowed to get a cluster-scoped object but not list its kind sees that a finding was withheld, without its payload. - Shared issue composition feeds REST/MCP issues, per-resource findings, diagnose, GitOps and CNPG. The home Active Issues panel uses `/api/issues`. AI resource-list and search summaries include the composed findings and incur their scan cost; `/api/dashboard`, MCP `get_dashboard` and `cluster://health` exclude the dynamic scan. - Fold deletion conditions on the same subject into its termination row. Time the issue from the deletion request; the UI labels it “since deletion.” ### Controller recognition and boundaries `pkg/finalizers/catalog.go` owns pure finalizer identity, workload/image matching and per-controller cleanup policy. It recognizes Argo CD, Flux, VictoriaMetrics and Karpenter. Detection is generic across watched CRDs; controller-specific diagnosis is catalog-driven. New recognition defaults to cleanup disabled. The catalog replaces the exported `insights.FinalizerOwner` / `insights.ResolveFinalizerOwner` names; current supported Hub code has no call sites for them. The integration guide documents where to add entries and the positive/negative fixtures required. Recognition uses workload names/labels, pod-template labels and published image repository paths. Registry hosts, tags and digests are ignored; mirrors must preserve the vendor repository path as trailing segments; proxy-cache prefixes are supported. Flux Helm requires the published image path because unrelated Helm controllers share its name and labels. A matching name/label with an unrecognized image leaves identity unconfirmed and blocks cleanup instead of counting as an absent controller. Role predicates distinguish Argo application controllers from other components sharing their image, and VictoriaMetrics operators from config reloaders. Name-only guesses for uncataloged finalizers remain candidates: they cannot establish ownership, change severity or unlock cleanup. The inventory checks Deployments, StatefulSets, DaemonSets and recognized pods elsewhere. All copies contribute to health; a healthy copy cannot hide a failing one. Reuse observations by resolved controller within each caller's composition. Health summaries live in `pkg/health` and are shared with GitOps. Argo CD and Karpenter can run outside the cluster and never opt into cleanup; Karpenter also releases infrastructure. ### Conditional cleanup No match is not proof of an uninstall. The finding says “No recognized controller found,” stays warning and leads with restoration. Only Flux and VictoriaMetrics catalog entries opt into `conditional_cleanup` after a complete negative inventory or permanent-scale-down candidate. The additive field contains the user-confirmed condition, agent guidance, caution and dry-run commands. Each JSON Patch tests the object UID and exact finalizer at its index before removing one finalizer. Re-read after each removal. The Issues UI keeps cleanup collapsed beneath Next step; it performs no mutation. Removal releases only this object’s finalizer. Older VM operators also finalize managed Kubernetes objects, so deleting the parent can leave children stuck and Pods running; restoring the operator after the parent is gone may not clean them up. Repair/restore advice says the controller can resume cleanup, without guaranteeing completion. MCP guidance prohibits presenting removal as the next step before the user confirms the condition. Mixed finalizers retain the existing warning policy when one controller is healthy, while preserving repair, restore and check advice for every other finalizer. Unknown controllers, weak candidates, incomplete access, off-cluster installations and protection finalizers get no removal commands. ### Validation - Documentation-only follow-up at `244e84263`: simplified all three changed Markdown files by 61 lines, corrected recovery wording and the conventional-namespace GitOps lookup description, and retained safety rules and extension points. Scope, relative links and whitespace checked; behavior is unchanged. - Full frontend/embed/Go build and affected Issues/MCP suites at `7fa46cb63`; conditional-cleanup UI tests passed. The broader root/`pkg` suites and 2,023 web / 4,504 k8s-ui tests (one existing skipped) remain evidence for unchanged paths from the preceding integration checks. - Regression coverage includes deletion thresholds, exact resource identity, per-namespace synchronization, namespace/Secret/cluster-subject authorization, inventory completeness, renamed/image-role recognition, multi-copy health, unknown/mixed finalizers, cleanup opt-in, condition folding and UID/index guards. - **Actual VM lifecycle on existing EKS, no kind:** Helm-renamed, namespace-scoped VictoriaMetrics operator v0.68.1 with real VMAgent/VMSingle workloads and operator-image reloaders. At `b7cd15ff5`, intentional operator uninstall left three real CRs deleting with healthy application/reloader Pods. No findings before ten minutes; afterward warning, restore-first advice and conditional cleanup. Reloaders were not mistaken for the operator. REST/MCP agreed; generated server dry run succeeded; wrong UID/finalizer guards failed unchanged; one reviewed apply deleted only its owned parent CR. - Restoring the operator deleted the two parents left intact and their main workloads. The bypassed parent’s seven built-in dependents stayed stuck, and all three auto-generated VMServiceScrapes remained deleting under this older operator. Those leftovers were inspected and cleaned separately with UID/finalizer guards. The operator release, owned namespace and all 26 created empty CRDs were removed. - At `72b521d1c`, the actual healthy `vmop` operator was recognized by image on all three naturally stuck scrape CRs: warning, no cleanup, REST/MCP agreement. After intentional scale-to-zero, the corrected dependent-finalizer caution and collapsed/expanded UI were verified; generated previews and guarded applies removed only those owned scrape CRs. The `7fa46cb63` follow-up changes recovery wording/documentation and test copy only; unchanged behavior reuses those live runs. - **Shared-mode EKS at `9e60c1573`:** stand-in fixtures with proxy authentication and three RBAC identities cover healthy, crashloop, image-pull failure, Deployment/DaemonSet without Pods, scaled-zero and unrecognized states; namespace-only callers get neither cleanup nor hidden controller names. REST/MCP agree; get-only cluster coverage reports a withheld finding; unreadable Secret evidence loses names/keys/onset. UI captures cover collapsed cleanup, restricted omission and withheld messaging. Those fixtures were 10–30 minutes deleting; the 30-minute critical ramp has tests and earlier live no-pod evidence at `07101583e`. **Limits:** object-level conditional cleanup is not a complete VM installation uninstall. The default Radar chart’s deployed ServiceAccount/Hub permissions, a live AI confirmation conversation, large-cluster performance and persistent/cloud-data cleanup remain unverified. v0.75.0 was installed and healthy during setup, but the intentional uninstall/recovery failure reproduction used v0.68.1. The live run used Helm for the operator and kubectl for the VM CRs; it did not reproduce the original application chart’s Helm-release bookkeeping. Earlier kind evidence is historical; this EKS run created no kind cluster. [Review packet](https://radar-pr-2053-review.nadav168665.chatgpt.site) (owner-private).
1 parent 072ddd1 commit fbfe986

37 files changed

Lines changed: 3526 additions & 373 deletions

‎docs/INTEGRATION_GUIDE.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,29 @@ shared renderers.
8888
or the generic pass reintroducing warnings for intentional states. Don't suppress
8989
a whole API group when only some kinds are covered.
9090

91+
### Finalizer controller recognition
92+
93+
Stuck-deletion detection covers already-watched, synchronized CRDs. Adding an
94+
owner enriches those findings; it needs no per-kind detector or renderer.
95+
96+
- [ ] Add documented keys and controller names, labels and image paths in
97+
[`pkg/finalizers/catalog.go`](../pkg/finalizers/catalog.go). Disambiguate shared
98+
keys by API group/kind, and shared images or identities by role/image checks.
99+
Name guesses for uncataloged finalizers remain candidates, never owners.
100+
- [ ] Review cleanup semantics before setting `AllowConditionalCleanup` (false
101+
by default). Leave it disabled for unreviewed cleanup, off-cluster controllers
102+
or infrastructure cleanup; record those limits on the entry.
103+
- [ ] Add identity fixtures in
104+
[`pkg/finalizers/catalog_test.go`](../pkg/finalizers/catalog_test.go) and
105+
guidance/eligibility cases in
106+
[`internal/issues/finalizers_test.go`](../internal/issues/finalizers_test.go),
107+
including unrelated workloads and incomplete visibility.
108+
- [ ] Keep reads and policy in `internal/issues/finalizers.go`, pod health in
109+
`pkg/health`. Catalog additions should not change the shared API/UI machinery.
110+
- [ ] Verify a real stuck deletion, including intentional uninstall if enabling
111+
removal. Update the [MCP contract](mcp.md#read-tools) when supported behavior
112+
changes; distinguish controller evidence from synthetic fixtures.
113+
91114
### MCP / AI context
92115

93116
- [ ] Check existing resource-tool output in `pkg/ai/context/summary_crd.go`,

‎docs/gitops.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ Radar treats Terminating as a distinct lifecycle phase that dominates other stat
9090
- **Detail header**: orange `[Terminating]` chip replaces Sync/Health badges. Source / Revision / Last reconcile / Sync mode metadata swaps to `Pending deletion · Finalizers`, with the original fields behind a "Show pre-deletion metadata" toggle
9191
- **Action buttons**: Sync, Reconcile, Suspend / Resume, Rollback, Sync-with-source disable with a tooltip explaining why. Refresh and Terminate stay enabled — they're read-only / cleanup-only verbs
9292
- **Lifecycle banner**: a dedicated orange banner above the Issues band; pre-deletion failures collapse behind a `Pre-deletion issues (N)` disclosure
93-
- **Severity ramp**: info <5min, warning 5-30min, alert >30min. Past 30min the Issue's Cause line names the controller responsible for the finalizer and reports its pod state ("helm-controller is not running in flux-system")
93+
- **Severity ramp**: info <5min, warning 5-30min, alert >30min. Past 30min the Issue's Cause line names the controller responsible for the finalizer and reports its pod state ("helm-controller is not running in flux-system"). No matching pods in Argo CD’s conventional namespace is inconclusive: its controller may run elsewhere or outside the cluster (Amazon EKS Capabilities)
9494
- **Fleet view**: `—` in Sync/Health columns, orange row stripe, `[TERMINATING]` chip in the leftmost slot, `Pending Nago` instead of "Last Sync"
9595
- **Topology**: orange left-stripe on the root + children; stale sync/health chips suppressed
9696
- **Cluster Audit**: `stuckTerminating` check across all typed K8s resources with the same 5-minute/30-minute thresholds, presented as Medium/High posture priority

‎docs/mcp.md‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,55 @@ API group is part of a resource's identity. Radar infers the canonical group for
491491

492492
This selects the Argo Rollout workload path; it does not imply that `diagnose` supports arbitrary custom resource shapes.
493493

494+
Watched, synchronized custom resources that remain deleting appear in `issues`
495+
as `termination_stuck`, timed from the deletion request. Radar starts no new
496+
watches for this scan; an empty result does not establish that unwatched kinds
497+
are healthy. These findings also feed Active Issues and AI resource/search
498+
summaries, but not dashboard health summaries or `cluster://health`.
499+
500+
Findings start at warning after 10 minutes. At 30 minutes, controller-owned
501+
finalizers follow this policy:
502+
503+
| Controller evidence | Severity | Next step |
504+
|---|---|---|
505+
| Recognized controller is fully ready | Warning | Check logs, permissions and deletion-blocking dependencies. No removal. |
506+
| Recognized controller is installed but not fully ready | Critical | Repair the controller so it can resume cleanup. No removal. |
507+
| No recognized controller, or all recognized copies are scaled to zero | Warning | Restore it if it should be running. Removal may be offered conditionally. |
508+
| Owner or controller state cannot be established | Warning | Identify and check the controller. No removal. |
509+
510+
A fully ready controller for any finalizer keeps a mixed-finalizer finding at
511+
warning; repair advice for other controllers remains. In-use guards and
512+
GC-only finalizers can reach critical at 30 minutes. Resolve their dependencies
513+
rather than removing them; orphaning preserves dependents.
514+
515+
The catalog recognizes Argo CD, Flux, VictoriaMetrics and Karpenter through
516+
known finalizer keys and workload identities. Name-only guesses for uncataloged
517+
finalizers never establish ownership. Renamed installs and mirrors can be
518+
recognized through published images; controller-specific identity rules are in
519+
[`pkg/finalizers/catalog.go`](../pkg/finalizers/catalog.go).
520+
521+
Ordinary namespaced subjects require namespace access; Secrets also require
522+
Secret access, and cluster-scoped subjects require list on their exact kind.
523+
Controller evidence follows namespace access for namespaced reads and exact-kind
524+
list for cluster-wide searches. A missing or down controller requires complete
525+
cluster-wide Pod, Deployment,
526+
StatefulSet and DaemonSet visibility. Incomplete inventory cannot enable removal.
527+
A negative search is not proof of an uninstall; a controller may run elsewhere
528+
or use an identity Radar does not recognize.
529+
530+
Only Flux and VictoriaMetrics currently opt into `conditional_cleanup`, separate
531+
from `action`. It contains `condition`, `agent_guidance`, `caution`, and `removals`
532+
with one entry per finalizer (`finalizer`, `patch_resource`, `kubectl`). An agent
533+
may propose removal only after the user confirms intentional uninstall or a
534+
permanent stop. The UI keeps these commands in a collapsed conditional section.
535+
536+
Each command defaults to a server dry run and tests the object's UID and the
537+
finalizer at its index. Review the preview before applying; re-read after every
538+
removal because indices change. Removal bypasses controller cleanup and can
539+
leave Kubernetes resources, infrastructure or storage behind. Dependents with
540+
their own finalizers can remain stuck even after the controller is restored;
541+
older VictoriaMetrics operators place such finalizers on managed objects.
542+
494543
For `issues`, read `timing_summary` when present; it explains timing combinations that are easy to misread without schema context. The raw provenance fields remain available for filtering. `first_seen` is an evidence-backed lower bound, `onset_unknown` means no contributing signal has a known onset, and `resource_created_at` is resource-age context rather than issue age. A missing `first_seen` is exposed to CEL as `0`; require `first_seen != 0` for any age filter, and also require `onset_coverage_unknown == 0` when the whole row must have exact timing.
495544

496545
Resource context distinguishes observed dependency references (`dependencies` and

‎internal/issues/compose.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
// inject fakes without standing up an informer stack.
1919
type Provider interface {
2020
DetectProblems(namespaces []string) []k8s.Detection
21+
DetectDynamicTerminatingProblems(namespaces []string, canList func(group, resource, namespace string) bool) []k8s.Detection
2122
DetectCAPIProblems(namespaces []string) []k8s.Detection
2223
// DetectGitOpsProblems returns failing ArgoCD Applications and Flux
2324
// Kustomizations/HelmReleases — the reconciler-health failure class the
@@ -141,6 +142,10 @@ func ComposeWithStats(p Provider, f Filters) ([]Issue, ComposeStats) {
141142
}
142143
return correlationAllowed
143144
}
145+
canList := func(group, resource, namespace string) bool {
146+
return f.CanReadEvidence != nil && f.CanReadEvidence(EvidenceRead{Group: group, Resource: resource, Namespace: namespace, Verb: "list"})
147+
}
148+
finalizerObservations := map[string]finalizerObservation{}
144149
emit := func(ps []k8s.Detection, source Source) {
145150
for _, pr := range ps {
146151
if pr.Severity == "info" {
@@ -155,17 +160,24 @@ func ComposeWithStats(p Provider, f Filters) ([]Issue, ComposeStats) {
155160
pr.Cause, pr.Action = "", ""
156161
}
157162
}
163+
if len(pr.TerminatingFinalizers) > 0 {
164+
pr = enrichTerminatingProblem(pr, p, canList, finalizerObservations)
165+
}
158166
out = append(out, fromProblem(pr, now, source))
159167
}
160168
}
161-
emit(p.DetectProblems(f.Namespaces), SourceProblem) // hardcoded per-kind checks
169+
emit(p.DetectProblems(f.Namespaces), SourceProblem) // hardcoded per-kind checks
170+
if f.CanReadEvidence != nil {
171+
emit(p.DetectDynamicTerminatingProblems(f.Namespaces, canList), SourceProblem)
172+
}
162173
emit(p.DetectCAPIProblems(f.Namespaces), SourceProblem) // Cluster API
163174
emit(p.DetectGitOpsProblems(f.Namespaces), SourceProblem) // Argo/Flux reconciler health
164175
emit(p.DetectMissingRefs(f.Namespaces), SourceMissingRef) // dangling by-name refs
165176
emit(p.DetectScheduling(f.Namespaces), SourceScheduling) // placement/admission/post-bind
166177
karpenterIssues, karpenterOwnedSubjects := detectKarpenterIssues(p, f)
167178
out = append(out, karpenterIssues...)
168179
out = append(out, detectGenericCRDIssues(p, f, karpenterOwnedSubjects)...) // generic CRD .status.conditions
180+
out = foldDeletingConditions(out)
169181
out = elevateAdmissionWebhookBackendSeverity(out, p)
170182

171183
// ---- 2. Evidence-level transforms (operate on flat rows) ---------

0 commit comments

Comments
 (0)