-
Notifications
You must be signed in to change notification settings - Fork 167
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
fix(promo): record failure message for Argo CD Application #2179
Conversation
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
✅ Deploy Preview for docs-kargo-akuity-io ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2179 +/- ##
==========================================
- Coverage 46.37% 46.31% -0.06%
==========================================
Files 242 242
Lines 16737 16763 +26
==========================================
+ Hits 7761 7763 +2
- Misses 8606 8629 +23
- Partials 370 371 +1 ☔ View full report in Codecov by Sentry. |
@@ -156,6 +157,16 @@ func (a *argoCDMechanism) Promote( | |||
logger.Info(err.Error()) | |||
} | |||
if phase.Failed() { | |||
// Record the reason for the failure if available. | |||
if app.Status.OperationState != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This piggybacks on the fact that we short-circuit. In the initial version, I collected the messages together with the phases. But this grew into something unnecessarily complex and over-engineered, which I would like to avoid as long as we do not need it.
My apologies on that. I think the memory of seeing error messages surface in the status and the UI fooled me into believing I'd seen the same of failure messages. |
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
This addresses #2162, which unlike the issue indicates, never actually worked in any Kargo version.