feat(api): add summary projection to ListStages/WatchStages + GetStageHealthOutputs RPC#6159
Closed
jacobboykin wants to merge 13 commits into
Closed
feat(api): add summary projection to ListStages/WatchStages + GetStageHealthOutputs RPC#6159jacobboykin wants to merge 13 commits into
jacobboykin wants to merge 13 commits into
Conversation
✅ Deploy Preview for docs-kargo-io ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6159 +/- ##
==========================================
+ Coverage 57.51% 57.61% +0.10%
==========================================
Files 481 482 +1
Lines 40656 40778 +122
==========================================
+ Hits 23382 23495 +113
- Misses 15874 15880 +6
- Partials 1400 1403 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment was marked as outdated.
This comment was marked as outdated.
krancour
reviewed
Apr 24, 2026
jacobboykin
force-pushed
the
jboykin/stage-summary-api
branch
from
493ec53 to
8e7b141
Compare
jacobboykin
changed the title
Adds a lightweight Stage projection for list and graph views that need
metadata and current state for many Stages at once but do not need the
full Stage CR. The summary omits FreightHistory entries beyond the
current FreightCollection, PromotionTemplate step configuration, and
Verification configuration. Use GetStage to retrieve the full Stage
resource when detail fields are needed.
Both endpoints are available in ConnectRPC and REST
(GET /v1beta1/projects/{project}/stage-summaries, including
?watch=true for SSE). Requests accept an optional freightOrigins
filter and a resourceVersion for the standard list-then-watch pattern.
Signed-off-by: Jacob Boykin <boykinmusic@gmail.com>
Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
Buf lint enforces lower_snake_case for proto field names. Updated the new StageSummary / ListStageSummaries / WatchStageSummaries messages accordingly. Generated Go struct names (StageSummaries, FreightOrigins, etc.) and protojson wire format (camelCase via the json= tag) are unchanged. Swagger and TypeScript bindings regenerated to match. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
…geSummary
Adds a batch RPC that returns the raw health output blob for a specified
set of Stages in a project, and updates stageToSummary to always leave
Status.Health.Output nil in the summary response. Intended for clients
that use ListStageSummaries for the stage list and want to resolve
per-argocd-app health only for the Stages currently in viewport (React
Flow virtualizes to ~10-30 visible nodes at a time).
The raw health output is typically ~2 KB per Stage and is the single
largest remaining field in StageSummary. Moving it behind a lazy fetch
drops the summary to roughly 1-2 KB per stage, reducing both wire
transfer and the heap footprint of the cached list.
The new endpoint is available in ConnectRPC and REST
(GET /v1beta1/projects/{project}/stage-health-outputs?stageNames=a&stageNames=b).
Stages that do not exist or have no recorded health output are omitted
from the response map; the endpoint has best-effort semantics.
Signed-off-by: Jacob Boykin <boykinmusic@gmail.com>
Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
…y flag Replaces the dedicated ListStageSummaries / WatchStageSummaries RPCs and their StageSummary / StageSpecSummary / StageStatusSummary messages with a `summary` bool on ListStagesRequest / WatchStagesRequest. When set, heavy fields are stripped from each returned Stage in place: - status.freightHistory truncated to the current element (index 0) - spec.promotionTemplate.spec.steps[*].config cleared (kind/as/name kept) - status.health.output cleared (use GetStageHealthOutputs for lazy fetch) The UI continues to derive has-verification from `spec.verification != nil` and promotion-step-count from `len(spec.promotionTemplate.spec.steps)`, so no derived fields or dedicated projection types are needed. ListStagesResponse gains resource_version and WatchStagesRequest gains summary and resource_version so list-then-watch works across both full and summary modes. GetStageHealthOutputs is unchanged — it remains a separate batch RPC for lazy-loading argocd health blobs per viewport. Addresses review feedback asking why a dedicated endpoint was used instead of a flag on the existing ListStages/WatchStages pair. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
Promotes two helpers that were living in pkg/server to pkg/api/stage.go, alongside GetStage, ListFreightAvailableToStage, and the rest of the Stage-domain operations. Both are now exported and unit-tested: - StripStageForSummary: in-place projection that clears heavy fields (freightHistory[1..], promotionTemplate step configs, health.output) from a Stage. Previously a package-private helper in pkg/server/list_stages_v1alpha1.go. - ListStageHealthOutputs: given a project and a set of Stage names, returns map[name]raw-health-output. Previously inlined (twice — once for the ConnectRPC handler and once for the REST handler) in pkg/server/get_stage_health_outputs_v1alpha1.go, alongside a private uniqueNonEmptyStrings helper that is now folded into the exported function. The server handlers are now thin adapters that validate transport-level concerns (batch-size cap) and delegate domain work to pkg/api. This matches the convention akuity#6163 establishes for StageMatchesAnyWarehouse / ListStagesByWarehouses. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
… cache update - Remove the meta/v1/generated.proto import from service.proto; it was only used by the StageSummary messages that this PR replaced with a summary flag on ListStages. - The new required ListStagesResponse.resource_version field requires UI callers constructing the response shape to populate it. The pipeline watcher's cache-update path has no authoritative RV on hand (it's merging incremental Stage watch events), so it passes an empty string. The list-then-watch RV flow itself continues to be handled by ListStages's own response, not by this cache-update path. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
- Switch GetStageHealthOutputsResponse.health_outputs from map<string, bytes> to map<string, string>. The REST handler serializes via Go's encoding/json, which encodes []byte as base64 strings -- so the wire format never matched the generated REST/TS clients (which were modeled as []int32 / number[]) nor the intent (raw JSON blobs from Stage.status.health.output.raw). - Regenerate pb.go, swagger.json, Go REST model, and TS client to pick up the new shape. - Fix the REST handler so an over-batch request returns 400 instead of 500. The error middleware only maps *libhttp.HTTPError and Kubernetes status errors; a plain fmt.Errorf falls through as an internal server error. Wrap with libhttp.Error(..., http.StatusBadRequest) and add a REST test for the cap. - Strengthen the REST test to assert the wire shape directly via json.RawMessage, so a future regression to []byte encoding would fail at the JSON layer. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
The protoc-gen-doc template rendered " " for messages with an empty Description (synthetic *Entry types from proto map fields), producing a single-space line after each Entry section header. That tripped git diff --check on the new GetStageHealthOutputsResponse.HealthOutputsEntry section, but the same artifact already existed for ~20 pre-existing Entry sections in the file. Guard the space and content rendering so empty descriptions emit nothing, then regenerate the API doc. Diff is whitespace- only and applies cleanup to all affected sections. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
ListStageHealthOutputs previously listed every Stage in the project then filtered in memory. That undermined the lazy-fetch contract of GetStageHealthOutputs: each viewport refresh hauled status.health.output for every Stage in the project from etcd, even though only the in-view subset was requested. Switch to per-name Get with errgroup-bounded concurrency (16). Server-side work now scales with the request, not the project size. Cost is M SubjectAccessReviews and M API round-trips for an M-sized batch -- bounded by the existing maxStageHealthOutputsBatch=1000 cap. Tests now exercise the Get path and explicitly assert that List is never called. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
StripStageForSummary mutates its argument. The watch paths in WatchStages and ListStages passed the watch event object through unchanged. Today's client-go decodes objects fresh per event so this is safe in practice, but the coupling between response shaping and the watch object's ownership semantics is fragile -- any future cache-backed watch implementation would silently leak the stripped state into shared state. DeepCopy before stripping in both watch handlers. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
The summary projection promised step skeletons but only cleared step config. Step If, Vars, Task, ContinueOnError, and Retry stayed in the payload. Strip If and Vars too -- expression-bearing fields that callers of summary do not need -- and document the surviving skeleton (uses, task, as, continueOnError, retry) honestly. Drop the stale "name kept" mention from proto comments; PromotionStep has no Name field. The REST list/watch path was also missing the new resource-version contract. ConnectRPC ListStages already returned it; ConnectRPC WatchStages already accepted it. Surface the same on REST: - ListStagesByWarehouses now returns *kargoapi.StageList so ListMeta (notably ResourceVersion) survives. Both Connect and REST list handlers now share this helper instead of one of them re-doing the list inline. - REST GET /stages?watch=true&resourceVersion=<rv> threads the value into the underlying Watch via metav1.ListOptions. Lock both behaviors in with handler tests: - TestListStages: summary=true strips fields; ResourceVersion is propagated from underlying StageList. - TestWatchStages: summary=true strips fields in streamed events. - Test_server_listStages: REST summary=true / =false; REST list body carries ListMeta.ResourceVersion. - Test_server_listStages_watch: REST summary=true strips event body; REST resourceVersion query param reaches the underlying Watch. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
The backend has supported summary-projection ListStages/WatchStages since this PR's earlier commits, but the UI never sent summary=true, so the user-visible perf gains in the PR description weren't being realised against the bundled UI. Wire it through: - pipelines.tsx asks for ListStages with summary=true. - useEventsWatcher takes an opts.summary flag and forwards it to Watcher.watchStages, which now sends summary on the watch request and uses it as part of the listStages cache key (so cache reads and writes hit the same bucket). - graph.tsx and list-view.tsx pass summary: true to useEventsWatcher. Cache-poisoning fix in Watcher: the existing watcher wrote every streamed Stage into the GetStage detail-cache. With summary mode that would seed detail views with stripped freightHistory, step bodies, and missing health output. Skip that write when the watch is in summary mode -- detail pages refetch the full Stage themselves. Lazy health output for the multi-app Argo CD dropdown: argocd-link previously read stage.status.health.output.raw, which summary mode strips. Lazy-fetch it via GetStageHealthOutputs, gated on "more than one app, blob missing" so single-app stages never trigger the call. The helper that derives per-app status now takes the raw JSON string directly so both code paths converge on the same parser. Signed-off-by: Jacob Boykin <boykinmusic@gmail.com> Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
Upstream main added a 4th `kargoNamespace string` parameter to the `NewInternalClient` callback in `pkg/server/kubernetes.ClientOptions` (akuity#6194). Update the new `get_stage_health_outputs_v1alpha1_test.go` to match the new signature. The other 40 server tests on this branch already came along with the upstream change during the rebase. Signed-off-by: Jacob Boykin <jacob.boykin@akuity.io>
jacobboykin
force-pushed
the
jboykin/stage-summary-api
branch
from
3f1ef32 to
133af73
Compare
Contributor
Author
|
I'm going to close this one for now. The underlying idea here is still valid: large Stage payloads can be expensive, and a summary projection plus lazy health-output fetches could be useful if Since opening this, we've validated a narrower backend fix for the more direct issue: list-then-watch flows should not replay the resources the UI just listed. That directly addresses the main performance problem we saw with large projects, including the Promotion watch case, with much less behavioral/API complexity. So I think the right move is to close this rather than keep rebasing and expanding it. If profiling after the watch/resourceVersion work lands still shows |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a lightweight Stage projection for clients rendering list and graph views of many Stages at once. Instead of a new endpoint, a
summaryflag is threaded through the existingListStagesandWatchStagesRPCs. When set, heavy fields are stripped from each returned Stage in place:status.freightHistorytruncated to the current element (index 0)spec.promotionTemplate.spec.steps[*]—config,if, andvarscleared;uses,task,as,continueOnError,retrypreserved so the UI can still derive step kind and countstatus.health.outputcleared — fetch lazily via the newGetStageHealthOutputsRPC for Stages currently in viewportThe UI continues to derive
hasVerificationfromspec.verification != nilandpromotionStepCountfromlen(spec.promotionTemplate.spec.steps).API
ListStages/WatchStagesgainsummary: boolListStagesResponseandWatchStagesRequestgainresource_versionfor list-then-watchGetStageHealthOutputs(project, stage_names[])RPC + REST endpoint for lazy per-Stage health output fetch (batch cap 1000)Structure
Reusable Stage-domain helpers are exported in
pkg/api/stage.goalongside the existingGetStage,ListFreightAvailableToStage, etc.:api.StripStageForSummary(*Stage)— in-place summary projectionapi.ListStageHealthOutputs(ctx, c, project, names)— parallel per-name health-output fetch (concurrency cap 16)Server handlers are thin adapters.
Measurements
Tested end-to-end against a live multi-project instance with around 1,900 Stages behind the public ingress, using the bundled UI on headless Chrome. With
summary=true, gzipped wire size onListStagesdrops by roughly 5–7× on large projects, JSON parse on the main thread (which blocks every cache write during a watch storm) is 2.4–3.9× faster, and heap retained per cached response drops about 4.8×. Browser-perceived fetch ends up 1.7–2.3× faster end-to-end. The wins scale with stage count, so they're most noticable where the pain is biggest.What's left on the table: each Stage still carries its promotion-step skeletons (kept so the UI can derive step kind and count without falling back to
GetStage), which puts a roughly 10 KB floor on stages with hundreds of steps. The response and watch request carryresourceVersionfor proper list-then-watch, but the controller-runtime cache reader doesn't populate it today, so resume currently lands as "watch from now"; a separate decision tracked in #6156.