-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(controller): avoid patching Stage from Promotions reconciler #1919
Conversation
✅ Deploy Preview for docs-kargo-akuity-io canceled.
|
b746183
to
8fbc1c2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1919 +/- ##
==========================================
+ Coverage 45.61% 45.90% +0.29%
==========================================
Files 238 238
Lines 16477 16545 +68
==========================================
+ Hits 7516 7595 +79
+ Misses 8590 8578 -12
- Partials 371 372 +1 ☔ View full report in Codecov by Sentry. |
4f61ba6
to
ff63007
Compare
682a803
to
121cf8e
Compare
8599114
to
121cf8e
Compare
Signed-off-by: Hidde Beydals <[email protected]>
121cf8e
to
0db849b
Compare
Signed-off-by: Hidde Beydals <[email protected]>
Signed-off-by: Hidde Beydals <[email protected]>
- Steady phase is now determined by the synchronization of Promotions. - Cache can no longer be stale, as we now determine and update this bit ourselves. Signed-off-by: Hidde Beydals <[email protected]>
This avoids a pre-mature reset of the phase when it is actually e.g. verifying. Signed-off-by: Hidde Beydals <[email protected]>
Signed-off-by: Hidde Beydals <[email protected]>
Without this, subsequent actions on e.g. the verification history in the same reconciliation run would end up being set in `status.lastPromotion`. Signed-off-by: Hidde Beydals <[email protected]>
This addresses an issue when multiple Promotions are (automatically) generated in a small timeframe. Which causes their `creationTimestamp` to equal, as it does not include nanoseconds. As the generated Promotion name does include this, use this to lexicographically order them. Signed-off-by: Hidde Beydals <[email protected]>
Assuming a scenario where Freight would be "repromoted" (i.e. A -> B -> C -> A), the history would not accurately reflect this as "A" would already be on the stack. To address this, push any newly promoted Freight to the top of the stack (instead of potentially updating it). Signed-off-by: Hidde Beydals <[email protected]>
This reverts the relying on a timestamp in the Status, in favor of simply lexicographically ordering the Promotion names. Signed-off-by: Hidde Beydals <[email protected]>
This prevents an AnalysisRun from being created if there are still pending Promotions in the queue. Signed-off-by: Hidde Beydals <[email protected]>
Signed-off-by: Hidde Beydals <[email protected]>
0db849b
to
3cdd53c
Compare
@hiddeco do you think we should remove the now-unused |
It is still being used by the queue for Promotions itself. |
Ah. I see it now. 👍 |
This pull request changes the way Stages keep track of their current Freight, by removing the patching of the Stage from the Promotion reconciler and replacing it with logic to allow the Stage itself to self-determine what its current Freight is based on the Promotions existing for it.
From a bird's-eye view, it performs the following steps during the reconciliation of a Stage:
In addition to this: