[SREP-3734] Fix AlertmanagerInhibitions test race condition

[smarthall]

Summary

• The inhibits ClusterOperatorDegraded test was using a fixed 3-minute sleep before querying Prometheus for alerts, which was occasionally too short
• Replace the fixed sleep with an Eventually poll (5m timeout, 30s interval) that returns as soon as the alert appears

Details

The test injects a bogus OIDC identity provider to degrade the authentication ClusterOperator, then checks Prometheus for ClusterOperatorDown or ClusterOperatorDegraded alerts. The alert enters "pending" state once the operator reports degraded and Prometheus scrapes + evaluates the rule (typically 2-3 min). The previous fixed 3-minute sleep was tight enough that slight timing variation caused failures.

The sleep was reduced from 10m to 3m in #2952 (OSD-29225), which was primarily fixing an unrelated deep-equality bug.

Example failures

• 4.21 Apr 25 — Expected <bool>: false to be true at inhibitions.go:181
• 4.21 Apr 24 — same failure
• 4.21 Apr 23 — same failure

Test plan

• Verify periodic-ci-openshift-osde2e-main-nightly-4.21-osd-aws passes with this change

🤖 Generated with Claude Code

[openshift-merge-bot]

There are test jobs defined for this repository which are not configured to run automatically. Comment /test ? to see a list of all defined jobs. Review these jobs and use /test <job> to manually trigger jobs most likely to be impacted by the proposed changes.Comment /pipeline required to trigger all required & necessary jobs.

[coderabbitai]

Walkthrough

Replaces a static 3-minute sleep and single Prometheus query with an Eventually-based polling loop. The new logic retries Prometheus Alerts retrieval until detecting either ClusterOperatorDown or ClusterOperatorDegraded alert for name="authentication", treating fetch errors as transient (returns false).

Changes

Cohort / File(s)	Summary
Test Retry Logic pkg/e2e/osd/inhibitions.go	Replaced fixed delay sleep with Eventually polling loop that retries Prometheus Alerts queries. Fetch errors are treated as transient rather than terminal failures. Assertion moved to Eventually condition.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Test Structure And Quality	⚠️ Warning	Missing assertion messages on lines 34, 38, 41, and 80 violates requirement 4 and established test patterns, causing diagnostic ambiguity on failure.	Add descriptive messages to all unmessaged assertions: line 34 'failed to fetch alertmanager config secret', line 38 'alertmanager config missing inhibit_rules', line 41 'failed to parse alertmanager config', line 80 'ClusterOperatorDegraded or ClusterOperatorDown alert for authentication operator did not appear within 5 minutes'.
Microshift Test Compatibility	⚠️ Warning	Test 'inhibits ClusterOperatorDegraded' uses OpenShift-specific APIs (OAuth config.openshift.io/v1, Prometheus monitoring) without MicroShift skip mechanisms.	Add [apigroup:config.openshift.io] tag to test name at line 102 to auto-skip on MicroShift, or use [Skipped:MicroShift] label or exutil.IsMicroShiftCluster() guard.

✅ Passed checks (10 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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	Modified test file contains two stable Ginkgo test names without dynamic values; all variable content appropriately contained within test bodies.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Test modifies Prometheus alert polling for operator degradation without multi-node assumptions or pod scheduling dependencies.
Topology-Aware Scheduling Compatibility	✅ Passed	Modified file is an end-to-end test (pkg/e2e/osd/inhibitions.go); no topology-dependent logic introduced.
Ote Binary Stdout Contract	✅ Passed	Pull request modifies test logic within ginkgo.It() block; no logging imports or process-level code present; changes execute exclusively within test scope and do not corrupt OTE binary JSON stdout contract.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Modified test contains no IPv4 assumptions or external connectivity requirements; uses hardcoded non-functional OIDC URL and cluster-internal Prometheus access.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the primary change: replacing a fixed sleep with polling to fix test flakiness in the AlertmanagerInhibitions test.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

• OWNERS

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

[smarthall]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

pkg/e2e/osd/inhibitions.go (1)

162-181: Propagate ctx and log transient Prometheus errors.

The test function receives ctx as a parameter; use it instead of context.Background() to honour test cancellation. Additionally, log Prometheus query errors to aid flake diagnosis.

1. Derive timeout from ctx via context.WithTimeout(ctx, ...) and pass ctx to Eventually(ctx, func(ctx context.Context) bool { ... }, ...) so Gomega respects upstream cancellation (Gomega v1.39.1 supports this).
2. Log errors from prometheusApiClient.Alerts() using ginkgo.GinkgoLogr.Error(err, "Unable to query prom API") before returning false, matching the established pattern in pkg/e2e/state/alerts.go.

♻️ Proposed refactor

-		Eventually(func() bool {
-			timeout, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+		Eventually(ctx, func(ctx context.Context) bool {
+			queryCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
 			defer cancel()
-			alerts, err := prometheusApiClient.Alerts(timeout)
+			alerts, err := prometheusApiClient.Alerts(queryCtx)
 			if err != nil {
+				ginkgo.GinkgoLogr.Error(err, "Unable to query prom API")
 				return false
 			}
 			for _, alert := range alerts.Alerts {
 				if (alert.Labels["alertname"] == "ClusterOperatorDown" ||
 					alert.Labels["alertname"] == "ClusterOperatorDegraded") &&
 					alert.Labels["name"] == "authentication" {
 					return true
 				}
 			}
 			return false
-		}, 5*time.Minute, 30*time.Second).Should(BeTrue())
+		}, 5*time.Minute, 30*time.Second).Should(BeTrue(), "ClusterOperatorDown/Degraded alert for authentication never appeared")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/e2e/osd/inhibitions.go` around lines 162 - 181, Replace
context.Background() with the test's ctx by deriving the short query timeout via
context.WithTimeout(ctx, 10*time.Second) and pass ctx into Eventually so Gomega
can honor upstream cancellation (Gomega v1.39.1 supports Eventually(ctx, ...)).
Inside the body that calls prometheusApiClient.Alerts(timeout), log transient
errors before returning false using ginkgo.GinkgoLogr.Error(err, "Unable to
query prom API"); keep the alert-name checks as-is (alert.Labels["alertname"]
and alert.Labels["name"] == "authentication") and ensure the cancel() defer
remains.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/e2e/osd/inhibitions.go`:
- Around line 162-181: Replace context.Background() with the test's ctx by
deriving the short query timeout via context.WithTimeout(ctx, 10*time.Second)
and pass ctx into Eventually so Gomega can honor upstream cancellation (Gomega
v1.39.1 supports Eventually(ctx, ...)). Inside the body that calls
prometheusApiClient.Alerts(timeout), log transient errors before returning false
using ginkgo.GinkgoLogr.Error(err, "Unable to query prom API"); keep the
alert-name checks as-is (alert.Labels["alertname"] and alert.Labels["name"] ==
"authentication") and ensure the cancel() defer remains.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: c0e8c436-cab8-4dda-a8a3-ea11dd554bef

📥 Commits

Reviewing files that changed from the base of the PR and between b25ce52 and 84727e0.

📒 Files selected for processing (1)

• pkg/e2e/osd/inhibitions.go

[openshift-ci]

@smarthall: 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.

[smarthall]

Closing to port this test from osde2e to CAMO instead.