OCPCLOUD-3009: Bootstrap OTE framework#597
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@pmeida: This pull request references OCPCLOUD-3009 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. 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
Bot
added
the
needs-ok-to-test
|
Hi @pmeida. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduces a new ChangesOpenShift Tests Extension Integration
Sequence Diagram(s)sequenceDiagram
participant CLI as cobra root cmd
participant Registry as extension registry
participant Extension as openshift extension
participant GinkgoSuite as Ginkgo suite
participant e2e as e2e.InitCommonVariables
CLI->>Registry: NewRegistry()
Registry->>Extension: NewExtension("openshift", "cluster-capi-operator")
Extension->>Extension: add suites (parallel, serial, disruptive)
Extension->>GinkgoSuite: BuildExtensionTestSpecsFromOpenShiftGinkgoSuite()
GinkgoSuite-->>Extension: test specs
Extension->>e2e: AddBeforeAll → InitCommonVariables()
Extension->>Registry: Register(extension)
Registry->>CLI: AddExtensionCommands(root)
CLI->>CLI: Execute() / os.Exit(1) on error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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. Comment |
![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
🧹 Nitpick comments (1)
Dockerfile.rhel (1)
4-5: ⚡ Quick winUse deterministic gzip output for the embedded extension binary.
gzipcurrently writes timestamp/name metadata, which can make rebuilds non-reproducible even when binary content is unchanged.Suggested change
-RUN make clean build && \ - gzip bin/cluster-capi-operator-tests-ext +RUN make clean build && \ + gzip -n bin/cluster-capi-operator-tests-extAs per coding guidelines, “keep vendoring deterministic … to avoid drift across builds.”
🤖 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 `@Dockerfile.rhel` around lines 4 - 5, The gzip command that compresses bin/cluster-capi-operator-tests-ext currently writes timestamp and filename metadata to the output, causing non-deterministic builds even when the binary content hasn't changed. Fix this by adding the `-n` flag to the gzip command, which omits the original filename and modification timestamp from the gzip header, ensuring consistent output across rebuilds.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.
Inline comments:
In `@openshift-tests-extension/go.mod`:
- Around line 5-22: The go.mod file contains four pseudo-versions that require
justification per supply-chain policy:
github.com/openshift-eng/openshift-tests-extension (direct dependency), and
golang.org/x/exp, google.golang.org/genproto/googleapis/api, and
google.golang.org/genproto/googleapis/rpc (indirect dependencies). Add comments
or documentation in the go.mod file or in a separate justification document
explaining why these pseudo-versions are necessary instead of using released
stable versions for each package, making clear the business or technical
rationale for pinning to unversioned commits rather than stable releases.
---
Nitpick comments:
In `@Dockerfile.rhel`:
- Around line 4-5: The gzip command that compresses
bin/cluster-capi-operator-tests-ext currently writes timestamp and filename
metadata to the output, causing non-deterministic builds even when the binary
content hasn't changed. Fix this by adding the `-n` flag to the gzip command,
which omits the original filename and modification timestamp from the gzip
header, ensuring consistent output across rebuilds.
🪄 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: 493293e5-1e97-4bd9-8f87-bac5e1445e27
⛔ Files ignored due to path filters (40)
go.workis excluded by!**/*.workopenshift-tests-extension/go.sumis excluded by!**/*.sumvendor/github.com/openshift-eng/openshift-tests-extension/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdimages/cmdimages.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdinfo/info.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdlist/list.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runsuite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runtest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdupdate/update.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/dbtime/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extension.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/environment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/result.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/result_writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/task.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/viewer.htmlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/component.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/concurrency.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/environment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/names.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/output.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/junit/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/byte.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/empty.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int32.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (5)
Dockerfile.rhelMakefilehack/vendor.shopenshift-tests-extension/cmd/main.goopenshift-tests-extension/go.mod
pmeida
changed the title
|
/jira refresh |
|
@pmeida: This pull request references OCPCLOUD-3009 which is a valid jira issue. 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. |
How to add a test to OTEThe extension uses ginkgo auto-discovery. Whether a test is visible to OTE depends entirely on the file suffix:
To add a test to OTE: write it in a regular To keep a test out of OTE: use the standard This was validated locally using the tests from #599 — renaming |
|
/ok-to-test |
openshift-ci
Bot
added
ok-to-test
| ext.AddSuite(e.Suite{ | ||
| Name: "capio/parallel", | ||
| Qualifiers: []string{`!labels.exists(l, l == "Serial") && !labels.exists(l, l == "Disruptive")`}, | ||
| Parents: []string{"openshift/conformance/parallel"}, | ||
| }) | ||
|
|
||
| ext.AddSuite(e.Suite{ | ||
| Name: "capio/serial", | ||
| Qualifiers: []string{`labels.exists(l, l == "Serial") && !labels.exists(l, l == "Disruptive")`}, | ||
| Parents: []string{"openshift/conformance/serial"}, | ||
| }) | ||
|
|
||
| ext.AddSuite(e.Suite{ | ||
| Name: "capio/disruptive", | ||
| Qualifiers: []string{`labels.exists(l, l == "Disruptive")`}, | ||
| Parents: []string{"openshift/disruptive-longrunning"}, | ||
| }) | ||
|
|
||
| ext.AddSuite(e.Suite{ | ||
| Name: "capio/presubmit", | ||
| Qualifiers: []string{`labels.exists(l, l == "PreSubmit")`}, | ||
| }) | ||
|
|
||
| ext.AddSuite(e.Suite{ | ||
| Name: "capio/periodic", | ||
| Qualifiers: []string{`labels.exists(l, l == "Periodic")`}, | ||
| }) | ||
|
|
||
| ext.AddSuite(e.Suite{ | ||
| Name: "capio/e2e", | ||
| Description: "All Cluster CAPI Operator tests", | ||
| }) |
There was a problem hiding this comment.
Let's chat about these.
What does OTE enforce WRT to what suites we define?
Do we want to define all these?
There was a problem hiding this comment.
OTE doesn't enforce any particular suite structure - suites are optional metadata. The two that actually matter for promotion are capio/parallel and capio/serial via their Parents field, which is how tests get included in the nightly conformance runs and feed Sippy for promotion gate evaluation.
The presubmit/periodic suites were added for future-use, but they are not used currently. Happy to drop them if you'd prefer to start minimal and add as needed. The idea behind those 2 was to do explicit label-based filtering, controlling which tests run in presubmit/periodics. For this to be possible we would have to change the jobs configuration to something like:
test:
- ref: openshift-e2e-test
env:
- name: TEST_SUITE
value: capio/presubmitThere was a problem hiding this comment.
On reflection, they're harder to use in presubmits than they appear - a job using openshift-e2e-test with TEST_SUITE: capio/presubmit would pull the binary from the ART/stable image, not the PR's freshly-built one, so new tests in a PR wouldn't be picked up. For presubmit validation of new tests make e2e from src remains the right approach.
Lets drop those.
There was a problem hiding this comment.
Kept only capio/parallel, capio/serial, and capio/disruptive — these are the only suites that map to suites actually run in nightly builds via their Parents field (openshift/conformance/parallel, openshift/conformance/serial, openshift/disruptive-longrunning).
If we want to increase testing frequency with an explicit periodic job in the future, we can reference the parent or the child suites individually in the job.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/cluster-capi-operator-tests-ext/main.go (1)
76-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse “Cluster API” in user-facing command text.
Line 76 uses “Cluster CAPI Operator …” in help output. Please switch to “Cluster API” for user-facing wording.
As per coding guidelines, for user-facing text like logs and errors, use “Cluster API” and “Machine API”.
Suggested fix
- Long: "Cluster CAPI Operator tests extension for OpenShift", + Long: "Cluster API Operator tests extension for OpenShift",🤖 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 `@cmd/cluster-capi-operator-tests-ext/main.go` at line 76, The Long field in the command definition contains "Cluster CAPI Operator" which does not follow user-facing text conventions. Replace "Cluster CAPI Operator" with "Cluster API" in the Long field to align with coding guidelines for user-facing command help text. Ensure the updated message reads "Cluster API Operator tests extension for OpenShift" or similar wording using the standardized "Cluster API" terminology.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.
Outside diff comments:
In `@cmd/cluster-capi-operator-tests-ext/main.go`:
- Line 76: The Long field in the command definition contains "Cluster CAPI
Operator" which does not follow user-facing text conventions. Replace "Cluster
CAPI Operator" with "Cluster API" in the Long field to align with coding
guidelines for user-facing command help text. Ensure the updated message reads
"Cluster API Operator tests extension for OpenShift" or similar wording using
the standardized "Cluster API" terminology.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d957a173-32a9-496a-b165-58e26c424238
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumvendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (6)
Dockerfile.rhelMakefilecmd/cluster-capi-operator-tests-ext/dependencymagnet.gocmd/cluster-capi-operator-tests-ext/main.gogo.modhack/vendor.sh
💤 Files with no reviewable changes (1)
- Makefile
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile.rhel
|
/test lint |
|
@pmeida: This PR was included in a payload test run from openshift/origin#31307
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5ec85100-6a20-11f1-9161-4db964fb4ac7-0 |
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 `@openshift-tests-extension/go.mod`:
- Around line 11-15: Add the complete set of k8s.io replace directives to the
replace block in openshift-tests-extension/go.mod (currently containing only
ginkgo, testutils, and e2e replaces) to match the k8s.io versions pinned in the
root go.mod, ensuring deterministic version pinning in all build contexts and
preventing version drift. Additionally, fix the version mismatch in the root
go.mod at lines 47 and 87 where k8s.io/kubernetes is pinned to v1.35.3 in the
replace directive but required as v1.35.1 - update both replace occurrences to
v1.35.1 to align with the require version.
🪄 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: 420363db-1ca6-4c12-ab25-659efa1f05ea
⛔ Files ignored due to path filters (3)
go.workis excluded by!**/*.workopenshift-tests-extension/go.sumis excluded by!**/*.sumvendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (5)
AGENTS.mdMakefilehack/vendor.shopenshift-tests-extension/cmd/main.goopenshift-tests-extension/go.mod
✅ Files skipped from review due to trivial changes (1)
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
Local OTE flow walk-throughTo answer the question about local invocation - here's the full flow:
Added a demo file used (
|
|
/test e2e-aws-capi-techpreview |
|
@pmeida: This PR was included in a payload test run from openshift/origin#31307
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/82c90270-6a52-11f1-957c-4b4344f0a90d-0 |
|
/test e2e-aws-capi-techpreview |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/retest e2e-aws-capi-techpreview |
|
/retest |
|
/retest-required |
openshift-ci-robot
removed
the
verified
Introduces the OpenShift Tests Extension (OTE) binary scaffold, enabling e2e tests to be discovered and executed by the openshift-tests orchestration framework. - Add openshift-tests-extension/ as a separate Go module to isolate the ginkgo fork replace-directive from the main module dependency graph - Register the component as openshift:payload:cluster-capi-operator with three suites mapping to the standard conformance hierarchy: capio/parallel → openshift/conformance/parallel capio/serial → openshift/conformance/serial capio/disruptive → openshift/disruptive-longrunning - Build and embed the gzipped binary in the operator image at /usr/bin/cluster-capi-operator-tests-ext.gz - Add bin/cluster-capi-operator-tests-ext build target to Makefile - Add openshift-tests-extension to go.work and hack/vendor.sh - Vendor github.com/openshift-eng/openshift-tests-extension - Add openshift-tests-extension/README.md with developer guide covering file suffix conventions, suite routing, feature gate labeling, blocking vs informing lifecycle, per-test timeouts, and local run instructions - Rename e2e/machine_migration_vap_tests.go to the standard _test.go suffix to prevent unintended OTE discovery - Align testutils replace directive with upstream (ff0f225cc3b5) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
had to rebase onto latest upstream main (post PR #598). The ovn presubmit builds were failing because openshift-tests-extension/go.mod still pinned cluster-api-actuator-pkg/testutils to the old version while root and e2e go.mod had already been bumped by #598. With go.work present, the Docker build runs in workspace mode and requires all modules to agree on replace directives - the conflict caused go build to abort before compiling anything. Aligned all three modules and re-vendored. some presubmits went through some didnt, just bad timing |
|
/lgtm |
|
Scheduling tests matching the |
|
@pmeida: The 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. |
|
/verified by @pmeida |
openshift-ci-robot
added
the
verified
|
@pmeida: 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. |
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
1 similar comment
|
/retest |
|
/hold depends on openshift/release#80814 |
openshift-ci
Bot
added
the
do-not-merge/hold
|
@pmeida: The following tests failed, say
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. |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
openshift-merge-bot
Bot
merged commit 5656e20
into
openshift:main
5 participants
Summary
openshift:payload:cluster-capi-operatoropenshift-tests-extension/as a separate Go module to keep OTE dependencies (ginkgo fork, test framework) isolated from the main module's dependency graph, following the cluster-control-plane-machine-set-operator conventionParallelismandClusterStabilityexecution parameters and wired to the standard conformance hierarchy viaParents/usr/bin/cluster-capi-operator-tests-ext.gz.gofile (not_test.go) is automatically discovered byopenshift-testswith no changes to the extensione2e/machine_migration_vap_tests.go→e2e/machine_migration_vap_test.go- corrects an accidental naming error that would have caused those tests to be picked up by OTE unintentionallySimilar approaches:
Test Suites
capio/parallelopenshift/conformance/parallelcapio/serialopenshift/conformance/serialcapio/disruptiveopenshift/disruptive-longrunningThe
Parentsfield is what causes tests to be automatically included in the nightly conformance runs and feed Sippy for promotion gate evaluation.Test plan
make bin/cluster-capi-operator-tests-extbuilds successfullybin/cluster-capi-operator-tests-ext inforeturns correct component metadata (openshift:payload:cluster-capi-operator)bin/cluster-capi-operator-tests-ext list testsreturns empty (zero tests registered, as intended)make unit)make vendorruns cleanlye2e presubmit testare not affected by the change/payload-job-with-prsto check if the binary is detected (and with 0 tests)_test.goruns withmake e2eLocal outputs
No tests are yet configured with OTE (that is for future tasks)
Validation with PR #599 tests (renaming
crd_compatibility_test.go→crd_compatibility.go):Next steps
et.ExtensionTestSpecAPIblocks openshift/origin#31307
Summary by CodeRabbit
New Features
Documentation