OCPBUGS-84961: support Parents field on origin test suites#31261
OCPBUGS-84961: support Parents field on origin test suites#31261stbenjam wants to merge 1 commit into
Conversation
Add a Parents field to TestSuite so static suites (not just OTE extensions) can declare parent relationships. Child qualifiers are merged into parents with dedup via a fixed-point loop that handles arbitrary-depth transitive chains. Make openshift/conformance/parallel and openshift/conformance/serial children of openshift/conformance, removing the parent's hardcoded qualifier so it inherits entirely from its children. This also means extension qualifiers injected into the child suites automatically propagate up to the conformance parent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci-robot
added
jira/valid-reference
|
@stbenjam: This pull request references Jira Issue OCPBUGS-84961, which is invalid:
Comment 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. |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughAdds a Parents field to TestSuite, implements mergeParentQualifiers (fixed-point propagation with deduplication), integrates it into suite builders and static suite definitions, adds tests covering propagation scenarios, and emits debug logging of suite qualifiers and spec-source counts. ChangesParent Suite Qualifier Propagation
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@stbenjam: This pull request references Jira Issue OCPBUGS-84961, which is valid. The bug has been moved to the POST state. 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. |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-serial-1of2 periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-ipv4 periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-ipv6 periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-gcp-ovn-rt-upgrade |
|
@stbenjam: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/0ed3d720-60b4-11f1-8a9b-bd6ff849bb8b-0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/testsuites/standard_suites.go`:
- Around line 520-523: When merging child.Qualifiers into parent.Qualifiers the
code appends a qualifier but doesn't mark it in the existing map, so repeated
qualifiers within the same child slice can be appended multiple times; in the
loop over child.Qualifiers (the block that checks if !existing[q]) set
existing[q] = true immediately after appending to parent.Qualifiers (and keep
setting changed = true) so subsequent duplicates in the same child don't get
appended again.
🪄 Autofix (Beta)
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: af1d8eb0-1ccb-4ff3-bab7-1d6180f0c36f
📒 Files selected for processing (3)
pkg/test/ginkgo/test_suite.gopkg/testsuites/parents_test.gopkg/testsuites/standard_suites.go
|
Scheduling required tests: |
openshift-ci
Bot
added
the
ready-for-human-review
|
/payload-job periodic-ci-openshift-release-main-nightly-4.21-opct-external-aws-ccm |
|
@mtulio: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/671e2520-60d7-11f1-9f64-4c10518d7df1-0 |
|
4.21 job is failing to install: |
|
@mtulio: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/cd36cc10-60df-11f1-8554-e2accbdddd43-0 |
|
/cherry-pick release-4.22 release-4.21 |
|
@stbenjam: 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. |
|
Still evaluating the results, it's not getting the results we wanted: 
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
wait, testing in 4.21 payload, probably needs to test against 5.0 or 4.23? |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-opct-platform-external-aws-ccm periodic-ci-openshift-release-main-nightly-5.0-opct-platform-external-aws |
|
@mtulio: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/042fcc30-6108-11f1-877b-5e913a41d258-0 |
|
additionally fyi: I am working to export the "build logs", openshift-tests container output in the CI step to help on further investigation |
|
Sorry, I didn't notice your jobs weren't 5.0 — it needs to match main. Feel free to /lgtm when you think it's ready. If you find any problems just let me know, I'll take a look on Monday. |
Bikash those tests finished w/ unexpected values, since 5.0 is not well exercised under opct env, would you mind validating this fix under 5.0/4.23 cluster and collect the result to us? /assign @bshaw7 |
|
No, this is not working the way I expected. The problem is here. I'll need to grab a live cluster next week and take a look. |
|
We must have container logs to GCS now: /payload-job periodic-ci-openshift-release-main-nightly-5.0-opct-platform-external-aws |
|
@mtulio: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/56301730-6156-11f1-8756-5dbf23a2695c-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-opct-platform-external-aws |
|
@stbenjam: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/7cf37e30-61a0-11f1-946c-e6371f9ee7af-0 |
|
@stbenjam: This pull request references Jira Issue OCPBUGS-84961, 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. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/test/ginkgo/cmd_runsuite.go (1)
363-379: ⚡ Quick winConsider adding a TODO comment to track removal of temporary debug logging.
The commit message indicates this is temporary debug code added to diagnose extension test filtering issues. Adding an in-code TODO comment would make it easier to track and remove later once the investigation is complete.
📝 Suggested addition
+ // TODO(OCPBUGS-84961): Remove temporary debug logging once extension test filtering issue is resolved // Debug: log suite qualifiers before filter logrus.Infof("Suite %q has %d qualifiers before filter", suite.Name, len(suite.Qualifiers))🤖 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/test/ginkgo/cmd_runsuite.go` around lines 363 - 379, Add a TODO comment marking the temporary debug logging for suite qualifiers and sourceCounts so it’s easy to find and remove later; specifically, annotate the block that logs suite.Name and suite.Qualifiers and the loop that builds sourceCounts from specs (using ExtensionTestSpec.Source) with a clear TODO like “TODO: temporary debug logging for extension test filtering investigation - remove when fixed” and include a short owner/date or issue reference to track follow-up.
🤖 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/test/ginkgo/cmd_runsuite.go`:
- Around line 363-379: Add a TODO comment marking the temporary debug logging
for suite qualifiers and sourceCounts so it’s easy to find and remove later;
specifically, annotate the block that logs suite.Name and suite.Qualifiers and
the loop that builds sourceCounts from specs (using ExtensionTestSpec.Source)
with a clear TODO like “TODO: temporary debug logging for extension test
filtering investigation - remove when fixed” and include a short owner/date or
issue reference to track follow-up.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 1e9f5c63-a331-4d5b-b392-6e2fb343872e
📒 Files selected for processing (2)
pkg/test/ginkgo/cmd_runsuite.gopkg/testsuites/standard_suites.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/testsuites/standard_suites.go
|
Scheduling required tests: |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Add a kubernetes/conformance suite that aggregates tests from kubernetes/conformance/parallel and kubernetes/conformance/serial, enabling `openshift-tests run kubernetes/conformance` to run all kubernetes conformance tests in a single invocation. Uses AddGlobalSuite (not AddSuite) so the umbrella starts with zero qualifiers and inherits exclusively from its children via the mergeParentQualifiers fixed-point loop in origin. AddSuite would prepend a catch-all source qualifier that bypasses the exclusion filters ([Disabled:], [Disruptive], [Slow], etc.) in the child qualifiers. Companion to openshift/origin#31261 which adds the Parents field and mergeParentQualifiers mechanism. Upstream: <carry> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… suite Add a kubernetes/conformance suite that aggregates tests from kubernetes/conformance/parallel and kubernetes/conformance/serial, enabling `openshift-tests run kubernetes/conformance` to run all kubernetes conformance tests in a single invocation. Uses AddGlobalSuite (not AddSuite) so the umbrella starts with zero qualifiers and inherits exclusively from its children via the mergeParentQualifiers fixed-point loop in origin. AddSuite would prepend a catch-all source qualifier that bypasses the exclusion filters ([Disabled:], [Disruptive], [Slow], etc.) in the child qualifiers. Companion to openshift/origin#31261 which adds the Parents field and mergeParentQualifiers mechanism. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a kubernetes/conformance suite that aggregates tests from kubernetes/conformance/parallel and kubernetes/conformance/serial, enabling `openshift-tests run kubernetes/conformance` to run all kubernetes conformance tests in a single invocation. Uses AddGlobalSuite (not AddSuite) so the umbrella starts with zero qualifiers and inherits exclusively from its children via the mergeParentQualifiers fixed-point loop in origin. AddSuite would prepend a catch-all source qualifier that bypasses the exclusion filters ([Disabled:], [Disruptive], [Slow], etc.) in the child qualifiers. Companion to openshift/origin#31261 which adds the Parents field and mergeParentQualifiers mechanism. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I am pretty sure this fix works. The OPCT non-upgrade conformance jobs just can't be validated via In ci-operator, Fix: openshift/release#80178 removes the dependency override from the workflow so non-upgrade jobs install from |
Add a kubernetes/conformance suite that aggregates tests from kubernetes/conformance/parallel and kubernetes/conformance/serial, enabling `openshift-tests run kubernetes/conformance` to run all kubernetes conformance tests in a single invocation. Uses AddGlobalSuite (not AddSuite) so the umbrella starts with zero qualifiers and inherits exclusively from its children via the mergeParentQualifiers fixed-point loop in origin. AddSuite would prepend a catch-all source qualifier that bypasses the exclusion filters ([Disabled:], [Disruptive], [Slow], etc.) in the child qualifiers. Companion to openshift/origin#31261 which adds the Parents field and mergeParentQualifiers mechanism. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6 participants
Summary
Parents []stringfield toginkgo.TestSuite, enabling static origin suites (not just OTE extensions) to declare parent relationshipsopenshift/conformance/parallelandopenshift/conformance/serialchildren ofopenshift/conformance, removing the parent's hardcoded qualifier so it inherits entirely from its childrenParentsintoTestSuite.Parentsfor extension suites, so all parent resolution goes through one unified code pathSupersedes #31259
Test plan
go build ./...compilesgo vet ./pkg/test/ginkgo/... ./pkg/testsuites/...passespkg/testsuites/parents_test.gocovering:openshift/conformanceinherits from parallel and serial🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests