OCPCLOUD-3571: Add OTE e2e tests for ClusterAPIMachineManagementAWS feature gate#613
OCPCLOUD-3571: Add OTE e2e tests for ClusterAPIMachineManagementAWS feature gate#613pmeida wants to merge 1 commit into
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-3571 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. |
WalkthroughThis PR adds a new e2e test suite for AWS-specific Cluster API Machine Management, verifying controller deployment availability, CRD establishment, region matching, and infrastructure readiness. It also updates the test extension entrypoint to auto-apply platform-based selectors to specs based on their labels. ChangesAWS Cluster API E2E Suite
Estimated code review effort: 2 (Simple) | ~15 minutes Platform Label Auto-Application in Test Extension
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/cluster_api_machine_management_aws.go (1)
58-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
DescribeTable/Entryfor the CRD list check.The manual
forloop over the three CRD names could be expressed withDescribeTable/Entry, giving per-CRD pass/fail reporting instead of a single aggregateIt.♻️ Suggested refactor
- It("should have core AWS Cluster API CRDs installed and established", func() { - for _, name := range []string{ - "awsclusters.infrastructure.cluster.x-k8s.io", - "awsmachines.infrastructure.cluster.x-k8s.io", - "awsmachinetemplates.infrastructure.cluster.x-k8s.io", - } { - crd := &apiextensionsv1.CustomResourceDefinition{ObjectMeta: metav1.ObjectMeta{Name: name}} - Eventually(komega.Object(crd)).WithTimeout(framework.WaitMedium).WithPolling(framework.RetryMedium).Should( - HaveField("Status.Conditions", ContainElement(SatisfyAll( - HaveField("Type", Equal(apiextensionsv1.Established)), - HaveField("Status", Equal(apiextensionsv1.ConditionTrue)), - ))), - "CRD %s should be established", name, - ) - } - }) + DescribeTable("should have core AWS Cluster API CRDs installed and established", + func(name string) { + crd := &apiextensionsv1.CustomResourceDefinition{ObjectMeta: metav1.ObjectMeta{Name: name}} + Eventually(komega.Object(crd)).WithTimeout(framework.WaitMedium).WithPolling(framework.RetryMedium).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", Equal(apiextensionsv1.Established)), + HaveField("Status", Equal(apiextensionsv1.ConditionTrue)), + ))), + ) + }, + Entry("awsclusters CRD", "awsclusters.infrastructure.cluster.x-k8s.io"), + Entry("awsmachines CRD", "awsmachines.infrastructure.cluster.x-k8s.io"), + Entry("awsmachinetemplates CRD", "awsmachinetemplates.infrastructure.cluster.x-k8s.io"), + )As per coding guidelines, "Use
DescribeTablewithEntryfor table-driven tests instead of manual loops".🤖 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 `@e2e/cluster_api_machine_management_aws.go` around lines 58 - 74, Refactor the AWS CRD establishment test in the existing `Context`/`It` block to use `DescribeTable` with `Entry` instead of the manual loop over the CRD names. Move the repeated CRD assertion logic into a table-driven test so each of `awsclusters.infrastructure.cluster.x-k8s.io`, `awsmachines.infrastructure.cluster.x-k8s.io`, and `awsmachinetemplates.infrastructure.cluster.x-k8s.io` is checked as its own entry. Keep the same `Eventually(komega.Object(crd))` and `HaveField("Status.Conditions", ...)` assertion logic, but parameterize the CRD name through the table.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.
Nitpick comments:
In `@e2e/cluster_api_machine_management_aws.go`:
- Around line 58-74: Refactor the AWS CRD establishment test in the existing
`Context`/`It` block to use `DescribeTable` with `Entry` instead of the manual
loop over the CRD names. Move the repeated CRD assertion logic into a
table-driven test so each of `awsclusters.infrastructure.cluster.x-k8s.io`,
`awsmachines.infrastructure.cluster.x-k8s.io`, and
`awsmachinetemplates.infrastructure.cluster.x-k8s.io` is checked as its own
entry. Keep the same `Eventually(komega.Object(crd))` and
`HaveField("Status.Conditions", ...)` assertion logic, but parameterize the CRD
name through the table.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d66ae42f-1422-469e-8ba6-d712ffce8e11
📒 Files selected for processing (2)
e2e/cluster_api_machine_management_aws.goopenshift-tests-extension/cmd/main.go
…eature gate
Adds 5 blocking OTE tests validating the CAPI AWS stack when
ClusterAPIMachineManagementAWS is enabled, plus a generic platform
Walk in the OTE extension registration.
## Tests
| # | Test | What it proves | Gated by |
|---|---|---|---|
| 1 | capa-controller-manager deployment available | AWS provider controller deployed by capi-installer | AWS gate only |
| 2 | Core AWS CRDs established (awsclusters, awsmachines, awsmachinetemplates) | CAPA CRD bundle installed by capi-installer | AWS gate only |
| 3 | AWSCluster.Spec.Region matches Infrastructure region | InfraClusterController correctly read Infrastructure CR | Both gates |
| 4 | AWSCluster.Status.Ready=True | InfraClusterController reconciled AWS infra end-to-end | Both gates |
| 5 | Cluster.InfrastructureReady=True | CAPI core accepted the AWS infra, completing the stack | Both gates |
Tests 1-2 are directly controlled by ClusterAPIMachineManagementAWS
(capi-installer deploys the CAPA bundle). Tests 3-5 require both gates —
ClusterAPIMachineManagement for capi-controllers to run and
ClusterAPIMachineManagementAWS for the AWS CRDs to exist. In practice
the base gate is promoted first, so both are always enabled together.
## Platform restriction (two-layer)
Layer 1 — OTE environmentSelector (periodic jobs):
A generic Walk over all specs reads Label("platform:<name>") and adds
spec.Include(PlatformEquals(platform)). This avoids per-platform clauses
in main.go — adding GCP/Azure tests only requires a Label on the Describe.
Layer 2 — BeforeEach guard (make e2e presubmits):
The Ginkgo BeforeEach checks both the feature gate and the cluster platform.
OTE environmentSelector is not evaluated in the make e2e path.
## Design decisions
- assertDeploymentAvailable not extracted: used once, no abstraction benefit.
- AWSClusterControllerIdentity object test excluded: it is created by CAPA
internals (not by this gate), and its absence is caught transitively by
AWSCluster.Status.Ready=True.
- metav1.ConditionTrue used directly instead of metav1.ConditionStatus("True").
- string() cast on ClusterInfrastructureReadyCondition removed (untyped const).
## Validation
Validated on a TechPreview AWS cluster-bot cluster:
OTE binary (run-suite capio/parallel):
passed — capa-controller-manager deployment available
passed — core AWS Cluster API CRDs installed and established
passed — AWSCluster region matching the cluster infrastructure region
passed — AWSCluster object present and ready
passed — management Cluster object with InfrastructureReady=True
make e2e (--focus=ClusterAPIMachineManagementAWS):
Ran 5 of 54 Specs in 2.423 seconds
SUCCESS! -- 5 Passed | 0 Failed | 2 Pending | 47 Skipped
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
@pmeida: 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. |
2 participants
Summary
Implements 5 blocking OTE e2e tests for the
ClusterAPIMachineManagementAWSfeature gate, plus a generic platform Walk in the OTE extension registration.Tests
capa-controller-managerdeployment available inopenshift-cluster-apiClusterAPIMachineManagementAWSonlyawsclusters.infrastructure.cluster.x-k8s.ioCRD establishedClusterAPIMachineManagementAWSonlyawsmachines.infrastructure.cluster.x-k8s.ioCRD establishedClusterAPIMachineManagementAWSonlyawsmachinetemplates.infrastructure.cluster.x-k8s.ioCRD establishedClusterAPIMachineManagementAWSonlyCluster.InfrastructureReady=TrueClusterAPIMachineManagementAWS+ClusterAPIMachineManagementTests 1-4 are directly controlled by
ClusterAPIMachineManagementAWS(capi-installer deploys the CAPA bundle). Test 5 requires both gates -ClusterAPIMachineManagementfor capi-controllers to run andClusterAPIMachineManagementAWSfor the AWS CRDs to exist. In practice the base gate is promoted first so both are always enabled together.InfrastructureReady=Trueis the end-to-end signal: the CAPI core controller only sets it after observingAWSCluster.Status.Ready=True, whichInfraClusterControllersets after creating the AWSCluster. This transitively proves the AWSCluster was created and ready without needing a separate test.Platform restriction (two-layer)
Layer 1 - OTE environmentSelector (periodic jobs):
A generic Walk reads
Label("platform:<name>")from each spec and addsspec.Include(PlatformEquals(platform)). No per-platform clauses inmain.go- adding GCP/Azure tests only requires aLabelon the Describe block.Layer 2 -
BeforeEachguard (make e2epresubmits):Checks both the feature gate and the cluster platform. The OTE
environmentSelectoris not evaluated in themake e2epath.Validation
Validated on a TechPreview AWS cluster-bot cluster:
OTE binary (
run-suite capio/parallel):{"name": "...capa-controller-manager deployment available", "result": "passed"} {"name": "...core AWS Cluster API CRDs installed and established", "result": "passed"} {"name": "...AWSCluster region matching the cluster infrastructure region", "result": "passed"} {"name": "...AWSCluster object present and ready", "result": "passed"} {"name": "...management Cluster object with InfrastructureReady=True", "result": "passed"}make e2e(--focus=ClusterAPIMachineManagementAWS):Test plan
e2e-aws-capi-techpreviewpasses with all 5 tests greene2e-aws-ovn-techpreviewpasses with all 5 OTE tests greene2e-gcp-ovn-techpreviewSummary by CodeRabbit