feat: create vpc objects in explicitly provided availability zones

Léonard Suslian · Léonard Suslian · commit 5f7baeee27fe · 2024-05-13T10:48:59.000+02:00
diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go
@@ -84,6 +84,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
@@ -269,6 +269,11 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
 		}
 	}
 
+	if r.Spec.NetworkSpec.VPC.AvailabilityZones != nil && (r.Spec.NetworkSpec.VPC.AvailabilityZoneSelection != nil || r.Spec.NetworkSpec.VPC.AvailabilityZoneUsageLimit != nil) {
+		availabilityZonesField := field.NewPath("spec", "networkSpec", "vpc", "availabilityZones")
+		allErrs = append(allErrs, field.Invalid(availabilityZonesField, r.Spec.NetworkSpec.VPC.AvailabilityZoneSelection, "availabilityZones cannot be set if availabilityZoneUsageLimit and availabilityZoneSelection are set"))
+	}
+
 	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,16 @@ 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
@@ -424,7 +424,6 @@ 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
 	// +kubebuilder:validation:Minimum=1
 	AvailabilityZoneUsageLimit *int `json:"availabilityZoneUsageLimit,omitempty"`
 
@@ -433,10 +432,14 @@ type VPCSpec struct {
 	// Ordered - selects based on alphabetical order
 	// Random - selects AZs randomly in a region
 	// Defaults to Ordered
-	// +kubebuilder:default=Ordered
 	// +kubebuilder:validation:Enum=Ordered;Random
 	AvailabilityZoneSelection *AZSelectionScheme `json:"availabilityZoneSelection,omitempty"`
 
+	// AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+	// If defined, both AvailabilityZoneUsageLimit and AvailabilityZoneSelection are ignored.
+	// +optional
+	AvailabilityZones []string `json:"availabilityZones,omitempty"`
+
 	// EmptyRoutesDefaultVPCSecurityGroup specifies whether the default VPC security group ingress
 	// and egress rules should be removed.
 	//
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
@@ -594,7 +594,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:
@@ -606,7 +605,6 @@ 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
@@ -622,6 +620,13 @@ spec:
                         x-kubernetes-validations:
                         - message: Carrier Gateway ID must start with 'cagw-'
                           rule: self.startsWith('cagw-')
+                      availabilityZones:
+                        description: |-
+                          AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+                          If defined, both AvailabilityZoneUsageLimit and AvailabilityZoneSelection are ignored.
+                        items:
+                          type: string
+                        type: array
                       cidrBlock:
                         description: |-
                           CidrBlock is the CIDR block to be used when the provider creates a managed VPC.
@@ -2544,7 +2549,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:
@@ -2556,7 +2560,6 @@ 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
@@ -2572,6 +2575,13 @@ spec:
                         x-kubernetes-validations:
                         - message: Carrier Gateway ID must start with 'cagw-'
                           rule: self.startsWith('cagw-')
+                      availabilityZones:
+                        description: |-
+                          AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+                          If defined, both AvailabilityZoneUsageLimit and AvailabilityZoneSelection are ignored.
+                        items:
+                          type: string
+                        type: array
                       cidrBlock:
                         description: |-
                           CidrBlock is the CIDR block to be used when the provider creates a managed 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
@@ -1530,7 +1530,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:
@@ -1542,7 +1541,6 @@ 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
@@ -1558,6 +1556,13 @@ spec:
                         x-kubernetes-validations:
                         - message: Carrier Gateway ID must start with 'cagw-'
                           rule: self.startsWith('cagw-')
+                      availabilityZones:
+                        description: |-
+                          AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+                          If defined, both AvailabilityZoneUsageLimit and AvailabilityZoneSelection are ignored.
+                        items:
+                          type: string
+                        type: array
                       cidrBlock:
                         description: |-
                           CidrBlock is the CIDR block to be used when the provider creates a managed 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
@@ -1128,7 +1128,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:
@@ -1140,7 +1139,6 @@ 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
@@ -1156,6 +1154,13 @@ spec:
                                 x-kubernetes-validations:
                                 - message: Carrier Gateway ID must start with 'cagw-'
                                   rule: self.startsWith('cagw-')
+                              availabilityZones:
+                                description: |-
+                                  AvailabilityZones defines a list of Availability Zones in which to create network resources in.
+                                  If defined, both AvailabilityZoneUsageLimit and AvailabilityZoneSelection are ignored.
+                                items:
+                                  type: string
+                                type: array
                               cidrBlock:
                                 description: |-
                                   CidrBlock is the CIDR block to be used when the provider creates a managed 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"
@@ -426,6 +427,11 @@ 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 r.Spec.NetworkSpec.VPC.AvailabilityZones != nil && (r.Spec.NetworkSpec.VPC.AvailabilityZoneSelection != nil || r.Spec.NetworkSpec.VPC.AvailabilityZoneUsageLimit != nil) {
+		availabilityZonesField := field.NewPath("spec", "networkSpec", "vpc", "availabilityZones")
+		allErrs = append(allErrs, field.Invalid(availabilityZonesField, r.Spec.NetworkSpec.VPC.AvailabilityZoneSelection, "availabilityZones cannot be set if availabilityZoneUsageLimit and availabilityZoneSelection are set"))
+	}
+
 	return allErrs
 }
 
@@ -452,6 +458,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
@@ -246,32 +246,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, we will take the set availability zones, if they are defined.
+		// if not, we fall back to the two other settings.
+		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
@@ -338,6 +324,38 @@ 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]
+			})
+		}
+		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
`@@ -84,6 +84,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {`
`84`	`84`	`}`
`85`	`85`
`86`	`86`	`dst.Spec.NetworkSpec.AdditionalControlPlaneIngressRules = restored.Spec.NetworkSpec.AdditionalControlPlaneIngressRules`
	`87`	`+ dst.Spec.NetworkSpec.VPC.AvailabilityZones = restored.Spec.NetworkSpec.VPC.AvailabilityZones`
`87`	`88`
`88`	`89`	`if restored.Spec.NetworkSpec.VPC.IPAMPool != nil {`
`89`	`90`	`if dst.Spec.NetworkSpec.VPC.IPAMPool == nil {`
Original file line number	Diff line number	Diff line change
`@@ -269,6 +269,11 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {`
`269`	`269`	`}`
`270`	`270`	`}`
`271`	`271`
	`272`	`+ if r.Spec.NetworkSpec.VPC.AvailabilityZones != nil && (r.Spec.NetworkSpec.VPC.AvailabilityZoneSelection != nil \|\| r.Spec.NetworkSpec.VPC.AvailabilityZoneUsageLimit != nil) {`
	`273`	`+ availabilityZonesField := field.NewPath("spec", "networkSpec", "vpc", "availabilityZones")`
	`274`	`+ allErrs = append(allErrs, field.Invalid(availabilityZonesField, r.Spec.NetworkSpec.VPC.AvailabilityZoneSelection, "availabilityZones cannot be set if availabilityZoneUsageLimit and availabilityZoneSelection are set"))`
	`275`	`+ }`
	`276`	`+`
`272`	`277`	`return allErrs`
`273`	`278`	`}`
`274`	`279`
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,16 @@ 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`	`+ }`
	`64`	`+`
`54`	`65`	`}`
`55`	`66`
`56`	`67`	`// SetDefaults_AWSClusterSpec is used by defaulter-gen.`