Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ feat: create vpc objects in explicitly provided availability zones #4950

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/v1beta1/awscluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions api/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions api/v1beta2/awscluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
10 changes: 10 additions & 0 deletions api/v1beta2/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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.
Expand Down
20 changes: 18 additions & 2 deletions api/v1beta2/network_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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"`

Expand All @@ -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"`
synthe102 marked this conversation as resolved.
Show resolved Hide resolved

// EmptyRoutesDefaultVPCSecurityGroup specifies whether the default VPC security group ingress
// and egress rules should be removed.
//
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand All @@ -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)
}
65 changes: 41 additions & 24 deletions pkg/cloud/services/network/subnets.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,32 +247,18 @@ func (s *Service) reconcileZoneInfo(subnets infrav1.Subnets) error {
}

func (s *Service) getDefaultSubnets() (infrav1.Subnets, error) {
synthe102 marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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")
Expand Down
Loading