CORS-4136, CORS-4157: Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support #2430

sadasu · 2025-07-31T15:30:38Z

No description provided.

openshift-ci · 2025-07-31T15:30:47Z

Hello @sadasu! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

openshift-ci-robot · 2025-07-31T15:33:07Z

@sadasu: This pull request references CORS-4136 which is a valid jira issue.

This pull request references CORS-4157 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.20.0" version, but no target version was set.

In 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 · 2025-07-31T15:34:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

patrickdillon · 2025-08-06T18:35:02Z

config/v1/types_infrastructure.go

+	// +kubebuilder:validation:Enum="IPFamiliesIPv4";"IPFamiliesDualStack"
+	// +openshift:enable:FeatureGate=AWSDualStackInstall
+	// +optional
+	IpFamily IPFamiliesType `json:"ipFamily,omitempty"`


I think based on go naming conventions this should be

Suggested change

IpFamily IPFamiliesType `json:"ipFamily,omitempty"`

IPFamily IPFamiliesType `json:"ipFamily,omitempty"`

JoelSpeed · 2025-08-07T10:34:08Z

config/v1/types_infrastructure.go

+	// +default="IPFamiliesIPv4"
+	// +kubebuilder:default:="IPFamiliesIPv4"


Are we sure we want to default this? Are there existing clusters where this could default to an incorrect value?

All currently installed AWS,Azure and GCP clusters are ipv4 only so this seemed like the correct default. So, when we upgrade to a version with this API change, the address family would be unambiguous.

Is there some way in the cluster that we could tell that the cluster is IPv4 and therefore set it based on something that already exists, rather than defaulting within the API based off of an assumption?

While all clusters today are ipv4, in theory in the future we could end up defaulting a cluster that is not ipv4 and this would then send the wrong message

We are adding an install-config field for customers to set the IPFamily for the cluster for new installs. There too we are defaulting to IPV4. And as part of the support for dualstack, the Installer will always be explicitly setting this field in the Infra CR when the manifest is generated.

For existing clusters, we know that they are all IPv4 and hence added this defaulting behavior. Its value should be immutable.

JoelSpeed · 2025-08-07T10:34:26Z

config/v1/types_infrastructure.go

+	// ipFamily indicates the IP Address family supported by cluster nodes.
+	// +default="IPFamiliesIPv4"
+	// +kubebuilder:default:="IPFamiliesIPv4"
+	// +kubebuilder:validation:Enum="IPFamiliesIPv4";"IPFamiliesDualStack"


This should be the string value, not the constant name

JoelSpeed · 2025-08-07T10:34:56Z

config/v1/types_infrastructure.go

@@ -536,6 +536,14 @@ type AWSPlatformStatus struct {
 	// +optional
 	// +nullable
 	CloudLoadBalancerConfig *CloudLoadBalancerConfig `json:"cloudLoadBalancerConfig,omitempty"`
+
+	// ipFamily indicates the IP Address family supported by cluster nodes.


Explain the valid values and what they mean as part of the godoc

JoelSpeed · 2025-08-07T10:35:29Z

..._generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml

@@ -1130,7 +1130,6 @@ spec:
                - HighlyAvailable
                - HighlyAvailableArbiter
                - SingleReplica
-                - DualReplica


Not sure why this has been removed?

JoelSpeed · 2025-08-07T10:36:33Z

machineconfiguration/v1/types.go

@@ -148,7 +148,7 @@ type ControllerConfigSpec struct {

 	// ipFamilies indicates the IP families in use by the cluster network
 	// +required
-	IPFamilies IPFamiliesType `json:"ipFamilies"`
+	IPFamilies configv1.IPFamiliesType `json:"ipFamilies"`


Do we need to do this? We tend to prefer local types over importing from other packages

IPFamiliesType was originally defined here : https://github.com/openshift/api/blob/master/machineconfiguration/v1/types.go#L177-L185.

Instead of redefining IPFamilies for use within types_infrastructure.go, i moved it to a common location in config/v1/types.go

Are we confident that their use cases will not diverge? It's generally safer to have the two as separate APIs and not to share a common place like this

I do not envision IP family types to diverge. But, let me try again by separating the APIs.

JoelSpeed · 2025-08-07T10:36:46Z

...generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yaml

@@ -669,7 +669,7 @@ spec:
                                            when type is Name, and forbidden otherwise
                                          rule: 'has(self.type) && self.type == ''Name''
                                            ?  has(self.name) : !has(self.name)'
-                                      maxItems: 32
+                                      maxItems: 1


This seems to be unrelated?

This and the deletion of DualReplica keeps showing up every time I run make update.

Make sure you rebase onto the latest main before you run the update command

JoelSpeed · 2025-08-12T16:37:16Z

config/v1/types_infrastructure.go

+	// infrastructure and all cluster resources are expected to have both IPv4 and IPv6 addresses.
+	// +default="IPv4"
+	// +kubebuilder:default:="IPv4"
+	// +kubebuilder:validation:Enum="IPv4";"DualStack"


Generally prefer to have this on the type alias itself, rather than at the field level

There was a question within the team regarding this field in the PlatformStatus. Could the same functionality be achieved using an existing API such as cloud provider config? My reasoning has been that we need a source of truth for the cluster that is easily available even on day-2 and within an API that we manage. Thoughts?

JoelSpeed · 2025-08-12T16:37:36Z

config/v1/types.go

@@ -429,3 +429,11 @@ const (
 	// IBMCloudServiceVPC is the name for IBM Cloud VPC.
 	IBMCloudServiceVPC IBMCloudServiceName = "VPC"
 )
+
+// IPFamilyType indicates whether the different IP address configurations supported like IPv4-only, dual-stack
+type IPFamilyType string


Add // +enum to allow the correct generation of openapi spec

This config is being added to be explicit about the IP address family for the Cluster so that we don't have to infer that from other configs that may not always be present. Updating PlatformStatus for AWS, Azure and GCP to enable dual stack installs on these platforms (initially in tech preview).

openshift-ci · 2025-08-14T06:18:42Z

@sadasu: The following tests 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/e2e-azure	`fb36988`	link	false	`/test e2e-azure`
ci/prow/verify	`fb36988`	link	true	`/test verify`
ci/prow/okd-scos-e2e-aws-ovn	`fb36988`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/e2e-aws-serial-techpreview-2of2	`fb36988`	link	true	`/test e2e-aws-serial-techpreview-2of2`
ci/prow/e2e-gcp	`fb36988`	link	false	`/test e2e-gcp`

Full PR test history. Your PR dashboard.

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.

JoelSpeed · 2025-08-14T11:33:35Z

config/v1/types_infrastructure.go

+	// +kubebuilder:default:="IPv4"
+	// +kubebuilder:validation:Enum="IPv4";"DualStack"
+	// +openshift:enable:FeatureGate=AWSDualStackInstall
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="ipFamily is immutable once set"


Do you have a test to show that if the value is DualStack, and then someone tries to remove it, that it doesn't default to IPv4?

JoelSpeed · 2025-08-14T11:34:06Z

config/v1/types_infrastructure.go

@@ -775,6 +801,19 @@ type GCPPlatformStatus struct {
 	// +optional
 	// +openshift:enable:FeatureGate=GCPCustomAPIEndpointsInstall
 	ServiceEndpoints []GCPServiceEndpoint `json:"serviceEndpoints,omitempty"`
+
+	// ipFamily indicates the IP Address family supported by cluster nodes.


Explain in the godoc that the value is set at installation time and it cannot be changed

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2025

openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 31, 2025

sadasu changed the title ~~Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support~~ CORS-4136, CORS-4157: Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support Jul 31, 2025

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 31, 2025

openshift-ci bot requested review from JoelSpeed and sinnykumari July 31, 2025 15:33

sadasu changed the title ~~CORS-4136, CORS-4157: Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support~~ [WIP] CORS-4136, CORS-4157: Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support Jul 31, 2025

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 31, 2025

sadasu force-pushed the aws-azure-gcp-dual-stack branch from ed2ad9d to 64aaf90 Compare July 31, 2025 15:57

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2025

patrickdillon reviewed Aug 6, 2025

View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2025

sadasu force-pushed the aws-azure-gcp-dual-stack branch from 64aaf90 to bf0c0db Compare August 6, 2025 21:24

sadasu changed the title ~~[WIP] CORS-4136, CORS-4157: Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support~~ CORS-4136, CORS-4157: Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support Aug 6, 2025

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 6, 2025

JoelSpeed reviewed Aug 7, 2025

View reviewed changes

sadasu force-pushed the aws-azure-gcp-dual-stack branch 2 times, most recently from 6652e10 to e7a5704 Compare August 8, 2025 19:17

sadasu mentioned this pull request Aug 11, 2025

CORS-4096: Add DNS Record Type for IPv6 Domain mapping #2361

Open

sadasu force-pushed the aws-azure-gcp-dual-stack branch 2 times, most recently from 7c13a6c to 0bcd978 Compare August 11, 2025 23:27

Add Featuregates for Dual Stack support on AWS, Azure and GCP

5e1eb05

sadasu force-pushed the aws-azure-gcp-dual-stack branch from 0bcd978 to 628d0cf Compare August 12, 2025 16:15

JoelSpeed reviewed Aug 12, 2025

View reviewed changes

sadasu force-pushed the aws-azure-gcp-dual-stack branch from 628d0cf to fb36988 Compare August 14, 2025 02:57

JoelSpeed reviewed Aug 14, 2025

View reviewed changes

	IpFamily IPFamiliesType `json:"ipFamily,omitempty"`
	IPFamily IPFamiliesType `json:"ipFamily,omitempty"`

		// +default="IPFamiliesIPv4"
		// +kubebuilder:default:="IPFamiliesIPv4"

CORS-4136, CORS-4157: Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support #2430

Are you sure you want to change the base?

CORS-4136, CORS-4157: Add Feature gates and update Infrastructure API for AWS, Azure and GCP Dual Stack support #2430

Uh oh!

Conversation

sadasu commented Jul 31, 2025

Uh oh!

openshift-ci bot commented Jul 31, 2025

Uh oh!

openshift-ci-robot commented Jul 31, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jul 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Aug 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-ci-robot commented Jul 31, 2025 •

edited by openshift-ci bot

Loading