From 558454c53a7dbd221f0f456d82928dfea4c29fe3 Mon Sep 17 00:00:00 2001 From: Rico Pahlisch Date: Fri, 1 Mar 2024 11:00:38 +0100 Subject: [PATCH 1/5] enable the use off an external control plane --- api/v1beta1/azurecluster_default.go | 48 +++++--- api/v1beta1/azurecluster_types.go | 5 + api/v1beta1/azurecluster_validation.go | 53 ++++---- api/v1beta1/azurecluster_validation_test.go | 8 +- api/v1beta1/azurecluster_webhook.go | 6 + .../azureclustertemplate_validation.go | 1 + api/v1beta1/types.go | 2 +- api/v1beta1/zz_generated.deepcopy.go | 6 +- azure/scope/cluster.go | 115 ++++++++++-------- config/capz/manager_image_patch.yaml | 2 +- ...ucture.cluster.x-k8s.io_azureclusters.yaml | 4 + controllers/azurecluster_controller.go | 14 ++- controllers/azurecluster_reconciler.go | 9 +- 13 files changed, 169 insertions(+), 104 deletions(-) diff --git a/api/v1beta1/azurecluster_default.go b/api/v1beta1/azurecluster_default.go index a0898a51901..51159a00827 100644 --- a/api/v1beta1/azurecluster_default.go +++ b/api/v1beta1/azurecluster_default.go @@ -17,7 +17,10 @@ limitations under the License. package v1beta1 import ( + "context" + "encoding/json" "fmt" + "sigs.k8s.io/controller-runtime/pkg/log" "k8s.io/utils/ptr" ) @@ -54,13 +57,26 @@ func (c *AzureCluster) setDefaults() { } func (c *AzureCluster) setNetworkSpecDefaults() { + logger := log.FromContext(context.TODO()) + logger.Info(fmt.Sprintf("Was ist hier los %v", c.Spec.ControlPlaneEnabled)) c.setVnetDefaults() c.setBastionDefaults() c.setSubnetDefaults() c.setVnetPeeringDefaults() - c.setAPIServerLBDefaults() + if c.Spec.ControlPlaneEnabled { + logger.Info("dont call me setAPIServerLBDefaults") + c.setAPIServerLBDefaults() + } c.SetNodeOutboundLBDefaults() - c.SetControlPlaneOutboundLBDefaults() + if c.Spec.ControlPlaneEnabled { + logger.Info("dont call me SetControlPlaneOutboundLBDefaults") + c.SetControlPlaneOutboundLBDefaults() + } + if c.Spec.ControlPlaneEnabled { + c.Spec.NetworkSpec.APIServerLB = nil + } + bolB, _ := json.Marshal(c.Spec) + logger.Info(fmt.Sprintf("Was ist hier los %s", string(bolB))) } func (c *AzureCluster) setResourceGroupDefault() { @@ -93,16 +109,18 @@ func (c *AzureCluster) setSubnetDefaults() { c.Spec.NetworkSpec.UpdateSubnet(clusterSubnet, SubnetCluster) } - /* if there is a cp subnet set defaults - if no cp subnet and cluster subnet create a default cp subnet */ - cpSubnet, errcp := c.Spec.NetworkSpec.GetSubnet(SubnetControlPlane) - if errcp == nil { - cpSubnet.setControlPlaneSubnetDefaults(c.ObjectMeta.Name) - c.Spec.NetworkSpec.UpdateSubnet(cpSubnet, SubnetControlPlane) - } else if !clusterSubnetExists { - cpSubnet = SubnetSpec{SubnetClassSpec: SubnetClassSpec{Role: SubnetControlPlane}} - cpSubnet.setControlPlaneSubnetDefaults(c.ObjectMeta.Name) - c.Spec.NetworkSpec.Subnets = append(c.Spec.NetworkSpec.Subnets, cpSubnet) + if c.Spec.ControlPlaneEnabled { + /* if there is a cp subnet set defaults + if no cp subnet and cluster subnet create a default cp subnet */ + cpSubnet, errcp := c.Spec.NetworkSpec.GetSubnet(SubnetControlPlane) + if errcp == nil { + cpSubnet.setControlPlaneSubnetDefaults(c.ObjectMeta.Name) + c.Spec.NetworkSpec.UpdateSubnet(cpSubnet, SubnetControlPlane) + } else if !clusterSubnetExists { + cpSubnet = SubnetSpec{SubnetClassSpec: SubnetClassSpec{Role: SubnetControlPlane}} + cpSubnet.setControlPlaneSubnetDefaults(c.ObjectMeta.Name) + c.Spec.NetworkSpec.Subnets = append(c.Spec.NetworkSpec.Subnets, cpSubnet) + } } var nodeSubnetFound bool @@ -210,7 +228,7 @@ func (c *AzureCluster) setVnetPeeringDefaults() { } func (c *AzureCluster) setAPIServerLBDefaults() { - lb := &c.Spec.NetworkSpec.APIServerLB + lb := c.Spec.NetworkSpec.APIServerLB lb.LoadBalancerClassSpec.setAPIServerLBDefaults() @@ -249,7 +267,7 @@ func (c *AzureCluster) setAPIServerLBDefaults() { // SetNodeOutboundLBDefaults sets the default values for the NodeOutboundLB. func (c *AzureCluster) SetNodeOutboundLBDefaults() { if c.Spec.NetworkSpec.NodeOutboundLB == nil { - if c.Spec.NetworkSpec.APIServerLB.Type == Internal { + if !c.Spec.ControlPlaneEnabled || c.Spec.NetworkSpec.APIServerLB.Type == Internal { return } @@ -314,7 +332,7 @@ func (c *AzureCluster) SetBackendPoolNameDefault() { // SetAPIServerLBBackendPoolNameDefault defaults the name of the backend pool for apiserver LB. func (c *AzureCluster) SetAPIServerLBBackendPoolNameDefault() { - apiServerLB := &c.Spec.NetworkSpec.APIServerLB + apiServerLB := c.Spec.NetworkSpec.APIServerLB if apiServerLB.BackendPool.Name == "" { apiServerLB.BackendPool.Name = generateBackendAddressPoolName(apiServerLB.Name) } diff --git a/api/v1beta1/azurecluster_types.go b/api/v1beta1/azurecluster_types.go index def39a9d13a..a842bbac63b 100644 --- a/api/v1beta1/azurecluster_types.go +++ b/api/v1beta1/azurecluster_types.go @@ -45,6 +45,11 @@ type AzureClusterSpec struct { // +optional BastionSpec BastionSpec `json:"bastionSpec,omitempty"` + // ControlPlaneEnabled enable control plane cluster components. + // +kubebuilder:default=true + // +optional + ControlPlaneEnabled bool `json:"controlPlaneEnabled"` + // ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. It is not recommended to set // this when creating an AzureCluster as CAPZ will set this for you. However, if it is set, CAPZ will not change it. // +optional diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go index a079e6ea855..f4c898eb9f6 100644 --- a/api/v1beta1/azurecluster_validation.go +++ b/api/v1beta1/azurecluster_validation.go @@ -17,10 +17,12 @@ limitations under the License. package v1beta1 import ( + "context" "fmt" "net" "reflect" "regexp" + "sigs.k8s.io/controller-runtime/pkg/log" valid "github.com/asaskevich/govalidator" corev1 "k8s.io/api/core/v1" @@ -85,12 +87,16 @@ func (c *AzureCluster) validateCluster(old *AzureCluster) (admission.Warnings, e // validateClusterSpec validates a ClusterSpec. func (c *AzureCluster) validateClusterSpec(old *AzureCluster) field.ErrorList { + logger := log.FromContext(context.TODO()) var allErrs field.ErrorList var oldNetworkSpec NetworkSpec if old != nil { oldNetworkSpec = old.Spec.NetworkSpec } - allErrs = append(allErrs, validateNetworkSpec(c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) + + logger.Info(fmt.Sprintf("ControlPlaneEnabled: %v", c.Spec.ControlPlaneEnabled)) + + allErrs = append(allErrs, validateNetworkSpec(c.Spec.ControlPlaneEnabled, c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) var oldCloudProviderConfigOverrides *CloudProviderConfigOverrides if old != nil { @@ -154,7 +160,7 @@ func validateIdentityRef(identityRef *corev1.ObjectReference, fldPath *field.Pat } // validateNetworkSpec validates a NetworkSpec. -func validateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *field.Path) field.ErrorList { +func validateNetworkSpec(controlPlaneEnabled bool, networkSpec NetworkSpec, old NetworkSpec, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList // If the user specifies a resourceGroup for vnet, it means // that they intend to use a pre-existing vnet. In this case, @@ -167,20 +173,21 @@ func validateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *fiel allErrs = append(allErrs, validateVnetCIDR(networkSpec.Vnet.CIDRBlocks, fldPath.Child("cidrBlocks"))...) - allErrs = append(allErrs, validateSubnets(networkSpec.Subnets, networkSpec.Vnet, fldPath.Child("subnets"))...) + allErrs = append(allErrs, validateSubnets(controlPlaneEnabled, networkSpec.Subnets, networkSpec.Vnet, fldPath.Child("subnets"))...) allErrs = append(allErrs, validateVnetPeerings(networkSpec.Vnet.Peerings, fldPath.Child("peerings"))...) } var cidrBlocks []string - controlPlaneSubnet, err := networkSpec.GetControlPlaneSubnet() - if err != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("subnets"), networkSpec.Subnets, "ControlPlaneSubnet invalid")) - } - - cidrBlocks = controlPlaneSubnet.CIDRBlocks + if controlPlaneEnabled { + controlPlaneSubnet, err := networkSpec.GetControlPlaneSubnet() + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("subnets"), networkSpec.Subnets, "ControlPlaneSubnet invalid")) + } - allErrs = append(allErrs, validateAPIServerLB(networkSpec.APIServerLB, old.APIServerLB, cidrBlocks, fldPath.Child("apiServerLB"))...) + cidrBlocks = controlPlaneSubnet.CIDRBlocks + allErrs = append(allErrs, validateAPIServerLB(networkSpec.APIServerLB, old.APIServerLB, cidrBlocks, fldPath.Child("apiServerLB"))...) + } var needOutboundLB bool for _, subnet := range networkSpec.Subnets { @@ -192,10 +199,14 @@ func validateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *fiel if needOutboundLB { allErrs = append(allErrs, validateNodeOutboundLB(networkSpec.NodeOutboundLB, old.NodeOutboundLB, networkSpec.APIServerLB, fldPath.Child("nodeOutboundLB"))...) } - - allErrs = append(allErrs, validateControlPlaneOutboundLB(networkSpec.ControlPlaneOutboundLB, networkSpec.APIServerLB, fldPath.Child("controlPlaneOutboundLB"))...) - - allErrs = append(allErrs, validatePrivateDNSZoneName(networkSpec.PrivateDNSZoneName, networkSpec.APIServerLB.Type, fldPath.Child("privateDNSZoneName"))...) + if controlPlaneEnabled { + allErrs = append(allErrs, validateControlPlaneOutboundLB(networkSpec.ControlPlaneOutboundLB, networkSpec.APIServerLB, fldPath.Child("controlPlaneOutboundLB"))...) + } + var lbType LBType = Internal + if networkSpec.APIServerLB != nil { + lbType = networkSpec.APIServerLB.Type + } + allErrs = append(allErrs, validatePrivateDNSZoneName(networkSpec.PrivateDNSZoneName, controlPlaneEnabled, lbType, fldPath.Child("privateDNSZoneName"))...) if len(allErrs) == 0 { return nil @@ -214,11 +225,11 @@ func validateResourceGroup(resourceGroup string, fldPath *field.Path) *field.Err // validateSubnets validates a list of Subnets. // When configuring a cluster, it is essential to include either a control-plane subnet and a node subnet, or a user can configure a cluster subnet which will be used as a control-plane subnet and a node subnet. -func validateSubnets(subnets Subnets, vnet VnetSpec, fldPath *field.Path) field.ErrorList { +func validateSubnets(controlPlaneEnabled bool, subnets Subnets, vnet VnetSpec, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList subnetNames := make(map[string]bool, len(subnets)) requiredSubnetRoles := map[string]bool{ - "control-plane": false, + "control-plane": !controlPlaneEnabled, "node": false, } clusterSubnet := false @@ -383,7 +394,7 @@ func validateSecurityRule(rule SecurityRule, fldPath *field.Path) (allErrs field return allErrs } -func validateAPIServerLB(lb LoadBalancerSpec, old LoadBalancerSpec, cidrs []string, fldPath *field.Path) field.ErrorList { +func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []string, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList lbClassSpec := lb.LoadBalancerClassSpec @@ -433,7 +444,7 @@ func validateAPIServerLB(lb LoadBalancerSpec, old LoadBalancerSpec, cidrs []stri return allErrs } -func validateNodeOutboundLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, apiserverLB LoadBalancerSpec, fldPath *field.Path) field.ErrorList { +func validateNodeOutboundLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, apiserverLB *LoadBalancerSpec, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList var lbClassSpec, oldClassSpec *LoadBalancerClassSpec @@ -483,7 +494,7 @@ func validateNodeOutboundLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, apiserv return allErrs } -func validateControlPlaneOutboundLB(lb *LoadBalancerSpec, apiserverLB LoadBalancerSpec, fldPath *field.Path) field.ErrorList { +func validateControlPlaneOutboundLB(lb *LoadBalancerSpec, apiserverLB *LoadBalancerSpec, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList var lbClassSpec *LoadBalancerClassSpec @@ -505,11 +516,11 @@ func validateControlPlaneOutboundLB(lb *LoadBalancerSpec, apiserverLB LoadBalanc } // validatePrivateDNSZoneName validates the PrivateDNSZoneName. -func validatePrivateDNSZoneName(privateDNSZoneName string, apiserverLBType LBType, fldPath *field.Path) field.ErrorList { +func validatePrivateDNSZoneName(privateDNSZoneName string, controlPlaneEnabled bool, apiserverLBType LBType, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList if len(privateDNSZoneName) > 0 { - if apiserverLBType != Internal { + if controlPlaneEnabled && apiserverLBType != Internal { allErrs = append(allErrs, field.Invalid(fldPath, apiserverLBType, "PrivateDNSZoneName is available only if APIServerLB.Type is Internal")) } diff --git a/api/v1beta1/azurecluster_validation_test.go b/api/v1beta1/azurecluster_validation_test.go index 31b3fcbf668..14709dfd646 100644 --- a/api/v1beta1/azurecluster_validation_test.go +++ b/api/v1beta1/azurecluster_validation_test.go @@ -316,7 +316,7 @@ func TestNetworkSpecWithPreexistingVnetValid(t *testing.T) { for _, test := range testCase { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - errs := validateNetworkSpec(test.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) + errs := validateNetworkSpec(true, test.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) g.Expect(errs).To(BeNil()) }) } @@ -338,7 +338,7 @@ func TestNetworkSpecWithPreexistingVnetLackRequiredSubnets(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateNetworkSpec(testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) + errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) g.Expect(errs).To(HaveLen(1)) g.Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired)) g.Expect(errs[0].Field).To(Equal("spec.networkSpec.subnets")) @@ -361,7 +361,7 @@ func TestNetworkSpecWithPreexistingVnetInvalidResourceGroup(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateNetworkSpec(testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) + errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) g.Expect(errs).To(HaveLen(1)) g.Expect(errs[0].Type).To(Equal(field.ErrorTypeInvalid)) g.Expect(errs[0].Field).To(Equal("spec.networkSpec.vnet.resourceGroup")) @@ -384,7 +384,7 @@ func TestNetworkSpecWithoutPreexistingVnetValid(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateNetworkSpec(testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) + errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) g.Expect(errs).To(BeNil()) }) } diff --git a/api/v1beta1/azurecluster_webhook.go b/api/v1beta1/azurecluster_webhook.go index 49cc7c7fe00..9685db94660 100644 --- a/api/v1beta1/azurecluster_webhook.go +++ b/api/v1beta1/azurecluster_webhook.go @@ -17,7 +17,9 @@ limitations under the License. package v1beta1 import ( + "context" "reflect" + "sigs.k8s.io/controller-runtime/pkg/log" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -43,11 +45,15 @@ var _ webhook.Defaulter = &AzureCluster{} // Default implements webhook.Defaulter so a webhook will be registered for the type. func (c *AzureCluster) Default() { + logger := log.FromContext(context.TODO()) + logger.Info("Defaulter") c.setDefaults() } // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. func (c *AzureCluster) ValidateCreate() (admission.Warnings, error) { + logger := log.FromContext(context.TODO()) + logger.Info("ValidateCreate") return c.validateCluster(nil) } diff --git a/api/v1beta1/azureclustertemplate_validation.go b/api/v1beta1/azureclustertemplate_validation.go index 8f0323d38a3..5f82bb9b0bd 100644 --- a/api/v1beta1/azureclustertemplate_validation.go +++ b/api/v1beta1/azureclustertemplate_validation.go @@ -162,6 +162,7 @@ func (c *AzureClusterTemplate) validatePrivateDNSZoneName() field.ErrorList { allErrs = append(allErrs, validatePrivateDNSZoneName( networkSpec.PrivateDNSZoneName, + true, networkSpec.APIServerLB.Type, fldPath, )...) diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 1f82bbca993..8fdd2429829 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -100,7 +100,7 @@ type NetworkSpec struct { // APIServerLB is the configuration for the control-plane load balancer. // +optional - APIServerLB LoadBalancerSpec `json:"apiServerLB,omitempty"` + APIServerLB *LoadBalancerSpec `json:"apiServerLB,omitempty"` // NodeOutboundLB is the configuration for the node outbound load balancer. // +optional diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index ee778ef4b24..4cc92dcaf94 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -3082,7 +3082,11 @@ func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - in.APIServerLB.DeepCopyInto(&out.APIServerLB) + if in.APIServerLB != nil { + in, out := &in.APIServerLB, &out.APIServerLB + *out = new(LoadBalancerSpec) + (*in).DeepCopyInto(*out) + } if in.NodeOutboundLB != nil { in, out := &in.NodeOutboundLB, &out.NodeOutboundLB *out = new(LoadBalancerSpec) diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 6838d0e5a01..d15fe3f0ff7 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -149,38 +149,40 @@ func (s *ClusterScope) PublicIPSpecs() []azure.ResourceSpecGetter { // Public IP specs for control plane lb var controlPlaneOutboundIPSpecs []azure.ResourceSpecGetter - if s.IsAPIServerPrivate() { - // Public IP specs for control plane outbound lb - if s.ControlPlaneOutboundLB() != nil { - for _, ip := range s.ControlPlaneOutboundLB().FrontendIPs { - controlPlaneOutboundIPSpecs = append(controlPlaneOutboundIPSpecs, &publicips.PublicIPSpec{ - Name: ip.PublicIP.Name, + if s.ControlPlaneEnabled() { + if s.IsAPIServerPrivate() { + // Public IP specs for control plane outbound lb + if s.ControlPlaneOutboundLB() != nil { + for _, ip := range s.ControlPlaneOutboundLB().FrontendIPs { + controlPlaneOutboundIPSpecs = append(controlPlaneOutboundIPSpecs, &publicips.PublicIPSpec{ + Name: ip.PublicIP.Name, + ResourceGroup: s.ResourceGroup(), + ClusterName: s.ClusterName(), + DNSName: "", // Set to default value + IsIPv6: false, // Set to default value + Location: s.Location(), + ExtendedLocation: s.ExtendedLocation(), + FailureDomains: s.FailureDomains(), + AdditionalTags: s.AdditionalTags(), + }) + } + } + } else { + controlPlaneOutboundIPSpecs = []azure.ResourceSpecGetter{ + &publicips.PublicIPSpec{ + Name: s.APIServerPublicIP().Name, ResourceGroup: s.ResourceGroup(), + DNSName: s.APIServerPublicIP().DNSName, + IsIPv6: false, // Currently azure requires an IPv4 lb rule to enable IPv6 ClusterName: s.ClusterName(), - DNSName: "", // Set to default value - IsIPv6: false, // Set to default value Location: s.Location(), ExtendedLocation: s.ExtendedLocation(), FailureDomains: s.FailureDomains(), AdditionalTags: s.AdditionalTags(), - }) + IPTags: s.APIServerPublicIP().IPTags, + }, } } - } else { - controlPlaneOutboundIPSpecs = []azure.ResourceSpecGetter{ - &publicips.PublicIPSpec{ - Name: s.APIServerPublicIP().Name, - ResourceGroup: s.ResourceGroup(), - DNSName: s.APIServerPublicIP().DNSName, - IsIPv6: false, // Currently azure requires an IPv4 lb rule to enable IPv6 - ClusterName: s.ClusterName(), - Location: s.Location(), - ExtendedLocation: s.ExtendedLocation(), - FailureDomains: s.FailureDomains(), - AdditionalTags: s.AdditionalTags(), - IPTags: s.APIServerPublicIP().IPTags, - }, - } } publicIPSpecs = append(publicIPSpecs, controlPlaneOutboundIPSpecs...) @@ -241,8 +243,9 @@ func (s *ClusterScope) PublicIPSpecs() []azure.ResourceSpecGetter { // LBSpecs returns the load balancer specs. func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter { - specs := []azure.ResourceSpecGetter{ - &loadbalancers.LBSpec{ + var specs []azure.ResourceSpecGetter + if s.ControlPlaneEnabled() { + specs = append(specs, &loadbalancers.LBSpec{ // API Server LB Name: s.APIServerLB().Name, ResourceGroup: s.ResourceGroup(), @@ -261,7 +264,7 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter { BackendPoolName: s.APIServerLB().BackendPool.Name, IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes, AdditionalTags: s.AdditionalTags(), - }, + }) } // Node outbound LB @@ -669,9 +672,14 @@ func (s *ClusterScope) ControlPlaneRouteTable() infrav1.RouteTable { return subnet.RouteTable } +// ControlPlaneEnabled returns true if the control plane is enabled. +func (s *ClusterScope) ControlPlaneEnabled() bool { + return s.AzureCluster.Spec.ControlPlaneEnabled +} + // APIServerLB returns the cluster API Server load balancer. func (s *ClusterScope) APIServerLB() *infrav1.LoadBalancerSpec { - return &s.AzureCluster.Spec.NetworkSpec.APIServerLB + return s.AzureCluster.Spec.NetworkSpec.APIServerLB } // NodeOutboundLB returns the cluster node outbound load balancer. @@ -691,7 +699,7 @@ func (s *ClusterScope) APIServerLBName() string { // IsAPIServerPrivate returns true if the API Server LB is of type Internal. func (s *ClusterScope) IsAPIServerPrivate() bool { - return s.APIServerLB().Type == infrav1.Internal + return s.APIServerLB() != nil && s.APIServerLB().Type == infrav1.Internal } // APIServerPublicIP returns the API Server public IP. @@ -926,6 +934,9 @@ func (s *ClusterScope) FailureDomains() []*string { // SetControlPlaneSecurityRules sets the default security rules of the control plane subnet. // Note that this is not done in a webhook as it requires a valid Cluster object to exist to get the API Server port. func (s *ClusterScope) SetControlPlaneSecurityRules() { + if !s.ControlPlaneEnabled() { + return + } if s.ControlPlaneSubnet().SecurityGroup.SecurityRules == nil { subnet := s.ControlPlaneSubnet() subnet.SecurityGroup.SecurityRules = infrav1.SecurityRules{ @@ -963,31 +974,33 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() { func (s *ClusterScope) SetDNSName() { // for back compat, set the old API Server defaults if no API Server Spec has been set by new webhooks. lb := s.APIServerLB() - if lb == nil || lb.Name == "" { - lbName := fmt.Sprintf("%s-%s", s.ClusterName(), "public-lb") - ip, dns := s.GenerateLegacyFQDN() - lb = &infrav1.LoadBalancerSpec{ - Name: lbName, - FrontendIPs: []infrav1.FrontendIP{ - { - Name: azure.GenerateFrontendIPConfigName(lbName), - PublicIP: &infrav1.PublicIPSpec{ - Name: ip, - DNSName: dns, + if s.ControlPlaneEnabled() { + if lb == nil || lb.Name == "" { + lbName := fmt.Sprintf("%s-%s", s.ClusterName(), "public-lb") + ip, dns := s.GenerateLegacyFQDN() + lb = &infrav1.LoadBalancerSpec{ + Name: lbName, + FrontendIPs: []infrav1.FrontendIP{ + { + Name: azure.GenerateFrontendIPConfigName(lbName), + PublicIP: &infrav1.PublicIPSpec{ + Name: ip, + DNSName: dns, + }, }, }, - }, - LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ - SKU: infrav1.SKUStandard, - Type: infrav1.Public, - }, + LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ + SKU: infrav1.SKUStandard, + Type: infrav1.Public, + }, + } + lb.DeepCopyInto(s.APIServerLB()) + } + // Generate valid FQDN if not set. + // Note: this function uses the AzureCluster subscription ID. + if !s.IsAPIServerPrivate() && s.APIServerPublicIP().DNSName == "" { + s.APIServerPublicIP().DNSName = s.GenerateFQDN(s.APIServerPublicIP().Name) } - lb.DeepCopyInto(s.APIServerLB()) - } - // Generate valid FQDN if not set. - // Note: this function uses the AzureCluster subscription ID. - if !s.IsAPIServerPrivate() && s.APIServerPublicIP().DNSName == "" { - s.APIServerPublicIP().DNSName = s.GenerateFQDN(s.APIServerPublicIP().Name) } } diff --git a/config/capz/manager_image_patch.yaml b/config/capz/manager_image_patch.yaml index 47f8612fd1f..01467e26f06 100644 --- a/config/capz/manager_image_patch.yaml +++ b/config/capz/manager_image_patch.yaml @@ -8,5 +8,5 @@ spec: spec: containers: # Change the value of image field below to your controller image URL - - image: gcr.io/k8s-staging-cluster-api-azure/cluster-api-azure-controller:main + - image: localhost:5000/cluster-api-azure-controller-amd64:dev name: manager 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 e579119e66f..685d62f7ade 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml @@ -537,6 +537,10 @@ spec: type: object type: array type: object + controlPlaneEnabled: + default: true + description: ControlPlaneEnabled enable control plane cluster components. + type: boolean controlPlaneEndpoint: description: ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. It is not recommended to set diff --git a/controllers/azurecluster_controller.go b/controllers/azurecluster_controller.go index d827291201a..c515e75532d 100644 --- a/controllers/azurecluster_controller.go +++ b/controllers/azurecluster_controller.go @@ -248,12 +248,14 @@ func (acr *AzureClusterReconciler) reconcileNormal(ctx context.Context, clusterS return reconcile.Result{}, wrappedErr } - // Set APIEndpoints so the Cluster API Cluster Controller can pull them - if azureCluster.Spec.ControlPlaneEndpoint.Host == "" { - azureCluster.Spec.ControlPlaneEndpoint.Host = clusterScope.APIServerHost() - } - if azureCluster.Spec.ControlPlaneEndpoint.Port == 0 { - azureCluster.Spec.ControlPlaneEndpoint.Port = clusterScope.APIServerPort() + if azureCluster.Spec.ControlPlaneEnabled { + // Set APIEndpoints so the Cluster API Cluster Controller can pull them + if azureCluster.Spec.ControlPlaneEndpoint.Host == "" { + azureCluster.Spec.ControlPlaneEndpoint.Host = clusterScope.APIServerHost() + } + if azureCluster.Spec.ControlPlaneEndpoint.Port == 0 { + azureCluster.Spec.ControlPlaneEndpoint.Port = clusterScope.APIServerPort() + } } // No errors, so mark us ready so the Cluster API Cluster Controller can pull it diff --git a/controllers/azurecluster_reconciler.go b/controllers/azurecluster_reconciler.go index ec83660e731..580e3043253 100644 --- a/controllers/azurecluster_reconciler.go +++ b/controllers/azurecluster_reconciler.go @@ -114,10 +114,11 @@ func (s *azureClusterService) reconcile(ctx context.Context) error { if err := s.setFailureDomainsForLocation(ctx); err != nil { return errors.Wrap(err, "failed to get availability zones") } - - s.scope.AzureCluster.SetBackendPoolNameDefault() - s.scope.SetDNSName() - s.scope.SetControlPlaneSecurityRules() + if s.scope.ControlPlaneEnabled() { + s.scope.AzureCluster.SetBackendPoolNameDefault() + s.scope.SetDNSName() + s.scope.SetControlPlaneSecurityRules() + } for _, service := range s.services { if err := service.Reconcile(ctx); err != nil { From 845077e653819b7ab927a14a934ac9a1b97af367 Mon Sep 17 00:00:00 2001 From: Rico Pahlisch Date: Mon, 8 Apr 2024 13:52:43 +0200 Subject: [PATCH 2/5] cleanup --- api/v1beta1/azurecluster_default.go | 10 ---- api/v1beta1/azurecluster_default_test.go | 60 ++++++++++----------- api/v1beta1/azurecluster_validation.go | 2 +- api/v1beta1/azurecluster_validation_test.go | 28 +++++----- azure/scope/cluster_test.go | 30 +++++------ azure/scope/machine_test.go | 16 +++--- controllers/azuremachine_controller_test.go | 2 +- 7 files changed, 69 insertions(+), 79 deletions(-) diff --git a/api/v1beta1/azurecluster_default.go b/api/v1beta1/azurecluster_default.go index 51159a00827..ad4f72e0673 100644 --- a/api/v1beta1/azurecluster_default.go +++ b/api/v1beta1/azurecluster_default.go @@ -17,11 +17,7 @@ limitations under the License. package v1beta1 import ( - "context" - "encoding/json" "fmt" - "sigs.k8s.io/controller-runtime/pkg/log" - "k8s.io/utils/ptr" ) @@ -57,26 +53,20 @@ func (c *AzureCluster) setDefaults() { } func (c *AzureCluster) setNetworkSpecDefaults() { - logger := log.FromContext(context.TODO()) - logger.Info(fmt.Sprintf("Was ist hier los %v", c.Spec.ControlPlaneEnabled)) c.setVnetDefaults() c.setBastionDefaults() c.setSubnetDefaults() c.setVnetPeeringDefaults() if c.Spec.ControlPlaneEnabled { - logger.Info("dont call me setAPIServerLBDefaults") c.setAPIServerLBDefaults() } c.SetNodeOutboundLBDefaults() if c.Spec.ControlPlaneEnabled { - logger.Info("dont call me SetControlPlaneOutboundLBDefaults") c.SetControlPlaneOutboundLBDefaults() } if c.Spec.ControlPlaneEnabled { c.Spec.NetworkSpec.APIServerLB = nil } - bolB, _ := json.Marshal(c.Spec) - logger.Info(fmt.Sprintf("Was ist hier los %s", string(bolB))) } func (c *AzureCluster) setResourceGroupDefault() { diff --git a/api/v1beta1/azurecluster_default_test.go b/api/v1beta1/azurecluster_default_test.go index 8ab61074ad0..5d4422142d1 100644 --- a/api/v1beta1/azurecluster_default_test.go +++ b/api/v1beta1/azurecluster_default_test.go @@ -122,7 +122,7 @@ func TestVnetDefaults(t *testing.T) { RouteTable: RouteTable{}, }, }, - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ Name: "my-lb", FrontendIPs: []FrontendIP{ { @@ -1227,7 +1227,7 @@ func TestAPIServerLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ Name: "cluster-test-public-lb", FrontendIPs: []FrontendIP{ { @@ -1259,7 +1259,7 @@ func TestAPIServerLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Internal, }, @@ -1273,7 +1273,7 @@ func TestAPIServerLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ FrontendIPs: []FrontendIP{ { Name: "cluster-test-internal-lb-frontEnd", @@ -1304,7 +1304,7 @@ func TestAPIServerLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Internal, }, @@ -1321,7 +1321,7 @@ func TestAPIServerLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ FrontendIPs: []FrontendIP{ { Name: "cluster-test-internal-lb-frontEnd", @@ -1457,7 +1457,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ @@ -1503,7 +1503,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { RouteTable: RouteTable{}, }, }, - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, }, @@ -1520,7 +1520,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ @@ -1568,7 +1568,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { RouteTable: RouteTable{}, }, }, - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, }, @@ -1603,7 +1603,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ @@ -1667,7 +1667,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { RouteTable: RouteTable{}, }, }, - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, }, @@ -1702,7 +1702,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ @@ -1780,7 +1780,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { RouteTable: RouteTable{}, }, }, - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, }, @@ -1797,7 +1797,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ { SubnetClassSpec: SubnetClassSpec{ @@ -1881,7 +1881,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { RouteTable: RouteTable{}, }, }, - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, }, @@ -1916,7 +1916,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Internal}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Internal}}, }, }, }, @@ -1926,7 +1926,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Internal, }, @@ -1943,7 +1943,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, NodeOutboundLB: &LoadBalancerSpec{ FrontendIPsCount: ptr.To[int32](2), BackendPool: BackendPool{ @@ -1962,7 +1962,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, }, @@ -2005,7 +2005,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ Name: "user-defined-name", LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, @@ -2062,7 +2062,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { RouteTable: RouteTable{}, }, }, - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ Name: "user-defined-name", LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, @@ -2123,7 +2123,7 @@ func TestControlPlaneOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, }, }, }, @@ -2133,7 +2133,7 @@ func TestControlPlaneOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, }, @@ -2150,7 +2150,7 @@ func TestControlPlaneOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Internal}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Internal}}, }, }, }, @@ -2160,7 +2160,7 @@ func TestControlPlaneOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Internal, }, @@ -2177,7 +2177,7 @@ func TestControlPlaneOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Internal}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Internal}}, ControlPlaneOutboundLB: &LoadBalancerSpec{ FrontendIPsCount: ptr.To[int32](2), LoadBalancerClassSpec: LoadBalancerClassSpec{ @@ -2193,7 +2193,7 @@ func TestControlPlaneOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Internal, }, @@ -2236,7 +2236,7 @@ func TestControlPlaneOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Internal}}, + APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Internal}}, ControlPlaneOutboundLB: &LoadBalancerSpec{ BackendPool: BackendPool{ Name: "custom-outbound-lb", @@ -2254,7 +2254,7 @@ func TestControlPlaneOutboundLBDefaults(t *testing.T) { }, Spec: AzureClusterSpec{ NetworkSpec: NetworkSpec{ - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Internal, }, diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go index f4c898eb9f6..c10648e5955 100644 --- a/api/v1beta1/azurecluster_validation.go +++ b/api/v1beta1/azurecluster_validation.go @@ -202,7 +202,7 @@ func validateNetworkSpec(controlPlaneEnabled bool, networkSpec NetworkSpec, old if controlPlaneEnabled { allErrs = append(allErrs, validateControlPlaneOutboundLB(networkSpec.ControlPlaneOutboundLB, networkSpec.APIServerLB, fldPath.Child("controlPlaneOutboundLB"))...) } - var lbType LBType = Internal + var lbType = Internal if networkSpec.APIServerLB != nil { lbType = networkSpec.APIServerLB.Type } diff --git a/api/v1beta1/azurecluster_validation_test.go b/api/v1beta1/azurecluster_validation_test.go index 14709dfd646..25d169baa39 100644 --- a/api/v1beta1/azurecluster_validation_test.go +++ b/api/v1beta1/azurecluster_validation_test.go @@ -551,7 +551,7 @@ func TestClusterSubnetsValid(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - errs := validateSubnets(tc.subnets, createValidVnet(), + errs := validateSubnets(true, tc.subnets, createValidVnet(), field.NewPath("spec").Child("networkSpec").Child("subnets")) g.Expect(errs).To(ConsistOf(tc.err)) }) @@ -571,7 +571,7 @@ func TestSubnetsValid(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateSubnets(testCase.subnets, createValidVnet(), + errs := validateSubnets(true, testCase.subnets, createValidVnet(), field.NewPath("spec").Child("networkSpec").Child("subnets")) g.Expect(errs).To(BeNil()) }) @@ -592,7 +592,7 @@ func TestSubnetsInvalidSubnetName(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateSubnets(testCase.subnets, createValidVnet(), + errs := validateSubnets(true, testCase.subnets, createValidVnet(), field.NewPath("spec").Child("networkSpec").Child("subnets")) g.Expect(errs).To(HaveLen(1)) g.Expect(errs[0].Type).To(Equal(field.ErrorTypeInvalid)) @@ -616,7 +616,7 @@ func TestSubnetsInvalidLackRequiredSubnet(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateSubnets(testCase.subnets, createValidVnet(), + errs := validateSubnets(true, testCase.subnets, createValidVnet(), field.NewPath("spec").Child("networkSpec").Child("subnets")) g.Expect(errs).To(HaveLen(1)) g.Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired)) @@ -641,7 +641,7 @@ func TestSubnetNamesNotUnique(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateSubnets(testCase.subnets, createValidVnet(), + errs := validateSubnets(true, testCase.subnets, createValidVnet(), field.NewPath("spec").Child("networkSpec").Child("subnets")) g.Expect(errs).To(HaveLen(1)) g.Expect(errs[0].Type).To(Equal(field.ErrorTypeDuplicate)) @@ -1034,7 +1034,7 @@ func TestValidateAPIServerLB(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() g := NewWithT(t) - err := validateAPIServerLB(test.lb, test.old, test.cpCIDRS, field.NewPath("apiServerLB")) + err := validateAPIServerLB(&test.lb, &test.old, test.cpCIDRS, field.NewPath("apiServerLB")) if test.wantErr { g.Expect(err).To(ContainElement(MatchError(test.expectedErr.Error()))) } else { @@ -1092,7 +1092,7 @@ func TestPrivateDNSZoneName(t *testing.T) { NetworkClassSpec: NetworkClassSpec{ PrivateDNSZoneName: "good.dns.io", }, - APIServerLB: LoadBalancerSpec{ + APIServerLB: &LoadBalancerSpec{ Name: "my-lb", LoadBalancerClassSpec: LoadBalancerClassSpec{ Type: Public, @@ -1114,7 +1114,7 @@ func TestPrivateDNSZoneName(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() g := NewWithT(t) - err := validatePrivateDNSZoneName(test.network.PrivateDNSZoneName, test.network.APIServerLB.Type, field.NewPath("spec", "networkSpec", "privateDNSZoneName")) + err := validatePrivateDNSZoneName(test.network.PrivateDNSZoneName, true, test.network.APIServerLB.Type, field.NewPath("spec", "networkSpec", "privateDNSZoneName")) if test.wantErr { g.Expect(err).To(ContainElement(MatchError(test.expectedErr.Error()))) } else { @@ -1271,7 +1271,7 @@ func TestValidateNodeOutboundLB(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() g := NewWithT(t) - err := validateNodeOutboundLB(test.lb, test.old, test.apiServerLB, field.NewPath("nodeOutboundLB")) + err := validateNodeOutboundLB(test.lb, test.old, &test.apiServerLB, field.NewPath("nodeOutboundLB")) if test.wantErr { g.Expect(err).To(ContainElement(MatchError(test.expectedErr.Error()))) } else { @@ -1351,7 +1351,7 @@ func TestValidateControlPlaneNodeOutboundLB(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() g := NewWithT(t) - err := validateControlPlaneOutboundLB(test.lb, test.apiServerLB, field.NewPath("controlPlaneOutboundLB")) + err := validateControlPlaneOutboundLB(test.lb, &test.apiServerLB, field.NewPath("controlPlaneOutboundLB")) if test.wantErr { g.Expect(err).To(ContainElement(MatchError(test.expectedErr.Error()))) } else { @@ -1560,8 +1560,8 @@ func createValidVnet() VnetSpec { } } -func createValidAPIServerLB() LoadBalancerSpec { - return LoadBalancerSpec{ +func createValidAPIServerLB() *LoadBalancerSpec { + return &LoadBalancerSpec{ Name: "my-lb", FrontendIPs: []FrontendIP{ { @@ -1585,8 +1585,8 @@ func createValidNodeOutboundLB() *LoadBalancerSpec { } } -func createValidAPIServerInternalLB() LoadBalancerSpec { - return LoadBalancerSpec{ +func createValidAPIServerInternalLB() *LoadBalancerSpec { + return &LoadBalancerSpec{ Name: "my-lb", FrontendIPs: []FrontendIP{ { diff --git a/azure/scope/cluster_test.go b/azure/scope/cluster_test.go index fa60e4262bf..7d6dcb65799 100644 --- a/azure/scope/cluster_test.go +++ b/azure/scope/cluster_test.go @@ -88,7 +88,7 @@ func TestAPIServerHost(t *testing.T) { }, }, NetworkSpec: infrav1.NetworkSpec{ - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ FrontendIPs: []infrav1.FrontendIP{ { PublicIP: &infrav1.PublicIPSpec{ @@ -116,7 +116,7 @@ func TestAPIServerHost(t *testing.T) { }, }, NetworkSpec: infrav1.NetworkSpec{ - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ FrontendIPs: []infrav1.FrontendIP{ { PublicIP: &infrav1.PublicIPSpec{ @@ -147,7 +147,7 @@ func TestAPIServerHost(t *testing.T) { NetworkClassSpec: infrav1.NetworkClassSpec{ PrivateDNSZoneName: "example.private", }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ Type: infrav1.Internal, }, @@ -321,7 +321,7 @@ func TestPublicIPSpecs(t *testing.T) { }, }, NetworkSpec: infrav1.NetworkSpec{ - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ Type: infrav1.Internal, }, @@ -368,7 +368,7 @@ func TestPublicIPSpecs(t *testing.T) { ControlPlaneOutboundLB: &infrav1.LoadBalancerSpec{ FrontendIPsCount: ptr.To[int32](0), }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ Type: infrav1.Internal, }, @@ -424,7 +424,7 @@ func TestPublicIPSpecs(t *testing.T) { }, LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{}, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ Type: infrav1.Internal, }, @@ -506,7 +506,7 @@ func TestPublicIPSpecs(t *testing.T) { }, LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{}, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ Type: infrav1.Internal, }, @@ -593,7 +593,7 @@ func TestPublicIPSpecs(t *testing.T) { ControlPlaneOutboundLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{}, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{}, FrontendIPs: []infrav1.FrontendIP{ { @@ -663,7 +663,7 @@ func TestPublicIPSpecs(t *testing.T) { NodeOutboundLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{}, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ FrontendIPs: []infrav1.FrontendIP{ { PublicIP: &infrav1.PublicIPSpec{ @@ -754,7 +754,7 @@ func TestPublicIPSpecs(t *testing.T) { NodeOutboundLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{}, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ FrontendIPs: []infrav1.FrontendIP{ { PublicIP: &infrav1.PublicIPSpec{ @@ -2161,7 +2161,7 @@ func TestAPIServerLBPoolName(t *testing.T) { }, Spec: infrav1.AzureClusterSpec{ NetworkSpec: infrav1.NetworkSpec{ - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: tc.lbName, }, }, @@ -2311,7 +2311,7 @@ func TestOutboundLBName(t *testing.T) { } if tc.apiServerLB != nil { - azureCluster.Spec.NetworkSpec.APIServerLB = *tc.apiServerLB + azureCluster.Spec.NetworkSpec.APIServerLB = tc.apiServerLB } if tc.controlPlaneOutboundLB != nil { @@ -2436,7 +2436,7 @@ func TestBackendPoolName(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "APIServerLBName", }, ControlPlaneOutboundLB: &infrav1.LoadBalancerSpec{ @@ -3006,7 +3006,7 @@ func TestClusterScope_LBSpecs(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "api-server-lb", BackendPool: infrav1.BackendPool{ Name: "api-server-lb-backend-pool", @@ -3180,7 +3180,7 @@ func TestClusterScope_LBSpecs(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "api-server-lb", BackendPool: infrav1.BackendPool{ Name: "api-server-lb-backend-pool", diff --git a/azure/scope/machine_test.go b/azure/scope/machine_test.go index 487d94499ae..928f58f657e 100644 --- a/azure/scope/machine_test.go +++ b/azure/scope/machine_test.go @@ -302,7 +302,7 @@ func TestMachineScope_PublicIPSpecs(t *testing.T) { }, }, NetworkSpec: infrav1.NetworkSpec{ - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ Type: infrav1.Internal, }, @@ -386,7 +386,7 @@ func TestMachineScope_InboundNatSpecs(t *testing.T) { SubscriptionID: "123", }, NetworkSpec: infrav1.NetworkSpec{ - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "foo-loadbalancer", FrontendIPs: []infrav1.FrontendIP{ { @@ -2206,7 +2206,7 @@ func TestMachineScope_NICSpecs(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "api-lb", LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ Type: infrav1.Internal, @@ -2319,7 +2319,7 @@ func TestMachineScope_NICSpecs(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "api-lb", BackendPool: infrav1.BackendPool{ Name: "api-lb-backendPool", @@ -2429,7 +2429,7 @@ func TestMachineScope_NICSpecs(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "api-lb", BackendPool: infrav1.BackendPool{ Name: "api-lb-backendPool", @@ -2540,7 +2540,7 @@ func TestMachineScope_NICSpecs(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "api-lb", }, NodeOutboundLB: &infrav1.LoadBalancerSpec{ @@ -2680,7 +2680,7 @@ func TestMachineScope_NICSpecs(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "api-lb", }, NodeOutboundLB: &infrav1.LoadBalancerSpec{ @@ -2818,7 +2818,7 @@ func TestMachineScope_NICSpecs(t *testing.T) { }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "api-lb", }, NodeOutboundLB: &infrav1.LoadBalancerSpec{ diff --git a/controllers/azuremachine_controller_test.go b/controllers/azuremachine_controller_test.go index 301decfa71e..7729d288f91 100644 --- a/controllers/azuremachine_controller_test.go +++ b/controllers/azuremachine_controller_test.go @@ -569,7 +569,7 @@ func getFakeAzureCluster(changes ...func(*infrav1.AzureCluster)) *infrav1.AzureC }, }, }, - APIServerLB: infrav1.LoadBalancerSpec{ + APIServerLB: &infrav1.LoadBalancerSpec{ Name: "my-cluster-public-lb", FrontendIPs: []infrav1.FrontendIP{ { From 18009a335a34f93fb9a913af305387885edba921 Mon Sep 17 00:00:00 2001 From: Rico Pahlisch Date: Mon, 8 Apr 2024 15:21:36 +0200 Subject: [PATCH 3/5] fix tests --- api/v1beta1/azurecluster_default.go | 11 +++- api/v1beta1/azurecluster_default_test.go | 12 +++- api/v1beta1/azurecluster_validation.go | 5 -- api/v1beta1/azurecluster_validation_test.go | 70 +++++++++++++++++++-- api/v1beta1/azurecluster_webhook.go | 6 -- azure/scope/cluster_test.go | 31 ++++++--- 6 files changed, 108 insertions(+), 27 deletions(-) diff --git a/api/v1beta1/azurecluster_default.go b/api/v1beta1/azurecluster_default.go index ad4f72e0673..bcbe32bbfd0 100644 --- a/api/v1beta1/azurecluster_default.go +++ b/api/v1beta1/azurecluster_default.go @@ -18,6 +18,7 @@ package v1beta1 import ( "fmt" + "k8s.io/utils/ptr" ) @@ -64,7 +65,7 @@ func (c *AzureCluster) setNetworkSpecDefaults() { if c.Spec.ControlPlaneEnabled { c.SetControlPlaneOutboundLBDefaults() } - if c.Spec.ControlPlaneEnabled { + if !c.Spec.ControlPlaneEnabled { c.Spec.NetworkSpec.APIServerLB = nil } } @@ -218,6 +219,14 @@ func (c *AzureCluster) setVnetPeeringDefaults() { } func (c *AzureCluster) setAPIServerLBDefaults() { + if c.Spec.NetworkSpec.APIServerLB == nil { + lbSpec := LoadBalancerSpec{ + LoadBalancerClassSpec: LoadBalancerClassSpec{ + Type: "Public", + }, + } + c.Spec.NetworkSpec.APIServerLB = &lbSpec + } lb := c.Spec.NetworkSpec.APIServerLB lb.LoadBalancerClassSpec.setAPIServerLBDefaults() diff --git a/api/v1beta1/azurecluster_default_test.go b/api/v1beta1/azurecluster_default_test.go index 5d4422142d1..b8e5abbcdb4 100644 --- a/api/v1beta1/azurecluster_default_test.go +++ b/api/v1beta1/azurecluster_default_test.go @@ -1218,7 +1218,8 @@ func TestAPIServerLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ - NetworkSpec: NetworkSpec{}, + ControlPlaneEnabled: true, + NetworkSpec: NetworkSpec{}, }, }, output: &AzureCluster{ @@ -1226,6 +1227,7 @@ func TestAPIServerLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ APIServerLB: &LoadBalancerSpec{ Name: "cluster-test-public-lb", @@ -1456,6 +1458,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ @@ -1484,6 +1487,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -1519,6 +1523,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ @@ -1548,6 +1553,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -1602,6 +1608,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ @@ -1639,6 +1646,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -1796,6 +1804,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ APIServerLB: &LoadBalancerSpec{LoadBalancerClassSpec: LoadBalancerClassSpec{Type: Public}}, Subnets: Subnets{ @@ -1843,6 +1852,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go index c10648e5955..4670b19e8c4 100644 --- a/api/v1beta1/azurecluster_validation.go +++ b/api/v1beta1/azurecluster_validation.go @@ -17,12 +17,10 @@ limitations under the License. package v1beta1 import ( - "context" "fmt" "net" "reflect" "regexp" - "sigs.k8s.io/controller-runtime/pkg/log" valid "github.com/asaskevich/govalidator" corev1 "k8s.io/api/core/v1" @@ -87,15 +85,12 @@ func (c *AzureCluster) validateCluster(old *AzureCluster) (admission.Warnings, e // validateClusterSpec validates a ClusterSpec. func (c *AzureCluster) validateClusterSpec(old *AzureCluster) field.ErrorList { - logger := log.FromContext(context.TODO()) var allErrs field.ErrorList var oldNetworkSpec NetworkSpec if old != nil { oldNetworkSpec = old.Spec.NetworkSpec } - logger.Info(fmt.Sprintf("ControlPlaneEnabled: %v", c.Spec.ControlPlaneEnabled)) - allErrs = append(allErrs, validateNetworkSpec(c.Spec.ControlPlaneEnabled, c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) var oldCloudProviderConfigOverrides *CloudProviderConfigOverrides diff --git a/api/v1beta1/azurecluster_validation_test.go b/api/v1beta1/azurecluster_validation_test.go index 25d169baa39..7a2c58772af 100644 --- a/api/v1beta1/azurecluster_validation_test.go +++ b/api/v1beta1/azurecluster_validation_test.go @@ -316,7 +316,21 @@ func TestNetworkSpecWithPreexistingVnetValid(t *testing.T) { for _, test := range testCase { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - errs := validateNetworkSpec(true, test.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) + errs := validateNetworkSpec(true, test.networkSpec, NetworkSpec{ + Vnet: VnetSpec{}, + Subnets: nil, + APIServerLB: &LoadBalancerSpec{ + ID: "", + Name: "", + FrontendIPs: nil, + FrontendIPsCount: nil, + BackendPool: BackendPool{}, + LoadBalancerClassSpec: LoadBalancerClassSpec{}, + }, + NodeOutboundLB: nil, + ControlPlaneOutboundLB: nil, + NetworkClassSpec: NetworkClassSpec{}, + }, field.NewPath("spec").Child("networkSpec")) g.Expect(errs).To(BeNil()) }) } @@ -338,7 +352,21 @@ func TestNetworkSpecWithPreexistingVnetLackRequiredSubnets(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) + errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{ + Vnet: VnetSpec{}, + Subnets: nil, + APIServerLB: &LoadBalancerSpec{ + ID: "", + Name: "", + FrontendIPs: nil, + FrontendIPsCount: nil, + BackendPool: BackendPool{}, + LoadBalancerClassSpec: LoadBalancerClassSpec{}, + }, + NodeOutboundLB: nil, + ControlPlaneOutboundLB: nil, + NetworkClassSpec: NetworkClassSpec{}, + }, field.NewPath("spec").Child("networkSpec")) g.Expect(errs).To(HaveLen(1)) g.Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired)) g.Expect(errs[0].Field).To(Equal("spec.networkSpec.subnets")) @@ -361,7 +389,21 @@ func TestNetworkSpecWithPreexistingVnetInvalidResourceGroup(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) + errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{ + Vnet: VnetSpec{}, + Subnets: nil, + APIServerLB: &LoadBalancerSpec{ + ID: "", + Name: "", + FrontendIPs: nil, + FrontendIPsCount: nil, + BackendPool: BackendPool{}, + LoadBalancerClassSpec: LoadBalancerClassSpec{}, + }, + NodeOutboundLB: nil, + ControlPlaneOutboundLB: nil, + NetworkClassSpec: NetworkClassSpec{}, + }, field.NewPath("spec").Child("networkSpec")) g.Expect(errs).To(HaveLen(1)) g.Expect(errs[0].Type).To(Equal(field.ErrorTypeInvalid)) g.Expect(errs[0].Field).To(Equal("spec.networkSpec.vnet.resourceGroup")) @@ -384,7 +426,21 @@ func TestNetworkSpecWithoutPreexistingVnetValid(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { g := NewWithT(t) - errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{}, field.NewPath("spec").Child("networkSpec")) + errs := validateNetworkSpec(true, testCase.networkSpec, NetworkSpec{ + Vnet: VnetSpec{}, + Subnets: nil, + APIServerLB: &LoadBalancerSpec{ + ID: "", + Name: "", + FrontendIPs: nil, + FrontendIPsCount: nil, + BackendPool: BackendPool{}, + LoadBalancerClassSpec: LoadBalancerClassSpec{}, + }, + NodeOutboundLB: nil, + ControlPlaneOutboundLB: nil, + NetworkClassSpec: NetworkClassSpec{}, + }, field.NewPath("spec").Child("networkSpec")) g.Expect(errs).To(BeNil()) }) } @@ -1457,7 +1513,8 @@ func createValidClusterWithClusterSubnet() *AzureCluster { Name: "test-cluster", }, Spec: AzureClusterSpec{ - NetworkSpec: createValidNetworkSpecWithClusterSubnet(), + ControlPlaneEnabled: true, + NetworkSpec: createValidNetworkSpecWithClusterSubnet(), AzureClusterClassSpec: AzureClusterClassSpec{ IdentityRef: &corev1.ObjectReference{ Kind: "AzureClusterIdentity", @@ -1473,7 +1530,8 @@ func createValidCluster() *AzureCluster { Name: "test-cluster", }, Spec: AzureClusterSpec{ - NetworkSpec: createValidNetworkSpec(), + ControlPlaneEnabled: true, + NetworkSpec: createValidNetworkSpec(), AzureClusterClassSpec: AzureClusterClassSpec{ IdentityRef: &corev1.ObjectReference{ Kind: AzureClusterIdentityKind, diff --git a/api/v1beta1/azurecluster_webhook.go b/api/v1beta1/azurecluster_webhook.go index 9685db94660..49cc7c7fe00 100644 --- a/api/v1beta1/azurecluster_webhook.go +++ b/api/v1beta1/azurecluster_webhook.go @@ -17,9 +17,7 @@ limitations under the License. package v1beta1 import ( - "context" "reflect" - "sigs.k8s.io/controller-runtime/pkg/log" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -45,15 +43,11 @@ var _ webhook.Defaulter = &AzureCluster{} // Default implements webhook.Defaulter so a webhook will be registered for the type. func (c *AzureCluster) Default() { - logger := log.FromContext(context.TODO()) - logger.Info("Defaulter") c.setDefaults() } // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. func (c *AzureCluster) ValidateCreate() (admission.Warnings, error) { - logger := log.FromContext(context.TODO()) - logger.Info("ValidateCreate") return c.validateCluster(nil) } diff --git a/azure/scope/cluster_test.go b/azure/scope/cluster_test.go index 7d6dcb65799..5288b69a84e 100644 --- a/azure/scope/cluster_test.go +++ b/azure/scope/cluster_test.go @@ -87,6 +87,7 @@ func TestAPIServerHost(t *testing.T) { Kind: infrav1.AzureClusterIdentityKind, }, }, + ControlPlaneEnabled: true, NetworkSpec: infrav1.NetworkSpec{ APIServerLB: &infrav1.LoadBalancerSpec{ FrontendIPs: []infrav1.FrontendIP{ @@ -115,6 +116,7 @@ func TestAPIServerHost(t *testing.T) { Kind: infrav1.AzureClusterIdentityKind, }, }, + ControlPlaneEnabled: true, NetworkSpec: infrav1.NetworkSpec{ APIServerLB: &infrav1.LoadBalancerSpec{ FrontendIPs: []infrav1.FrontendIP{ @@ -143,6 +145,7 @@ func TestAPIServerHost(t *testing.T) { Kind: infrav1.AzureClusterIdentityKind, }, }, + ControlPlaneEnabled: true, NetworkSpec: infrav1.NetworkSpec{ NetworkClassSpec: infrav1.NetworkClassSpec{ PrivateDNSZoneName: "example.private", @@ -241,6 +244,7 @@ func TestGettingSecurityRules(t *testing.T) { Kind: infrav1.AzureClusterIdentityKind, }, }, + ControlPlaneEnabled: true, NetworkSpec: infrav1.NetworkSpec{ Subnets: infrav1.Subnets{ { @@ -352,7 +356,8 @@ func TestPublicIPSpecs(t *testing.T) { }, }, Spec: infrav1.AzureClusterSpec{ - ResourceGroup: "my-rg", + ResourceGroup: "my-rg", + ControlPlaneEnabled: true, AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ SubscriptionID: "123", Location: "centralIndia", @@ -399,7 +404,8 @@ func TestPublicIPSpecs(t *testing.T) { }, }, Spec: infrav1.AzureClusterSpec{ - ResourceGroup: "my-rg", + ResourceGroup: "my-rg", + ControlPlaneEnabled: true, AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ SubscriptionID: "123", Location: "centralIndia", @@ -469,7 +475,8 @@ func TestPublicIPSpecs(t *testing.T) { }, }, Spec: infrav1.AzureClusterSpec{ - ResourceGroup: "my-rg", + ResourceGroup: "my-rg", + ControlPlaneEnabled: true, AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ SubscriptionID: "123", Location: "centralIndia", @@ -577,7 +584,8 @@ func TestPublicIPSpecs(t *testing.T) { }, }, Spec: infrav1.AzureClusterSpec{ - ResourceGroup: "my-rg", + ResourceGroup: "my-rg", + ControlPlaneEnabled: true, AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ SubscriptionID: "123", Location: "centralIndia", @@ -644,7 +652,8 @@ func TestPublicIPSpecs(t *testing.T) { }, }, Spec: infrav1.AzureClusterSpec{ - ResourceGroup: "my-rg", + ResourceGroup: "my-rg", + ControlPlaneEnabled: true, AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ SubscriptionID: "123", Location: "centralIndia", @@ -714,7 +723,8 @@ func TestPublicIPSpecs(t *testing.T) { }, }, Spec: infrav1.AzureClusterSpec{ - ResourceGroup: "my-rg", + ResourceGroup: "my-rg", + ControlPlaneEnabled: true, BastionSpec: infrav1.BastionSpec{ AzureBastion: &infrav1.AzureBastion{ PublicIP: infrav1.PublicIPSpec{ @@ -2291,6 +2301,7 @@ func TestOutboundLBName(t *testing.T) { }, }, Spec: infrav1.AzureClusterSpec{ + ControlPlaneEnabled: true, AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ SubscriptionID: "123", IdentityRef: &corev1.ObjectReference{ @@ -2427,6 +2438,7 @@ func TestBackendPoolName(t *testing.T) { Kind: infrav1.AzureClusterIdentityKind, }, }, + ControlPlaneEnabled: true, NetworkSpec: infrav1.NetworkSpec{ Subnets: infrav1.Subnets{ { @@ -2549,6 +2561,7 @@ func TestOutboundPoolName(t *testing.T) { }, }, Spec: infrav1.AzureClusterSpec{ + ControlPlaneEnabled: true, AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ SubscriptionID: "123", IdentityRef: &corev1.ObjectReference{ @@ -2986,7 +2999,8 @@ func TestClusterScope_LBSpecs(t *testing.T) { Kind: infrav1.AzureClusterIdentityKind, }, }, - ResourceGroup: "my-rg", + ControlPlaneEnabled: true, + ResourceGroup: "my-rg", NetworkSpec: infrav1.NetworkSpec{ Vnet: infrav1.VnetSpec{ Name: "my-vnet", @@ -3160,7 +3174,8 @@ func TestClusterScope_LBSpecs(t *testing.T) { Kind: infrav1.AzureClusterIdentityKind, }, }, - ResourceGroup: "my-rg", + ControlPlaneEnabled: true, + ResourceGroup: "my-rg", NetworkSpec: infrav1.NetworkSpec{ Vnet: infrav1.VnetSpec{ Name: "my-vnet", From 0883d45da4090a627e6049ff2761cf821e665448 Mon Sep 17 00:00:00 2001 From: Rico Pahlisch Date: Tue, 9 Apr 2024 09:11:35 +0200 Subject: [PATCH 4/5] fix tests --- api/v1beta1/azurecluster_default_test.go | 43 ++++++++++++++++++++---- api/v1beta1/azurecluster_validation.go | 8 +++-- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/api/v1beta1/azurecluster_default_test.go b/api/v1beta1/azurecluster_default_test.go index b8e5abbcdb4..507bb86f139 100644 --- a/api/v1beta1/azurecluster_default_test.go +++ b/api/v1beta1/azurecluster_default_test.go @@ -95,6 +95,7 @@ func TestVnetDefaults(t *testing.T) { Name: "test-cluster", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Vnet: VnetSpec{ ResourceGroup: "custom-vnet", @@ -158,7 +159,8 @@ func TestVnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ - ResourceGroup: "cluster-test", + ControlPlaneEnabled: true, + ResourceGroup: "cluster-test", AzureClusterClassSpec: AzureClusterClassSpec{ IdentityRef: &corev1.ObjectReference{ Kind: AzureClusterIdentityKind, @@ -171,7 +173,8 @@ func TestVnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ - ResourceGroup: "cluster-test", + ControlPlaneEnabled: true, + ResourceGroup: "cluster-test", NetworkSpec: NetworkSpec{ Vnet: VnetSpec{ ResourceGroup: "cluster-test", @@ -196,7 +199,8 @@ func TestVnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ - ResourceGroup: "cluster-test", + ControlPlaneEnabled: true, + ResourceGroup: "cluster-test", NetworkSpec: NetworkSpec{ Vnet: VnetSpec{ VnetClassSpec: VnetClassSpec{ @@ -216,7 +220,8 @@ func TestVnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ - ResourceGroup: "cluster-test", + ControlPlaneEnabled: true, + ResourceGroup: "cluster-test", NetworkSpec: NetworkSpec{ Vnet: VnetSpec{ ResourceGroup: "cluster-test", @@ -241,7 +246,8 @@ func TestVnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ - ResourceGroup: "cluster-test", + ControlPlaneEnabled: true, + ResourceGroup: "cluster-test", NetworkSpec: NetworkSpec{ Vnet: VnetSpec{ VnetClassSpec: VnetClassSpec{ @@ -261,7 +267,8 @@ func TestVnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ - ResourceGroup: "cluster-test", + ControlPlaneEnabled: true, + ResourceGroup: "cluster-test", NetworkSpec: NetworkSpec{ Vnet: VnetSpec{ ResourceGroup: "cluster-test", @@ -308,7 +315,8 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ - NetworkSpec: NetworkSpec{}, + ControlPlaneEnabled: true, + NetworkSpec: NetworkSpec{}, }, }, output: &AzureCluster{ @@ -316,6 +324,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -352,6 +361,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -382,6 +392,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -422,6 +433,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -445,6 +457,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -486,6 +499,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -509,6 +523,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -540,6 +555,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -557,6 +573,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -588,6 +605,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -614,6 +632,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -654,6 +673,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -671,6 +691,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -712,6 +733,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Vnet: VnetSpec{ VnetClassSpec: VnetClassSpec{ @@ -742,6 +764,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Vnet: VnetSpec{ VnetClassSpec: VnetClassSpec{ @@ -779,6 +802,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -838,6 +862,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -913,6 +938,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -948,6 +974,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -1005,6 +1032,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { @@ -1030,6 +1058,7 @@ func TestSubnetDefaults(t *testing.T) { Name: "cluster-test", }, Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, NetworkSpec: NetworkSpec{ Subnets: Subnets{ { diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go index 4670b19e8c4..692be9b1e10 100644 --- a/api/v1beta1/azurecluster_validation.go +++ b/api/v1beta1/azurecluster_validation.go @@ -393,7 +393,11 @@ func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []st var allErrs field.ErrorList lbClassSpec := lb.LoadBalancerClassSpec - olLBClassSpec := old.LoadBalancerClassSpec + var olLBClassSpec LoadBalancerClassSpec + if old != nil { + olLBClassSpec = old.LoadBalancerClassSpec + } + allErrs = append(allErrs, validateClassSpecForAPIServerLB(lbClassSpec, &olLBClassSpec, fldPath)...) // Name should be valid. @@ -401,7 +405,7 @@ func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []st allErrs = append(allErrs, err) } // Name should be immutable. - if old.Name != "" && old.Name != lb.Name { + if old != nil && old.Name != "" && old.Name != lb.Name { allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer name should not be modified after AzureCluster creation.")) } From 125c187e66626755def8edbf6c33f288243d2f78 Mon Sep 17 00:00:00 2001 From: Rico Pahlisch Date: Fri, 26 Jul 2024 09:08:42 +0200 Subject: [PATCH 5/5] PR comments --- api/v1beta1/azurecluster_types.go | 2 +- api/v1beta1/azurecluster_validation.go | 6 +- azure/scope/cluster.go | 85 +++++++++++++------------- config/capz/manager_image_patch.yaml | 2 +- controllers/azurecluster_controller.go | 8 +++ 5 files changed, 57 insertions(+), 46 deletions(-) diff --git a/api/v1beta1/azurecluster_types.go b/api/v1beta1/azurecluster_types.go index a842bbac63b..e6d1799ae9a 100644 --- a/api/v1beta1/azurecluster_types.go +++ b/api/v1beta1/azurecluster_types.go @@ -48,7 +48,7 @@ type AzureClusterSpec struct { // ControlPlaneEnabled enable control plane cluster components. // +kubebuilder:default=true // +optional - ControlPlaneEnabled bool `json:"controlPlaneEnabled"` + ControlPlaneEnabled bool `json:"controlPlaneEnabled,omitempty"` // ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. It is not recommended to set // this when creating an AzureCluster as CAPZ will set this for you. However, if it is set, CAPZ will not change it. diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go index 692be9b1e10..1f5f58d30b2 100644 --- a/api/v1beta1/azurecluster_validation.go +++ b/api/v1beta1/azurecluster_validation.go @@ -224,8 +224,10 @@ func validateSubnets(controlPlaneEnabled bool, subnets Subnets, vnet VnetSpec, f var allErrs field.ErrorList subnetNames := make(map[string]bool, len(subnets)) requiredSubnetRoles := map[string]bool{ - "control-plane": !controlPlaneEnabled, - "node": false, + "node": false, + } + if controlPlaneEnabled { + requiredSubnetRoles["control-plane"] = false } clusterSubnet := false numberofClusterSubnets := 0 diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index d15fe3f0ff7..47b1d099e37 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -149,25 +149,25 @@ func (s *ClusterScope) PublicIPSpecs() []azure.ResourceSpecGetter { // Public IP specs for control plane lb var controlPlaneOutboundIPSpecs []azure.ResourceSpecGetter - if s.ControlPlaneEnabled() { - if s.IsAPIServerPrivate() { - // Public IP specs for control plane outbound lb - if s.ControlPlaneOutboundLB() != nil { - for _, ip := range s.ControlPlaneOutboundLB().FrontendIPs { - controlPlaneOutboundIPSpecs = append(controlPlaneOutboundIPSpecs, &publicips.PublicIPSpec{ - Name: ip.PublicIP.Name, - ResourceGroup: s.ResourceGroup(), - ClusterName: s.ClusterName(), - DNSName: "", // Set to default value - IsIPv6: false, // Set to default value - Location: s.Location(), - ExtendedLocation: s.ExtendedLocation(), - FailureDomains: s.FailureDomains(), - AdditionalTags: s.AdditionalTags(), - }) - } + if s.IsAPIServerPrivate() { + // Public IP specs for control plane outbound lb + if s.ControlPlaneOutboundLB() != nil { + for _, ip := range s.ControlPlaneOutboundLB().FrontendIPs { + controlPlaneOutboundIPSpecs = append(controlPlaneOutboundIPSpecs, &publicips.PublicIPSpec{ + Name: ip.PublicIP.Name, + ResourceGroup: s.ResourceGroup(), + ClusterName: s.ClusterName(), + DNSName: "", // Set to default value + IsIPv6: false, // Set to default value + Location: s.Location(), + ExtendedLocation: s.ExtendedLocation(), + FailureDomains: s.FailureDomains(), + AdditionalTags: s.AdditionalTags(), + }) } - } else { + } + } else { + if s.ControlPlaneEnabled() { controlPlaneOutboundIPSpecs = []azure.ResourceSpecGetter{ &publicips.PublicIPSpec{ Name: s.APIServerPublicIP().Name, @@ -972,35 +972,36 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() { // SetDNSName sets the API Server public IP DNS name. // Note: this logic exists only for purposes of ensuring backwards compatibility for old clusters created without an APIServerLB, and should be removed in the future. func (s *ClusterScope) SetDNSName() { + if !s.ControlPlaneEnabled() { + return + } // for back compat, set the old API Server defaults if no API Server Spec has been set by new webhooks. lb := s.APIServerLB() - if s.ControlPlaneEnabled() { - if lb == nil || lb.Name == "" { - lbName := fmt.Sprintf("%s-%s", s.ClusterName(), "public-lb") - ip, dns := s.GenerateLegacyFQDN() - lb = &infrav1.LoadBalancerSpec{ - Name: lbName, - FrontendIPs: []infrav1.FrontendIP{ - { - Name: azure.GenerateFrontendIPConfigName(lbName), - PublicIP: &infrav1.PublicIPSpec{ - Name: ip, - DNSName: dns, - }, + if lb == nil || lb.Name == "" { + lbName := fmt.Sprintf("%s-%s", s.ClusterName(), "public-lb") + ip, dns := s.GenerateLegacyFQDN() + lb = &infrav1.LoadBalancerSpec{ + Name: lbName, + FrontendIPs: []infrav1.FrontendIP{ + { + Name: azure.GenerateFrontendIPConfigName(lbName), + PublicIP: &infrav1.PublicIPSpec{ + Name: ip, + DNSName: dns, }, }, - LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ - SKU: infrav1.SKUStandard, - Type: infrav1.Public, - }, - } - lb.DeepCopyInto(s.APIServerLB()) - } - // Generate valid FQDN if not set. - // Note: this function uses the AzureCluster subscription ID. - if !s.IsAPIServerPrivate() && s.APIServerPublicIP().DNSName == "" { - s.APIServerPublicIP().DNSName = s.GenerateFQDN(s.APIServerPublicIP().Name) + }, + LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{ + SKU: infrav1.SKUStandard, + Type: infrav1.Public, + }, } + lb.DeepCopyInto(s.APIServerLB()) + } + // Generate valid FQDN if not set. + // Note: this function uses the AzureCluster subscription ID. + if !s.IsAPIServerPrivate() && s.APIServerPublicIP().DNSName == "" { + s.APIServerPublicIP().DNSName = s.GenerateFQDN(s.APIServerPublicIP().Name) } } diff --git a/config/capz/manager_image_patch.yaml b/config/capz/manager_image_patch.yaml index 01467e26f06..47f8612fd1f 100644 --- a/config/capz/manager_image_patch.yaml +++ b/config/capz/manager_image_patch.yaml @@ -8,5 +8,5 @@ spec: spec: containers: # Change the value of image field below to your controller image URL - - image: localhost:5000/cluster-api-azure-controller-amd64:dev + - image: gcr.io/k8s-staging-cluster-api-azure/cluster-api-azure-controller:main name: manager diff --git a/controllers/azurecluster_controller.go b/controllers/azurecluster_controller.go index c515e75532d..811fef2dc14 100644 --- a/controllers/azurecluster_controller.go +++ b/controllers/azurecluster_controller.go @@ -256,6 +256,14 @@ func (acr *AzureClusterReconciler) reconcileNormal(ctx context.Context, clusterS if azureCluster.Spec.ControlPlaneEndpoint.Port == 0 { azureCluster.Spec.ControlPlaneEndpoint.Port = clusterScope.APIServerPort() } + } else { + if azureCluster.Spec.ControlPlaneEndpoint.Host == "" { + conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, "ExternallyManagedControlPlane", clusterv1.ConditionSeverityInfo, "Waiting for the Control Plane host") + return reconcile.Result{}, nil + } else if azureCluster.Spec.ControlPlaneEndpoint.Port == 0 { + conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, "ExternallyManagedControlPlane", clusterv1.ConditionSeverityInfo, "Waiting for the Control Plane port") + return reconcile.Result{}, nil + } } // No errors, so mark us ready so the Cluster API Cluster Controller can pull it