OCPBUGS-100385: MonitorTest: measure the union of ClusterOperator wait intervals

[stbenjam]

These bugs and fixes were automatically generated by a payload-agent experiment to improve resilience and diagnostics for infrastructure failures. Please review the PR and either shepherd it to merge or close it. If the work is incorrect or unhelpful, a brief comment would help us improve. Thanks, and apologies if we missed the mark.

What this fixes

The upgrade monitor decides whether CVO reported waiting at least three minutes for a ClusterOperator. Its input consists of constructed intervals representing periods when the ClusterVersion Progressing message named that operator.

The existing calculation subtracts the earliest interval start from the latest interval end. When CVO names an operator in multiple disjoint episodes, this counts all intervening time—even though CVO did not report waiting on that operator during the gap.

This change sorts and merges overlapping or adjacent intervals, then applies the threshold to the duration of their union. This restores the accumulation intended by origin#30333 without double-counting overlaps. Failure diagnostics now report:

• total CVO-reported waiting duration;
• episode count;
• longest episode;
• full observation span; and
• each merged episode range.

These intervals are a CVO status-message metric. They do not prove operator availability, pod readiness, or overall cluster health during the gaps.

Visualization of the new behavior is here

Payload and RCA context

The false attribution surfaced during the 4.22→5.0 regression introduced by cluster-version-operator#1427 and reverted by cluster-version-operator#1431. The old CVO could not render its 5.0 Deployment, never self-updated, and repeatedly restarted its sync between the embedded 4.22 payload and desired 5.0 payload. That produced several short waiting on config-operator episodes separated by unrelated payload work and restarts.

The CVO defect was the primary payload regression; this PR fixes a secondary monitor calculation that turned those disjoint episodes into a misleading config-operator failure. The CVO, etcd, alert, and disruption signals for the underlying product regression remain unaffected.

• Evidence-backed RCA
• Failed Payload Agent analysis
• Representative AWS timeline artifact
• Representative Azure timeline artifact

For payload 5.0.0-0.ci-2026-07-25-102741, 13 raw AWS/Azure child failures had CVO-reported interval unions of 13–150 seconds with no episode longer than 150 seconds, while the old calculation reported observation spans from 3m27s to 58m53s. One representative result combined a 15-second interval with a later point observation and reported 45m14s.

The Payload Agent consequently recommended reverting unrelated openshift/api#2920 and #2923 at 90 confidence. Experimental revert jobs continued failing, falsifying both recommendations.

• Experimental revert #2953, AWS: 0/5 passed
• Experimental revert #2954, AWS: 3/5 passed

Validation

• go test ./pkg/monitortests/clusterversionoperator/legacycvomonitortests
• go vet ./pkg/monitortests/clusterversionoperator/legacycvomonitortests
• Added table-driven coverage for separated point observations, overlaps, adjacency, multiple episodes, unsorted input, and invalid intervals.

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes
  • Improved upgrade progress reporting by accurately summarizing operator waiting periods.
  • Combined overlapping or adjacent periods while keeping separate episodes distinct.
  • Enhanced failure messages with total waiting time, longest episode, observation span, and episode details for clearer diagnostics.
• Tests
  • Added coverage for overlapping, invalid, adjacent, separate, and empty waiting intervals.

[openshift-merge-bot]

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

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stbenjam

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 [stbenjam]

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

[coderabbitai]

Walkthrough

Upgrade Progressing checks now aggregate CVO waiting intervals into merged episodes, total and longest durations, observation spans, and episode ranges. Exception output and tests use the new summary instead of a single overall interval.

Changes

Waiting interval summarization

Layer / File(s)	Summary
Interval aggregation and reporting pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go	Intervals are filtered, sorted, and merged when they overlap or touch. Summary metrics are used for duration checks and failure messages.
Summary validation pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators_test.go	Tests validate summarized totals, merged episodes, gaps, invalid intervals, empty input, and formatted output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: deads2k, p0lyn0mial

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes measuring the union of ClusterOperator wait intervals, which is the main change.
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	Changed tests use static table names and Test_summarizeWaitingIntervals; no Ginkgo titles or dynamic values such as timestamps, pod names, nodes, namespaces, or IPs were added.
Test Structure And Quality	✅ Passed	Changed tests use standard Go testing/testify table-driven unit tests, not Ginkgo; they create no cluster resources and use no waits that need cleanup or timeouts.
Microshift Test Compatibility	✅ Passed	The PR adds only Go unit-test coverage (Test_summarizeWaitingIntervals); it adds no Ginkgo e2e tests, so MicroShift compatibility checks do not apply.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR changes only Go implementation and testing.T unit tests; no new It/Describe/Context/When e2e tests or multi-node assumptions are present.
Topology-Aware Scheduling Compatibility	✅ Passed	The complete PR changes only interval-summary logic and tests in two Go files; they add no manifests, controllers, replicas, affinity, topology spread, selectors, tolerations, or PDB constraints.
Ote Binary Stdout Contract	✅ Passed	The PR adds only fmt.Sprintf summary formatting and test assertions; affected package has no fmt.Print, log.Print, klog, or os.Stdout writes in process-level code.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The patch adds only Go unit tests (Test_summarizeWaitingIntervals) and a comment; no new Ginkgo e2e tests, IPv4 assumptions, or external connectivity.
No-Weak-Crypto	✅ Passed	The PR diff only adds interval sorting, merging, duration reporting, and tests; it adds no crypto imports, weak algorithms, custom crypto, or secret comparisons.
Container-Privileges	✅ Passed	The PR changes only two Go source/test files; the diff adds no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, root, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs	✅ Passed	The PR adds only aggregate durations, counts, fixed operator names, and RFC3339 interval timestamps to JUnit diagnostics; it does not log secrets, PII, hostnames, IDs, or customer data.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go (1)

915-984: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a brief "why" comment on the merge/union algorithm.

summarizeWaitingIntervals implements non-trivial interval-union semantics (sort, merge overlapping/adjacent into episodes, compute total/longest/span) with no comment explaining the intent (avoiding false failures from healthy gaps between disjoint waits). A one-line comment would help future maintainers reasoning about this CI-flake-diagnosis logic without re-deriving the algorithm.

As per coding guidelines, **/*.go: "Favor clarity and maintainability over cleverness in Go code. Keep comments minimal and helpful, explaining why rather than what."

📝 Example comment

+// summarizeWaitingIntervals merges overlapping/adjacent waiting intervals into
+// episodes so that healthy gaps between disjoint observations aren't counted
+// as waiting time (see false failures caused by disjoint aggregator gaps).
 func summarizeWaitingIntervals(intervals monitorapi.Intervals) waitingIntervalSummary {

🤖 Prompt for 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.

In `@pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go`
around lines 915 - 984, Add a concise why-focused comment above the interval
sorting/merging logic in summarizeWaitingIntervals, explaining that unioning
overlapping or adjacent waits into episodes prevents healthy gaps between
disjoint waits from producing misleading CI-flake failures. Keep the existing
interval-union and summary calculations unchanged.

Source: Coding guidelines

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

Nitpick comments:
In `@pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go`:
- Around line 915-984: Add a concise why-focused comment above the interval
sorting/merging logic in summarizeWaitingIntervals, explaining that unioning
overlapping or adjacent waits into episodes prevents healthy gaps between
disjoint waits from producing misleading CI-flake failures. Keep the existing
interval-union and summary calculations unchanged.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: da7f5c77-8b94-456a-bad8-be8695bb23da

📥 Commits

Reviewing files that changed from the base of the PR and between 118982e and 8f138af.

📒 Files selected for processing (2)

• pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go
• pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators_test.go

[openshift-ci-robot]

@stbenjam: This pull request references Jira Issue OCPBUGS-100385, which is invalid:

• expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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

Details

In response to this:

These bugs and fixes were automatically generated by a payload-agent experiment to improve resilience and diagnostics for infrastructure failures. Please review the PR and either shepherd it to merge or close it. If the work is incorrect or unhelpful, a brief comment would help us improve. Thanks, and apologies if we missed the mark.

What this fixes

The upgrade monitor currently decides whether CVO waited at least three minutes for a ClusterOperator by subtracting the earliest interval start from the latest interval end. When the operator is healthy between disjoint observations, that healthy gap is incorrectly counted as waiting time.

This change sorts and merges overlapping/adjacent intervals, then applies the threshold to the union of actual waiting time. Failure diagnostics now report:

• total waiting duration;
• episode count;
• longest episode;
• full observation span; and
• each merged episode range.

Payload impact

This arithmetic defect caused repeated false failures in the AWS and Azure major-upgrade aggregators for 5.0.0-0.ci-2026-07-25-102741. Across 13 raw failing children, actual wait totaled 13–150 seconds with no episode longer than 150 seconds, while the test reported spans from 3m27s to 58m53s. One representative result combined a 15-second interval with a later point observation and reported 45m14s.

The Payload Agent then recommended reverting unrelated openshift/api#2920 and #2923 at 90 confidence. Experimental revert jobs continued failing, falsifying both recommendations.

• Failed Payload Agent analysis
• Representative AWS timeline artifact
• Representative Azure timeline artifact
• Experimental revert #2953, AWS: 0/5 passed
• Experimental revert #2954, AWS: 3/5 passed

Validation

• go test ./pkg/monitortests/clusterversionoperator/legacycvomonitortests
• go vet ./pkg/monitortests/clusterversionoperator/legacycvomonitortests
• Added table-driven coverage for separated point observations, overlaps, adjacency, multiple episodes, unsorted input, and invalid intervals.

AI disclosure

Generated with GPT-5.6 Sol from an adversarial Payload Agent audit and verified against the raw monitor timeline artifacts.

Summary by CodeRabbit

• Bug Fixes
• Improved upgrade progress reporting by accurately summarizing periods when cluster operators are waiting.
• Overlapping or adjacent waiting periods are combined, while separate episodes are reported independently.
• Failure messages now include total waiting time, longest episode, overall span, and episode count for clearer diagnostics.
• Tests
• Added coverage for overlapping, invalid, adjacent, and non-overlapping waiting intervals.

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.

[stbenjam]

/payload-job periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

[openshift-ci]

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

• periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/8d5e6b80-8ce9-11f1-8b39-f47c20246fc6-0

[redhat-chai-bot]

Review: Interval Union for CVO Wait Measurement

I reviewed the diff, the CVO source code (specifically how convertErrorToProgressingForUpdateEffectNone generates the "waiting on" Progressing messages), and the linked Jira issue.

The Bug

During an upgrade, CVO periodically publishes its Progressing condition with messages like "working towards VERSION: X of Y done, waiting on etcd, kube-apiserver". The monitor test (testUpgradeOperatorProgressingStateTransitions) captures each such observation as an interval and tracks how long CVO spent "waiting on" each ClusterOperator.

The old fromAndTo() computed this as max(To) - min(From) — the wall-clock span from the first observation to the last. The problem is that CVO's Progressing message is sampled, not continuous. An operator might appear in a "waiting on" message at T=0s, disappear for 45 minutes while CVO is healthy, and then reappear in a later message. The old code would report ~45 minutes of waiting when the actual waiting was only a few seconds.

Algorithm Correctness

The new summarizeWaitingIntervals() implements a standard interval-union (sweep-line) approach and is correct:

• Adjacent intervals merge correctly — interval.From.After(last.To) is false when From == last.To, so touching intervals are properly unified. This is the right behavior since CVO observations are contiguous in time.
• Point observations (From == To) are valid and not skipped — they create 0-duration episodes. This is correct: a point observation proves CVO was "waiting on" the operator at that instant but contributes no duration. The test case "separated point does not fill healthy gap" specifically validates the exact pattern that caused the production false failures.
• Overlapping intervals are handled without double-counting — the merge extends To only if the new interval extends beyond the current episode.
• The String() method produces excellent diagnostics (total, episode count, longest, observation span, each merged range in RFC3339), which will make future debugging much easier.

Test Coverage

The new Test_summarizeWaitingIntervals covers the key patterns well:

Case	What it validates
separated point does not fill healthy gap	The exact production failure pattern
overlapping and adjacent intervals form one union	Standard merge behavior
multiple real episodes sum without double counting	Disjoint episodes with overlap within each
invalid interval is ignored	Defensive against bad data

The existing Test_updateCOWaiting expectations are updated correctly — for example, kube-apiserver with a 6-minute existing interval + 2-minute new interval now correctly expects 8 minutes (the union of adjacent intervals) instead of the old 5 minutes (which was a test bug reflecting the same defective arithmetic).

Minor Suggestions (non-blocking)

1. Missing godoc on summarizeWaitingIntervals — a one-liner explaining why this exists (healthy gaps between disjoint observations shouldn't count as waiting) would help future maintainers. The algorithm is clear but the motivation requires CVO domain knowledge.
2. No test for empty input — summarizeWaitingIntervals(nil) should return a zero summary with "no valid waiting intervals". Works from code inspection, but an explicit test case would complete coverage.
3. No test for all-invalid input — e.g., a slice with only To < From entries. Also minor.

Overall

✅ This is a correct and well-motivated fix. The interval-union algorithm is sound, test coverage is solid for the production failure patterns, and the improved diagnostics are a nice bonus. The real-world impact is significant — this defect caused the Payload Agent to falsely recommend reverting unrelated PRs at 90% confidence.

[openshift-merge-bot]

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

[coderabbitai]

🧹 Nitpick comments (1)

pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators_test.go (1)

314-315: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the formatted summary for valid intervals too.

expectedString is set only for empty and all-invalid inputs. This test does not detect regressions in non-empty diagnostics, including total duration, episode count, observation span, or merged ranges. Add expected strings for representative separated, overlapping, and multi-episode cases, or add a dedicated table-driven string test.

Also applies to: 383-385

🤖 Prompt for 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.

In
`@pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators_test.go`
around lines 314 - 315, Extend the table-driven tests around the existing
expectedString cases to assert formatted summaries for valid intervals,
including separated, overlapping, and multi-episode inputs. Populate
expectedString with exact output covering total duration, episode count,
observation span, and merged ranges, or add a dedicated table-driven test for
the summary formatter.

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

Nitpick comments:
In
`@pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators_test.go`:
- Around line 314-315: Extend the table-driven tests around the existing
expectedString cases to assert formatted summaries for valid intervals,
including separated, overlapping, and multi-episode inputs. Populate
expectedString with exact output covering total duration, episode count,
observation span, and merged ranges, or add a dedicated table-driven test for
the summary formatter.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: c71a9c51-2388-4393-a7e6-7f2818d289e5

📥 Commits

Reviewing files that changed from the base of the PR and between 8f138af and 4a51d57.

📒 Files selected for processing (2)

• pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go
• pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/monitortests/clusterversionoperator/legacycvomonitortests/operators.go

[openshift-ci]

@stbenjam: The following tests 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-aws-csi	4a51d57	link	true	/test e2e-aws-csi
ci/prow/e2e-metal-ipi-ovn-ipv6	4a51d57	link	true	/test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-aws-ovn-fips	4a51d57	link	true	/test e2e-aws-ovn-fips

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.