Skip to content

no-jira: aws: set IPv6 block on AWSCluster NetworkSpec for dual-stack ipFamily#593

Open
tthvo wants to merge 1 commit into
openshift:mainfrom
tthvo:awscluster-dualstack
Open

no-jira: aws: set IPv6 block on AWSCluster NetworkSpec for dual-stack ipFamily#593
tthvo wants to merge 1 commit into
openshift:mainfrom
tthvo:awscluster-dualstack

Conversation

@tthvo

@tthvo tthvo commented Jun 10, 2026

Copy link
Copy Markdown
Member

Description

When the infrastructure ipFamily is DualStackIPv4Primary or DualStackIPv6Primary, set the VPC IPv6 block on the AWSCluster so CAPA can create IPv6-capable resources.

Note: CAPA requires the VPC IPv6 block to be set to assign primary IPv6 addresses to instances. This is only applicable to IPv6 primary, but we set it for both dual-stack families to indicate that the VPC has IPv6 addressing enabled.

Summary by CodeRabbit

  • New Features

    • Added support for configurable IP family settings in AWS clusters, including dual-stack IPv6 primary configurations.
  • Tests

    • Added test coverage for IP family configuration scenarios.

When the infrastructure ipFamily is DualStackIPv4Primary or
DualStackIPv6Primary, set the VPC IPv6 block on the AWSCluster so
CAPA can create IPv6-capable resources.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

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 Jun 10, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@tthvo: This pull request explicitly references no jira issue.

Details

In response to this:

Description

When the infrastructure ipFamily is DualStackIPv4Primary or DualStackIPv6Primary, set the VPC IPv6 block on the AWSCluster so CAPA can create IPv6-capable resources.

Note: CAPA requires the VPC IPv6 block to be set to assign primary IPv6 addresses to instances. This is only applicable to IPv6 primary, but we set it for both dual-stack families to indicate that the VPC has IPv6 addressing enabled.

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 commented Jun 10, 2026

Copy link
Copy Markdown

Walkthrough

The pull request adds IPv6 configuration support to AWSCluster. The implementation extends newAWSCluster to accept an IP family parameter and conditionally configure IPv6 in the network spec. Tests are refactored to isolate per-test state and validate IPv6 behavior across dual-stack and IPv4-only configurations.

Changes

IPv6 Support for AWSCluster

Layer / File(s) Summary
AWSCluster IPv6 Implementation
pkg/controllers/infracluster/aws.go
configv1 import is added; newAWSCluster function signature is extended to accept ipFamily configv1.IPFamilyType; the caller in ensureAWSCluster passes r.Infra.Status.PlatformStatus.AWS.IPFamily; conditional logic sets Spec.NetworkSpec.VPC.IPv6 for dual-stack IPv6-primary and dual-stack IPv4-primary families.
Test Fixture Refactoring
pkg/controllers/infracluster/infracluster_controller_test.go
ocpInfraAWS is moved from a single declaration into a per-test variable initialized in BeforeEach, preventing state leakage across test cases; control-plane machine variables (machine1, machine2, machine3) are similarly refactored to enable per-test mutation of IP family configuration.
IPv6 Configuration Test Coverage
pkg/controllers/infracluster/infracluster_controller_test.go
New test context verifies AWSCluster Spec.NetworkSpec.VPC.IPv6 behavior across DualStackIPv4Primary, DualStackIPv6Primary, and IPv4 family settings, confirming IPv6 is configured only when dual-stack IPv6-semantics are present.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: setting IPv6 configuration on AWSCluster NetworkSpec when dual-stack IP family is configured.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All Ginkgo test names in the PR are stable and deterministic. New ipFamily tests use static string literals without dynamic content, variable interpolation, or generated identifiers. Dynamic values...
Test Structure And Quality ✅ Passed Tests meet quality requirements: (1) Single responsibility—each new It block tests one specific IPv6/ipFamily behavior; (2) Setup/cleanup—tests use BeforeEach for setup, AfterEach for cleanup; (3)...
Microshift Test Compatibility ✅ Passed PR adds controller unit/integration tests, not Ginkgo e2e tests. Custom check applies only to e2e tests (package e2e, using e2e/framework), which this PR does not add.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The PR adds unit tests (not e2e tests) to pkg/controllers/infracluster/ using envtest. The new tests verify AWSCluster IPv6 configuration based on ipFamily settings and do not assume multi-node clu...
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies only infrastructure controller code to configure IPv6 on AWSCluster NetworkSpec; introduces no workload scheduling constraints, pod affinity, topology spread, node selectors, or PodDisr...
Ote Binary Stdout Contract ✅ Passed PR changes don't introduce stdout violations. klog properly redirected via GinkgoWriter in BeforeSuite; all new test code isolated in test blocks; no process-level stdout writes added.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The new Ginkgo test block "When the infrastructure ipFamily is configured" contains no hardcoded IPv4 addresses, external connectivity requirements, or IPv4-only assumptions. It dynamically tests m...
No-Weak-Crypto ✅ Passed No weak cryptographic algorithms, custom crypto implementations, or insecure secret comparisons detected in the PR changes.
Container-Privileges ✅ Passed The PR modifies only Go source files (aws.go and infracluster_controller_test.go), not container/Kubernetes manifests. The custom check for container privileges is not applicable to this codebase c...
No-Sensitive-Data-In-Logs ✅ Passed No logging of sensitive data found. The PR adds an ipFamily parameter used only for conditional logic (lines 139-140) without any logging. Existing logs only include AWSCluster name/namespace via k...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign theobarberbany for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@tthvo

tthvo commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

/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 Jun 10, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/controllers/infracluster/aws.go`:
- Around line 136-138: The existing comment in
pkg/controllers/infracluster/aws.go is misleading about when CAPA requires a VPC
IPv6 block; update the comment near the code that sets IPv6 for both
DualStackIPv4Primary and DualStackIPv6Primary to clearly state that CAPA
requires the VPC IPv6 block whenever IPv6 addressing is enabled (including both
IPv6-primary and IPv4-primary dual-stack clusters), and that the code
intentionally sets the IPv6 block for both DualStackIPv4Primary and
DualStackIPv6Primary to indicate IPv6 addressing is enabled; reference the
DualStackIPv4Primary and DualStackIPv6Primary symbols in the comment so readers
immediately see which branches are affected.
- Line 86: The code calls r.newAWSCluster(providerSpec, apiURL, int32(port),
r.Infra.Status.PlatformStatus.AWS.IPFamily) without guarding that
r.Infra.Status.PlatformStatus.AWS is non-nil; add a nil-check before accessing
IPFamily (e.g., verify r.Infra != nil && r.Infra.Status.PlatformStatus != nil &&
r.Infra.Status.PlatformStatus.AWS != nil) and handle the nil case by returning
an error or using a safe default prior to invoking r.newAWSCluster so the code
never dereferences a nil AWS pointer.
🪄 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: f791e52c-9f19-409f-9f12-ea1b270b86e8

📥 Commits

Reviewing files that changed from the base of the PR and between 05c113e and 15b74ce.

📒 Files selected for processing (2)
  • pkg/controllers/infracluster/aws.go
  • pkg/controllers/infracluster/infracluster_controller_test.go

Comment thread pkg/controllers/infracluster/aws.go
Comment thread pkg/controllers/infracluster/aws.go
@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@tthvo: all tests passed!

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.

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

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants