Skip to content

Make console status flush cancellation-aware - #1228

Open
redhat-chai-bot wants to merge 2 commits into
openshift:mainfrom
redhat-chai-bot:OCPBUGS-121388-cancellation-safe-status-flush
Open

Make console status flush cancellation-aware#1228
redhat-chai-bot wants to merge 2 commits into
openshift:mainfrom
redhat-chai-bot:OCPBUGS-121388-cancellation-safe-status-flush

Conversation

@redhat-chai-bot

@redhat-chai-bot redhat-chai-bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

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 canceled result from an in-flight configuration read could be persisted as a real Degraded condition. 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.

  • Azure OVN upgrade — 2026-08-20 22:05 UTC — run 2090566102311505920
  • Azure OVN upgrade — 2026-08-21 03:56 UTC — run 2090649310461956096
  • AWS OVN upgrade — 2026-08-21 09:56 UTC — run 2090739875228684288
  • AWS OVN upgrade with FIPS — 2026-09-02 19:49 UTC — run 2095237695084171264
  • AWS OVN upgrade with FIPS — 2026-09-01 19:35 UTC — run 2094871740508278784
  • AWS OVN upgrade with FIPS — 2026-08-31 09:14 UTC — run 2094353176554639360
  • AWS OVN upgrade with FIPS — 2026-08-28 01:47 UTC — run 2093153570210713600
  • AWS OVN upgrade with FIPS — 2026-08-26 18:17 UTC — run 2092677761968640000
  • AWS OVN upgrade with FIPS — 2026-08-25 17:21 UTC — run 2092301518152667136
  • GCP OVN RT upgrade — 2026-08-27 19:27 UTC — run 2093057874388324352

The 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 reason ConfigMapSync_FailedGetOLMConfig, message identifier ConfigMapSyncDegraded, and an olmconfigs/cluster read ending in context canceled. One FIPS run also showed the corresponding custom-route cleanup variant.

Changes

  • Pass the reconciliation context through status flush callers.
  • Skip status persistence when the context is canceled, including no-op update paths.
  • Preserve error precedence for active status-update failures.
  • Add focused coverage for no-op cancellation, mid-update cancellation, and active update errors.

Validation

  • Exact reproduced cancellation case returns context.Canceled.
  • GOFLAGS=-mod=vendor go test ./pkg/console/status
  • Focused cancellation suite: 100 repetitions passed.
  • Race-enabled cancellation suite: 20 repetitions passed.
  • make verify
  • make check (unit tests, formatting, and vet)
  • git diff --check

Review

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.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@redhat-chai-bot

Copy link
Copy Markdown
Contributor Author

/label reliability


AI-generated. Review for accuracy.

@redhat-chai-bot

Copy link
Copy Markdown
Contributor Author

🗣️ We’d really appreciate your feedback here


AI-generated. Review for accuracy.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Team

Run ID: 8c94ea68-eafe-40a7-9e5e-60fce35996ca

📥 Commits

Reviewing files that changed from the base of the PR and between 7fa0a80 and a3d4541.

📒 Files selected for processing (17)
  • pkg/console/controllers/clidownloads/controller.go
  • pkg/console/controllers/clioidcclientstatus/controller.go
  • pkg/console/controllers/downloadsdeployment/controller.go
  • pkg/console/controllers/healthcheck/controller.go
  • pkg/console/controllers/oauthclients/oauthclients.go
  • pkg/console/controllers/oauthclientsecret/oauthclientsecret.go
  • pkg/console/controllers/oidcsetup/oidcsetup.go
  • pkg/console/controllers/poddisruptionbudget/controller.go
  • pkg/console/controllers/route/controller.go
  • pkg/console/controllers/service/controller.go
  • pkg/console/controllers/serviceaccounts/controller.go
  • pkg/console/controllers/storageversionmigration/controller.go
  • pkg/console/controllers/upgradenotification/controller.go
  • pkg/console/operator/operator.go
  • pkg/console/operator/sync_v400.go
  • pkg/console/status/status.go
  • pkg/console/status/status_test.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • openshift/console (manual)

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:

  • pkg/console/controllers/service/controller.go
  • pkg/console/controllers/clidownloads/controller.go
  • pkg/console/controllers/route/controller.go
  • pkg/console/controllers/upgradenotification/controller.go
  • pkg/console/controllers/poddisruptionbudget/controller.go
  • pkg/console/controllers/healthcheck/controller.go
  • pkg/console/controllers/oidcsetup/oidcsetup.go
  • pkg/console/controllers/serviceaccounts/controller.go
  • pkg/console/controllers/clioidcclientstatus/controller.go
  • pkg/console/operator/sync_v400.go
  • pkg/console/operator/operator.go
  • pkg/console/controllers/storageversionmigration/controller.go
  • pkg/console/controllers/oauthclientsecret/oauthclientsecret.go
  • pkg/console/controllers/oauthclients/oauthclients.go
  • pkg/console/status/status.go
  • pkg/console/status/status_test.go
  • pkg/console/controllers/downloadsdeployment/controller.go
Review test code for quality and patterns.

⚙️ CodeRabbit configuration file

Files:

  • pkg/console/status/status_test.go
Review Go code following OpenShift operator patterns.

⚙️ CodeRabbit configuration file

Files:

  • pkg/console/controllers/service/controller.go
  • pkg/console/controllers/clidownloads/controller.go
  • pkg/console/controllers/route/controller.go
  • pkg/console/controllers/upgradenotification/controller.go
  • pkg/console/controllers/poddisruptionbudget/controller.go
  • pkg/console/controllers/healthcheck/controller.go
  • pkg/console/controllers/oidcsetup/oidcsetup.go
  • pkg/console/controllers/serviceaccounts/controller.go
  • pkg/console/controllers/clioidcclientstatus/controller.go
  • pkg/console/operator/sync_v400.go
  • pkg/console/operator/operator.go
  • pkg/console/controllers/storageversionmigration/controller.go
  • pkg/console/controllers/oauthclientsecret/oauthclientsecret.go
  • pkg/console/controllers/oauthclients/oauthclients.go
  • pkg/console/status/status.go
  • pkg/console/status/status_test.go
  • pkg/console/controllers/downloadsdeployment/controller.go
🔇 Additional comments (17)
pkg/console/status/status.go (1)

182-192: LGTM!

Also applies to: 201-208

pkg/console/status/status_test.go (1)

4-5: LGTM!

Also applies to: 10-10, 127-178, 180-199, 201-250, 252-273, 275-312, 314-341

pkg/console/controllers/clioidcclientstatus/controller.go (1)

95-95: LGTM!

Also applies to: 119-119, 143-143, 145-145

pkg/console/controllers/oauthclients/oauthclients.go (1)

131-131: LGTM!

Also applies to: 157-157, 182-182, 194-194, 197-197

pkg/console/controllers/oauthclientsecret/oauthclientsecret.go (1)

125-125: LGTM!

Also applies to: 130-130, 136-136, 142-142, 148-148, 153-153

pkg/console/controllers/oidcsetup/oidcsetup.go (1)

127-127: LGTM!

Also applies to: 156-156, 180-180, 182-182

pkg/console/controllers/storageversionmigration/controller.go (1)

75-75: LGTM!

pkg/console/operator/sync_v400.go (1)

75-75: LGTM!

Also applies to: 79-79, 98-98, 105-105, 112-112, 128-128, 155-155, 161-161, 167-167, 173-173, 194-194, 200-200, 206-206, 219-219, 226-226, 232-232, 252-252, 282-282, 289-289, 293-293

pkg/console/controllers/clidownloads/controller.go (1)

130-130: LGTM!

Also applies to: 134-134, 138-138, 143-143, 172-172, 175-175

pkg/console/controllers/downloadsdeployment/controller.go (1)

104-104: LGTM!

Also applies to: 110-110, 114-114

pkg/console/controllers/healthcheck/controller.go (1)

99-99: LGTM!

Also applies to: 119-119, 124-124, 143-143, 153-153

pkg/console/controllers/poddisruptionbudget/controller.go (1)

105-105: LGTM!

pkg/console/controllers/route/controller.go (1)

135-135: LGTM!

Also applies to: 139-139, 145-145, 150-150, 157-157, 162-162, 167-167, 179-179, 193-193, 197-197

pkg/console/controllers/service/controller.go (1)

115-115: LGTM!

Also applies to: 119-119, 123-123, 139-139, 148-148

pkg/console/controllers/serviceaccounts/controller.go (1)

115-115: LGTM!

Also applies to: 118-118

pkg/console/controllers/upgradenotification/controller.go (1)

98-98: LGTM!

pkg/console/operator/operator.go (1)

419-419: LGTM!


Walkthrough

FlushAndReturn now uses reconciliation contexts, checks cancellation before and after status persistence, and uses the context for status updates. All console controller and operator call sites pass ctx. Tests cover cancellation, update errors, and degraded status persistence.

Changes

Status context propagation

Layer / File(s) Summary
Context-aware status handler and tests
pkg/console/status/status.go, pkg/console/status/status_test.go
FlushAndReturn accepts context.Context, handles cancellation around status persistence, and preserves defined error precedence. Tests cover cancellation timing, update failures, and degraded status updates.
Controller call-site updates
pkg/console/controllers/*
Console controllers pass ctx on error, requeue, and success paths.
Operator call-site updates
pkg/console/operator/*
Operator removal and versioned synchronization paths pass ctx to FlushAndReturn.

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to a3d45

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: jhadvig, sg00dwin, therealjon

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 17 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PASS. The pull request adds only standard Go tests in pkg/console/status/status_test.go; it does not add Ginkgo It, Describe, Context, or similar titles. The five Test... names and the two `…
Test Structure And Quality ✅ Passed PASS: The pull request changes only pkg/console/status/status_test.go for tests, and those tests use Go's standard testing package, not Ginkgo. They use in-memory fake operator clients, so no clus…
Microshift Test Compatibility ✅ Passed PASS: The pull request adds only ordinary Go unit tests in pkg/console/status/status_test.go using testing.T and t.Run. It adds no Ginkgo It, Describe, Context, or When e2e tests. The ot…
Single Node Openshift (Sno) Test Compatibility ✅ Passed The pull request adds no Ginkgo e2e tests. The only added tests are standard Go Test... unit tests in pkg/console/status/status_test.go; the other changes update production call sites. Therefore, …
Topology-Aware Scheduling Compatibility ✅ Passed PASS — The pull request does not introduce topology-sensitive scheduling behavior. The diff changes only FlushAndReturn context propagation, cancellation handling, and status tests. It adds no manif…
Ote Binary Stdout Contract ✅ Passed PASS: The pull request does not introduce any stdout write or logging configuration change in process-level code. The diff only updates FlushAndReturn calls to pass ctx, changes cancellation handl…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS: The pull request adds only standard Go unit tests in pkg/console/status/status_test.go, using Test... functions and testing.T. It adds no Ginkgo e2e tests, IPv4 assumptions, or external ne…
No-Weak-Crypto ✅ Passed PASS. The PR diff only changes FlushAndReturn calls to pass context.Context, updates status flushing, and adds cancellation tests. No added MD5, SHA-1, DES/3DES, RC4, Blowfish, ECB, custom crypto,…
Container-Privileges ✅ Passed PASS: The pull request changes only Go source and Go tests. The diff adds no container or Kubernetes manifest fields for privileged mode, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEsc…
No-Sensitive-Data-In-Logs ✅ Passed No sensitive-data logging was introduced. The PR diff adds only context arguments, cancellation handling, and tests; the added-line scan found no new logging API calls. Existing klog statements and …
Title check ✅ Passed The title is concise, specific, and accurately describes the main change: making console status flushing cancellation-aware.
Description check ✅ Passed The description clearly covers the root cause, solution, failure evidence, validation, and review outcome. It does not use the repository template headings and omits browser and reviewer metadata, but…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot added the reliability Categorizes an issue as related to the Product Reliability Agent. label Sep 8, 2026
@openshift-ci
openshift-ci Bot requested review from jhadvig and spadgett September 8, 2026 00:12
@openshift-ci

openshift-ci Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: redhat-chai-bot
Once this PR has been reviewed and has the lgtm label, please assign therealjon for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci

openshift-ci Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

@redhat-chai-bot: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-images a3d4541 link true /test okd-scos-images

Full PR test history. Your PR dashboard.

Details

Instructions 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.

@redhat-chai-bot

Copy link
Copy Markdown
Contributor Author

Failure-job links

These public Prow runs carry the console Degraded assertion and cancellation signature associated with this change:

The recurring discriminator is [Monitor:legacy-cvo-invariants][bz-Management Console] clusteroperator/console should not change condition/Degraded, with reason ConfigMapSync_FailedGetOLMConfig, message identifier ConfigMapSyncDegraded, and an olmconfigs/cluster read ending in context canceled. The Azure run from 2026-08-20 has the strongest direct causal audit trail; the other listed runs share the same test and event signature.


AI-generated. Review for accuracy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reliability Categorizes an issue as related to the Product Reliability Agent.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant