CORS-4180: Allow AWS and Azure as platforms that support dual-stack on Day-0 #2804

sadasu · 2025-09-23T00:34:23Z

Added AWS and Azure as platforms that support DualStack on Day-0
Added AWS, Azure and OpenStack as platforms that do not support conversion to DualStack on Day-2.
Updated tests to reflect the newly supported platforms and when they support DualStack.
Currently DualStack for AWS and Azure are behind featuregates. Updated to check that these featuregates are enabled before DualStack support is extended to these platforms.

openshift-ci-robot · 2025-09-23T00:35:29Z

@sadasu: This pull request references CORS-4180 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.21.0" version, but no target version was set.

In response to this:

Added AWS and Azure as platforms that support DualStack on Day-0

Added AWS, Azure and OpenStack as platforms that do not support conversion to DualStack on Day-2.

Updated tests to reflect the newly supported platforms and when they support DualStack.

Developed with Cursor.

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 · 2025-09-23T00:35:40Z

@sadasu: This pull request references CORS-4180 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.21.0" version, but no target version was set.

In response to this:

Added AWS and Azure as platforms that support DualStack on Day-0

Added AWS, Azure and OpenStack as platforms that do not support conversion to DualStack on Day-2.

Updated tests to reflect the newly supported platforms and when they support DualStack.

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.

sadasu · 2025-09-23T12:39:06Z

/retest-required

patrickdillon · 2025-09-23T18:12:10Z

This looks good to me. AWS tests are failing to find a lease... i think we're hitting ci infra problems.

sadasu · 2025-09-24T03:23:03Z

/retest-required

openshift-ci-robot · 2025-10-03T14:04:35Z

@sadasu: This pull request references CORS-4180 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.21.0" version, but no target version was set.

In response to this:

Added AWS and Azure as platforms that support DualStack on Day-0

Added AWS, Azure and OpenStack as platforms that do not support conversion to DualStack on Day-2.

Updated tests to reflect the newly supported platforms and when they support DualStack.

Currently DualStack for AWS and Azure are behind featuregates. Updated to check that these featuregates are enabled before DualStack support is extended to these platforms.

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.

sadasu · 2025-10-08T15:02:22Z

/jira-refresh

sadasu · 2025-10-14T13:42:55Z

/retest-required

jhixson74 · 2025-10-15T20:38:08Z

/cc

jhixson74 · 2025-10-15T23:10:00Z

/retest-required

jhixson74 · 2025-10-16T03:31:07Z

/retest-required

tthvo · 2025-10-21T00:01:29Z

/retest

jhixson74 · 2025-10-23T17:13:53Z

/lgtm

tthvo · 2025-10-23T18:06:54Z

pkg/network/render.go

 		return nil, progressing, err
 	}

+	updateDualStackPlatforms(featureGates)


Should we move the function to earlier in the func Render so that any rendering, if any later on, can always pick up the most up-to-date list of supported dualstack platform?

pkg/network/render.go

tthvo · 2025-10-23T18:14:06Z

pkg/network/render.go

+var conversionToDualStackPlatforms = sets.NewString(
+	string(configv1.BareMetalPlatformType),
+	string(configv1.NonePlatformType),
+	string(configv1.VSpherePlatformType),
+)


question: Previously, only OpenStack is rejected. Just wondering if we confirmed this is OK?

The code comments seem to indicate so, but I have 0 clue 😞

This is the "allow" list so it contains the list of platforms that allow conversion to DualStack on Day-2. The code earlier was looking at OpenStack platform as the reject list. It could be implemented either ways.

Oh sounds good! I was just unsure if there are any other platforms that allow dualstack, which is not yet accounted for here...

tthvo · 2025-10-23T20:39:16Z

Maybe another rebase?

Bringing in the feature gates added for AWS and Azure DualStack support.

This is required to bring in the lastest featuregates

1. Added AWS and Azure as platforms that support DualStack on Day-0 2. Created another set of platforms that support DualStack on Day-2 3. Updated Render logic to take enabled featuregates into consideration to determine supported DualStack platforms 4. Updated tests to reflect the newly supported platforms and when they support DualStack.

tthvo

/lgtm

openshift-ci · 2025-10-24T01:38:07Z

@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/4.20-upgrade-from-stable-4.19-e2e-azure-ovn-upgrade	`9fbf4db`	link	false	`/test 4.20-upgrade-from-stable-4.19-e2e-azure-ovn-upgrade`
ci/prow/4.20-upgrade-from-stable-4.19-e2e-aws-ovn-upgrade	`9fbf4db`	link	false	`/test 4.20-upgrade-from-stable-4.19-e2e-aws-ovn-upgrade`
ci/prow/e2e-aws-hypershift-ovn-kubevirt	`9fbf4db`	link	false	`/test e2e-aws-hypershift-ovn-kubevirt`
ci/prow/security	`d95a14b`	link	false	`/test security`
ci/prow/4.21-upgrade-from-stable-4.20-e2e-gcp-ovn-upgrade	`d95a14b`	link	false	`/test 4.21-upgrade-from-stable-4.20-e2e-gcp-ovn-upgrade`
ci/prow/e2e-metal-ipi-ovn-dualstack-bgp	`d95a14b`	link	true	`/test e2e-metal-ipi-ovn-dualstack-bgp`
ci/prow/e2e-gcp-ovn	`d95a14b`	link	true	`/test e2e-gcp-ovn`
ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw	`d95a14b`	link	true	`/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw`
ci/prow/4.21-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade	`d95a14b`	link	false	`/test 4.21-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade`
ci/prow/okd-scos-e2e-aws-ovn	`d95a14b`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/4.21-upgrade-from-stable-4.20-e2e-azure-ovn-upgrade	`d95a14b`	link	false	`/test 4.21-upgrade-from-stable-4.20-e2e-azure-ovn-upgrade`

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.

tthvo · 2025-10-24T20:11:41Z

pkg/network/render.go

+	// Update the list of supported DualStack platforms based on enabled feature gates.
+	updateDualStackPlatforms(featureGates)
+


@sadasu this is added in the Render function is, I believe, is called when rendering manifests for bootstrap.

This means when the cluster-network-operator is running in the cluster, such function is not called, right? And the list is not modified at all?

/lgtm cancel

Pending question above 👀

I think we should have the function isSupportedDualStackPlatform keeps the list of supported platforms (i.e. both static and feature-gated items), instead of having a global variable and modifying them at runtime.

WDYT?

Whoops, nvmind... It is also called during reconcilation 😓 so its fine. Please ignore it :D

openshift-ci · 2025-10-24T20:15:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jhixson74, sadasu
Once this PR has been reviewed and has the lgtm label, please assign jacobtanenbaum 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

kyrtapz

Thanks @sadasu!
There are a few changes we need to make and I think we should be good.

kyrtapz · 2025-10-28T16:03:21Z

pkg/network/render.go

+// of platforms that support DualStack on Day-0. Currently the 2 platforms added here
+// donot support conversion to DualStack on Day-2. When that happens, we will need to
+// update `conversionToDualStackPlatforms` too.
+func updateDualStackPlatforms(featureGates featuregates.FeatureGate) {


I don't like the fact that we are modifying the global variable through this function. My proposal is to set the supported platforms as part of the bootstrap.InfraStatus and read it from there. Same applies to conversionToDualStackPlatforms.

@kyrtapz happy to make that change but I am not sure how future proof that is. As long as the dualStackPlatforms is in place, we run the risk of someone using that global instead of the InfraStatus.

With the change I proposed we should get rid of the global dualStackPlatforms variable.

kyrtapz · 2025-10-28T16:04:55Z

pkg/network/render.go

 	objs := []*uns.Unstructured{}

+	// Update the list of supported DualStack platforms based on enabled feature gates.
+	updateDualStackPlatforms(featureGates)


~~Calling it here would mean adding elements to dualStackPlatforms every time Render is called, we cannot do that.~~

Edit: this isn't a problem since we use a set. I still don't like the fact that it tries to modify the global variable so often.

kyrtapz · 2025-10-28T16:09:15Z

pkg/network/render.go

 			return errors.Errorf("%s is not one of the supported platforms for dual stack (%s)", infraRes.PlatformType,
 				strings.Join(dualStackPlatforms.List(), ", "))
-		} else if string(configv1.OpenStackPlatformType) == string(infraRes.PlatformType) {
+		} else if !isConversionSupportedDualStackPlatform(infraRes.PlatformType) {


I know you didn't introduce this but we can simplify the code and ditch the else if since the if returns.
something like this:

if !isSupportedDualStackPlatform(infraRes.PlatformType) { return ... } if !isConversionSupportedDualStackPlatform(infraRes.PlatformType) { return ... }

openshift-ci bot requested review from pperiyasamy and ricky-rav September 23, 2025 00:35

sadasu changed the title ~~Allow AWS and Azure as platforms that support dual-stack on Day-0~~ CORS-4180: Allow AWS and Azure as platforms that support dual-stack on Day-0 Sep 23, 2025

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

sadasu mentioned this pull request Sep 23, 2025

CORS-4180: Allow AWS, Azure and GCP as platforms that support dual-stack #2775

Closed

sadasu force-pushed the update-for-dualstack branch from 4696a48 to 621b294 Compare October 3, 2025 14:02

sadasu force-pushed the update-for-dualstack branch from 621b294 to 9fbf4db Compare October 3, 2025 14:12

openshift-ci bot requested a review from jhixson74 October 15, 2025 20:38

openshift-ci bot assigned jhixson74 Oct 23, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2025

tthvo reviewed Oct 23, 2025

View reviewed changes

pkg/network/render.go Outdated Show resolved Hide resolved

tthvo reviewed Oct 23, 2025

View reviewed changes

sadasu force-pushed the update-for-dualstack branch from 9fbf4db to 43ceb4d Compare October 23, 2025 20:29

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2025

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

sadasu added 3 commits October 23, 2025 16:40

Update openshift/api and openshift/client-go versions

dca3ee3

Bringing in the feature gates added for AWS and Azure DualStack support.

Update openshift/library-go version

d565d54

This is required to bring in the lastest featuregates

sadasu force-pushed the update-for-dualstack branch from 43ceb4d to d95a14b Compare October 23, 2025 21:08

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

tthvo reviewed Oct 23, 2025

View reviewed changes

openshift-ci bot assigned tthvo Oct 23, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2025

tthvo reviewed Oct 24, 2025

View reviewed changes

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2025

kyrtapz suggested changes Oct 28, 2025

View reviewed changes

tthvo mentioned this pull request Oct 31, 2025

CORS-4072: [Draft] Dual stack support for AWS openshift/installer#9930

Open

		// Update the list of supported DualStack platforms based on enabled feature gates.
		updateDualStackPlatforms(featureGates)

CORS-4180: Allow AWS and Azure as platforms that support dual-stack on Day-0 #2804

Are you sure you want to change the base?

CORS-4180: Allow AWS and Azure as platforms that support dual-stack on Day-0 #2804

Uh oh!

Conversation

sadasu commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Sep 23, 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-robot commented Sep 23, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sadasu commented Sep 23, 2025

Uh oh!

patrickdillon commented Sep 23, 2025

Uh oh!

sadasu commented Sep 24, 2025

Uh oh!

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

Uh oh!

Uh oh!

sadasu commented Oct 8, 2025

Uh oh!

sadasu commented Oct 14, 2025

Uh oh!

jhixson74 commented Oct 15, 2025

Uh oh!

jhixson74 commented Oct 15, 2025

Uh oh!

jhixson74 commented Oct 16, 2025

Uh oh!

tthvo commented Oct 21, 2025

Uh oh!

jhixson74 commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

tthvo commented Oct 23, 2025

Uh oh!

tthvo left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 24, 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!

openshift-ci bot commented Oct 24, 2025

Uh oh!

kyrtapz left a comment

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!

kyrtapz Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sadasu commented Sep 23, 2025 •

edited

Loading

openshift-ci-robot commented Sep 23, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Sep 23, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Oct 3, 2025 •

edited by openshift-ci bot

Loading

kyrtapz Oct 28, 2025 •

edited

Loading