From a964b65048a00d8e2d564f02742b91f1610ea322 Mon Sep 17 00:00:00 2001 From: shysank Date: Thu, 18 Mar 2021 01:00:20 +0530 Subject: [PATCH] add separate loadbalancer spec for node outbound lb --- api/v1alpha3/azurecluster_conversion.go | 8 +- api/v1alpha3/zz_generated.conversion.go | 18 +- api/v1alpha4/azurecluster_default.go | 65 ++++++ api/v1alpha4/azurecluster_default_test.go | 203 ++++++++++++++++++ api/v1alpha4/azurecluster_validation.go | 54 +++-- api/v1alpha4/azurecluster_validation_test.go | 195 ++++++++++++----- api/v1alpha4/types.go | 15 +- api/v1alpha4/zz_generated.deepcopy.go | 13 +- azure/scope/cluster.go | 77 +++---- ...ucture.cluster.x-k8s.io_azureclusters.yaml | 47 +++- 10 files changed, 550 insertions(+), 145 deletions(-) diff --git a/api/v1alpha3/azurecluster_conversion.go b/api/v1alpha3/azurecluster_conversion.go index a03d6d04dbf..e38aa73d553 100644 --- a/api/v1alpha3/azurecluster_conversion.go +++ b/api/v1alpha3/azurecluster_conversion.go @@ -42,7 +42,8 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint return err } - dst.Spec.NetworkSpec.LoadBalancerNodeOutboundIPs = restored.Spec.NetworkSpec.LoadBalancerNodeOutboundIPs + dst.Spec.NetworkSpec.APIServerLB.FrontendIPsCount = restored.Spec.NetworkSpec.APIServerLB.FrontendIPsCount + dst.Spec.NetworkSpec.NodeOutboundLB = restored.Spec.NetworkSpec.NodeOutboundLB return nil } @@ -221,3 +222,8 @@ func Convert_v1alpha3_APIEndpoint_To_v1alpha4_APIEndpoint(in *apiv1alpha3.APIEnd func Convert_v1alpha4_APIEndpoint_To_v1alpha3_APIEndpoint(in *apiv1alpha4.APIEndpoint, out *apiv1alpha3.APIEndpoint, s apiconversion.Scope) error { return apiv1alpha3.Convert_v1alpha4_APIEndpoint_To_v1alpha3_APIEndpoint(in, out, s) } + +// Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec is an autogenerated conversion function. +func Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(in *v1alpha4.LoadBalancerSpec, out *LoadBalancerSpec, s apiconversion.Scope) error { + return autoConvert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(in, out, s) +} diff --git a/api/v1alpha3/zz_generated.conversion.go b/api/v1alpha3/zz_generated.conversion.go index f786cc66b04..3bb43cfe818 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -275,11 +275,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1alpha4.LoadBalancerSpec)(nil), (*LoadBalancerSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(a.(*v1alpha4.LoadBalancerSpec), b.(*LoadBalancerSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*ManagedDisk)(nil), (*v1alpha4.ManagedDisk)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk(a.(*ManagedDisk), b.(*v1alpha4.ManagedDisk), scope) }); err != nil { @@ -435,6 +430,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha4.LoadBalancerSpec)(nil), (*LoadBalancerSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(a.(*v1alpha4.LoadBalancerSpec), b.(*LoadBalancerSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha4.ManagedDisk)(nil), (*ManagedDisk)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha4_ManagedDisk_To_v1alpha3_ManagedDisk(a.(*v1alpha4.ManagedDisk), b.(*ManagedDisk), scope) }); err != nil { @@ -1299,14 +1299,10 @@ func autoConvert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(in *v1al out.SKU = SKU(in.SKU) out.FrontendIPs = *(*[]FrontendIP)(unsafe.Pointer(&in.FrontendIPs)) out.Type = LBType(in.Type) + // WARNING: in.FrontendIPsCount requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec is an autogenerated conversion function. -func Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(in *v1alpha4.LoadBalancerSpec, out *LoadBalancerSpec, s conversion.Scope) error { - return autoConvert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(in, out, s) -} - func autoConvert_v1alpha3_ManagedDisk_To_v1alpha4_ManagedDisk(in *ManagedDisk, out *v1alpha4.ManagedDisk, s conversion.Scope) error { out.StorageAccountType = in.StorageAccountType out.DiskEncryptionSet = (*v1alpha4.DiskEncryptionSetParameters)(unsafe.Pointer(in.DiskEncryptionSet)) @@ -1363,7 +1359,7 @@ func autoConvert_v1alpha4_NetworkSpec_To_v1alpha3_NetworkSpec(in *v1alpha4.Netwo if err := Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(&in.APIServerLB, &out.APIServerLB, s); err != nil { return err } - // WARNING: in.LoadBalancerNodeOutboundIPs requires manual conversion: does not exist in peer-type + // WARNING: in.NodeOutboundLB requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1alpha4/azurecluster_default.go b/api/v1alpha4/azurecluster_default.go index 79e1983f886..c9160b6cd37 100644 --- a/api/v1alpha4/azurecluster_default.go +++ b/api/v1alpha4/azurecluster_default.go @@ -18,6 +18,8 @@ package v1alpha4 import ( "fmt" + + "k8s.io/utils/pointer" ) const ( @@ -40,6 +42,7 @@ func (c *AzureCluster) setNetworkSpecDefaults() { c.setVnetDefaults() c.setSubnetDefaults() c.setAPIServerLBDefaults() + c.setNodeOutboundLBDefaults() } func (c *AzureCluster) setResourceGroupDefault() { @@ -139,6 +142,58 @@ func (c *AzureCluster) setAPIServerLBDefaults() { } } +func (c *AzureCluster) setNodeOutboundLBDefaults() { + if c.Spec.NetworkSpec.NodeOutboundLB == nil { + if c.Spec.NetworkSpec.APIServerLB.Type == Internal { + return + } + c.Spec.NetworkSpec.NodeOutboundLB = &LoadBalancerSpec{} + } + + lb := c.Spec.NetworkSpec.NodeOutboundLB + if lb.Type == "" { + lb.Type = Public + } + if lb.SKU == "" { + lb.SKU = SKUStandard + } + + if lb.Name == "" { + lb.Name = c.ObjectMeta.Name + } + if len(lb.FrontendIPs) == 0 { + + if lb.FrontendIPsCount == nil { + lb.FrontendIPsCount = pointer.Int32Ptr(1) + } + + switch *lb.FrontendIPsCount { + case 0: // do nothing + case 1: + lb.FrontendIPs = []FrontendIP{ + { + Name: generateFrontendIPConfigName(lb.Name), + PublicIP: &PublicIPSpec{ + Name: generateNodeOutboundIPName(c.ObjectMeta.Name), + }, + }, + } + default: + for i := 0; i < int(*lb.FrontendIPsCount); i++ { + lb.FrontendIPs = append(lb.FrontendIPs, FrontendIP{ + Name: withIndex(generateFrontendIPConfigName(lb.Name), i+1), + PublicIP: &PublicIPSpec{ + Name: withIndex(generateNodeOutboundIPName(c.ObjectMeta.Name), i+1), + }, + }) + } + + } + } else { + lb.FrontendIPsCount = pointer.Int32Ptr(int32(len(lb.FrontendIPs))) + } +} + // generateVnetName generates a virtual network name, based on the cluster name. func generateVnetName(clusterName string) string { return fmt.Sprintf("%s-%s", clusterName, "vnet") @@ -188,3 +243,13 @@ func generatePublicIPName(clusterName string) string { func generateFrontendIPConfigName(lbName string) string { return fmt.Sprintf("%s-%s", lbName, "frontEnd") } + +// generateNodeOutboundIPName generates a public IP name, based on the cluster name. +func generateNodeOutboundIPName(clusterName string) string { + return fmt.Sprintf("pip-%s-node-outbound", clusterName) +} + +// withIndex appends the index as suffix to a generated name +func withIndex(name string, n int) string { + return fmt.Sprintf("%s-%d", name, n) +} diff --git a/api/v1alpha4/azurecluster_default_test.go b/api/v1alpha4/azurecluster_default_test.go index f30179c3244..9cd0699ef70 100644 --- a/api/v1alpha4/azurecluster_default_test.go +++ b/api/v1alpha4/azurecluster_default_test.go @@ -21,6 +21,8 @@ import ( "reflect" "testing" + "k8s.io/utils/pointer" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -128,6 +130,20 @@ func TestVnetDefaults(t *testing.T) { }, Type: Public, }, + NodeOutboundLB: &LoadBalancerSpec{ + Name: "my-node-outbound-lb", + SKU: SKUStandard, + FrontendIPs: []FrontendIP{ + { + Name: "ip-config", + PublicIP: &PublicIPSpec{ + Name: "public-ip", + DNSName: "myfqdn.azure.com", + }, + }, + }, + Type: Public, + }, }, }, }, @@ -628,3 +644,190 @@ func TestAPIServerLBDefaults(t *testing.T) { }) } } + +func TestNodeOutboundLBDefaults(t *testing.T) { + cases := []struct { + name string + cluster *AzureCluster + output *AzureCluster + }{ + { + name: "default lb for public clusters", + cluster: &AzureCluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: LoadBalancerSpec{Type: Public}, + }, + }, + }, + output: &AzureCluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: LoadBalancerSpec{ + Type: Public, + }, + NodeOutboundLB: &LoadBalancerSpec{ + Name: "cluster-test", + SKU: SKUStandard, + FrontendIPs: []FrontendIP{{ + Name: "cluster-test-frontEnd", + PublicIP: &PublicIPSpec{ + Name: "pip-cluster-test-node-outbound", + }, + }}, + Type: Public, + FrontendIPsCount: pointer.Int32Ptr(1), + }, + }, + }, + }, + }, + { + name: "no lb for private clusters", + cluster: &AzureCluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: LoadBalancerSpec{Type: Internal}, + }, + }, + }, + output: &AzureCluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: LoadBalancerSpec{ + Type: Internal, + }, + }, + }, + }, + }, + { + name: "frontendIPsCount > 1", + cluster: &AzureCluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: LoadBalancerSpec{Type: Public}, + NodeOutboundLB: &LoadBalancerSpec{FrontendIPsCount: pointer.Int32Ptr(2)}, + }, + }, + }, + output: &AzureCluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: LoadBalancerSpec{ + Type: Public, + }, + NodeOutboundLB: &LoadBalancerSpec{ + Name: "cluster-test", + SKU: SKUStandard, + FrontendIPs: []FrontendIP{ + { + Name: "cluster-test-frontEnd-1", + PublicIP: &PublicIPSpec{ + Name: "pip-cluster-test-node-outbound-1", + }, + }, + { + Name: "cluster-test-frontEnd-2", + PublicIP: &PublicIPSpec{ + Name: "pip-cluster-test-node-outbound-2", + }, + }, + }, + Type: Public, + FrontendIPsCount: pointer.Int32Ptr(2), + }, + }, + }, + }, + }, + { + name: "when frontend ips are configured", + cluster: &AzureCluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: LoadBalancerSpec{Type: Public}, + NodeOutboundLB: &LoadBalancerSpec{FrontendIPs: []FrontendIP{ + { + Name: "fip-1", + PublicIP: &PublicIPSpec{ + Name: "pip-1", + }, + }, + { + Name: "fip-2", + PublicIP: &PublicIPSpec{ + Name: "pip-2", + }, + }, + }}, + }, + }, + }, + output: &AzureCluster{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: LoadBalancerSpec{Type: Public}, + NodeOutboundLB: &LoadBalancerSpec{ + Name: "cluster-test", + Type: Public, + SKU: SKUStandard, + FrontendIPs: []FrontendIP{ + { + Name: "fip-1", + PublicIP: &PublicIPSpec{ + Name: "pip-1", + }, + }, + { + Name: "fip-2", + PublicIP: &PublicIPSpec{ + Name: "pip-2", + }, + }, + }, + FrontendIPsCount: pointer.Int32Ptr(2), + }, + }, + }, + }, + }, + } + + for _, c := range cases { + tc := c + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + tc.cluster.setNodeOutboundLBDefaults() + if !reflect.DeepEqual(tc.cluster, tc.output) { + expected, _ := json.MarshalIndent(tc.output, "", "\t") + actual, _ := json.MarshalIndent(tc.cluster, "", "\t") + t.Errorf("Expected %s, got %s", string(expected), string(actual)) + } + }) + } +} diff --git a/api/v1alpha4/azurecluster_validation.go b/api/v1alpha4/azurecluster_validation.go index 3c715ce9e8d..c15b2198571 100644 --- a/api/v1alpha4/azurecluster_validation.go +++ b/api/v1alpha4/azurecluster_validation.go @@ -108,9 +108,7 @@ func validateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *fiel allErrs = append(allErrs, validateAPIServerLB(networkSpec.APIServerLB, old.APIServerLB, cidrBlocks, fldPath.Child("apiServerLB"))...) - if err := validateLoadBalancerNodeOutboundIPs(networkSpec, fldPath); err != nil { - allErrs = append(allErrs, err) - } + allErrs = append(allErrs, validateNodeOutboundLB(networkSpec.NodeOutboundLB, old.NodeOutboundLB, networkSpec.APIServerLB, fldPath.Child("nodeOutboundLB"))...) if len(allErrs) == 0 { return nil @@ -272,20 +270,50 @@ func validateAPIServerLB(lb LoadBalancerSpec, old LoadBalancerSpec, cidrs []stri return allErrs } -func validateLoadBalancerNodeOutboundIPs(networkSpec NetworkSpec, fldPath *field.Path) *field.Error { - if networkSpec.LoadBalancerNodeOutboundIPs == nil { - return nil +func validateNodeOutboundLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, apiserverLB LoadBalancerSpec, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + // LB can be nil when disabled for private clusters. + if lb == nil && apiserverLB.Type == Internal { + return allErrs } - if *networkSpec.LoadBalancerNodeOutboundIPs < 1 { - return field.Invalid(fldPath.Child("load_balancer_node_outbound_ips"), networkSpec.LoadBalancerNodeOutboundIPs, - "LoadBalancerNodeOutboundIPs value should be greater than 0") + if lb == nil { + allErrs = append(allErrs, field.Invalid(fldPath, lb, "Node outbound load balancer cannot be nil for public clusters.")) + return allErrs } - if *networkSpec.LoadBalancerNodeOutboundIPs > MaxLoadBalancerOutboundIPs { - return field.Invalid(fldPath.Child("load_balancer_node_outbound_ips"), networkSpec.LoadBalancerNodeOutboundIPs, - fmt.Sprintf("LoadBalancerNodeOutboundIPs value cannot be greater than %d", MaxLoadBalancerOutboundIPs)) + // SKU should be Standard and is immutable. + if lb.SKU != SKUStandard { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("sku"), lb.SKU, []string{string(SKUStandard)})) + } + if old != nil && old.SKU != "" && old.SKU != lb.SKU { + allErrs = append(allErrs, field.Invalid(fldPath.Child("sku"), lb.SKU, "Node outbound load balancer SKU should not be modified after AzureCluster creation.")) } - return nil + // Type should be Public. + if lb.Type != Public { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("type"), lb.Type, []string{string(Public)})) + } + if old != nil && old.Type != "" && old.Type != lb.Type { + allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), lb.Type, "Node outbound load balancer type should not be modified after AzureCluster creation.")) + } + + // Name should be valid. + if err := validateLoadBalancerName(lb.Name, fldPath.Child("name")); err != nil { + allErrs = append(allErrs, err) + } + if old != nil && old.Name != "" && old.Name != lb.Name { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), lb.Type, "Node outbound load balancer name should not be modified after AzureCluster creation.")) + } + + for i, frontEndIps := range lb.FrontendIPs { + + if frontEndIps.PrivateIPAddress != "" { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(i).Child("privateIP"), + "Public Load Balancers cannot have a Private IP")) + } + } + + return allErrs } diff --git a/api/v1alpha4/azurecluster_validation_test.go b/api/v1alpha4/azurecluster_validation_test.go index 74a18e1ffc7..b2a11e76512 100644 --- a/api/v1alpha4/azurecluster_validation_test.go +++ b/api/v1alpha4/azurecluster_validation_test.go @@ -22,7 +22,6 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/utils/pointer" ) func TestClusterNameValidation(t *testing.T) { @@ -325,59 +324,6 @@ func TestNetworkSpecWithoutPreexistingVnetValid(t *testing.T) { }) } -func TestNetworkSpecWithLoadBalancingNodeOutboundIPsInValid(t *testing.T) { - g := NewWithT(t) - - type test struct { - name string - networkSpec NetworkSpec - lbNodeOutboundIps *int32 - } - - testCases := []test{ - { - name: "azurecluster networkspec with load balancing node outbound ips less than 1 fails", - networkSpec: createValidNetworkSpec(), - lbNodeOutboundIps: pointer.Int32Ptr(0), - }, - { - name: "azurecluster networkspec with load balancing node outbound ips greater than max value fails", - networkSpec: createValidNetworkSpec(), - lbNodeOutboundIps: pointer.Int32Ptr(100), - }, - } - - for _, testCase := range testCases { - testCase.networkSpec.LoadBalancerNodeOutboundIPs = testCase.lbNodeOutboundIps - t.Run(testCase.name, func(t *testing.T) { - errs := validateNetworkSpec(testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) - g.Expect(errs).To(Not(BeNil())) - }) - } - -} - -func TestNetworkSpecWithLoadBalancingNodeOutboundIPsValid(t *testing.T) { - g := NewWithT(t) - - type test struct { - name string - networkSpec NetworkSpec - } - - testCase := test{ - name: "azurecluster networkspec with load balancing node outbound ips - valid", - networkSpec: createValidNetworkSpec(), - } - - testCase.networkSpec.LoadBalancerNodeOutboundIPs = pointer.Int32Ptr(5) - - t.Run(testCase.name, func(t *testing.T) { - errs := validateNetworkSpec(testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) - g.Expect(errs).To(BeNil()) - }) -} - func TestResourceGroupValid(t *testing.T) { g := NewWithT(t) @@ -848,6 +794,125 @@ func TestValidateAPIServerLB(t *testing.T) { } } +func TestValidateNodeOutboundLB(t *testing.T) { + g := NewWithT(t) + + testcases := []struct { + name string + lb *LoadBalancerSpec + old *LoadBalancerSpec + apiServerLB LoadBalancerSpec + wantErr bool + expectedErr field.Error + }{ + { + name: "no lb for public clusters", + lb: nil, + apiServerLB: LoadBalancerSpec{Type: Public}, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueInvalid", + Field: "nodeOutboundLB", + BadValue: nil, + Detail: "Node outbound load balancer cannot be nil for public clusters.", + }, + }, + { + name: "no lb allowed for internal clusters", + lb: nil, + apiServerLB: LoadBalancerSpec{Type: Internal}, + wantErr: false, + }, + { + name: "invalid SKU", + lb: &LoadBalancerSpec{ + Name: "my-awesome-lb", + SKU: "Awesome", + FrontendIPs: []FrontendIP{ + { + Name: "ip-config", + }, + }, + Type: Public, + }, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueNotSupported", + Field: "nodeOutboundLB.sku", + BadValue: "Awesome", + Detail: "supported values: \"Standard\"", + }, + }, + { + name: "invalid Type", + lb: &LoadBalancerSpec{ + Type: "Foo", + }, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueNotSupported", + Field: "nodeOutboundLB.type", + BadValue: "Foo", + Detail: "supported values: \"Public\"", + }, + }, + { + name: "invalid Name", + lb: &LoadBalancerSpec{ + Name: "***", + }, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueInvalid", + Field: "nodeOutboundLB.name", + BadValue: "***", + Detail: "name of load balancer doesn't match regex ^[-\\w\\._]+$", + }, + }, + { + name: "public LB with private IP", + lb: &LoadBalancerSpec{ + Type: Public, + FrontendIPs: []FrontendIP{ + { + Name: "ip-1", + }, + { + Name: "ip-2", + PrivateIPAddress: "10.0.0.4", + }, + }, + }, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueForbidden", + Field: "nodeOutboundLB.frontendIPConfigs[1].privateIP", + Detail: "Public Load Balancers cannot have a Private IP", + }, + }, + } + + for _, test := range testcases { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + err := validateNodeOutboundLB(test.lb, test.old, test.apiServerLB, field.NewPath("nodeOutboundLB")) + if test.wantErr { + g.Expect(err).NotTo(HaveLen(0)) + found := false + for _, actual := range err { + if actual.Error() == test.expectedErr.Error() { + found = true + } + } + g.Expect(found).To(BeTrue()) + } else { + g.Expect(err).To(HaveLen(0)) + } + }) + } +} + func createValidCluster() *AzureCluster { return &AzureCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -865,8 +930,9 @@ func createValidNetworkSpec() NetworkSpec { ResourceGroup: "custom-vnet", Name: "my-vnet", }, - Subnets: createValidSubnets(), - APIServerLB: createValidAPIServerLB(), + Subnets: createValidSubnets(), + APIServerLB: createValidAPIServerLB(), + NodeOutboundLB: createValidNodeOutboundLB(), } } @@ -899,3 +965,20 @@ func createValidAPIServerLB() LoadBalancerSpec { Type: Public, } } + +func createValidNodeOutboundLB() *LoadBalancerSpec { + return &LoadBalancerSpec{ + Name: "my-node-outbound-lb", + SKU: SKUStandard, + FrontendIPs: []FrontendIP{ + { + Name: "ip-config", + PublicIP: &PublicIPSpec{ + Name: "public-ip", + DNSName: "myfqdn.azure.com", + }, + }, + }, + Type: Public, + } +} diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go index 9b5eea3ead9..14b9020c7b4 100644 --- a/api/v1alpha4/types.go +++ b/api/v1alpha4/types.go @@ -59,9 +59,9 @@ type NetworkSpec struct { // +optional APIServerLB LoadBalancerSpec `json:"apiServerLB,omitempty"` - // LoadBalancerNodeOutboundIPs is the configuration for multiple frontend ips for node outbound load balancer. + // NodeOutboundLB is the configuration for the node outbound load balancer. // +optional - LoadBalancerNodeOutboundIPs *int32 `json:"load_balancer_node_outbound_ips,omitempty"` + NodeOutboundLB *LoadBalancerSpec `json:"nodeOutboundLB,omitempty"` } // VnetSpec configures an Azure virtual network. @@ -164,11 +164,12 @@ type IngressRules []IngressRule // LoadBalancerSpec defines an Azure load balancer. type LoadBalancerSpec struct { - ID string `json:"id,omitempty"` - Name string `json:"name,omitempty"` - SKU SKU `json:"sku,omitempty"` - FrontendIPs []FrontendIP `json:"frontendIPs,omitempty"` - Type LBType `json:"type,omitempty"` + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` + SKU SKU `json:"sku,omitempty"` + FrontendIPs []FrontendIP `json:"frontendIPs,omitempty"` + Type LBType `json:"type,omitempty"` + FrontendIPsCount *int32 `json:"frontendIPsCount,omitempty"` } // SKU defines an Azure load balancer SKU. diff --git a/api/v1alpha4/zz_generated.deepcopy.go b/api/v1alpha4/zz_generated.deepcopy.go index 8715a46b2e3..3a79e069e8f 100644 --- a/api/v1alpha4/zz_generated.deepcopy.go +++ b/api/v1alpha4/zz_generated.deepcopy.go @@ -791,6 +791,11 @@ func (in *LoadBalancerSpec) DeepCopyInto(out *LoadBalancerSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.FrontendIPsCount != nil { + in, out := &in.FrontendIPsCount, &out.FrontendIPsCount + *out = new(int32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerSpec. @@ -835,10 +840,10 @@ func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { } } in.APIServerLB.DeepCopyInto(&out.APIServerLB) - if in.LoadBalancerNodeOutboundIPs != nil { - in, out := &in.LoadBalancerNodeOutboundIPs, &out.LoadBalancerNodeOutboundIPs - *out = new(int32) - **out = **in + if in.NodeOutboundLB != nil { + in, out := &in.NodeOutboundLB, &out.NodeOutboundLB + *out = new(LoadBalancerSpec) + (*in).DeepCopyInto(*out) } } diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index b103e114a27..a6b6c09ae38 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -133,20 +133,24 @@ func (s *ClusterScope) PublicIPSpecs() []azure.PublicIPSpec { publicIPSpecs = append(publicIPSpecs, controlPlaneOutboundIP) // Public IP specs for node outbound lb - loadBalancerNodeOutboundIPs := s.LoadBalancerNodeOutboundIPs() var nodeOutboundIPSpecs []azure.PublicIPSpec - - if loadBalancerNodeOutboundIPs == 1 { - nodeOutboundIPSpecs = append(nodeOutboundIPSpecs, azure.PublicIPSpec{ - Name: azure.GenerateNodeOutboundIPName(s.ClusterName()), - }) - } else { - for i := 0; i < int(loadBalancerNodeOutboundIPs); i++ { - publicIPSpecs = append(publicIPSpecs, azure.PublicIPSpec{ - Name: azure.WithIndex(azure.GenerateNodeOutboundIPName(s.ClusterName()), i+1), + if s.NodeOutboundLB() != nil { + loadBalancerNodeOutboundIPs := s.NodeOutboundLB().FrontendIPsCount + if loadBalancerNodeOutboundIPs == nil || *loadBalancerNodeOutboundIPs == 0 { + // do nothing + } else if *loadBalancerNodeOutboundIPs == 1 { + nodeOutboundIPSpecs = append(nodeOutboundIPSpecs, azure.PublicIPSpec{ + Name: azure.GenerateNodeOutboundIPName(s.ClusterName()), }) + } else { + for i := 0; i < int(*loadBalancerNodeOutboundIPs); i++ { + publicIPSpecs = append(publicIPSpecs, azure.PublicIPSpec{ + Name: azure.WithIndex(azure.GenerateNodeOutboundIPName(s.ClusterName()), i+1), + }) + } } } + publicIPSpecs = append(publicIPSpecs, nodeOutboundIPSpecs...) return publicIPSpecs @@ -166,15 +170,18 @@ func (s *ClusterScope) LBSpecs() []azure.LBSpec { Role: infrav1.APIServerRole, BackendPoolName: s.APIServerLBPoolName(s.APIServerLB().Name), }, - { - // Public Node outbound LB + } + + // Public Node outbound LB + if s.NodeOutboundLB() != nil { + specs = append(specs, azure.LBSpec{ Name: s.NodeOutboundLBName(), - FrontendIPConfigs: s.nodeOutboundFrontendIPConfigs(), - Type: infrav1.Public, - SKU: infrav1.SKUStandard, + FrontendIPConfigs: s.NodeOutboundLB().FrontendIPs, + Type: s.NodeOutboundLB().Type, + SKU: s.NodeOutboundLB().SKU, BackendPoolName: s.OutboundPoolName(s.NodeOutboundLBName()), Role: infrav1.NodeOutboundRole, - }, + }) } if !s.IsAPIServerPrivate() { @@ -201,31 +208,6 @@ func (s *ClusterScope) LBSpecs() []azure.LBSpec { return specs } -// nodeOutboundFrontendIPConfigs returns frontend ip configs for node outbound lb. -func (s *ClusterScope) nodeOutboundFrontendIPConfigs() []infrav1.FrontendIP { - if s.LoadBalancerNodeOutboundIPs() == 1 { - return []infrav1.FrontendIP{ - { - Name: azure.GenerateFrontendIPConfigName(s.NodeOutboundLBName()), - PublicIP: &infrav1.PublicIPSpec{ - Name: azure.GenerateNodeOutboundIPName(s.ClusterName()), - }, - }, - } - } - - frontendIPs := make([]infrav1.FrontendIP, s.LoadBalancerNodeOutboundIPs()) - for i := 0; i < int(s.LoadBalancerNodeOutboundIPs()); i++ { - frontendIPs[i] = infrav1.FrontendIP{ - Name: azure.WithIndex(azure.GenerateFrontendIPConfigName(s.NodeOutboundLBName()), i+1), - PublicIP: &infrav1.PublicIPSpec{ - Name: azure.WithIndex(azure.GenerateNodeOutboundIPName(s.ClusterName()), i+1), - }, - } - } - return frontendIPs -} - // RouteTableSpecs returns the node route table func (s *ClusterScope) RouteTableSpecs() []azure.RouteTableSpec { routetables := []azure.RouteTableSpec{} @@ -357,6 +339,11 @@ func (s *ClusterScope) APIServerLB() *infrav1.LoadBalancerSpec { return &s.AzureCluster.Spec.NetworkSpec.APIServerLB } +// NodeOutboundLB returns the cluster node outbound load balancer. +func (s *ClusterScope) NodeOutboundLB() *infrav1.LoadBalancerSpec { + return s.AzureCluster.Spec.NetworkSpec.NodeOutboundLB +} + // APIServerLBName returns the API Server LB name. func (s *ClusterScope) APIServerLBName() string { return s.APIServerLB().Name @@ -428,14 +415,6 @@ func (s *ClusterScope) AvailabilitySetEnabled() bool { return len(s.AzureCluster.Status.FailureDomains) == 0 } -// LoadBalancerNodeOutboundIPs returns the number node outbound ips that needs to be configured in the load balancer. -func (s *ClusterScope) LoadBalancerNodeOutboundIPs() int32 { - if s.AzureCluster.Spec.NetworkSpec.LoadBalancerNodeOutboundIPs == nil { - return 1 - } - return *s.AzureCluster.Spec.NetworkSpec.LoadBalancerNodeOutboundIPs -} - // GenerateFQDN generates a fully qualified domain name, based on a hash, cluster name and cluster location. func (s *ClusterScope) GenerateFQDN(ipName string) string { h := fnv.New32a() diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml index 50ddc5d8e3e..f59c21cf9e4 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml @@ -437,6 +437,49 @@ spec: - name type: object type: array + frontendIPsCount: + format: int32 + type: integer + id: + type: string + name: + type: string + sku: + description: SKU defines an Azure load balancer SKU. + type: string + type: + description: LBType defines an Azure load balancer Type. + type: string + type: object + nodeOutboundLB: + description: NodeOutboundLB is the configuration for the node outbound load balancer. + properties: + frontendIPs: + items: + description: FrontendIP defines a load balancer frontend IP configuration. + properties: + name: + minLength: 1 + type: string + privateIP: + type: string + publicIP: + description: PublicIPSpec defines the inputs to create an Azure public IP address. + properties: + dnsName: + type: string + name: + type: string + required: + - name + type: object + required: + - name + type: object + type: array + frontendIPsCount: + format: int32 + type: integer id: type: string name: @@ -448,10 +491,6 @@ spec: description: LBType defines an Azure load balancer Type. type: string type: object - load_balancer_node_outbound_ips: - description: LoadBalancerNodeOutboundIPs is the configuration for multiple frontend ips for node outbound load balancer. - format: int32 - type: integer subnets: description: Subnets is the configuration for the control-plane subnet and the node subnet. items: