NO-ISSUE: Fix TestOperator_upgradeableSync by hongkailiu · Pull Request #1373 · openshift/cluster-version-operator

hongkailiu · 2026-04-11T15:34:50Z

We have discussed internally about this a while ago and thanks to @DavidHurta to provide the solution.

$ go test -run TestOperator_upgradeableSync ./pkg/cvo/... -count=1000
ok      github.com/openshift/cluster-version-operator/pkg/cvo   134.432s
ok      github.com/openshift/cluster-version-operator/pkg/cvo/configuration     0.746s [no tests to run]
ok      github.com/openshift/cluster-version-operator/pkg/cvo/internal  1.951s [no tests to run]
?       github.com/openshift/cluster-version-operator/pkg/cvo/internal/dynamicclient    [no test files]

It seems to bite us more often than before in CI, e.g., job1, and
job2 and
job3.

I still do not fully understand why we have to add configManagedInformer.WaitForCacheSync(ctx.Done()) or cannot prove that it is the best way to avoid racing but we use it in the core code too. Considering we change only testing code, I would like to have it to avoid unnecessary failures CI.

Summary by CodeRabbit

Tests
- Improved test reliability by ensuring cache synchronization completes before test execution.

@DavidHurta

We have discussed [internally](https://redhat-internal.slack.com/archives/CJ1J9C3V4/p1727780218907479?thread_ts=1727759381.029409&cid=CJ1J9C3V4) about this a while ago and thanks to @DavidHurta to provide the solution. ```console $ go test -run TestOperator_upgradeableSync ./pkg/cvo/... -count=1000 ok github.com/openshift/cluster-version-operator/pkg/cvo 134.432s ok github.com/openshift/cluster-version-operator/pkg/cvo/configuration 0.746s [no tests to run] ok github.com/openshift/cluster-version-operator/pkg/cvo/internal 1.951s [no tests to run] ? github.com/openshift/cluster-version-operator/pkg/cvo/internal/dynamicclient [no test files] ``` It seems to bite us more often than before in CI, e.g., [job1](https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-version-operator/1371/pull-ci-openshift-cluster-version-operator-main-unit/2042797754249383936), and [job2](https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-version-operator/1360/pull-ci-openshift-cluster-version-operator-main-unit/2039307795836178432) and [job3](https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-version-operator/1355/pull-ci-openshift-cluster-version-operator-main-unit/2035211529078444032). I still do not fully understand why we have to add `configManagedInformer.WaitForCacheSync(ctx.Done())` or cannot prove that it is the best way to avoid racing but we use it in [the core code](https://github.com/openshift/cluster-version-operator/blob/e9c1c39e21ec353ff5993d386c42bc1b15063dbf/pkg/start/start.go#L257) too. Considering we change only testing code, I would like to have it to avoid unnecessary failures CI.

coderabbitai · 2026-04-11T15:35:09Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: f2c4dcf8-d43e-4179-b6ad-d9dac2f24ca6

📥 Commits

Reviewing files that changed from the base of the PR and between e9c1c39 and fc09e31.

📒 Files selected for processing (1)

pkg/cvo/cvo_test.go

Walkthrough

A test fix that adds cache synchronization waiting for the ConfigMap-backed shared informer in TestOperator_upgradeableSync. The test now blocks until the cache is fully synced before proceeding with ConfigMap operations and event notifications.

Changes

Cohort / File(s)	Summary
Test Cache Synchronization `pkg/cvo/cvo_test.go`	Added `WaitForCacheSync(ctx.Done())` call in `TestOperator_upgradeableSync` to ensure the ConfigMap informer cache is fully synchronized before test proceeds.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

wking

/lgtm

openshift-ci · 2026-04-11T16:35:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hongkailiu, wking

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [hongkailiu,wking]

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

hongkailiu · 2026-04-13T12:53:32Z

/verified by @hongkailiu

openshift-ci-robot · 2026-04-13T12:54:50Z

@hongkailiu: This PR has been marked as verified by @hongkailiu.

Details

In response to this:

/verified by @hongkailiu

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 openshift-eng/jira-lifecycle-plugin repository.

wking · 2026-04-15T17:09:40Z

Internal test-cases only affect e2e-agnostic-operator and/or unit, so these other failures are orthogonal:

/override ci/prow/e2e-aws-ovn-techpreview
/override ci/prow/e2e-hypershift-conformance

openshift-ci · 2026-04-15T17:10:02Z

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-aws-ovn-techpreview, ci/prow/e2e-hypershift-conformance

Details

In response to this:

Internal e2e only affects e2e-agnostic-operator and/or unit, so these other failures are orthogonal:

/override ci/prow/e2e-aws-ovn-techpreview
/override ci/prow/e2e-hypershift-conformance

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.

wking · 2026-04-15T17:14:47Z

/label acknowledge-critical-fixes-only

openshift-ci-robot · 2026-04-15T17:21:54Z

@hongkailiu: This pull request explicitly references no jira issue.

Details

In response to this:

We have discussed internally about this a while ago and thanks to @DavidHurta to provide the solution.
$ go test -run TestOperator_upgradeableSync ./pkg/cvo/... -count=1000
ok      github.com/openshift/cluster-version-operator/pkg/cvo   134.432s
ok      github.com/openshift/cluster-version-operator/pkg/cvo/configuration     0.746s [no tests to run]
ok      github.com/openshift/cluster-version-operator/pkg/cvo/internal  1.951s [no tests to run]
?       github.com/openshift/cluster-version-operator/pkg/cvo/internal/dynamicclient    [no test files]
It seems to bite us more often than before in CI, e.g., job1, and
job2 and
job3.

I still do not fully understand why we have to add configManagedInformer.WaitForCacheSync(ctx.Done()) or cannot prove that it is the best way to avoid racing but we use it in the core code too. Considering we change only testing code, I would like to have it to avoid unnecessary failures CI.

Summary by CodeRabbit

Tests

Improved test reliability by ensuring cache synchronization completes before test execution.

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-merge-bot · 2026-04-15T18:32:31Z

/retest-required

Remaining retests: 0 against base HEAD 6de1611 and 2 for PR HEAD fc09e31 in total

wking · 2026-04-15T18:36:33Z

/override ci/prow/e2e-agnostic-operator
/override ci/prow/e2e-agnostic-ovn
/override ci/prow/e2e-agnostic-ovn-techpreview-serial-1of3
/override ci/prow/e2e-agnostic-ovn-techpreview-serial-2of3
/override ci/prow/e2e-agnostic-ovn-techpreview-serial-3of3
/override ci/prow/e2e-agnostic-ovn-upgrade-into-change
/override ci/prow/e2e-agnostic-ovn-upgrade-out-of-change
/override ci/prow/e2e-hypershift

openshift-ci · 2026-04-15T18:37:10Z

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-agnostic-operator, ci/prow/e2e-agnostic-ovn, ci/prow/e2e-agnostic-ovn-techpreview-serial-1of3, ci/prow/e2e-agnostic-ovn-techpreview-serial-2of3, ci/prow/e2e-agnostic-ovn-techpreview-serial-3of3, ci/prow/e2e-agnostic-ovn-upgrade-into-change, ci/prow/e2e-agnostic-ovn-upgrade-out-of-change, ci/prow/e2e-hypershift

Details

In response to this:

/override ci/prow/e2e-agnostic-operator
/override ci/prow/e2e-agnostic-ovn
/override ci/prow/e2e-agnostic-ovn-techpreview-serial-1of3
/override ci/prow/e2e-agnostic-ovn-techpreview-serial-2of3
/override ci/prow/e2e-agnostic-ovn-techpreview-serial-3of3
/override ci/prow/e2e-agnostic-ovn-upgrade-into-change
/override ci/prow/e2e-agnostic-ovn-upgrade-out-of-change
/override ci/prow/e2e-hypershift

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.

openshift-ci · 2026-04-15T18:41:15Z

@hongkailiu: all tests passed!

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.

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 11, 2026

wking approved these changes Apr 11, 2026

View reviewed changes

openshift-ci bot assigned wking Apr 11, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Apr 13, 2026

openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Apr 15, 2026

wking changed the title ~~Fix TestOperator_upgradeableSync~~ NO-ISSUE: Fix TestOperator_upgradeableSync Apr 15, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 15, 2026

openshift-merge-bot bot merged commit d3df6c0 into openshift:main Apr 15, 2026
19 checks passed

Conversation

hongkailiu commented Apr 11, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

wking left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Apr 11, 2026

Uh oh!

hongkailiu commented Apr 13, 2026

Uh oh!

openshift-ci-robot commented Apr 13, 2026

Uh oh!

wking commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Apr 15, 2026

Uh oh!

wking commented Apr 15, 2026

Uh oh!

openshift-ci-robot commented Apr 15, 2026

Summary by CodeRabbit

Uh oh!

openshift-merge-bot bot commented Apr 15, 2026

Uh oh!

wking commented Apr 15, 2026

Uh oh!

openshift-ci bot commented Apr 15, 2026

Uh oh!

Uh oh!

openshift-ci bot commented Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hongkailiu commented Apr 11, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 11, 2026 •

edited

Loading

wking commented Apr 15, 2026 •

edited

Loading