Skip to content

feat(controller): record a Stage's current and last PromotionRequest - #6843

Open
jsbroks wants to merge 1 commit into
mainfrom
justin/promotion-request-status-writer
Open

feat(controller): record a Stage's current and last PromotionRequest#6843
jsbroks wants to merge 1 commit into
mainfrom
justin/promotion-request-status-writer

Conversation

@jsbroks

@jsbroks jsbroks commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Builds on #6842 (merged), which added the types these fields use.

Summary

A new syncPromotionRequests step in the Stage reconciler lists the
PromotionRequests promoting on the Stage's behalf and records two of them in
status:

  • currentPromotionRequest — the request the Stage is promoting through,
    cleared when there is none, so a finished request never looks active.
  • lastPromotionRequest — the newest request to have reached a terminal phase.

Selection matches syncPromotions, because the situation is the same one

A Stage can have several PromotionRequests in flight. Auto-promotion creates one
only when none exists in any phase, but promote_to_stage and
promote_downstream create one per call with no in-flight check, so consecutive
promotions queue up — the same way consecutive Promotions do.

So the requests are sorted with api.ComparePromotionRequestByPhaseAndCreationTime,
the PromotionRequest counterpart of ComparePromotionByPhaseAndCreationTime
(Running first, then non-terminal by ULID ascending, then terminal by ULID
descending), and the Stage records Items[0] as current — exactly as
syncPromotions takes the first sorted Promotion as its current one. That
parallel matters beyond cosmetics: the Promotion reconciler gates on
stage.Status.CurrentPromotion.Name == promo.Name, so a Stage's status is what
serializes its queue. Keeping the same shape lets the Enterprise fan-out
reconciler gate on currentPromotionRequest the same way.

As in syncPromotions, name order stands in for creation order. A generated
PromotionRequest name is <stage>.<ulid>.<short-hash>, so among one Stage's
requests everything left of the ULID is identical and comparing names whole is
comparing the ULIDs. Child Promotion names carry a Target segment before the
ULID; PromotionRequest names carry no such segment, so the divergence that bit
there cannot arise here, and no ULID-extraction machinery is needed.

lastPromotionRequest only moves forward

Terminal requests sort newest-first, so the first terminal request in the sorted
list is the newest. It is recorded only when it is newer than the one already
recorded: a Stage's account of how its last round of fan-out ended should outlive
the request that produced it, so garbage collection of the newest request must
not let an older one take its place.

Other pieces

  • indexer.PromotionRequestsByStage, to list a Stage's requests.
  • kargo.PromotionRequestPhaseChanged, so a phase change — or the deletion of a
    non-terminal request, which must clear the current reference — enqueues the
    Stage. Both mirror what already exists for Promotions.
  • The reference's freight is a PromotionRequestFreightReference carrying
    only the Freight's name, copied straight from spec.freight — no lookup, no
    error path, and nothing to go stale if the Freight is garbage-collected.
  • OSS, not EE. feat(api): add currentPromotionRequest and latestPromotionRequest #6842 documented both fields as ignored in OSS; this writer is
    in OSS, so that note is replaced. OSS maintains the fields, and the request
    they refer to simply never gets past being marked Errored, because fanning
    Freight out to Targets is an Enterprise-only feature.
  • The stale reasoning in createAutoPromotionRequest about Stage status not
    tracking PromotionRequests is updated. Its stricter guard is unchanged and
    still justified: these fields mirror a request's own phase, not a Stage having
    absorbed its outcome.

Testing

  • TestRegularStageReconciler_syncPromotionRequests — 9 cases: list error,
    clearing a stale current reference, other Stages' requests ignored, a
    non-terminal request becoming current (with its name-only freight reference),
    Running over Pending, older Pending over newer, terminal becoming last, both
    references at once, and last never moving backwards. Fixtures use
    GeneratePromotionRequestName, so the ordering cases run against real
    generated names.
  • TestComparePromotionRequestByPhaseAndCreationTime (with an antisymmetry
    check per case), TestComparePromotionRequestPhase,
    TestPromotionRequestsByStage, TestPromotionRequestPhaseChanged.
  • go test -race green over pkg/controller/..., pkg/api/...,
    pkg/server/..., pkg/garbage/....
  • make lint-go 0 issues; codegen-controller / codegen-openapi /
    codegen-ui re-run for the doc-comment changes.

@kargo-governance-bot kargo-governance-bot Bot added needs/area Issue or PR needs to be labeled to indicate what parts of the code base are affected needs/kind Issue or PR needs to be labeled to clarify its nature needs/priority Priority has not yet been determined; a good signal that maintainers aren't fully committed labels Aug 19, 2026
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.15873% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.12%. Comparing base (9ba0795) to head (456d00a).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
pkg/controller/stages/regular_stages.go 65.75% 24 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6843      +/-   ##
==========================================
+ Coverage   31.04%   31.12%   +0.07%     
==========================================
  Files         835      835              
  Lines       81453    81579     +126     
==========================================
+ Hits        25291    25391     +100     
- Misses      54690    54716      +26     
  Partials     1472     1472              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jsbroks
jsbroks force-pushed the justin/promotion-request-status-writer branch 2 times, most recently from 3663ade to 7ef55e3 Compare August 19, 2026 20:19
@jsbroks
jsbroks force-pushed the justin/promotion-request-status-types branch from 0c72185 to c192ee3 Compare August 20, 2026 00:22
@jsbroks
jsbroks force-pushed the justin/promotion-request-status-writer branch from 7ef55e3 to 185329f Compare August 20, 2026 13:49
@jsbroks
jsbroks force-pushed the justin/promotion-request-status-types branch from c192ee3 to 902d7bb Compare August 20, 2026 20:50
Base automatically changed from justin/promotion-request-status-types to main August 21, 2026 16:07
@jsbroks
jsbroks force-pushed the justin/promotion-request-status-writer branch from 185329f to 2ad48c3 Compare August 21, 2026 17:51
@netlify

netlify Bot commented Aug 21, 2026

Copy link
Copy Markdown

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit 456d00a
🔍 Latest deploy log https://app.netlify.com/projects/docs-kargo-io/deploys/6a8c50f89db63600086a106a
😎 Deploy Preview https://deploy-preview-6843.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@jsbroks
jsbroks force-pushed the justin/promotion-request-status-writer branch from 2ad48c3 to 97e19e5 Compare August 21, 2026 17:56
The Stage reconciler gains a syncPromotionRequests step that lists the
PromotionRequests promoting on the Stage's behalf and records two of them in
status: the one the Stage is promoting through, and the newest to have reached a
terminal phase.

Selection matches syncPromotions exactly, because the situation is the same one.
A Stage can have several PromotionRequests in flight -- auto-promotion creates
one only when none exists in any phase, but the promote endpoints create one per
call, so consecutive promotions queue up. So the requests are sorted with
ComparePromotionRequestByPhaseAndCreationTime, the PromotionRequest counterpart
of ComparePromotionByPhaseAndCreationTime -- Running first, then non-terminal by
ULID ascending, then terminal by ULID descending -- and the Stage records the
first as current, exactly as it takes the first sorted Promotion as its current
Promotion.

As there, name order stands in for creation order. A generated PromotionRequest
name is <stage>.<ulid>.<short-hash>, so among the requests of a single Stage
everything left of the ULID is identical and comparing names whole is comparing
the ULIDs. (Child Promotion names carry a Target segment before the ULID;
PromotionRequest names carry no such segment, so the divergence possible there
cannot arise here.)

The last PromotionRequest only ever moves forward. Terminal requests sort
newest-first, so the first terminal request in the sorted list is the newest, and
it is recorded only when it is newer than the one already recorded: a Stage's
account of how its last round of fan-out ended should outlive the request that
produced it, so garbage collection of the newest request must not let an older
one take its place.

Supporting pieces: a PromotionRequestsByStage index to list a Stage's requests,
and a PromotionRequestPhaseChanged predicate so a phase change -- or the
deletion of a non-terminal request, which must clear the current reference --
enqueues the Stage. Both mirror what already exists for Promotions.

The two StageStatus fields are no longer documented as ignored in OSS, since
this writer is in OSS: OSS maintains them, and the request they refer to simply
never gets further than being marked Errored because fanning Freight out to
Targets is an Enterprise-only feature. The stale reasoning in
createAutoPromotionRequest about status not tracking PromotionRequests is
updated too; its stricter guard is unchanged, and still justified.

Signed-off-by: Justin Brooks <justin.brooks@akuity.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jsbroks
jsbroks force-pushed the justin/promotion-request-status-writer branch from 97e19e5 to 456d00a Compare August 24, 2026 14:11
@jsbroks
jsbroks marked this pull request as ready for review August 24, 2026 15:52
@jsbroks
jsbroks requested a review from a team as a code owner August 24, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs/area Issue or PR needs to be labeled to indicate what parts of the code base are affected needs/kind Issue or PR needs to be labeled to clarify its nature needs/priority Priority has not yet been determined; a good signal that maintainers aren't fully committed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant