Skip to content

Commit

Permalink
move defaulting of amcp sku into defaulting webhook
Browse files Browse the repository at this point in the history
  • Loading branch information
devigned committed Oct 28, 2021
1 parent 66fa35c commit b71c3e3
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 13 deletions.
2 changes: 1 addition & 1 deletion azure/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}

Expand Down
6 changes: 0 additions & 6 deletions azure/services/managedclusters/managedclusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions exp/api/v1alpha4/zz_generated.conversion.go

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

8 changes: 8 additions & 0 deletions exp/api/v1beta1/azuremanagedcontrolplane_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
14 changes: 12 additions & 2 deletions exp/api/v1beta1/azuremanagedcontrolplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,21 @@ type AADProfile struct {
AdminGroupObjectIDs []string `json:"adminGroupObjectIDs"`
}

// AzureManagedControlPlaneSkuTier - Tier of a managed cluster SKU.
// +kubebuilder:validation:Enum=Free;Paid
type AzureManagedControlPlaneSkuTier string

const (
// FreeManagedControlPlaneTier is the free tier of AKS without corresponding SLAs.
FreeManagedControlPlaneTier AzureManagedControlPlaneSkuTier = "Free"
// PaidManagedControlPlaneTier is the paid tier of AKS with corresponding SLAs.
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.
Expand Down
1 change: 1 addition & 0 deletions exp/api/v1beta1/azuremanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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))
Expand All @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion exp/controllers/azuremanagedmachinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controllers

import (
"context"
"fmt"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -227,7 +228,7 @@ func (ammpr *AzureManagedMachinePoolReconciler) Reconcile(ctx context.Context, r
}

func (ammpr *AzureManagedMachinePoolReconciler) reconcileNormal(ctx context.Context, scope *scope.ManagedControlPlaneScope) (reconcile.Result, error) {
ctx, _, done := tele.StartSpanWithLogger(ctx, "controllers.AzureManagedMachinePoolReconciler.reconcileNormal")
ctx, log, done := tele.StartSpanWithLogger(ctx, "controllers.AzureManagedMachinePoolReconciler.reconcileNormal")
defer done()

scope.Logger.Info("Reconciling AzureManagedMachinePool")
Expand All @@ -241,6 +242,7 @@ func (ammpr *AzureManagedMachinePoolReconciler) reconcileNormal(ctx context.Cont

if err := ammpr.createAzureManagedMachinePoolService(scope).Reconcile(ctx); err != nil {
if IsAgentPoolVMSSNotFoundError(err) {
log.V(2).Info(fmt.Sprintf("unable to find agent pool %s/%s", scope.InfraMachinePool.Namespace, scope.InfraMachinePool.Name))
// if the underlying VMSS is not yet created, requeue for 30s in the future
return reconcile.Result{
RequeueAfter: 30 * time.Second,
Expand Down
2 changes: 1 addition & 1 deletion exp/controllers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit b71c3e3

Please sign in to comment.