Support DualStack(IPv4+IPv6) #848

mohamed-rafraf · 2024-09-16T13:32:04Z

How to categorize this PR?

/area networking
/kind api-change
/platform gcp

What this PR does / why we need it:
This PR introduces DualStack support for infrastructure objects. A new field DualStack { enabled: bool} has been added. When enabled, the controller will create a VPC and Subnet in GCP with IPv6 access type set to external, supporting both external and internal IPv6 communication.

The controller can provision dual-stack infrastructure using the InfraFlow approach (via GCP SDK).

apiVersion: extensions.gardener.cloud/v1alpha1
kind: Infrastructure
metadata:
  name: gcp-infra
  namespace: shoot--med--gcp
    annotations:
      provider.extensions.gardener.cloud/use-flow: "true"
spec:
  type: gcp
  region: europe-west1
  secretRef:
    namespace: shoot--med--gcp
    name: core-gcp
  providerConfig:
    apiVersion: gcp.provider.extensions.gardener.cloud/v1alpha1
    kind: InfrastructureConfig
    networks:
      dualStack: 
        enabled: true
      workers: 10.250.0.0/16

Which issue(s) this PR fixes:
Fixes #829

Special notes for your reviewer:

Release note:

NONE

CLAassistant · 2024-09-16T13:32:11Z

All committers have signed the CLA.

gardener-robot · 2024-09-16T13:32:30Z

@mohamed-rafraf Thank you for your contribution.

gardener-robot-ci-2 · 2024-09-16T13:32:41Z

Thank you @mohamed-rafraf for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

ScheererJ · 2024-09-19T07:29:49Z

Please remove the ULA part from the PR description and sign the contributors agreement.

After internal discussion, we would like the machine-controller-manager change (if required) in this PR as well. This pull should provide dual-stack VPC/subnet and enable VMs to also have IPv6 addresses if necessary.

ScheererJ · 2024-09-19T07:30:03Z

/ok-to-test

gardener-robot · 2024-10-04T15:04:01Z

@mohamed-rafraf You need rebase this pull request with latest master branch. Please check.

charts/internal/seed-controlplane/charts/ingress-gce/templates/service-account.yaml

test/integration/infrastructure/infrastructure_test.go

axel7born · 2024-12-04T12:04:52Z

/ok-to-test

DockToFuture · 2024-12-04T16:10:59Z

/ok-to-test

see gardener#848 Signed-off-by: Artiom Diomin <[email protected]>

hebelsan · 2024-12-12T11:04:41Z

charts/internal/seed-controlplane/charts/ingress-gce/templates/deployment.yaml

+  name: ingress-gce
+  namespace: {{ .Release.Namespace }}
+  annotations:
+    scheduler.alpha.kubernetes.io/critical-pod: ''


Is this annotation needed?

It's here from the upstream. I will replace it with priorityClassName: system-cluster-critical as such annotations don't work anymore.

hebelsan · 2024-12-12T14:09:29Z

imagevector/images.yaml

+      integrity_requirement: 'high'
+      availability_requirement: 'low'
+- name: default-http-backend
+  sourceRepository: github.com/kubernetes/ingress-gce


Suggested change

sourceRepository: github.com/kubernetes/ingress-gce

sourceRepository: https://github.com/gardener/ingress-default-backend

That's a different one, not related to GCP

hebelsan · 2024-12-12T14:17:33Z

pkg/apis/gcp/types_infrastructure.go

@@ -69,6 +72,8 @@ const (
 	PurposeNodes SubnetPurpose = "nodes"
 	// PurposeInternal is a SubnetPurpose for internal use.
 	PurposeInternal SubnetPurpose = "internal"
+	// PurposeServices is a SubnetPurpose for internal use.


Suggested change

// PurposeServices is a SubnetPurpose for internal use.

// PurposeServices is a SubnetPurpose for services.

hebelsan · 2024-12-12T14:18:09Z

pkg/apis/gcp/v1alpha1/types_infrastructure.go

@@ -74,6 +77,8 @@ const (
 	PurposeNodes SubnetPurpose = "nodes"
 	// PurposeInternal is a SubnetPurpose for internal use.
 	PurposeInternal SubnetPurpose = "internal"
+	// PurposeServices is a SubnetPurpose for internal use.


Suggested change

// PurposeServices is a SubnetPurpose for internal use.

// PurposeServices is a SubnetPurpose for services.

hebelsan · 2024-12-12T16:23:19Z

pkg/controller/infrastructure/flow_reconciler.go

+	},
+	)


Suggested change

},

)

})

hebelsan · 2024-12-12T16:31:27Z

pkg/controller/infrastructure/infraflow/ensure.go

-		region = fctx.infra.Spec.Region
-	)
+func (fctx *FlowContext) ensureIPv6CIDRs(ctx context.Context) error {
+	nodeSubnet := fctx.whiteboard.GetObject(ObjectKeyNodeSubnet).(*compute.Subnetwork)


Consider using: GetObject[*compute.Subnetwork](fctx.whiteboard, ObjectKeyNodeSubnet)

Suggested change

nodeSubnet := fctx.whiteboard.GetObject(ObjectKeyNodeSubnet).(*compute.Subnetwork)

nodeSubnet := GetObject[*compute.Subnetwork](fctx.whiteboard, ObjectKeyNodeSubnet)

GetObject() checks if object exists and return new(T) in case if object is not found, the result will never be nil. But I need nil to check for the error case.

hebelsan · 2024-12-12T16:32:38Z

pkg/controller/infrastructure/infraflow/ensure.go

+	}
+	fctx.whiteboard.Set(NodesSubnetIPv6CIDR, nodesIPv6Range)
+
+	srvSubnet := fctx.whiteboard.GetObject(ObjectKeyServicesSubnet).(*compute.Subnetwork)


Again consider using: GetObject

Suggested change

srvSubnet := fctx.whiteboard.GetObject(ObjectKeyServicesSubnet).(*compute.Subnetwork)

srvSubnet := GetObject[*compute.Subnetwork](fctx.whiteboard, ObjectKeyServicesSubnet)

hebelsan · 2024-12-12T16:43:26Z

pkg/controller/infrastructure/infraflow/ensure.go

+	cidrsipv6 := []*string{}
+	if nodesipv6 := fctx.whiteboard.Get(NodesSubnetIPv6CIDR); ptr.Deref(nodesipv6, "") != "" {
+		cidrsipv6 = append(cidrsipv6, nodesipv6)
+	}
+	if servicesipv6 := fctx.whiteboard.Get(ServicesSubnetIPv6CIDR); ptr.Deref(servicesipv6, "") != "" {
+		cidrsipv6 = append(cidrsipv6, servicesipv6)
+	}


To stays consistent we should use camel case variable naming in cidrsipv6 = cidrsIPv6, nodesipv6 = nodesIPv6 and servicesipv6 = servicesIPv6

hebelsan · 2024-12-12T16:47:38Z

pkg/gcp/types.go

@@ -14,6 +14,10 @@ const (

 	// CloudControllerManagerImageName is the name of the cloud-controller-manager image.
 	CloudControllerManagerImageName = "cloud-controller-manager"
+	// IngressGCEImageName is the name of the ingress-gce image.
+	IngressGCEImageName = "ingress-gce"
+	// DefaultHTTPBackendImageName is the name of the csi-driver image.


Probably copy paste error?

see gardener#848 Signed-off-by: Artiom Diomin <[email protected]>

kron4eg · 2024-12-13T09:57:59Z

I've rebased whole PR against current master.

pkg/apis/gcp/types_infrastructure.go

pkg/controller/infrastructure/infraflow/shared/firewall.go

pkg/controller/infrastructure/infraflow/reconciler.go

pkg/controller/worker/machines.go

charts/internal/seed-controlplane/charts/ingress-gce/values.yaml

kon-angelo · 2025-01-27T14:31:50Z

pkg/controller/controlplane/valuesprovider.go

+	subnetPurpose := apisgcp.PurposeInternal
+	if dualstack {
+		subnetPurpose = apisgcp.PurposeNodes
+	}


I think this should be validated against right? Having an internal subnet on a dualStack cluster makes no sense AFAIK because the internal loadbalancers will now be created now in the internal subnet.

I think there is no validation for that yet.

pkg/controller/infrastructure/infraflow/graph.go

mohamed-rafraf requested review from a team as code owners September 16, 2024 13:32

gardener-robot added the needs/review Needs review label Sep 16, 2024

gardener-robot added kind/api-change API change with impact on API users needs/second-opinion Needs second review by someone else area/networking Networking related platform/gcp Google cloud platform/infrastructure labels Sep 16, 2024

gardener-robot added the size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) label Sep 16, 2024

gardener-robot added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 19, 2024

gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 19, 2024

gardener-robot added size/l Size of pull request is large (see gardener-robot robot/bots/size.py) and removed size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Sep 20, 2024

gardener-robot added the needs/rebase Needs git rebase label Oct 4, 2024

ScheererJ mentioned this pull request Oct 10, 2024

Support IPv6 gardener/gardener#8899

Open

60 tasks

mohamed-rafraf force-pushed the dual-stack branch from 586844b to 748b10f Compare October 16, 2024 08:24

mohamed-rafraf marked this pull request as draft October 18, 2024 10:32

gardener-robot added size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) and removed size/l Size of pull request is large (see gardener-robot robot/bots/size.py) labels Nov 5, 2024

kron4eg force-pushed the dual-stack branch from 7241993 to a4564cd Compare November 5, 2024 22:26

kron4eg force-pushed the dual-stack branch from 5b5e4c4 to 2ff5081 Compare November 20, 2024 13:55

kron4eg mentioned this pull request Nov 22, 2024

WIP Ingress-gce helm chart #852

Closed

mohamed-rafraf changed the title ~~Support DualStack(IPv4+IPv6) for Infrastructure~~ Support DualStack(IPv4+IPv6) Nov 22, 2024

axel7born reviewed Nov 25, 2024

View reviewed changes

charts/internal/seed-controlplane/charts/ingress-gce/templates/service-account.yaml Outdated Show resolved Hide resolved

kron4eg force-pushed the dual-stack branch 2 times, most recently from 29014f7 to 00ee2a1 Compare November 29, 2024 15:05

axel7born suggested changes Nov 29, 2024

View reviewed changes

test/integration/infrastructure/infrastructure_test.go Outdated Show resolved Hide resolved

test/integration/infrastructure/infrastructure_test.go Outdated Show resolved Hide resolved

test/integration/infrastructure/infrastructure_test.go Show resolved Hide resolved

kron4eg force-pushed the dual-stack branch from 00ee2a1 to ec9fff7 Compare November 29, 2024 15:06

mohamed-rafraf marked this pull request as ready for review December 4, 2024 09:45

gardener-robot added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 4, 2024

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 4, 2024

gardener-robot added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 4, 2024

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 4, 2024

mohamed-rafraf marked this pull request as draft December 4, 2024 22:23

kron4eg force-pushed the dual-stack branch from e54b9ec to b76ba5a Compare December 9, 2024 15:18

kron4eg added a commit to mohamed-rafraf/gardener-extension-provider-gcp that referenced this pull request Dec 9, 2024

New values for machineclass

a305a3f

see gardener#848 Signed-off-by: Artiom Diomin <[email protected]>

mohamed-rafraf marked this pull request as ready for review December 11, 2024 12:26

hebelsan reviewed Dec 12, 2024

View reviewed changes

kron4eg added a commit to mohamed-rafraf/gardener-extension-provider-gcp that referenced this pull request Dec 13, 2024

New values for machineclass

28ec40b

see gardener#848 Signed-off-by: Artiom Diomin <[email protected]>

kron4eg force-pushed the dual-stack branch from 9efa122 to 942e1e4 Compare December 13, 2024 09:55

add dual-stack support for gcp

4a20257

kon-angelo reviewed Jan 27, 2025

View reviewed changes

DockToFuture force-pushed the dual-stack branch from 942e1e4 to 4a20257 Compare January 30, 2025 12:12

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 30, 2025

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 30, 2025

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 3, 2025

address pr review

26c78cf

DockToFuture force-pushed the dual-stack branch from 63a1dd4 to 26c78cf Compare February 4, 2025 13:52

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support DualStack(IPv4+IPv6) #848

Support DualStack(IPv4+IPv6) #848

mohamed-rafraf commented Sep 16, 2024 •

edited

Loading

CLAassistant commented Sep 16, 2024 •

edited

Loading

gardener-robot commented Sep 16, 2024

gardener-robot-ci-2 commented Sep 16, 2024

ScheererJ commented Sep 19, 2024

ScheererJ commented Sep 19, 2024

gardener-robot commented Oct 4, 2024

axel7born commented Dec 4, 2024

DockToFuture commented Dec 4, 2024

hebelsan Dec 12, 2024

kron4eg Dec 13, 2024

hebelsan Dec 12, 2024

kron4eg Dec 13, 2024

hebelsan Dec 12, 2024

hebelsan Dec 12, 2024

hebelsan Dec 12, 2024

hebelsan Dec 12, 2024

kron4eg Dec 13, 2024

hebelsan Dec 12, 2024

hebelsan Dec 12, 2024

hebelsan Dec 12, 2024

kron4eg commented Dec 13, 2024

kon-angelo Jan 27, 2025

	sourceRepository: github.com/kubernetes/ingress-gce
	sourceRepository: https://github.com/gardener/ingress-default-backend

	// PurposeServices is a SubnetPurpose for internal use.
	// PurposeServices is a SubnetPurpose for services.

	nodeSubnet := fctx.whiteboard.GetObject(ObjectKeyNodeSubnet).(*compute.Subnetwork)
	nodeSubnet := GetObject[*compute.Subnetwork](fctx.whiteboard, ObjectKeyNodeSubnet)

	srvSubnet := fctx.whiteboard.GetObject(ObjectKeyServicesSubnet).(*compute.Subnetwork)
	srvSubnet := GetObject[*compute.Subnetwork](fctx.whiteboard, ObjectKeyServicesSubnet)

Support DualStack(IPv4+IPv6) #848

Are you sure you want to change the base?

Support DualStack(IPv4+IPv6) #848

Conversation

mohamed-rafraf commented Sep 16, 2024 • edited Loading

CLAassistant commented Sep 16, 2024 • edited Loading

gardener-robot commented Sep 16, 2024

gardener-robot-ci-2 commented Sep 16, 2024

ScheererJ commented Sep 19, 2024

ScheererJ commented Sep 19, 2024

gardener-robot commented Oct 4, 2024

axel7born commented Dec 4, 2024

DockToFuture commented Dec 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kron4eg commented Dec 13, 2024

Choose a reason for hiding this comment

mohamed-rafraf commented Sep 16, 2024 •

edited

Loading

CLAassistant commented Sep 16, 2024 •

edited

Loading