Address review comments for dual-stack IPv4/IPv6 support

- Remove deprecated subnetID field from machine class spec and template;
  subnetIDs already contains the same information
- Use core.IsDualStack() instead of !core.IsIPv4SingleStack() for clarity
- Move isDualStack check to flow graph level via shared.DoIf() for all
  IPv6 tasks; remove redundant early-return guards from task functions
- Refactor PatchProviderStatusAndState: extract
  computeInfrastructureNetworkingStatus() on FlowContext to keep the
  function signature clean
- Clarify docs: Option 2 (explicit IPv6 CIDRs) does not require a
  subnetPoolID; if both are set, explicit CIDRs take precedence

Author: axel7born

charts/internal/machineclass/templates/machineclass.yaml

@@ -56,7 +56,6 @@ providerSpec:
     imageName: {{ $machineClass.imageName }}
 {{- end }}
     networkID: {{ $machineClass.networkID }}
-    subnetID: {{ $machineClass.subnetID }}
     subnetIDs:
 {{ toYaml $machineClass.subnetIDs | indent 6 }}
     podNetworkCIDRs:

docs/usage/usage.md

@@ -110,7 +110,9 @@ Unlike IPv4 where you explicitly specify the CIDR in `networks.workers`, IPv6 ad
 
 #### Option 2: Explicit IPv6 CIDR Configuration
 
-Alternatively, you can explicitly specify IPv6 CIDRs for nodes, pods, and services using the `networks.ipv6` field:
+Alternatively, you can explicitly specify IPv6 CIDRs for nodes, pods, and services using the `networks.ipv6` field.
+In this case, IPv6 subnets are created without a subnet pool (using the provided CIDRs directly), so no `subnetPoolID` is required.
+If a `subnetPoolID` is also set, the explicit CIDRs from `networks.ipv6` take precedence and the pool is not used for CIDR allocation.
 
 ```yaml
 apiVersion: openstack.provider.extensions.gardener.cloud/v1alpha1

pkg/admission/validator/shoot.go

@@ -179,8 +179,9 @@ func (s *shoot) validateShoot(ctx context.Context, context *validationContext) f
 		allErrs = append(allErrs, openstackvalidation.ValidateNetworking(context.shoot.Spec.Networking, nwPath)...)
 		allErrs = append(allErrs, openstackvalidation.ValidateInfrastructureConfig(context.infraConfig, context.shoot.Spec.Networking.Nodes, infraConfigPath)...)
 
-		// Validate that either SubnetPoolID or IPv6 config is set for dual-stack shoots
-		if !core.IsIPv4SingleStack(context.shoot.Spec.Networking.IPFamilies) {
+		// Validate that either SubnetPoolID or IPv6 config is set for dual-stack shoots.
+		// It is valid to set both; in that case the explicit networks.ipv6 CIDRs take precedence.
+		if core.IsDualStack(context.shoot.Spec.Networking.IPFamilies) {
 			if context.infraConfig.SubnetPoolID == nil && context.infraConfig.Networks.IPv6 == nil {
 				allErrs = append(allErrs, field.Required(infraConfigPath, "either subnetPoolID or networks.ipv6 must be set for dual-stack shoots"))
 			}

pkg/controller/infrastructure/actuator_reconcile.go

@@ -102,5 +102,5 @@ func (a *actuator) migrateFromTerraform(ctx context.Context, log logr.Logger, in
 	// we mark that there are infra resources created.
 	state.Data[infraflow.CreatedResourcesExistKey] = "true"
 
-	return state, infraflow.PatchProviderStatusAndState(ctx, a.client, infra, nil, nil, &runtime.RawExtension{Object: state}, nil, nil, nil)
+	return state, infraflow.PatchProviderStatusAndState(ctx, a.client, infra, nil, &runtime.RawExtension{Object: state}, nil)
 }

pkg/controller/infrastructure/infraflow/context.go

@@ -156,50 +156,25 @@ func NewFlowContext(opts Opts) (*FlowContext, error) {
 }
 
 func (fctx *FlowContext) persistState(ctx context.Context) error {
-	return PatchProviderStatusAndState(ctx, fctx.client, fctx.infra, fctx.shootNetworking, nil, fctx.computeInfrastructureState(), nil, nil, nil)
+	return PatchProviderStatusAndState(ctx, fctx.client, fctx.infra, nil, fctx.computeInfrastructureState(), nil)
 }
 
 // PatchProviderStatusAndState patches the infrastructure status with the given provider specific status and state.
 func PatchProviderStatusAndState(
 	ctx context.Context,
 	runtimeClient client.Client,
 	infra *extensionsv1alpha1.Infrastructure,
-	networking *gardencorev1beta1.Networking,
 	status *openstackv1alpha1.InfrastructureStatus,
 	state *runtime.RawExtension,
-	nodesSubnetIPv6CIDR *string,
-	podSubnetIPv6CIDR *string,
-	servicesSubnetIPv6CIDR *string,
+	networkingStatus *extensionsv1alpha1.InfrastructureStatusNetworking,
 ) error {
 	patch := client.MergeFrom(infra.DeepCopy())
 	if status != nil {
 		infra.Status.ProviderStatus = &runtime.RawExtension{Object: status}
 		infra.Status.EgressCIDRs = ComputeEgressCIDRs(status.Networks.Router.ExternalFixedIPs)
 	}
 
-	infra.Status.Networking = &extensionsv1alpha1.InfrastructureStatusNetworking{}
-
-	if networking != nil {
-		if networking.Nodes != nil {
-			infra.Status.Networking.Nodes = append(infra.Status.Networking.Nodes, *networking.Nodes)
-		}
-		if networking.Pods != nil {
-			infra.Status.Networking.Pods = append(infra.Status.Networking.Pods, *networking.Pods)
-		}
-		if networking.Services != nil {
-			infra.Status.Networking.Services = append(infra.Status.Networking.Services, *networking.Services)
-		}
-	}
-
-	if nodesSubnetIPv6CIDR != nil {
-		infra.Status.Networking.Nodes = append(infra.Status.Networking.Nodes, *nodesSubnetIPv6CIDR)
-	}
-	if podSubnetIPv6CIDR != nil {
-		infra.Status.Networking.Pods = append(infra.Status.Networking.Pods, *podSubnetIPv6CIDR)
-	}
-	if servicesSubnetIPv6CIDR != nil {
-		infra.Status.Networking.Services = append(infra.Status.Networking.Services, *servicesSubnetIPv6CIDR)
-	}
+	infra.Status.Networking = networkingStatus
 
 	if state != nil {
 		infra.Status.State = state
@@ -215,6 +190,36 @@ func PatchProviderStatusAndState(
 	return runtimeClient.Status().Patch(ctx, infra, patch)
 }
 
+// computeInfrastructureNetworkingStatus builds the InfrastructureStatusNetworking from the shoot networking
+// spec and any IPv6 CIDRs allocated during reconciliation.
+func (fctx *FlowContext) computeInfrastructureNetworkingStatus() *extensionsv1alpha1.InfrastructureStatusNetworking {
+	networking := &extensionsv1alpha1.InfrastructureStatusNetworking{}
+
+	if fctx.shootNetworking != nil {
+		if fctx.shootNetworking.Nodes != nil {
+			networking.Nodes = append(networking.Nodes, *fctx.shootNetworking.Nodes)
+		}
+		if fctx.shootNetworking.Pods != nil {
+			networking.Pods = append(networking.Pods, *fctx.shootNetworking.Pods)
+		}
+		if fctx.shootNetworking.Services != nil {
+			networking.Services = append(networking.Services, *fctx.shootNetworking.Services)
+		}
+	}
+
+	if nodeCIDR := fctx.state.Get(IdentifierNodeSubnetIPv6CIDR); nodeCIDR != nil {
+		networking.Nodes = append(networking.Nodes, *nodeCIDR)
+	}
+	if podCIDR := fctx.state.Get(IdentifierPodSubnetIPv6CIDR); podCIDR != nil {
+		networking.Pods = append(networking.Pods, *podCIDR)
+	}
+	if svcCIDR := fctx.state.Get(IdentifierServiceSubnetIPv6CIDR); svcCIDR != nil {
+		networking.Services = append(networking.Services, *svcCIDR)
+	}
+
+	return networking
+}
+
 func (fctx *FlowContext) computeInfrastructureState() *runtime.RawExtension {
 	return &runtime.RawExtension{
 		Object: &openstackv1alpha1.InfrastructureState{

pkg/controller/infrastructure/infraflow/reconcile.go

@@ -45,11 +45,8 @@ func (fctx *FlowContext) Reconcile(ctx context.Context) error {
 
 	state := fctx.computeInfrastructureState()
 	status := fctx.computeInfrastructureStatus()
-	nodeCIDR := fctx.state.Get(IdentifierNodeSubnetIPv6CIDR)
-	podCIDR := fctx.state.Get(IdentifierPodSubnetIPv6CIDR)
-	svcCIDR := fctx.state.Get(IdentifierServiceSubnetIPv6CIDR)
-	return PatchProviderStatusAndState(ctx, fctx.client, fctx.infra, fctx.shootNetworking, status, state, nodeCIDR, podCIDR, svcCIDR)
-	//return PatchProviderStatusAndState(ctx, fctx.client, fctx.infra, fctx.shootNetworking, status, state, nil, nil, nil)
+	networkingStatus := fctx.computeInfrastructureNetworkingStatus()
+	return PatchProviderStatusAndState(ctx, fctx.client, fctx.infra, status, state, networkingStatus)
 }
 
 func (fctx *FlowContext) buildReconcileGraph() *flow.Graph {
@@ -79,20 +76,20 @@ func (fctx *FlowContext) buildReconcileGraph() *flow.Graph {
 
 	ensureSubnetIPv6 := fctx.AddTask(g, "ensure IPv6 subnet",
 		fctx.ensureSubnetIPv6,
-		shared.Timeout(defaultTimeout), shared.Dependencies(ensureNetwork))
+		shared.Timeout(defaultTimeout), shared.Dependencies(ensureNetwork), shared.DoIf(gardencorev1beta1.IsDualStack(fctx.shootNetworking.IPFamilies)))
 
 	_ = fctx.AddTask(g, "ensure router interface",
 		fctx.ensureRouterInterface,
 		shared.Timeout(defaultTimeout), shared.Dependencies(ensureRouter, ensureSubnet))
 
 	_ = fctx.AddTask(g, "ensure IPv6 router interface",
 		fctx.ensureRouterInterfaceIPv6,
-		shared.Timeout(defaultTimeout), shared.Dependencies(ensureRouter, ensureSubnetIPv6))
+		shared.Timeout(defaultTimeout), shared.Dependencies(ensureRouter, ensureSubnetIPv6), shared.DoIf(gardencorev1beta1.IsDualStack(fctx.shootNetworking.IPFamilies)))
 
 	_ = fctx.AddTask(g, "ensure IPv6 CIDR services", fctx.ensureIPv6CIDRs,
 		shared.Timeout(defaultTimeout),
 		shared.Dependencies(ensureSubnetIPv6),
-		shared.DoIf(fctx.isDualStack()),
+		shared.DoIf(gardencorev1beta1.IsDualStack(fctx.shootNetworking.IPFamilies)),
 	)
 
 	ensureSecGroup := fctx.AddTask(g, "ensure security group",
@@ -337,9 +334,6 @@ func (fctx *FlowContext) ensureSubnet(ctx context.Context) error {
 func (fctx *FlowContext) ensureSubnetIPv6(ctx context.Context) error {
 	log := shared.LogFromContext(ctx)
 
-	if !fctx.isDualStack() {
-		return nil
-	}
 	if fctx.state.Get(IdentifierNetwork) == nil {
 		return fmt.Errorf("missing cluster network ID")
 	}
@@ -415,10 +409,6 @@ func (fctx *FlowContext) ensureSubnetIPv6(ctx context.Context) error {
 func (fctx *FlowContext) ensureIPv6CIDRs(ctx context.Context) error {
 	log := shared.LogFromContext(ctx)
 
-	if !fctx.isDualStack() {
-		return nil
-	}
-
 	// If explicit IPv6 CIDRs are configured, use them directly
 	if fctx.hasExplicitIPv6Config() {
 		nodeCIDR := fctx.config.Networks.IPv6.NodeCIDR
@@ -559,9 +549,6 @@ func (fctx *FlowContext) ensureRouterInterface(ctx context.Context) error {
 
 func (fctx *FlowContext) ensureRouterInterfaceIPv6(ctx context.Context) error {
 	log := shared.LogFromContext(ctx)
-	if !fctx.isDualStack() {
-		return nil
-	}
 	routerID := fctx.state.Get(IdentifierRouter)
 	if routerID == nil {
 		return fmt.Errorf("internal error: missing routerID")
@@ -660,7 +647,7 @@ func (fctx *FlowContext) ensureSecGroupRules(ctx context.Context) error {
 		},
 	}
 
-	if fctx.isDualStack() {
+	if gardencorev1beta1.IsDualStack(fctx.shootNetworking.IPFamilies) {
 		desiredRules = append(desiredRules, []rules.SecGroupRule{
 			{
 				Direction:     string(rules.DirIngress),

pkg/controller/infrastructure/infraflow/utils.go

@@ -13,7 +13,6 @@ import (
 	"sync"
 	"time"
 
-	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	"github.com/go-logr/logr"
 	"github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/loadbalancers"
 	"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/layer3/routers"
@@ -213,12 +212,6 @@ func (fctx *FlowContext) workersCIDR() string {
 	return fctx.config.WorkersCIDR()
 }
 
-// isDualStack returns true if the cluster is configured for dual-stack networking
-// or has migrated from dual-stack to single-stack (indicated by having 2 node CIDRs).
-func (fctx *FlowContext) isDualStack() bool {
-	return !gardencorev1beta1.IsIPv4SingleStack(fctx.shootNetworking.IPFamilies)
-}
-
 // hasExplicitIPv6Config returns true if the IPv6 configuration with explicit CIDRs is set
 func (fctx *FlowContext) hasExplicitIPv6Config() bool {
 	return fctx.config.Networks.IPv6 != nil

pkg/controller/worker/machines.go

@@ -207,7 +207,6 @@ func (w *WorkerDelegate) generateMachineConfig(ctx context.Context) error {
 				subnetIDs = append(subnetIDs, subnet.ID)
 			}
 
-			machineClassSpec["subnetID"] = subnets[0].ID
 			machineClassSpec["subnetIDs"] = subnetIDs
 
 			if volumeSize > 0 {

pkg/controller/worker/machines_test.go

@@ -582,7 +582,6 @@ var _ = Describe("Machines", func() {
 						"region":          region,
 						"keyName":         keyName,
 						"networkID":       networkID,
-						"subnetID":        subnetID,
 						"podNetworkCIDRs": []string{podCIDR},
 						"securityGroups":  []string{securityGroupName},
 						"subnetIDs":       []string{subnetID},