OCPBUGS-84257: fix openshift/network/third-party suite selecting zero tests#31084
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
jira/severity-critical
|
@petr-muller: This pull request references Jira Issue OCPBUGS-84257, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
|
@petr-muller: This pull request references Jira Issue OCPBUGS-84257, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
|
/cherry-pick release-4.22 |
|
@petr-muller: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/testsuites/suites_test.go (1)
67-116: Add one negative control to tighten semantic coverage.Consider adding a
source=hyperkube+[sig-network]test that has none of the allowed feature conditions, and assert it does not match. This protects the OR clause from accidental broadening.Suggested test hardening
hyperkubeTests := extensiontests.ExtensionTestSpecs{ { Name: "[sig-network] NetworkPolicy API should support creating NetworkPolicy API operations [Conformance]", Source: "openshift:payload:hyperkube", }, @@ { Name: "[sig-network] some origin network test [Conformance]", Source: "openshift:payload:origin", }, + { + Name: "[sig-network] generic networking test without required qualifiers", + Source: "openshift:payload:hyperkube", + }, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/testsuites/suites_test.go` around lines 67 - 116, Add a negative-control entry to hyperkubeTests: insert a test with Source "openshift:payload:hyperkube" and a Name that starts with "[sig-network]" but has no allowed feature or Conformance tags (e.g. "[sig-network] some hyperkube network test") so it should NOT match thirdPartySuite.Qualifiers; do not add this name to expectedNames, and add an explicit assertion that this name is absent from filtered (or rely on the existing unexpected-test check) to ensure the OR clause doesn't overmatch; locate hyperkubeTests, the Filter call (hyperkubeTests.Filter), and the filtered/expectedNames checks to implement this.
🤖 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/testsuites/suites_test.go`:
- Around line 67-116: Add a negative-control entry to hyperkubeTests: insert a
test with Source "openshift:payload:hyperkube" and a Name that starts with
"[sig-network]" but has no allowed feature or Conformance tags (e.g.
"[sig-network] some hyperkube network test") so it should NOT match
thirdPartySuite.Qualifiers; do not add this name to expectedNames, and add an
explicit assertion that this name is absent from filtered (or rely on the
existing unexpected-test check) to ensure the OR clause doesn't overmatch;
locate hyperkubeTests, the Filter call (hyperkubeTests.Filter), and the
filtered/expectedNames checks to implement this.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ca140eae-2ab1-4316-906b-cc6f947358a6
📒 Files selected for processing (2)
pkg/testsuites/standard_suites.gopkg/testsuites/suites_test.go
openshift-merge-bot
Bot
added
the
ready-for-human-review
petr-muller
force-pushed
the
ocpbugs-84257-investigate-agentic
branch
from
5913cf7 to
44e8acb
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/testsuites/suites_test.go (1)
99-115: Tighten assertion to explicitly detect missing expected matches.Current checks catch count mismatches and unexpected names, but not missing expected names directly. A full set comparison makes failures more precise.
Proposed assertion hardening
- expectedNames := map[string]bool{ + expectedNames := map[string]struct{}{ "[sig-network] NetworkPolicy API should support creating NetworkPolicy API operations [Conformance]": true, "[sig-network] Networking should provide Internet connection for containers [Feature:Networking-IPv4] [Conformance]": true, "[sig-network] NetworkPolicy should enforce policy based on Ports [Feature:IPv6DualStack]": true, } + matchedNames := map[string]struct{}{} if len(filtered) != len(expectedNames) { t.Errorf("expected %d tests, got %d", len(expectedNames), len(filtered)) for _, s := range filtered { t.Logf(" matched: %s", s.Name) } } for _, s := range filtered { - if !expectedNames[s.Name] { + if _, ok := expectedNames[s.Name]; !ok { t.Errorf("unexpected test matched: %s", s.Name) } + matchedNames[s.Name] = struct{}{} + } + for name := range expectedNames { + if _, ok := matchedNames[name]; !ok { + t.Errorf("expected test missing from matches: %s", name) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/testsuites/suites_test.go` around lines 99 - 115, The test currently only checks counts and unexpected names but doesn't explicitly detect missing expected matches; update the assertion logic around the variables expectedNames and filtered to perform a full set comparison: after building expectedNames and collecting actual names from filtered (e.g., iterate filtered to record actualNames), iterate expectedNames and error if any expected name is not present in actualNames (log which expected names are missing), and keep the existing check for unexpected names to ensure precise, informative failures in the test function containing filtered.
🤖 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/testsuites/suites_test.go`:
- Around line 99-115: The test currently only checks counts and unexpected names
but doesn't explicitly detect missing expected matches; update the assertion
logic around the variables expectedNames and filtered to perform a full set
comparison: after building expectedNames and collecting actual names from
filtered (e.g., iterate filtered to record actualNames), iterate expectedNames
and error if any expected name is not present in actualNames (log which expected
names are missing), and keep the existing check for unexpected names to ensure
precise, informative failures in the test function containing filtered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d316fa5d-7204-49ce-b54d-de3eb5232686
📒 Files selected for processing (2)
pkg/testsuites/standard_suites.gopkg/testsuites/suites_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/testsuites/standard_suites.go
|
Scheduling required tests: |
Reproducer with unfixedReproducer with fixed/verified by @petr-muller |
openshift-ci-robot
added
the
verified
|
@petr-muller: This PR has been marked as verified by DetailsIn response to this:
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-robot
removed
the
verified
|
@petr-muller: This pull request references Jira Issue OCPBUGS-84257, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
… tests
The openshift/network/third-party suite CEL qualifier used
name.contains("[Suite:k8s]") to identify upstream Kubernetes tests, but
no tests carry that tag after the OTE migration. Replace with
source == "openshift:payload:hyperkube" which is the correct way to
identify upstream k8s tests in the extension framework.
Add a semantic test that verifies the qualifier actually matches expected
test specs, preventing future regressions.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
petr-muller
force-pushed
the
ocpbugs-84257-investigate-agentic
branch
from
a87b58e to
8695b69
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: coderabbitai[bot], petr-muller, smg247 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Scheduling required tests: |
|
/verified by @petr-muller |
openshift-ci-robot
added
the
verified
|
@petr-muller: This PR has been marked as verified by DetailsIn response to this:
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. |
|
/hold Revision 8695b69 was retested 3 times: holding |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/hold cancel unrelated |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
@petr-muller: Overrode contexts on behalf of petr-muller: ci/prow/e2e-aws-ovn-fips, ci/prow/e2e-gcp-ovn-upgrade DetailsIn response to this:
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. |
|
Job Failure Risk Analysis for sha: 8695b69
|
|
@petr-muller: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
@petr-muller: Jira Issue Verification Checks: Jira Issue OCPBUGS-84257 Jira Issue OCPBUGS-84257 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
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. |
|
@petr-muller: new pull request created: #31089 DetailsIn response to this:
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. |
|
@petr-muller: new pull request created: #31090 DetailsIn response to this:
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. |
|
@petr-muller: new pull request created: #31091 DetailsIn response to this:
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. |
|
Fix included in release 5.0.0-0.nightly-2026-04-29-125648 |
5 participants
Summary
openshift/network/third-partysuite CEL qualifier to usesource == "openshift:payload:hyperkube"instead ofname.contains("[Suite:k8s]"), which matches zero tests after the OTE migrationTestThirdPartySuiteMatchesHyperkubeTests) verifying the qualifier correctly includes/excludes tests based on source, sig, and feature tagsRoot Cause
The OTE migration (commit c4895d7) converted the suite's
Matchesfunction to a CEL expression that still checks for[Suite:k8s]in test names. No tests carry that tag in the OTE framework — upstream k8s tests now use thesourcefield ("openshift:payload:hyperkube") to identify their origin.Reproduced across 4.21.6, 4.21.12, 4.22-ec.5, and 5.0 nightly: zero tests have
[Suite:k8s], while 101 tests fromk8s-tests-extmatch the intended criteria.🤖 Generated with Claude Code
Summary by CodeRabbit