Skip to content

CNTRLPLANE-3037: Introduce envtest for integration tests - #8034

Closed
enxebre wants to merge 9 commits into
openshift:mainfrom
enxebre:envtest-poc
Closed

CNTRLPLANE-3037: Introduce envtest for integration tests#8034
enxebre wants to merge 9 commits into
openshift:mainfrom
enxebre:envtest-poc

Conversation

@enxebre

@enxebre enxebre commented Mar 23, 2026

Copy link
Copy Markdown
Member

What this PR does / why we need it:

Drops existing api ux test which requires real infra in favour of YAML-driven integration tests that validate HyperShift CRD schemas
(including CEL validation rules) using envtest.

Tests run against multiple Kubernetes and OCP API server versions to catch compatibility issues
across releases.

The YAML format is compatible with
openshift/api tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • New Features

    • Envtest-based API validation suite with multi-version OCP and Kubernetes support.
  • Tests

    • YAML-driven create/update test format, generator, and runner covering extensive NodePool validation.
    • New envtest runner across OCP/Kubernetes version matrix; removed legacy API UX tests and example manifests.
  • Documentation

    • Added envtest README with usage and test-writing guidelines.
  • Chores

    • Tooling and build automation updated to provision and run envtest assets across versions.

@openshift-ci-robot

Copy link
Copy Markdown

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: LGTM mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 23, 2026
@openshift-ci-robot

openshift-ci-robot commented Mar 23, 2026

Copy link
Copy Markdown

@enxebre: This pull request references CNTRLPLANE-3037 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 "4.22.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:

Drops existing api ux test which requires real infra in favour of YAML-driven integration tests that validate HyperShift CRD schemas
(including CEL validation rules) using envtest.

Tests run against multiple Kubernetes and OCP API server versions to catch compatibility issues
across releases.

The YAML format is compatible with
openshift/api tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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.

@enxebre

enxebre commented Mar 23, 2026

Copy link
Copy Markdown
Member Author

/test unit
/test verify

@coderabbitai

coderabbitai Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 6b29a802-632b-406b-a0db-38e8903f133b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds an envtest-based, YAML-driven API validation test framework and Makefile support to build and use setup-envtest and manage KUBEBUILDER_ASSETS for multiple OCP and upstream Kubernetes versions. It introduces envtest package types, a Ginkgo suite bootstrap, a generator that converts .testsuite.yaml files into table-driven tests, a NodePool testsuite manifest, and README documentation. hack/tools is updated to include setup-envtest. Several legacy E2E tests and YAML assets were removed. New Makefile variables and phony targets were added to compile/setup setup-envtest, resolve envtest asset directories for OCP and vanilla kube, and run envtest suites across configured versions.

Sequence Diagram(s)

sequenceDiagram
    participant Makefile
    participant Tools as "hack/tools (go build)"
    participant SetupEnv as setup-envtest
    participant TestRunner as "go test / Ginkgo"
    participant EnvTest as "envtest.Environment"
    participant SuiteLoader as "LoadTestSuiteSpecs"
    participant Generator as "GenerateTestSuite"
    participant APIServer as "API Server (with CRDs)"
    participant K8sClient as "controller-runtime Client"

    Makefile->>Tools: build setup-envtest binary (SETUP_ENVTEST_VER)
    Tools-->>SetupEnv: produce setup-envtest executable
    Makefile->>SetupEnv: invoke setup-envtest use (--index for OCP) to resolve assets
    SetupEnv-->>Makefile: return KUBEBUILDER_ASSETS path(s)
    Makefile->>TestRunner: invoke TestAPIs with KUBEBUILDER_ASSETS (loop per configured version)
    TestRunner->>EnvTest: start envtest.Environment and install CRDs (allCRDs(featureSet))
    EnvTest->>APIServer: launch API server with installed CRDs
    TestRunner->>SuiteLoader: Load .testsuite.yaml files from test/envtest
    SuiteLoader-->>TestRunner: []SuiteSpec
    TestRunner->>Generator: GenerateTestSuite for each SuiteSpec
    Generator->>K8sClient: create/update/delete unstructured test resources
    K8sClient->>APIServer: send create/update requests
    APIServer-->>K8sClient: validation responses / persisted objects
    K8sClient-->>Generator: return results
    Generator->>TestRunner: assert expected errors/state
    TestRunner->>EnvTest: stop envtest.Environment
Loading
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@enxebre

enxebre commented Mar 23, 2026

Copy link
Copy Markdown
Member Author

follow up for #8016
/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2026
@openshift-ci
openshift-ci Bot requested review from csrwng and devguyio March 23, 2026 14:23
@openshift-ci openshift-ci Bot added area/ci-tooling Indicates the PR includes changes for CI or tooling area/testing Indicates the PR includes changes for e2e testing labels Mar 23, 2026
@openshift-ci

openshift-ci Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

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:

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

@openshift-ci openshift-ci Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/needs-area labels Mar 23, 2026
@openshift-ci-robot

openshift-ci-robot commented Mar 23, 2026

Copy link
Copy Markdown

@enxebre: This pull request references CNTRLPLANE-3037 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 "4.22.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:

Drops existing api ux test which requires real infra in favour of YAML-driven integration tests that validate HyperShift CRD schemas
(including CEL validation rules) using envtest.

Tests run against multiple Kubernetes and OCP API server versions to catch compatibility issues
across releases.

The YAML format is compatible with
openshift/api tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

Release Notes

  • New Features

  • Added comprehensive API validation test suite for HyperShift CRDs using envtest.

  • Introduced testing support across multiple OCP and vanilla Kubernetes versions.

  • Tests

  • Refactored validation tests with a new YAML-driven test format for improved maintainability and extensibility.

  • Chores

  • Updated tooling dependencies to support enhanced testing infrastructure.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
test/envtest/generator.go (1)

58-67: Validate case GVKs against the suite header.

Lines 63-65 only check that crdName is present, and version is never enforced. Line 73 then derives the cleanup GVK from the first case's YAML instead. That leaves the suite header as inert metadata: a copied initial/updated document can silently exercise the wrong resource/version while the suite still reads as valid.

Also applies to: 72-85

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/envtest/generator.go` around lines 58 - 67, The suite validation
currently only checks s.CRDName and never enforces s.Version nor validates that
each case's GVK matches the suite header; update the parsing/validation (around
SuiteSpec unmarshalling and the code that derives cleanup GVK from the first
case) to: ensure SuiteSpec has both CRDName and Version present (e.g., s.CRDName
and s.Version), iterate over the parsed cases (e.g., cases or CaseSpec entries)
and verify each case's Group/Kind/Version matches the suite header CRDName and
Version, returning an error if any mismatch is found, and stop deriving the
cleanup GVK from the first case YAML—use the suite header GVK (CRDName+Version)
as the canonical cleanup GVK instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Makefile`:
- Around line 316-318: The Makefile test target currently runs the prerequisite
test-envtest-api-all and then runs go test ./... which re-runs the envtest
packages; update the test rule so it does not re-run envtest: either remove
test-envtest-api-all from the test target prerequisites and keep the existing go
test ./... invocation, or (preferably) change the go test invocation in the test
target to exclude the envtest packages (e.g., exclude packages under
./test/envtest) so the explicit test-envtest-api-all prerequisite handles
envtest bootstrapping and coverage only; adjust the KUBEBUILDER_ASSETS/GO
invocation accordingly to ensure coverage remains collected once.

In `@test/envtest/generator.go`:
- Around line 88-93: The AfterEach cleanup currently calls
k8sClient.DeleteAllOf(ctx, u, client.InNamespace("default")) but constructs an
Expect without applying a matcher, so failures are ignored; update the AfterEach
to assert the delete call succeeds by wrapping the call with
Expect(...).To(Succeed()) (or Expect(...).ToNot(HaveOccurred())) using the same
cleanupGVK, u.SetGroupVersionKind and k8sClient.DeleteAllOf symbols so any
deletion error will fail the test and prevent leaked objects.

In `@test/envtest/nodepools.testsuite.yaml`:
- Around line 412-435: The "when image is empty it should fail" test duplicates
the bad-format case because release.image is still set to "@"; change the
NodePool fixture's spec.release.image in that test to an empty string (or remove
the image field) so it truly tests the empty-image branch. Similarly, for the
"defaults to amd64" test remove the spec.arch: amd64 entry from that NodePool
fixture so the code exercises the omitted-arch/default path instead of always
using amd64.

---

Nitpick comments:
In `@test/envtest/generator.go`:
- Around line 58-67: The suite validation currently only checks s.CRDName and
never enforces s.Version nor validates that each case's GVK matches the suite
header; update the parsing/validation (around SuiteSpec unmarshalling and the
code that derives cleanup GVK from the first case) to: ensure SuiteSpec has both
CRDName and Version present (e.g., s.CRDName and s.Version), iterate over the
parsed cases (e.g., cases or CaseSpec entries) and verify each case's
Group/Kind/Version matches the suite header CRDName and Version, returning an
error if any mismatch is found, and stop deriving the cleanup GVK from the first
case YAML—use the suite header GVK (CRDName+Version) as the canonical cleanup
GVK instead.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 4227ca0f-0294-4036-b192-890e26dd5bb5

📥 Commits

Reviewing files that changed from the base of the PR and between 02bd099 and 35f92f7.

⛔ Files ignored due to path filters (49)
  • hack/tools/go.sum is excluded by !**/*.sum
  • hack/tools/vendor/github.com/go-logr/logr/slogr/slogr.go is excluded by !**/vendor/**
  • hack/tools/vendor/github.com/go-logr/zapr/.gitignore is excluded by !**/vendor/**
  • hack/tools/vendor/github.com/go-logr/zapr/.golangci.yaml is excluded by !**/vendor/**
  • hack/tools/vendor/github.com/go-logr/zapr/LICENSE is excluded by !**/vendor/**
  • hack/tools/vendor/github.com/go-logr/zapr/README.md is excluded by !**/vendor/**
  • hack/tools/vendor/github.com/go-logr/zapr/slogzapr.go is excluded by !**/vendor/**
  • hack/tools/vendor/github.com/go-logr/zapr/zapr.go is excluded by !**/vendor/**
  • hack/tools/vendor/github.com/go-logr/zapr/zapr_noslog.go is excluded by !**/vendor/**
  • hack/tools/vendor/github.com/go-logr/zapr/zapr_slog.go is excluded by !**/vendor/**
  • hack/tools/vendor/modules.txt is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/LICENSE is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/README.md is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/env/env.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/env/exit.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/env/helpers.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/main.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/remote/client.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/remote/http_client.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/remote/read_body.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/store/helpers.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/store/store.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/version/version.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/versions/parse.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/versions/platform.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/versions/version.go is excluded by !**/vendor/**
  • hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/workflows/workflows.go is excluded by !**/vendor/**
  • vendor/modules.txt is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/envtest/crd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/envtest/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/envtest/helper.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/envtest/server.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/envtest/webhook.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/errors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/flock_other.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/flock_unix.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/addr/manager.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/certs/tinyca.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/apiserver.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/auth.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/etcd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/kubectl.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/plane.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/arguments.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/bin_path_finder.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/procattr_other.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/procattr_unix.go is excluded by !vendor/**, !**/vendor/**
  • vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/process.go is excluded by !vendor/**, !**/vendor/**
📒 Files selected for processing (13)
  • Makefile
  • hack/tools/go.mod
  • hack/tools/tools.go
  • test/e2e/assets/hostedcluster-base.yaml
  • test/e2e/assets/nodepool-base.yaml
  • test/e2e/create_cluster_test.go
  • test/e2e/v2/tests/api_ux_validation_test.go
  • test/envtest/README.md
  • test/envtest/generator.go
  • test/envtest/hostedclusters.testsuite.yaml
  • test/envtest/nodepools.testsuite.yaml
  • test/envtest/suite_test.go
  • test/envtest/types.go
💤 Files with no reviewable changes (4)
  • test/e2e/assets/hostedcluster-base.yaml
  • test/e2e/create_cluster_test.go
  • test/e2e/v2/tests/api_ux_validation_test.go
  • test/e2e/assets/nodepool-base.yaml

Comment thread Makefile Outdated
Comment thread test/envtest/generator.go Outdated
Comment thread test/envtest/nodepools.testsuite.yaml Outdated
Comment on lines +412 to +435
- name: when image is empty it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: "@"
replicas: 0
platform:
aws:
instanceProfile: a-profile
instanceType: m6a.2xlarge
rootVolume:
size: 120
type: gp3
subnet:
id: "subnet-any"
type: AWS
expectedError: "Image must start with a word character (letters, digits, or underscores) and contain no white spaces"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

These two cases don't hit the branch named in the test.

Line 423 still uses @, so the “image is empty” case just duplicates the bad-format case. Line 1297 still sets arch: amd64, so the “defaults to amd64” case never exercises the omitted-arch path.

Suggested fixture correction
   - name: when image is empty it should fail
     initial: |
       apiVersion: hypershift.openshift.io/v1beta1
       kind: NodePool
       spec:
         arch: amd64
         clusterName: some-cluster
         management:
           autoRepair: false
           upgradeType: Replace
         release:
-          image: "@"
+          image: ""
         replicas: 0
         platform:
           aws:
             instanceProfile: a-profile
             instanceType: m6a.2xlarge

   - name: should pass when imageType is Windows without arch (defaults to amd64)
     initial: |
       apiVersion: hypershift.openshift.io/v1beta1
       kind: NodePool
       spec:
-        arch: amd64
         clusterName: some-cluster
         management:
           autoRepair: false
           upgradeType: Replace
         release:
           image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64

Also applies to: 1292-1315

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/envtest/nodepools.testsuite.yaml` around lines 412 - 435, The "when
image is empty it should fail" test duplicates the bad-format case because
release.image is still set to "@"; change the NodePool fixture's
spec.release.image in that test to an empty string (or remove the image field)
so it truly tests the empty-image branch. Similarly, for the "defaults to amd64"
test remove the spec.arch: amd64 entry from that NodePool fixture so the code
exercises the omitted-arch/default path instead of always using amd64.

@openshift-ci-robot

openshift-ci-robot commented Mar 23, 2026

Copy link
Copy Markdown

@enxebre: This pull request references CNTRLPLANE-3037 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 "4.22.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:

Drops existing api ux test which requires real infra in favour of YAML-driven integration tests that validate HyperShift CRD schemas
(including CEL validation rules) using envtest.

Tests run against multiple Kubernetes and OCP API server versions to catch compatibility issues
across releases.

The YAML format is compatible with
openshift/api tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • New Features

  • Added an envtest-based API validation suite for HyperShift CRDs with multi-version OCP and Kubernetes support.

  • Tests

  • Introduced YAML-driven test format, generator and runner to execute create/update validation suites (including comprehensive NodePool tests).

  • Added test entrypoint and versioned test targets for broad coverage.

  • Documentation

  • Added envtest README with usage and test-writing guidelines.

  • Chores

  • Updated tooling/dependencies and Makefile targets to support the new test workflow.

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

openshift-ci-robot commented Mar 23, 2026

Copy link
Copy Markdown

@enxebre: This pull request references CNTRLPLANE-3037 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 "4.22.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:

Drops existing api ux test which requires real infra in favour of YAML-driven integration tests that validate HyperShift CRD schemas
(including CEL validation rules) using envtest.

Tests run against multiple Kubernetes and OCP API server versions to catch compatibility issues
across releases.

The YAML format is compatible with
openshift/api tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • New Features

  • Envtest-based API validation suite for HyperShift CRDs with multi-version OCP and Kubernetes support.

  • Tests

  • YAML-driven test format, generator, and runner for create/update validation, including comprehensive NodePool scenarios.

  • New test entrypoint and Make targets to run suites across OCP/kube versions.

  • Removed legacy API UX tests and example manifests.

  • Documentation

  • Added envtest README with usage and test-writing guidelines.

  • Chores

  • Updated tooling/dependencies and Makefile to support the new test workflow.

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

openshift-ci-robot commented Mar 23, 2026

Copy link
Copy Markdown

@enxebre: This pull request references CNTRLPLANE-3037 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 "4.22.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:

Drops existing api ux test which requires real infra in favour of YAML-driven integration tests that validate HyperShift CRD schemas
(including CEL validation rules) using envtest.

Tests run against multiple Kubernetes and OCP API server versions to catch compatibility issues
across releases.

The YAML format is compatible with
openshift/api tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • New Features

  • Envtest-based API validation suite with multi-version OCP and Kubernetes support.

  • Tests

  • YAML-driven create/update test format, generator, and runner covering comprehensive NodePool validation.

  • New test runner supporting OCP/kubernetes version matrix; removed legacy API UX tests and example manifests.

  • Documentation

  • Added envtest README with usage and test-writing guidelines.

  • Chores

  • Tooling and build automation updated to support the new test workflow.

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

openshift-ci-robot commented Mar 23, 2026

Copy link
Copy Markdown

@enxebre: This pull request references CNTRLPLANE-3037 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 "4.22.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:

Drops existing api ux test which requires real infra in favour of YAML-driven integration tests that validate HyperShift CRD schemas
(including CEL validation rules) using envtest.

Tests run against multiple Kubernetes and OCP API server versions to catch compatibility issues
across releases.

The YAML format is compatible with
openshift/api tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • New Features

  • Envtest-based API validation suite with multi-version OCP and Kubernetes support.

  • Tests

  • YAML-driven create/update test format, generator, and runner covering extensive NodePool validation.

  • New envtest runner across OCP/Kubernetes version matrix; removed legacy API UX tests and example manifests.

  • Documentation

  • Added envtest README with usage and test-writing guidelines.

  • Chores

  • Tooling and build automation updated to provision and run envtest assets across versions.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
test/envtest/generator.go (1)

91-97: ⚠️ Potential issue | 🟠 Major

Still missing a matcher on cleanup.

Line 96 wraps DeleteAllOf in Expect(...) but never calls .To(...), so cleanup errors are ignored and leaked objects can contaminate later table entries.

Run the following script to verify the current assertion:

#!/bin/bash
set -euo pipefail
nl -ba test/envtest/generator.go | sed -n '91,97p'

Expected result: the cleanup block contains Expect(k8sClient.DeleteAllOf(...)) without a matcher.

Suggested fix
 		AfterEach(func() {
 			if cleanupGVK.Kind != "" {
 				u := &unstructured.Unstructured{}
 				u.SetGroupVersionKind(cleanupGVK)
-				Expect(k8sClient.DeleteAllOf(ctx, u, client.InNamespace("default")))
+				Expect(k8sClient.DeleteAllOf(ctx, u, client.InNamespace("default"))).To(Succeed())
 			}
 		})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/envtest/generator.go` around lines 91 - 97, The AfterEach cleanup calls
Expect(k8sClient.DeleteAllOf(...)) without a matcher, so deletion errors are
ignored; update the assertion around k8sClient.DeleteAllOf(ctx, u,
client.InNamespace("default")) (used with cleanupGVK and
unstructured.Unstructured) to include a matcher such as .To(Succeed()) or
.ToNot(HaveOccurred()) so failures surface (e.g.,
Expect(k8sClient.DeleteAllOf(...)).To(Succeed())).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Makefile`:
- Around line 320-324: The ENVTEST_OCP_INDEX currently points to the moving
master branch (ENVTEST_OCP_INDEX :=
https://raw.githubusercontent.com/openshift/api/master/envtest-releases.yaml),
which makes test matrix non-reproducible; change it to a commit-pinned URL or
vendor the envtest index into the repo and point ENVTEST_OCP_INDEX at that
static file. Update the Makefile to replace the master raw URL with a specific
commit SHA raw URL (or a local vendored path) and ensure
ENVTEST_OCP_K8S_VERSIONS remains compatible with the pinned index.
- Around line 316-318: The default Makefile target "test" currently depends on
the full envtest matrix "test-envtest-api-all", making everyday runs slow and
network-dependent; change "test" to run only a single representative envtest
target (or no envtest) instead of depending on "test-envtest-api-all", and keep
"test-envtest-api-all" as a separate CI-only target; update any other targets
that inherit this dependency (notably "pre-commit") so they do not depend on the
full matrix, and ensure the cover output behavior (writing cover.out) remains
correct for the representative local run while the matrix target remains solely
for CI.

In `@test/envtest/suite_test.go`:
- Around line 59-71: The package-level Describe block is executed at package
init time but suites is populated in TestAPIs, so the generated specs register
zero tests; move the call to LoadTestSuiteSpecs so suites is populated before
any Describe evaluation (e.g., call LoadTestSuiteSpecs during init() or in
TestMain) and remove or adjust the LoadTestSuiteSpecs invocation inside TestAPIs
to avoid double-loading; ensure the global variable suites is set by the earlier
initialization so the Describe block (and its loop over suites) sees the loaded
specs.

---

Duplicate comments:
In `@test/envtest/generator.go`:
- Around line 91-97: The AfterEach cleanup calls
Expect(k8sClient.DeleteAllOf(...)) without a matcher, so deletion errors are
ignored; update the assertion around k8sClient.DeleteAllOf(ctx, u,
client.InNamespace("default")) (used with cleanupGVK and
unstructured.Unstructured) to include a matcher such as .To(Succeed()) or
.ToNot(HaveOccurred()) so failures surface (e.g.,
Expect(k8sClient.DeleteAllOf(...)).To(Succeed())).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: ec95d69f-bd2a-48a6-95e9-07eb7a24650f

📥 Commits

Reviewing files that changed from the base of the PR and between 82e9324 and 37096c9.

📒 Files selected for processing (12)
  • .golangci.yml
  • Makefile
  • test/e2e/assets/hostedcluster-base.yaml
  • test/e2e/assets/nodepool-base.yaml
  • test/e2e/create_cluster_test.go
  • test/e2e/v2/tests/api_ux_validation_test.go
  • test/envtest/README.md
  • test/envtest/generator.go
  • test/envtest/hostedclusters.testsuite.yaml
  • test/envtest/nodepools.testsuite.yaml
  • test/envtest/suite_test.go
  • test/envtest/types.go
💤 Files with no reviewable changes (4)
  • test/e2e/assets/nodepool-base.yaml
  • test/e2e/create_cluster_test.go
  • test/e2e/assets/hostedcluster-base.yaml
  • test/e2e/v2/tests/api_ux_validation_test.go
✅ Files skipped from review due to trivial changes (3)
  • test/envtest/README.md
  • test/envtest/types.go
  • test/envtest/nodepools.testsuite.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .golangci.yml

Comment thread Makefile
Comment thread Makefile
Comment thread test/envtest/suite_test.go
@enxebre enxebre changed the title CNTRLPLANE-3037: Introduce envtest for integration tests CNTRLPLANE-3037,OCPBUGS-78837: Introduce envtest for integration tests Mar 23, 2026
@openshift-ci-robot

openshift-ci-robot commented Mar 23, 2026

Copy link
Copy Markdown

@enxebre: This pull request references CNTRLPLANE-3037 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 "4.22.0" version, but no target version was set.

This pull request references Jira Issue OCPBUGS-78837, which is invalid:

  • expected the bug to target the "4.22.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:

What this PR does / why we need it:

Drops existing api ux test which requires real infra in favour of YAML-driven integration tests that validate HyperShift CRD schemas
(including CEL validation rules) using envtest.

Tests run against multiple Kubernetes and OCP API server versions to catch compatibility issues
across releases.

The YAML format is compatible with
openshift/api tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • New Features

  • Envtest-based API validation suite with multi-version OCP and Kubernetes support.

  • Tests

  • YAML-driven create/update test format, generator, and runner covering extensive NodePool validation.

  • New envtest runner across OCP/Kubernetes version matrix; removed legacy API UX tests and example manifests.

  • Documentation

  • Added envtest README with usage and test-writing guidelines.

  • Chores

  • Tooling and build automation updated to provision and run envtest assets across versions.

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 openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Mar 23, 2026
@enxebre

enxebre commented Mar 23, 2026

Copy link
Copy Markdown
Member Author

/test e2e-aws

@cwbotbot

Copy link
Copy Markdown

Test Results

e2e-aws

@enxebre

enxebre commented Mar 25, 2026

Copy link
Copy Markdown
Member Author
  • Each test suite get either all stable or techpreview CRDs installed and uninstalled before and after running.
  • Test suites are defined as {stable/techpreview}.{CRDName}.{suiteCase}.testsuite.yaml files following the openshift/api tests format
  • Split test cases into suites including a ratcheting example.
    ptal @JoelSpeed

@enxebre

enxebre commented Mar 25, 2026

Copy link
Copy Markdown
Member Author

install/unistall per suite has considerable impact on timing.
I'll update to keep a single CRD per suite then have a dedicated validation for installing all together.

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 25, 2026
@enxebre
enxebre force-pushed the envtest-poc branch 2 times, most recently from 1ae4450 to d1a360d Compare March 25, 2026 14:11
- Add an envtest setup goal on the Makefile
- Add a new unit test using envtest to cover the same as the create
  cluster e2e test.

Signed-off-by: Borja Clemente <bclement@redhat.com>
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 25, 2026
Introducing envtest requires adding new dependencies, which are being
vendored in a separate commit to ease review.

Signed-off-by: Borja Clemente <bclement@redhat.com>
enxebre and others added 7 commits March 25, 2026 17:04
Replace the Go-based envtest test with a YAML-driven test framework
following the openshift/api tests pattern. The framework:

- Loads .testsuite.yaml files defining onCreate/onUpdate test cases
- Per suite, installs/uninstalls the CRD under test for each feature
  set variant (derived from stable./techpreview. file prefix)
- Validates all CRDs install via dedicated GenerateCRDInstallTest
- Generates Ginkgo test tables from YAML specs
- Supports expectedError, expectedStatusError for validation tests
- Supports initialCRDPatches for ratcheting validation via yaml-patch
- Uses //go:build envtest tag so tests are excluded from go test ./...

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@enxebre

enxebre commented Mar 25, 2026

Copy link
Copy Markdown
Member Author

install/unistall per suite has considerable impact on timing.
I'll update to keep a single CRD per suite then have a dedicated validation for installing all together.

done PTAL @JoelSpeed

@enxebre

enxebre commented Mar 26, 2026

Copy link
Copy Markdown
Member Author

@JoelSpeed would it make sense to try to align the assumptions that the CRD filtering does and then this repo can just vendor o/api/test logic?
I think current main broken assumption is payload-manifests/featuregates/ doesn't exist, here are within /api/hypershift/v1beta1/featuregates and the suites don't live in tests/

@enxebre

enxebre commented Mar 27, 2026

Copy link
Copy Markdown
Member Author

@JoelSpeed would it make sense to try to align the assumptions that the CRD filtering does and then this repo can just vendor o/api/test logic?
I think current main broken assumption is payload-manifests/featuregates/ doesn't exist, here are within /api/hypershift/v1beta1/featuregates and the suites don't live in tests/

This turned impractical because it would require openshift/api#2637 which relies on write-available-featuresets using features.AllFeatureSets() which is consuming the gates from openshift/api.

I created an alternative though that's closer to o/api but use spec.featureGate instead of the annotation for CRD filtering.
Tests takes longer now as they run against all CRD variants Default, TechPreviewNoUpgrade and CustomNoUpgrade #8089

@openshift-ci

openshift-ci Bot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

@enxebre: The following test 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/security 0c2e64e link true /test security

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.

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2026
@openshift-ci

openshift-ci Bot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

PR needs rebase.

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.

@JoelSpeed

Copy link
Copy Markdown
Contributor

/close

An alternative PR merged #8089

@openshift-ci openshift-ci Bot closed this Mar 31, 2026
@openshift-ci

openshift-ci Bot commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

@JoelSpeed: Closed this PR.

Details

In response to this:

/close

An alternative PR merged #8089

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/ci-tooling Indicates the PR includes changes for CI or tooling area/platform/aws PR/issue for AWS (AWSPlatform) platform area/platform/azure PR/issue for Azure (AzurePlatform) platform area/platform/gcp PR/issue for GCP (GCPPlatform) platform area/testing Indicates the PR includes changes for e2e testing jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants