Skip to content

OCPBUGS-105606: Allow DualReplica stable-system CVO Available blips from NoExecuteTaintManager - #31501

Open
eggfoobar wants to merge 2 commits into
openshift:mainfrom
eggfoobar:allow-dualreplica-stable-cvo-available-blips
Open

OCPBUGS-105606: Allow DualReplica stable-system CVO Available blips from NoExecuteTaintManager#31501
eggfoobar wants to merge 2 commits into
openshift:mainfrom
eggfoobar:allow-dualreplica-stable-cvo-available-blips

Conversation

@eggfoobar

@eggfoobar eggfoobar commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Serial NoExecuteTaintManager tests taint a control-plane node on DualReplica (masters are also workers), briefly driving csi-snapshot-controller and packageserver Available=False. Mirror the existing upgrade-path exceptions in the stable-system monitor so TNF serial jobs do not fail on these expected blips.

Summary by CodeRabbit

  • Tests
    • Improved upgrade and disruption testing for dual-replica configurations.
    • Added a one-minute grace period to accommodate expected temporary availability conditions during recovery.
    • Scoped exceptions for transient snapshot-controller and package-server status changes to relevant recovery windows.
    • Continued to treat unrelated availability failures as test failures.
    • Added coverage for conditions occurring during, immediately after, and outside disruptive test intervals.

@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Aug 11, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@eggfoobar: This pull request references Jira Issue OCPBUGS-105606, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Serial NoExecuteTaintManager tests taint a control-plane node on DualReplica (masters are also workers), briefly driving csi-snapshot-controller and packageserver Available=False. Mirror the existing upgrade-path exceptions in the stable-system monitor so TNF serial jobs do not fail on these expected blips.

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.

@coderabbitai

coderabbitai Bot commented Aug 11, 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: Enterprise

Run ID: 06da34b7-6a83-499b-b94b-c167a6d97946

📥 Commits

Reviewing files that changed from the base of the PR and between 64bb99a and 638822f.

📒 Files selected for processing (2)
  • pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go
  • pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators_test.go

Walkthrough

The stable operator transition monitor detects DualReplica topology and scopes specific transient conditions to NoExecuteTaintManager test intervals plus a one-minute grace period.

Changes

DualReplica exception handling

Layer / File(s) Summary
NoExecuteTaintManager interval scoping and tests
pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go, pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators_test.go
Helpers identify NoExecuteTaintManager intervals and include a one-minute grace period. Table-driven tests cover overlapping, post-grace, and pre-test intervals.
Scoped operator-condition exceptions
pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go
DualReplica handling passes condition intervals to exception evaluation. CSI snapshot controller Waiting for Deployment and package-server ClusterServiceVersionNotSucceeded conditions are excepted only during matching windows.

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

Mergeability Score: ⚪ Minimal · up to 63882

The change is limited to allowing expected DualReplica availability blips during NoExecuteTaintManager tests; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested labels: approved, lgtm, verified

Suggested reviewers: p0lyn0mial, sjenning, hongkailiu

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality ❓ Inconclusive The added test uses testify assertions without diagnostic messages, which may conflict with the assertion-message requirement; scope and repository convention need confirmation. Confirm whether this check applies to non-Ginkgo Go unit tests and whether existing testify assertions without messages are accepted.
✅ Passed checks (13 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the DualReplica CVO availability exceptions added for NoExecuteTaintManager tests.
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 The diff adds only static Go test and subtest names; no Ginkgo titles or dynamic pod, node, namespace, timestamp, IP, UUID, or generated values appear in titles.
Microshift Test Compatibility ✅ Passed The PR adds a standard Go TestOverlapsNoExecuteTaintManagerTest, not a Ginkgo e2e test; it only constructs monitorapi intervals and references no MicroShift-incompatible API or feature.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The diff adds a standard Go unit test and interval helpers, not a new Ginkgo e2e test; it introduces no SNO multi-node assumption requiring review.
Topology-Aware Scheduling Compatibility ✅ Passed The diff changes only legacy monitor interval filtering and tests; it adds no manifests, controllers, replicas, affinities, spread constraints, selectors, tolerations, or PDBs.
Ote Binary Stdout Contract ✅ Passed The diff adds interval logic and unit-test assertions only; no main/init/suite setup or stdout-writing calls appear in the changed package.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The added test is a standard Go unit test, not a Ginkgo e2e test; it only builds monitorapi intervals and uses time arithmetic, with no IPv4 assumptions or network access.
No-Weak-Crypto ✅ Passed The commit only adds interval filtering and tests; its added lines contain no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret-comparison code.
Container-Privileges ✅ Passed The patch changes only two Go files and adds no container or Kubernetes manifest privilege settings such as privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs ✅ Passed The PR adds interval matching and test fixtures only; it adds no logging calls or sensitive literals, and the existing JUnit event serialization remains unchanged.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci-robot

Copy link
Copy Markdown

@eggfoobar: This pull request references Jira Issue OCPBUGS-105606, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

Serial NoExecuteTaintManager tests taint a control-plane node on DualReplica (masters are also workers), briefly driving csi-snapshot-controller and packageserver Available=False. Mirror the existing upgrade-path exceptions in the stable-system monitor so TNF serial jobs do not fail on these expected blips.

Summary by CodeRabbit

  • Tests
  • Improved upgrade and disruption testing for dual-replica and highly available arbiter configurations.
  • Test validation now accommodates expected temporary availability conditions while system components recover.
  • Added coverage for transient snapshot-controller and package-server status changes during two-node operations.

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-ci
openshift-ci Bot requested review from p0lyn0mial and sjenning August 11, 2026 13:39
@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: automatic mode

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go`:
- Around line 90-97: Update the csi-snapshot-controller exception comment and
returned message in the isTwoNode branch to describe both DualReplica and
HighlyAvailableArbiterMode, or use the generic “two-node topology” wording. Keep
the existing condition matching and exception behavior unchanged.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 716ea3a2-b3de-4880-8dad-975b9d716178

📥 Commits

Reviewing files that changed from the base of the PR and between 29be8bd and f4a867c.

📒 Files selected for processing (1)
  • pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go

@eggfoobar
eggfoobar force-pushed the allow-dualreplica-stable-cvo-available-blips branch from f4a867c to 64bb99a Compare August 11, 2026 14:01
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e-aws-csi
/test e2e-aws-ovn-fips
/test e2e-aws-ovn-microshift
/test e2e-aws-ovn-microshift-serial
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-gcp-csi
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-ipv6
/test e2e-vsphere-ovn
/test e2e-vsphere-ovn-upi

@openshift-ci openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Aug 12, 2026
@eggfoobar

Copy link
Copy Markdown
Contributor Author

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-serial

@openshift-ci

openshift-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

@eggfoobar: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-serial-1of3
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-serial-2of3
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-serial-3of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/b64786d0-9635-11f1-9dbe-1e0a0336105d-0

@fonta-rh

Copy link
Copy Markdown
Contributor

/approve

@fonta-rh

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2026
Comment thread pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go Outdated
eggfoobar and others added 2 commits August 13, 2026 11:18
…ntManager

Serial NoExecuteTaintManager tests taint a control-plane node on DualReplica
(masters are also workers), briefly driving csi-snapshot-controller and
packageserver Available=False. Mirror the existing upgrade-path exceptions in
the stable-system monitor so TNF serial jobs do not fail on these expected
blips.

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: ehila <ehila@redhat.com>
…tions

Scope the stable-system csi-snapshot-controller and packageserver Available
allowlist to blips that overlap a NoExecuteTaintManager e2e window, plus a
one-minute grace for post-taint recovery, instead of any DualReplica run.

Do not key off a serial job signal: serial only describes how NoExecuteTaintManager
enters the suite, not that it caused a given Available blip. Overlap with the
test interval is the causal check and still fails unrelated DualReplica
Available=False regressions outside that window.

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: ehila <ehila@redhat.com>
@eggfoobar
eggfoobar force-pushed the allow-dualreplica-stable-cvo-available-blips branch from 64bb99a to 638822f Compare August 13, 2026 09:19
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e-aws-csi
/test e2e-aws-ovn-fips
/test e2e-aws-ovn-microshift
/test e2e-aws-ovn-microshift-serial
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-gcp-csi
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-ipv6
/test e2e-vsphere-ovn
/test e2e-vsphere-ovn-upi

@eggfoobar

Copy link
Copy Markdown
Contributor Author

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-serial

@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@eggfoobar: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-serial-1of3
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-serial-2of3
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-serial-3of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d79dae40-9702-11f1-87f2-59784e8fad07-0

@eggfoobar

Copy link
Copy Markdown
Contributor Author

/verified by CI

Serial lanes now pass with out the NoExecuteTaintManager error.

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

Copy link
Copy Markdown

@eggfoobar: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

Serial lanes now pass with out the NoExecuteTaintManager error.

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.

@neisw

neisw commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2026
@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eggfoobar, fonta-rh, neisw

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:

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 13, 2026
@eggfoobar

Copy link
Copy Markdown
Contributor Author

/retest-required

@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@eggfoobar: 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/e2e-metal-ipi-ovn-ipv6 638822f link true /test e2e-metal-ipi-ovn-ipv6

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-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 6e25f2a and 2 for PR HEAD 638822f in total

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants