Skip to content

CNF-26150: Fix flaky e2e ConditionMatcher.Matches early-return when Any mode is true - #464

Open
sebrandon1 wants to merge 1 commit into
openshift:masterfrom
sebrandon1:fix/condition-matcher-any-logic
Open

CNF-26150: Fix flaky e2e ConditionMatcher.Matches early-return when Any mode is true#464
sebrandon1 wants to merge 1 commit into
openshift:masterfrom
sebrandon1:fix/condition-matcher-any-logic

Conversation

@sebrandon1

@sebrandon1 sebrandon1 commented Jul 27, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix ConditionMatcher.Matches() early-return bug that short-circuits in Any mode on the first status mismatch, causing false negatives when iteration order places a non-matching condition before a matching one
  • Add regression test reproducing the exact CI failure scenario (static-resources Degraded=False iterated before deployment Degraded=True)
  • Guard the early-return with !m.Any so Any mode continues searching for at least one match

Problem

The Matches function short-circuits with return false when it finds a condition that matches the type pattern but not the expected status. This is correct for All mode but wrong for Any mode -- Any should keep looking for at least one match.

Since the Kubernetes API returns conditions in non-deterministic order, this produces flaky e2e failures in the overrides tests. The e2e-operator-tech-preview job fails when cert-manager-webhook-static-resources-Degraded (False) is iterated before cert-manager-webhook-deploymentDegraded (True).

Affected PRs

This flaky failure has been causing spurious CI failures on these open PRs:

Related PRs

PR Title Status
#462 CNF-26102: Fix istiocsr updateCondition error aggregation bug Open
#461 CNF-26153: Add unit tests across codebase to close coverage gaps Open
#463 CNF-26103: Add default case in istiocsr getIssuer Open

Jira

  • CNF-26150 -- Fix flaky e2e ConditionMatcher.Matches early-return (To Do)
  • Epic: CNF-23509 -- cert-manager-operator Tuning (In Progress)

Test Plan

  • CI passes (e2e-operator, e2e-operator-tech-preview)
  • Existing unit test TestVerifyOperatorStatusCondition passes (5 cases)
  • New test case "Any mode succeeds when matching condition appears after non-matching one" passes

@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

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

Copy link
Copy Markdown

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

Details

In response to this:

Summary

  • Fixes a bug in ConditionMatcher.Matches() where the early-return on status mismatch fires even when Any=true, causing false negatives when a non-matching condition is iterated before a matching one
  • Adds a unit test reproducing the exact CI failure scenario (static-resources Degraded=False before deployment Degraded=True)

Problem

The Matches function short-circuits with return false when it finds a condition that matches the type pattern but not the expected status. This is correct for All mode but wrong for Any mode — Any should keep looking for at least one match.

Since the Kubernetes API returns conditions in non-deterministic order, this produces flaky e2e failures in the overrides tests. The e2e-operator-tech-preview job fails when cert-manager-webhook-static-resources-Degraded (False) is iterated before cert-manager-webhook-deploymentDegraded (True).

Fix

Guard the early-return with !m.Any so Any mode continues searching instead of short-circuiting on the first mismatch.

Test plan

  • Existing unit test TestVerifyOperatorStatusCondition passes (5 cases, 360s)
  • New test case "Any mode succeeds when matching condition appears after non-matching one" passes instantly (would timeout at 180s without the fix)

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 Jul 27, 2026

Copy link
Copy Markdown

Walkthrough

The condition matcher now continues evaluation after a status mismatch in match-any mode. An e2e test verifies that a later matching condition succeeds after an earlier mismatch.

Changes

Condition matcher behavior

Layer / File(s) Summary
Match-any status evaluation
test/e2e/condition_matcher_test.go, test/e2e/condition_matcher_unit_test.go
The matcher fails immediately on a status mismatch only in match-all mode. The test verifies a later matching Degraded condition in match-any mode.

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

Suggested reviewers: trilokgeer, mytreya-rh

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 added test uses the static title "Any mode succeeds when matching condition appears after non-matching one"; no dynamic values or Ginkgo titles were added.
Test Structure And Quality ✅ Passed The added table case targets one behavior, uses a local fake client, and exercises PollUntilContextTimeout with the repository's 3-minute timeout; no cleanup or Ginkgo lifecycle is required.
Microshift Test Compatibility ✅ Passed The added case is a standard Go table-driven unit test, not a Ginkgo e2e test, and it uses only a fake CertManager client with no MicroShift-incompatible API or feature.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The added case is a table-driven Go unit test using a fake client and operator conditions only; it adds no Ginkgo spec or multi-node/HA assumption.
Topology-Aware Scheduling Compatibility ✅ Passed The pull request changes only e2e condition-matching code and tests; it adds no deployment, controller, operator, or scheduling constraints.
Ote Binary Stdout Contract ✅ Passed The PR adds only matcher logic and test data; no added stdout calls, and main.go plus suite setup files are unchanged.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The added case is a Go table-driven unit test using a fake client and in-memory conditions. It adds no Ginkgo test, IPv4 assumption, or external connectivity.
No-Weak-Crypto ✅ Passed The PR changes only condition matching logic and test data. Added lines contain no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons.
Container-Privileges ✅ Passed The PR changes only two Go test files; the patch adds no Kubernetes/container manifests or privilege-related settings such as privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilege...
No-Sensitive-Data-In-Logs ✅ Passed The PR adds no logging. New test data uses fixed synthetic identifiers and statuses, with no passwords, tokens, API keys, PII, hostnames, or customer data.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the fix to ConditionMatcher.Matches in Any mode and the resulting flaky e2e behavior.
✨ 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 requested review from TrilokGeer and mytreya-rh July 27, 2026 19:43
@openshift-ci

openshift-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sebrandon1
Once this PR has been reviewed and has the lgtm label, please assign bharath-b-rh 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

@sebrandon1 sebrandon1 changed the title NO-JIRA: Fix ConditionMatcher.Matches early-return when Any is true CNF-26150: Fix flaky e2e ConditionMatcher.Matches early-return when Any mode is true Jul 27, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 27, 2026

Copy link
Copy Markdown

@sebrandon1: This pull request references CNF-26150 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

  • Fixes a bug in ConditionMatcher.Matches() where the early-return on status mismatch fires even when Any=true, causing false negatives when a non-matching condition is iterated before a matching one
  • Adds a unit test reproducing the exact CI failure scenario (static-resources Degraded=False before deployment Degraded=True)

Problem

The Matches function short-circuits with return false when it finds a condition that matches the type pattern but not the expected status. This is correct for All mode but wrong for Any mode — Any should keep looking for at least one match.

Since the Kubernetes API returns conditions in non-deterministic order, this produces flaky e2e failures in the overrides tests. The e2e-operator-tech-preview job fails when cert-manager-webhook-static-resources-Degraded (False) is iterated before cert-manager-webhook-deploymentDegraded (True).

Fix

Guard the early-return with !m.Any so Any mode continues searching instead of short-circuiting on the first mismatch.

Test plan

  • Existing unit test TestVerifyOperatorStatusCondition passes (5 cases, 360s)
  • New test case "Any mode succeeds when matching condition appears after non-matching one" passes instantly (would timeout at 180s without the fix)

Summary by CodeRabbit

  • Bug Fixes

  • Improved condition matching in “Any” mode so a non-matching condition does not prevent later matching conditions from succeeding.

  • Tests

  • Added coverage confirming that a valid matching condition is recognized even when it follows a non-matching condition.

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.

@sebrandon1
sebrandon1 force-pushed the fix/condition-matcher-any-logic branch from 3aafea9 to 6680ada Compare August 3, 2026 15:23
When Any is true, the Matches function should return true if at least
one condition matching the type pattern has the expected status. However,
the early-return on status mismatch fires before checking all conditions,
causing false negatives when a non-matching condition appears before a
matching one in the iteration order.

This produces flaky e2e failures in overrides tests because the API
returns conditions in non-deterministic order: if the static-resources
Degraded=False condition is iterated before the deployment Degraded=True
condition, the matcher incorrectly returns false.

Guard the early-return with !m.Any so that Any mode continues searching
for at least one match instead of short-circuiting on the first mismatch.
@sebrandon1

Copy link
Copy Markdown
Member Author

/retest

@openshift-ci

openshift-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@sebrandon1: 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-operator-tech-preview b6c869a link false /test e2e-operator-tech-preview

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.

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

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants