Make console status flush cancellation-aware - #1228
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
/label reliability AI-generated. Review for accuracy. |
|
🗣️ We’d really appreciate your feedback here AI-generated. Review for accuracy. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (17)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (3)Injection prevention (prodsec-skills): SQL: parameterized queries only; no string concatenation Command: no shell=True, os.system, or backtick exec with user input LDAP/XPath: escape special characters in filters Path traversal: canonicaliz...⚙️ CodeRabbit configuration file Files:
Review test code for quality and patterns.⚙️ CodeRabbit configuration file Files:
Review Go code following OpenShift operator patterns.⚙️ CodeRabbit configuration file Files:
🔇 Additional comments (17)
Walkthrough
ChangesStatus context propagation
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to Canceled reconciliations now avoid persisting transient Degraded status while genuine active status-update failures retain their existing behavior. No current merge-blocking risk is identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: redhat-chai-bot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@redhat-chai-bot: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Failure-job linksThese public Prow runs carry the console Degraded assertion and cancellation signature associated with this change:
The recurring discriminator is AI-generated. Review for accuracy. |
This pull request was generated by the Product Reliability Agent. To help us improve product stability and CI, please focus review on functional correctness and material concerns. The agent will automatically address feedback, but we’d appreciate keeping non-blocking nits from delaying an otherwise correct PR. Within two business days, please merge, provide blocking feedback, or close the PR with a reason.
Why
During an upgrade or leader handoff, shutdown can cancel reconciliation that is still running in the outgoing console-operator leader. Before this change, status flushing outlived that reconciliation context, so an expected
context canceledresult from an in-flight configuration read could be persisted as a realDegradedcondition. A replacement leader could then publish the stale condition until its first successful synchronization cleared it. This made a healthy upgrade appear to have a console degradation and caused upgrade verification to reject otherwise healthy runs.This change makes status flushing honor the active reconciliation context: cancellation is propagated and is not persisted as a product degradation, while genuine errors from an active context retain their normal handling.
Failure evidence
The following public Prow runs carry the console Degraded assertion and cancellation signature associated with this issue. Direct Prow links for these runs are posted in the PR discussion.
2090566102311505920209064931046195609620907398752286842882095237695084171264209487174050827878420943531765546393602093153570210713600209267776196864000020923015181526671362093057874388324352The Azure run from 2026-08-20 has the strongest direct causal audit trail. The other listed runs share the same test and event signature; the GCP run independently contains the same cancellation signature, while the exact leader-handoff sequence is inferred from the audited case.
The recurring discriminator is
[Monitor:legacy-cvo-invariants][bz-Management Console] clusteroperator/console should not change condition/Degraded, with reasonConfigMapSync_FailedGetOLMConfig, message identifierConfigMapSyncDegraded, and anolmconfigs/clusterread ending incontext canceled. One FIPS run also showed the corresponding custom-route cleanup variant.Changes
Validation
context.Canceled.GOFLAGS=-mod=vendor go test ./pkg/console/statusmake verifymake check(unit tests, formatting, and vet)git diff --checkReview
A dedicated adversarial review independently reproduced and then verified the cancellation edge case. The final review disposition was APPROVE with no actionable or blocking findings.