feat: create vpc objects in explicitly provided availability zones

Léonard Suslian · Léonard Suslian · commit f0fbb06b331b · 2024-08-06T13:21:49.000+02:00
diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go
@@ -85,6 +85,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {
 	}
 
 	dst.Spec.NetworkSpec.AdditionalControlPlaneIngressRules = restored.Spec.NetworkSpec.AdditionalControlPlaneIngressRules
+	dst.Spec.NetworkSpec.VPC.AvailabilityZones = restored.Spec.NetworkSpec.VPC.AvailabilityZones
 
 	if restored.Spec.NetworkSpec.VPC.IPAMPool != nil {
 		if dst.Spec.NetworkSpec.VPC.IPAMPool == nil {
diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go
diff --git a/api/v1beta2/awscluster_webhook.go b/api/v1beta2/awscluster_webhook.go
@@ -291,6 +291,10 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
 		}
 	}
 
+	if err := r.Spec.NetworkSpec.VPC.ValidateAvailabilityZones(); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
 	return allErrs
 }
 
diff --git a/api/v1beta2/defaults.go b/api/v1beta2/defaults.go
@@ -18,6 +18,7 @@ package v1beta2
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 
 	clusterv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 )
@@ -51,6 +52,15 @@ func SetDefaults_NetworkSpec(obj *NetworkSpec) { //nolint:golint,stylecheck
 			},
 		}
 	}
+	// If AvailabilityZones are not set, set defaults for AZ selection
+	if obj.VPC.AvailabilityZones == nil {
+		if obj.VPC.AvailabilityZoneUsageLimit == nil {
+			obj.VPC.AvailabilityZoneUsageLimit = ptr.To(3)
+		}
+		if obj.VPC.AvailabilityZoneSelection == nil {
+			obj.VPC.AvailabilityZoneSelection = &AZSelectionSchemeOrdered
+		}
+	}
 }
 
 // SetDefaults_AWSClusterSpec is used by defaulter-gen.
diff --git a/api/v1beta2/network_types.go b/api/v1beta2/network_types.go
@@ -23,6 +23,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/utils/ptr"
 )
 
@@ -437,7 +438,7 @@ type VPCSpec struct {
 	// should be used in a region when automatically creating subnets. If a region has more
 	// than this number of AZs then this number of AZs will be picked randomly when creating
 	// default subnets. Defaults to 3
-	// +kubebuilder:default=3
+	// +optional
 	// +kubebuilder:validation:Minimum=1
 	AvailabilityZoneUsageLimit *int `json:"availabilityZoneUsageLimit,omitempty"`
 
@@ -446,10 +447,16 @@ type VPCSpec struct {
 	// Ordered - selects based on alphabetical order
 	// Random - selects AZs randomly in a region
 	// Defaults to Ordered
-	// +kubebuilder:default=Ordered
+	// +optional
 	// +kubebuilder:validation:Enum=Ordered;Random
 	AvailabilityZoneSelection *AZSelectionScheme `json:"availabilityZoneSelection,omitempty"`
 
+	// AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+	// Cannot be defined at the same time as AvailabilityZoneSelection and AvailabilityZoneUsageLimit.
+	// +optional
+	// +kubebuilder:validation:MinItems=1
+	AvailabilityZones []string `json:"availabilityZones,omitempty"`
+
 	// EmptyRoutesDefaultVPCSecurityGroup specifies whether the default VPC security group ingress
 	// and egress rules should be removed.
 	//
@@ -522,6 +529,15 @@ func (v *VPCSpec) GetPublicIpv4Pool() *string {
 	return nil
 }
 
+// ValidateAvailabilityZones returns an error if the availability zones field combination is invalid.
+func (v *VPCSpec) ValidateAvailabilityZones() *field.Error {
+	if len(v.AvailabilityZones) > 0 && (v.AvailabilityZoneSelection != nil || v.AvailabilityZoneUsageLimit != nil) {
+		availabilityZonesField := field.NewPath("spec", "network", "vpc", "availabilityZones")
+		return field.Invalid(availabilityZonesField, v.AvailabilityZoneSelection, "availabilityZones cannot be set if availabilityZoneUsageLimit and availabilityZoneSelection are set")
+	}
+	return nil
+}
+
 // SubnetSpec configures an AWS Subnet.
 type SubnetSpec struct {
 	// ID defines a unique identifier to reference this resource.
diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml
@@ -598,7 +598,6 @@ spec:
                     description: VPC configuration.
                     properties:
                       availabilityZoneSelection:
-                        default: Ordered
                         description: |-
                           AvailabilityZoneSelection specifies how AZs should be selected if there are more AZs
                           in a region than specified by AvailabilityZoneUsageLimit. There are 2 selection schemes:
@@ -610,14 +609,21 @@ spec:
                         - Random
                         type: string
                       availabilityZoneUsageLimit:
-                        default: 3
                         description: |-
                           AvailabilityZoneUsageLimit specifies the maximum number of availability zones (AZ) that
                           should be used in a region when automatically creating subnets. If a region has more
                           than this number of AZs then this number of AZs will be picked randomly when creating
                           default subnets. Defaults to 3
                         minimum: 1
                         type: integer
+                      availabilityZones:
+                        description: |-
+                          AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+                          Cannot be defined at the same time as AvailabilityZoneSelection and AvailabilityZoneUsageLimit.
+                        items:
+                          type: string
+                        minItems: 1
+                        type: array
                       carrierGatewayId:
                         description: |-
                           CarrierGatewayID is the id of the internet gateway associated with the VPC,
@@ -2624,7 +2630,6 @@ spec:
                     description: VPC configuration.
                     properties:
                       availabilityZoneSelection:
-                        default: Ordered
                         description: |-
                           AvailabilityZoneSelection specifies how AZs should be selected if there are more AZs
                           in a region than specified by AvailabilityZoneUsageLimit. There are 2 selection schemes:
@@ -2636,14 +2641,21 @@ spec:
                         - Random
                         type: string
                       availabilityZoneUsageLimit:
-                        default: 3
                         description: |-
                           AvailabilityZoneUsageLimit specifies the maximum number of availability zones (AZ) that
                           should be used in a region when automatically creating subnets. If a region has more
                           than this number of AZs then this number of AZs will be picked randomly when creating
                           default subnets. Defaults to 3
                         minimum: 1
                         type: integer
+                      availabilityZones:
+                        description: |-
+                          AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+                          Cannot be defined at the same time as AvailabilityZoneSelection and AvailabilityZoneUsageLimit.
+                        items:
+                          type: string
+                        minItems: 1
+                        type: array
                       carrierGatewayId:
                         description: |-
                           CarrierGatewayID is the id of the internet gateway associated with the VPC,
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -1538,7 +1538,6 @@ spec:
                     description: VPC configuration.
                     properties:
                       availabilityZoneSelection:
-                        default: Ordered
                         description: |-
                           AvailabilityZoneSelection specifies how AZs should be selected if there are more AZs
                           in a region than specified by AvailabilityZoneUsageLimit. There are 2 selection schemes:
@@ -1550,14 +1549,21 @@ spec:
                         - Random
                         type: string
                       availabilityZoneUsageLimit:
-                        default: 3
                         description: |-
                           AvailabilityZoneUsageLimit specifies the maximum number of availability zones (AZ) that
                           should be used in a region when automatically creating subnets. If a region has more
                           than this number of AZs then this number of AZs will be picked randomly when creating
                           default subnets. Defaults to 3
                         minimum: 1
                         type: integer
+                      availabilityZones:
+                        description: |-
+                          AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+                          Cannot be defined at the same time as AvailabilityZoneSelection and AvailabilityZoneUsageLimit.
+                        items:
+                          type: string
+                        minItems: 1
+                        type: array
                       carrierGatewayId:
                         description: |-
                           CarrierGatewayID is the id of the internet gateway associated with the VPC,
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml
@@ -1136,7 +1136,6 @@ spec:
                             description: VPC configuration.
                             properties:
                               availabilityZoneSelection:
-                                default: Ordered
                                 description: |-
                                   AvailabilityZoneSelection specifies how AZs should be selected if there are more AZs
                                   in a region than specified by AvailabilityZoneUsageLimit. There are 2 selection schemes:
@@ -1148,14 +1147,21 @@ spec:
                                 - Random
                                 type: string
                               availabilityZoneUsageLimit:
-                                default: 3
                                 description: |-
                                   AvailabilityZoneUsageLimit specifies the maximum number of availability zones (AZ) that
                                   should be used in a region when automatically creating subnets. If a region has more
                                   than this number of AZs then this number of AZs will be picked randomly when creating
                                   default subnets. Defaults to 3
                                 minimum: 1
                                 type: integer
+                              availabilityZones:
+                                description: |-
+                                  AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+                                  Cannot be defined at the same time as AvailabilityZoneSelection and AvailabilityZoneUsageLimit.
+                                items:
+                                  type: string
+                                minItems: 1
+                                type: array
                               carrierGatewayId:
                                 description: |-
                                   CarrierGatewayID is the id of the internet gateway associated with the VPC,
diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go
@@ -27,6 +27,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apimachinery/pkg/util/version"
 	"k8s.io/klog/v2"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -474,6 +475,10 @@ func (r *AWSManagedControlPlane) validateNetwork() field.ErrorList {
 		allErrs = append(allErrs, field.Invalid(ipamPoolField, r.Spec.NetworkSpec.VPC.IPv6.IPAMPool, "ipamPool must have either id or name"))
 	}
 
+	if err := r.Spec.NetworkSpec.VPC.ValidateAvailabilityZones(); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
 	return allErrs
 }
 
@@ -500,6 +505,16 @@ func (r *AWSManagedControlPlane) Default() {
 		}
 	}
 
+	// If AvailabilityZones are not set, set defaults for AZ selection
+	if r.Spec.NetworkSpec.VPC.AvailabilityZones == nil {
+		if r.Spec.NetworkSpec.VPC.AvailabilityZoneUsageLimit == nil {
+			r.Spec.NetworkSpec.VPC.AvailabilityZoneUsageLimit = ptr.To(3)
+		}
+		if r.Spec.NetworkSpec.VPC.AvailabilityZoneSelection == nil {
+			r.Spec.NetworkSpec.VPC.AvailabilityZoneSelection = &infrav1.AZSelectionSchemeOrdered
+		}
+	}
+
 	infrav1.SetDefaults_Bastion(&r.Spec.Bastion)
 	infrav1.SetDefaults_NetworkSpec(&r.Spec.NetworkSpec)
 }
diff --git a/pkg/cloud/services/network/subnets.go b/pkg/cloud/services/network/subnets.go
@@ -247,32 +247,18 @@ func (s *Service) reconcileZoneInfo(subnets infrav1.Subnets) error {
 }
 
 func (s *Service) getDefaultSubnets() (infrav1.Subnets, error) {
-	zones, err := s.getAvailableZones()
-	if err != nil {
-		return nil, err
-	}
-
-	maxZones := defaultMaxNumAZs
-	if s.scope.VPC().AvailabilityZoneUsageLimit != nil {
-		maxZones = *s.scope.VPC().AvailabilityZoneUsageLimit
-	}
-	selectionScheme := infrav1.AZSelectionSchemeOrdered
-	if s.scope.VPC().AvailabilityZoneSelection != nil {
-		selectionScheme = *s.scope.VPC().AvailabilityZoneSelection
-	}
+	var (
+		// By default, take the set availability zones. If they are not defined,
+		// fall back to `availabilityZoneUsageLimit`/`availabilityZoneSelection`
+		zones = s.scope.VPC().AvailabilityZones
+		err   error
+	)
 
-	if len(zones) > maxZones {
-		s.scope.Debug("region has more than AvailabilityZoneUsageLimit availability zones, picking zones to use", "region", s.scope.Region(), "AvailabilityZoneUsageLimit", maxZones)
-		if selectionScheme == infrav1.AZSelectionSchemeRandom {
-			rand.Shuffle(len(zones), func(i, j int) {
-				zones[i], zones[j] = zones[j], zones[i]
-			})
-		}
-		if selectionScheme == infrav1.AZSelectionSchemeOrdered {
-			sort.Strings(zones)
+	if len(zones) == 0 {
+		zones, err = s.selectZones()
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to select availability zones")
 		}
-		zones = zones[:maxZones]
-		s.scope.Debug("zones selected", "region", s.scope.Region(), "zones", zones)
 	}
 
 	// 1 private subnet for each AZ plus 1 other subnet that will be further sub-divided for the public subnets or vice versa if
@@ -362,6 +348,37 @@ func (s *Service) getDefaultSubnets() (infrav1.Subnets, error) {
 	return subnets, nil
 }
 
+func (s *Service) selectZones() ([]string, error) {
+	zones, err := s.getAvailableZones()
+	if err != nil {
+		return nil, err
+	}
+
+	maxZones := defaultMaxNumAZs
+	if s.scope.VPC().AvailabilityZoneUsageLimit != nil {
+		maxZones = *s.scope.VPC().AvailabilityZoneUsageLimit
+	}
+	selectionScheme := infrav1.AZSelectionSchemeOrdered
+	if s.scope.VPC().AvailabilityZoneSelection != nil {
+		selectionScheme = *s.scope.VPC().AvailabilityZoneSelection
+	}
+
+	if len(zones) > maxZones {
+		s.scope.Debug("region has more than AvailabilityZoneUsageLimit availability zones, picking zones to use", "region", s.scope.Region(), "AvailabilityZoneUsageLimit", maxZones)
+		if selectionScheme == infrav1.AZSelectionSchemeRandom {
+			rand.Shuffle(len(zones), func(i, j int) {
+				zones[i], zones[j] = zones[j], zones[i]
+			})
+		} else if selectionScheme == infrav1.AZSelectionSchemeOrdered {
+			sort.Strings(zones)
+		}
+		zones = zones[:maxZones]
+		s.scope.Debug("zones selected", "region", s.scope.Region(), "zones", zones)
+	}
+
+	return zones, nil
+}
+
 func (s *Service) deleteSubnets() error {
 	if s.scope.VPC().IsUnmanaged(s.scope.Name()) {
 		s.scope.Trace("Skipping subnets deletion in unmanaged mode")
diff --git a/pkg/cloud/services/network/subnets_test.go b/pkg/cloud/services/network/subnets_test.go

Original file line number	Diff line number	Diff line change
`@@ -85,6 +85,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {`
`85`	`85`	`}`
`86`	`86`
`87`	`87`	`dst.Spec.NetworkSpec.AdditionalControlPlaneIngressRules = restored.Spec.NetworkSpec.AdditionalControlPlaneIngressRules`
	`88`	`+ dst.Spec.NetworkSpec.VPC.AvailabilityZones = restored.Spec.NetworkSpec.VPC.AvailabilityZones`
`88`	`89`
`89`	`90`	`if restored.Spec.NetworkSpec.VPC.IPAMPool != nil {`
`90`	`91`	`if dst.Spec.NetworkSpec.VPC.IPAMPool == nil {`
Original file line number	Diff line number	Diff line change
`@@ -291,6 +291,10 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {`
`291`	`291`	`}`
`292`	`292`	`}`
`293`	`293`
	`294`	`+ if err := r.Spec.NetworkSpec.VPC.ValidateAvailabilityZones(); err != nil {`
	`295`	`+ allErrs = append(allErrs, err)`
	`296`	`+ }`
	`297`	`+`
`294`	`298`	`return allErrs`
`295`	`299`	`}`
`296`	`300`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@ package v1beta2`
`18`	`18`
`19`	`19`	`import (`
`20`	`20`	`metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"`
	`21`	`+ "k8s.io/utils/ptr"`
`21`	`22`
`22`	`23`	`clusterv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"`
`23`	`24`	`)`
`@@ -51,6 +52,15 @@ func SetDefaults_NetworkSpec(obj *NetworkSpec) { //nolint:golint,stylecheck`
`51`	`52`	`},`
`52`	`53`	`}`
`53`	`54`	`}`
	`55`	`+ // If AvailabilityZones are not set, set defaults for AZ selection`
	`56`	`+ if obj.VPC.AvailabilityZones == nil {`
	`57`	`+ if obj.VPC.AvailabilityZoneUsageLimit == nil {`
	`58`	`+ obj.VPC.AvailabilityZoneUsageLimit = ptr.To(3)`
	`59`	`+ }`
	`60`	`+ if obj.VPC.AvailabilityZoneSelection == nil {`
	`61`	`+ obj.VPC.AvailabilityZoneSelection = &AZSelectionSchemeOrdered`
	`62`	`+ }`
	`63`	`+ }`
`54`	`64`	`}`
`55`	`65`
`56`	`66`	`// SetDefaults_AWSClusterSpec is used by defaulter-gen.`