feat(rollouts): deployment-plugin parity - #1625
Conversation
PR Summary by QodoExpand Argo Rollouts visibility and AnalysisRun history
AI Description
Diagram
High-Level Assessment
Files changed (18)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e8c2f9f. Configure here.
hisco
left a comment
hisco
left a comment
There was a problem hiding this comment.
Thanks for this. It closes a real gap, and your replies to the bot round were good judgment, including the two you pushed back on.
I've pushed four commits to your branch rather than sending it back, and merged main in since it had drifted. Your commits are untouched. What changed:
Formatting. The comments inside AnalysisRunSummary split gofmt's struct tag alignment into separate blocks. That was the only thing failing CI. Worth running gofmt -l . locally, go test won't catch it.
The analysis history sat above the live progression. On a paused rollout you scrolled past finished runs to reach the step the rollout is actually on. It's now below the ReplicaSets and collapsed. That one is taste rather than a bug, so say if you'd rather have it back.
A running analysis rendered as a warning. The inline badge on the current step mapped four phases and fell back to the alert tone for everything else, so Running and Pending read as problems. getAnalysisRunStatus and analysisStatusClass already carried the same mapping, so this was a third copy that had drifted. All three now share analysisPhaseLevel.
Also corrected the rollout-type label values in a comment, Argo writes Background rather than BackgroundAnalysis, and gave an Inconclusive Experiment the warning banner the AnalysisRun renderer already uses
Today the only backend AnalysisRun awareness is topology's
activeAnalysisRuns(), deliberately restricted to the Rollout's 4
"currently active" status slots. Adds pkg/rollouts.ListAnalysisRuns,
following revisions.go's exact shape (reuses its unexported ownedBy
helper) to list every AnalysisRun owned by a Rollout, newest first,
with per-run metric pass/fail counts computed the same way
resource-utils.ts's summarizeAnalysisMetrics does (dryRun results
excluded from the tally).
New GET /rollouts/{ns}/{name}/analysisruns handler, gated on listing
AnalysisRuns directly rather than the Rollout's own patch grant —
same reverse-lookup pattern as the RBAC/Policy/Velero/CNPG endpoints
elsewhere in this file's siblings.
…derer Both kinds were already discovered/cached but fell through to the generic raw-YAML renderer. New AnalysisTemplateRenderer shows each metric's provider (Prometheus/Web/Job/Kubernetes/etc.), interval, count, success/failure conditions, and provider-specific config — same shape AnalysisRunRenderer already parses for a run's live results, applied here to the static template definition instead. Registered in ResourceRendererDispatch (no collision guard needed — both kind strings are unique across the whole repo) and given curated table columns (Name, Namespace where applicable, Metrics count, Age).
status.canary.currentExperiment was a bare string with no drill-down anywhere. New ExperimentRenderer shows spec.templates[] (replicas, weight) with live status.templateStatuses[] (ready/available counts, phase), spec.analyses[] with their AnalysisTemplate references and live status.analysisRuns[] verdicts. No demo fixture exists for this CRD, so it was verified against a real Experiment applied directly to the rollouts-demo cluster rather than an official scenario — which caught a genuine field-name bug: the live controller's status shape is analysisRuns[]/phase, not the analysisRunStatuses[]/status shape the initial implementation assumed from the public docs' examples. Fixed against the real object.
…story Mirrors kubectl-argo-rollouts and the ArgoCD UI extension's Rollout view, and closes the three gaps the current renderer had against them: - Canary Steps upgraded from a flat status-dot list to a connected vertical stepper (CanaryStepTimeline) — same connecting-line visual as ConditionsSection. Steps are a strictly linear sequence, so this stays a simple top-to-bottom timeline rather than reaching for a graph library the way Tekton's DAG needed to. The current step's live analysis status (if it has one) now shows inline, and analysis steps get clickable links to the AnalysisTemplate/ ClusterAnalysisTemplate they reference (canaryStepTemplateRefs). - blueGreen gets an equivalent derived phase list (blueGreenPhases) — it has no steps[] array to iterate, so this crosses strategy config against live status.blueGreen fields. Verified live against the demo fixture's real BlueGreenPause state and fixed a real ordering bug: scaleUpPreviewCheckPoint is absent on real Rollouts (a narrow field, not a general "preview is up" signal) — "preview scaled up" now also treats a recorded pre-promotion analysis, or the Rollout having reached any blueGreen pause, as equally valid completion evidence, since neither can happen before the preview is scaled. - New ReplicaSetProgression section: a read-only revision tree (Rollout -> ReplicaSet revision -> pods), reusing the already-shipped useWorkloadRevisions/useWorkloadPods hooks and revisionRoleBadges() from the existing rollback dialog — no new backend endpoint needed, the per-pod revisionIdentity hash already joins pods to their revision. Rollback itself stays exclusively on the existing dialog. - New AnalysisRun History subsection surfaces the backend's new history endpoint (name, phase, trigger, metrics passing/total), alongside the existing 4 "currently active" slots. - status.canary.currentExperiment is now a nav link to the new Experiment renderer instead of a bare string. Host wrapper (web/) fetches the two new data sources (useRolloutAnalysisRuns, useWorkloadRevisions/useWorkloadPods) and passes them down as optional props, keeping packages/k8s-ui pure — a library consumer that skips the fetch loses only the new sections, nothing else breaks. Also removes RolloutRenderer.tsx from badge-no-handrolled.test.tsx's BASELINE: replacing the old hand-rolled status-dot markup with the new stepper left it with zero hand-rolled chip colors.
Flips AnalysisTemplate/ClusterAnalysisTemplate/Experiment's Detail View column to Yes and adds prose for the step timeline, ReplicaSet progression, and AnalysisRun history sections.
…ueGreen completion detection Two real bugs in RolloutRenderer.tsx, both confirmed against the real Argo Rollouts API schema and cross-checked against docs/mcp fixtures: - canaryStepTemplateRefs assumed a distinct clusterTemplateName field name for cluster-scoped refs. The real schema uses ONE field (templateName) for both scopes, disambiguated by a separate clusterScope boolean — that field never exists in real objects, so every ref was always treated as namespaced, and a step referencing a ClusterAnalysisTemplate opened the wrong kind. Also extended it to handle Experiment spec.analyses[] entries, which carry templateName/clusterScope flat on the entry itself rather than nested under a templates[] array — previously always returned empty for Experiments, so template links never appeared there at all. - blueGreenPhases required activeSelector === previewSelector to consider a Rollout promoted, but Argo Rollouts clears previewSelector once there's no active preview to track — a settled, fully-Healthy blueGreen commonly has none, so completion could never be detected past the narrow just-promoted window. Now also accepts activeSelector matching status.currentPodHash (a generic, always-populated field naming the newest ReplicaSet) as equally valid completion evidence. Also removed three comments that only restated the section title immediately below them, and one that described a dependency as "already-shipped" instead of just its current behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ral CRD; give it a real status/phase Two real gaps: - The experiments render branch and status dispatch had no group guard at all — Katib (kubeflow.org) ships its own, unrelated Experiment CRD sharing this plural, and got the Argo Rollouts Experiment renderer's mostly-empty status view instead of its actual resource details. Gated both on argoproj.io, matching every other collision guard already in this file (Istio Gateway, CNPG Cluster, Kyverno Policy, etc.) and CLAUDE.md's own documented pattern for exactly this shape of collision. - getResourceStatus() had no branch for experiments at all (present for analysisruns, missing here), and the Resources table routed the Phase column through GenericCell — whose generic phase heuristic doesn't recognize 'Successful' (Experiment's actual terminal phase) and treats 'Running' as healthy, neither correct for the AnalysisPhase vocabulary Experiment actually reports. Both now use getAnalysisRunStatus, which already has the right mapping for that exact vocabulary (Experiment and AnalysisRun share it) — a new ExperimentCell for the table, same pattern as the existing AnalysisRunCell, rather than touching the generic heuristic other kinds still rely on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d-hoc classes and a parallel vocabulary - Metric provider badge (AnalysisTemplate renderer) hand-wrote `badge-sm status-neutral` instead of the shared <Badge> component — everywhere else in a drawer body (not a table cell, where .status-* is the documented exception) uses it directly. - Arguments, AnalysisRun History, and ReplicaSets sections explicitly defaulted to collapsed with real, non-empty, non-low-priority content — every other populated section on the same pages already relies on Section's own default (expanded). Pod Template stays collapsed deliberately — it's not part of this fix; the same raw pod-spec dump already collapses by default in the generic WorkloadRenderer, a pre-existing, intentional convention. - CanaryStepTimeline's step-analysis dot/badge used a hand-rolled 'ok'/'warning'/'fail' vocabulary with hardcoded red/amber Tailwind backgrounds, duplicating (and requiring hand-translation back to) the canonical HealthLevel tones the badge already used one line away. Switched to HealthLevel + the shared healthColors token map throughout — same visual states (a failed/inconclusive analysis on the current step still gets its own color; a successful one doesn't change the dot, matching the original behavior exactly), no parallel vocabulary or raw colors left. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…values The comments interleaved in AnalysisRunSummary split gofmt's struct tag alignment into separate blocks, so the original spacing no longer matched what gofmt produces. CI's gofmt check fails on it. The rollout-type label values in the comment were also wrong. Argo Rollouts writes Step, Background, PrePromotion and PostPromotion, not the longer "...Analysis" spellings.
AnalysisRun History rendered above the canary step timeline and the blue-green progression, so an operator opening a paused rollout scrolled past finished runs to reach the step it is actually sitting on. That step is the most decision-relevant thing on the screen. History now sits below ReplicaSets and starts collapsed. It answers "has this been flaky before", which is a question you go looking for, not one you need in front of you mid-rollout.
An Inconclusive phase fell through to status-unknown and showed no banner, so an Experiment whose analysis matched neither its success nor its failure condition read as "no information" rather than as something needing a decision. AnalysisRunRenderer already treats Inconclusive as a warning. Inconclusive now maps to status-alert and gets a warning banner, matching that renderer.
The inline badge on the current canary step mapped Successful, Failed, Error and Inconclusive, then fell back to the alert tone for everything else. Running and Pending are the usual states while a step is live, so an analysis that was simply still going read as a warning. getAnalysisRunStatus and analysisStatusClass already carried the same phase to tone mapping, so this was a third copy that had drifted. All three now share analysisPhaseLevel, which also gives an unrecognised phase the unknown tone rather than alert.
00e36d0 to
27528ff
Compare
2 participants
Brings Radar's Argo Rollouts support up to parity with the
kubectl-argo-rollouts/Argo CD UI extension's own resource coverage:AnalysisRunhistory (not just the current run) for a Rollout.AnalysisTemplate/ClusterAnalysisTemplatedetail renderer: metric definitions, args, and provider config.Experimentdetail renderer: templates, weights, and status.AnalysisRunhistory — the same "what's this rollout actually doing right now" story the plugin's own CLI/UI tells, inline in Radar.Test plan
go test ./...make tscnpx vitest run(k8s-ui)🤖 Generated with Claude Code
Note
Medium Risk
Adds a namespace-scoped AnalysisRun list API and richer Rollout UI wiring; risk is moderate due to RBAC boundaries and potential list cost on busy namespaces, but changes are mostly read-only presentation.
Overview
Expands Argo Rollouts coverage so Radar matches the kubectl/Argo CD plugin story for progressive delivery visibility and related CRDs.
Rollout detail now shows a connected canary step or derived blue-green phase timeline (inline step analysis status and links to AnalysisTemplates), a read-only ReplicaSet → pod progression (reusing existing workload revision/pod APIs), and an AnalysisRun history list beyond the four active status slots.
Backend:
GET /api/rollouts/{ns}/{name}/analysisrunslists owned AnalysisRuns (phase, trigger, metric counts), gated onlistAnalysisRuns RBAC separately from Rollout patch capabilities.New detail renderers for
AnalysisTemplate/ClusterAnalysisTemplate(metrics, providers, args) and ArgoExperiment(templates, analyses, status), plus resource-browser columns and dispatch guards so Kubeflow’s unrelatedExperimentCRD is not mis-rendered.Docs and tests updated for the new views and listing behavior.
Reviewed by Cursor Bugbot for commit 27528ff. Bugbot is set up for automated code reviews on this repo. Configure here.