-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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: cert-manager.io/certificate health.lua for consistent issuing (Issue #16523) #16520
Conversation
Signed-off-by: Chris Murray <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #16520 +/- ##
=======================================
Coverage 49.49% 49.49%
=======================================
Files 270 270
Lines 47485 47485
=======================================
+ Hits 23501 23502 +1
+ Misses 21673 21672 -1
Partials 2311 2311 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Chris Murray <[email protected]>
Signed-off-by: Chris Murray <[email protected]>
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.
Thanks @chr15murray!! LGTM!!
…ssue argoproj#16523) (argoproj#16520) * Update cert-manager.opcertificate health.lua Signed-off-by: Chris Murray <[email protected]> * adding test case for cert issuing Signed-off-by: Chris Murray <[email protected]> * fixing typo Signed-off-by: Chris Murray <[email protected]> --------- Signed-off-by: Chris Murray <[email protected]>
…ssue argoproj#16523) (argoproj#16520) * Update cert-manager.opcertificate health.lua Signed-off-by: Chris Murray <[email protected]> * adding test case for cert issuing Signed-off-by: Chris Murray <[email protected]> * fixing typo Signed-off-by: Chris Murray <[email protected]> --------- Signed-off-by: Chris Murray <[email protected]>
…ssue argoproj#16523) (argoproj#16520) * Update cert-manager.opcertificate health.lua Signed-off-by: Chris Murray <[email protected]> * adding test case for cert issuing Signed-off-by: Chris Murray <[email protected]> * fixing typo Signed-off-by: Chris Murray <[email protected]> --------- Signed-off-by: Chris Murray <[email protected]>
…ssue argoproj#16523) (argoproj#16520) * Update cert-manager.opcertificate health.lua Signed-off-by: Chris Murray <[email protected]> * adding test case for cert issuing Signed-off-by: Chris Murray <[email protected]> * fixing typo Signed-off-by: Chris Murray <[email protected]> --------- Signed-off-by: Chris Murray <[email protected]> Signed-off-by: Kevin Lyda <[email protected]>
With this change, certificates always show as "Progressing". Given that it loops over all status first, looking for type As an example, here is the status field
The certificate renewal was issued, but the logic will see the second status and set the status to Progressing. |
@cyrus-mc can you open an issue? |
@crenshaw-dev Issue created. |
…ssue argoproj#16523) (argoproj#16520) * Update cert-manager.opcertificate health.lua Signed-off-by: Chris Murray <[email protected]> * adding test case for cert issuing Signed-off-by: Chris Murray <[email protected]> * fixing typo Signed-off-by: Chris Murray <[email protected]> --------- Signed-off-by: Chris Murray <[email protected]>
Fixes [ISSUE #16523]
Checklist:
The health of a cert-manager.io/certificate is not always consistent within ArgoCD when Issuing and Ready states both exist. This occurs because the
Ready: False
state may come first or second in the status conditions.Example Degraded
Example Issuing
This PR checks for Issuing first and places the resource health into 'Progressing' state if True before considering the Ready status.
I've also add a new test case for this. With the previous health check that test case would be in a Degraded state.