-
Notifications
You must be signed in to change notification settings - Fork 37
fix: make controllers resilient to out-of-date status fields #1008
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
base: main
Are you sure you want to change the base?
fix: make controllers resilient to out-of-date status fields #1008
Conversation
7344437 to
91a7f95
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1008 +/- ##
==========================================
- Coverage 50.92% 50.25% -0.68%
==========================================
Files 51 51
Lines 5492 5562 +70
==========================================
- Hits 2797 2795 -2
- Misses 2399 2471 +72
Partials 296 296 ☔ View full report in Codecov by Sentry. |
ea60c7a to
d89d3b8
Compare
Bundle ReportBundle size has no change ✅ |
3d37272 to
bef951f
Compare
Fixes reconciliation loops in PullRequest and CommitStatus controllers See commit body for full details Signed-off-by: ZeBidule <[email protected]>
bef951f to
5035e44
Compare
| // Check if the status is already set correctly to avoid redundant API calls | ||
| // This prevents GitLab state transition errors when status is already set | ||
| if cs.Status.Sha == cs.Spec.Sha && cs.Status.Phase == cs.Spec.Phase { | ||
| logger.Info("Commit status already set correctly, skipping API call", "sha", cs.Spec.Sha, "phase", cs.Spec.Phase) | ||
| return ctrl.Result{}, 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 change would make it so that other changes (like the description changing) wouldn't be written to the SCM.
I think instead of making the change in commitstatus_controller (i.e. for all providers), we should make it on a per-provider basis. This github-specific fix is a good example: #1002
Fixes reconciliation loops in PullRequest and CommitStatus controllers that occur when status fields in Kubernetes resources are out of sync with the actual state in SCM providers.
Problem
Issue 1: PullRequest Reconciliation Loop
status.statefield is sometimes not updated in the K8S resourceExample error:
Issue 2: CommitStatus Reconciliation Loop
Example error:
Solution
1. PullRequest Providers (GitLab, GitHub, Forgejo)
2. CommitStatus Controller
status.shaandstatus.phasewithspecvalues before making API callsImpact
✅ Eliminates reconciliation loops caused by stale status fields
✅ Reduces unnecessary SCM API calls
✅ Prevents pipeline spam from repeated reconciliation attempts
✅ Improves resilience to transient status update failures
Testing
Related
This fix addresses the issue reported in the community discussion about promotion blocking due to out-of-sync status fields.