From 98fb4b619129c8d81406c1ed92b1443d1016db6d Mon Sep 17 00:00:00 2001 From: David Justice Date: Wed, 27 Oct 2021 18:56:19 -0400 Subject: [PATCH] move defaulting of amcp sku into defaulting webhook --- azure/scope/managedcontrolplane.go | 2 +- azure/services/managedclusters/managedclusters.go | 6 ------ exp/api/v1alpha4/zz_generated.conversion.go | 4 ++-- exp/api/v1beta1/azuremanagedcontrolplane_default.go | 8 ++++++++ exp/api/v1beta1/azuremanagedcontrolplane_types.go | 12 ++++++++++-- exp/api/v1beta1/azuremanagedcontrolplane_webhook.go | 1 + .../v1beta1/azuremanagedcontrolplane_webhook_test.go | 3 +++ exp/controllers/helpers.go | 2 +- 8 files changed, 26 insertions(+), 12 deletions(-) diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 6ac1515615cc..bbcb9bd90053 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -424,7 +424,7 @@ func (s *ManagedControlPlaneScope) ManagedClusterSpec() (azure.ManagedClusterSpe if s.ControlPlane.Spec.SKU != nil { managedClusterSpec.SKU = &azure.SKU{ - Tier: s.ControlPlane.Spec.SKU.Tier, + Tier: string(s.ControlPlane.Spec.SKU.Tier), } } diff --git a/azure/services/managedclusters/managedclusters.go b/azure/services/managedclusters/managedclusters.go index 9bd0381b0fe0..5f195f2c7f83 100644 --- a/azure/services/managedclusters/managedclusters.go +++ b/azure/services/managedclusters/managedclusters.go @@ -259,12 +259,6 @@ func (s *Service) Reconcile(ctx context.Context) error { Name: containerservice.ManagedClusterSKUNameBasic, Tier: tierName, } - } else { - // Add the default sku so that the diff will match if no sku is specified by the spec. - managedCluster.Sku = &containerservice.ManagedClusterSKU{ - Name: containerservice.ManagedClusterSKUNameBasic, - Tier: containerservice.ManagedClusterSKUTierFree, - } } if managedClusterSpec.LoadBalancerProfile != nil { diff --git a/exp/api/v1alpha4/zz_generated.conversion.go b/exp/api/v1alpha4/zz_generated.conversion.go index bf819586e87c..2d75e1f19233 100644 --- a/exp/api/v1alpha4/zz_generated.conversion.go +++ b/exp/api/v1alpha4/zz_generated.conversion.go @@ -1311,7 +1311,7 @@ func Convert_v1beta1_ManagedControlPlaneVirtualNetwork_To_v1alpha4_ManagedContro } func autoConvert_v1alpha4_SKU_To_v1beta1_SKU(in *SKU, out *v1beta1.SKU, s conversion.Scope) error { - out.Tier = in.Tier + out.Tier = v1beta1.AzureManagedControlPlaneSkuTier(in.Tier) return nil } @@ -1321,7 +1321,7 @@ func Convert_v1alpha4_SKU_To_v1beta1_SKU(in *SKU, out *v1beta1.SKU, s conversion } func autoConvert_v1beta1_SKU_To_v1alpha4_SKU(in *v1beta1.SKU, out *SKU, s conversion.Scope) error { - out.Tier = in.Tier + out.Tier = string(in.Tier) return nil } diff --git a/exp/api/v1beta1/azuremanagedcontrolplane_default.go b/exp/api/v1beta1/azuremanagedcontrolplane_default.go index be0bb691875c..e22c8358c1d0 100644 --- a/exp/api/v1beta1/azuremanagedcontrolplane_default.go +++ b/exp/api/v1beta1/azuremanagedcontrolplane_default.go @@ -73,3 +73,11 @@ func (r *AzureManagedControlPlane) setDefaultSubnet() { r.Spec.VirtualNetwork.Subnet.CIDRBlock = defaultAKSNodeSubnetCIDR } } + +func (r *AzureManagedControlPlane) setDefaultSku() { + if r.Spec.SKU == nil { + r.Spec.SKU = &SKU{ + Tier: FreeManagedControlPlaneTier, + } + } +} diff --git a/exp/api/v1beta1/azuremanagedcontrolplane_types.go b/exp/api/v1beta1/azuremanagedcontrolplane_types.go index 381e99c4ddc4..7a01e8012d25 100644 --- a/exp/api/v1beta1/azuremanagedcontrolplane_types.go +++ b/exp/api/v1beta1/azuremanagedcontrolplane_types.go @@ -122,11 +122,19 @@ type AADProfile struct { AdminGroupObjectIDs []string `json:"adminGroupObjectIDs"` } +// AzureManagedControlPlaneSkuTier - Tier of a managed cluster SKU. +// +kubebuilder:validation:Enum=Free;Paid +type AzureManagedControlPlaneSkuTier string + +const ( + FreeManagedControlPlaneTier AzureManagedControlPlaneSkuTier = "Free" + PaidManagedControlPlaneTier AzureManagedControlPlaneSkuTier = "Paid" +) + // SKU - AKS SKU. type SKU struct { // Tier - Tier of a managed cluster SKU. - // +kubebuilder:validation:Enum=Free;Paid - Tier string `json:"tier"` + Tier AzureManagedControlPlaneSkuTier `json:"tier"` } // LoadBalancerProfile - Profile of the cluster load balancer. diff --git a/exp/api/v1beta1/azuremanagedcontrolplane_webhook.go b/exp/api/v1beta1/azuremanagedcontrolplane_webhook.go index 5e0aab3e1dd8..2f78ab6e1187 100644 --- a/exp/api/v1beta1/azuremanagedcontrolplane_webhook.go +++ b/exp/api/v1beta1/azuremanagedcontrolplane_webhook.go @@ -80,6 +80,7 @@ func (r *AzureManagedControlPlane) Default() { r.setDefaultNodeResourceGroupName() r.setDefaultVirtualNetwork() r.setDefaultSubnet() + r.setDefaultSku() } // +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedcontrolplane,mutating=false,failurePolicy=fail,groups=infrastructure.cluster.x-k8s.io,resources=azuremanagedcontrolplanes,versions=v1beta1,name=validation.azuremanagedcontrolplanes.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 diff --git a/exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go b/exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go index ae52fdbf0d9b..9f0307a2b60d 100644 --- a/exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go +++ b/exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go @@ -49,6 +49,7 @@ func TestDefaultingWebhook(t *testing.T) { g.Expect(amcp.Spec.NodeResourceGroupName).To(Equal("MC_fooRg_fooName_fooLocation")) g.Expect(amcp.Spec.VirtualNetwork.Name).To(Equal("fooName")) g.Expect(amcp.Spec.VirtualNetwork.Subnet.Name).To(Equal("fooName")) + g.Expect(amcp.Spec.SKU.Tier).To(Equal(FreeManagedControlPlaneTier)) t.Logf("Testing amcp defaulting webhook with baseline") netPlug := "kubenet" @@ -62,6 +63,7 @@ func TestDefaultingWebhook(t *testing.T) { amcp.Spec.NodeResourceGroupName = "fooNodeRg" amcp.Spec.VirtualNetwork.Name = "fooVnetName" amcp.Spec.VirtualNetwork.Subnet.Name = "fooSubnetName" + amcp.Spec.SKU.Tier = PaidManagedControlPlaneTier amcp.Default() g.Expect(*amcp.Spec.NetworkPlugin).To(Equal(netPlug)) g.Expect(*amcp.Spec.LoadBalancerSKU).To(Equal(lbSKU)) @@ -71,6 +73,7 @@ func TestDefaultingWebhook(t *testing.T) { g.Expect(amcp.Spec.NodeResourceGroupName).To(Equal("fooNodeRg")) g.Expect(amcp.Spec.VirtualNetwork.Name).To(Equal("fooVnetName")) g.Expect(amcp.Spec.VirtualNetwork.Subnet.Name).To(Equal("fooSubnetName")) + g.Expect(amcp.Spec.SKU.Tier).To(Equal(PaidManagedControlPlaneTier)) } func TestValidatingWebhook(t *testing.T) { diff --git a/exp/controllers/helpers.go b/exp/controllers/helpers.go index ab2c3d5eca95..62152e80e258 100644 --- a/exp/controllers/helpers.go +++ b/exp/controllers/helpers.go @@ -414,7 +414,7 @@ func MachinePoolToAzureManagedControlPlaneMapFunc(ctx context.Context, c client. ammp := &infrav1exp.AzureManagedMachinePool{} key := types.NamespacedName{Namespace: infraMachinePoolRef.Namespace, Name: infraMachinePoolRef.Name} if err := c.Get(ctx, key, ammp); err != nil { - log.Error(err, "failed to fetch azure managed machine pool for Machinepool: %s", infraMachinePoolRef.Name) + log.Error(err, fmt.Sprintf("failed to fetch azure managed machine pool for Machinepool: %s", infraMachinePoolRef.Name)) // If we get here, we might want to reconcile but aren't sure. // Do it anyway to be safe. Worst case we reconcile a few extra times with no-ops. return []reconcile.Request{