Skip to content

Commit

Permalink
Fix AKS reconciliation of taints
Browse files Browse the repository at this point in the history
  • Loading branch information
willie-yao committed Oct 3, 2023
1 parent 337e5d5 commit fc44a4e
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
25 changes: 24 additions & 1 deletion azure/services/agentpools/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ func (s *AgentPoolSpec) Parameters(ctx context.Context, existing interface{}) (p
defer done()

nodeLabels := s.NodeLabels
nodeTaints := azure.PtrSlice(&s.NodeTaints)
if existing != nil {
existingPool, ok := existing.(armcontainerservice.AgentPool)
if !ok {
Expand Down Expand Up @@ -260,6 +261,9 @@ func (s *AgentPoolSpec) Parameters(ctx context.Context, existing interface{}) (p
nodeLabels = mergeSystemNodeLabels(normalizedProfile.Properties.NodeLabels, existingPool.Properties.NodeLabels)
normalizedProfile.Properties.NodeLabels = nodeLabels

nodeTaints = mergeSystemNodeTaints(normalizedProfile.Properties.NodeTaints, existingPool.Properties.NodeTaints)
normalizedProfile.Properties.NodeTaints = nodeTaints

// Compute a diff to check if we require an update
diff := cmp.Diff(normalizedProfile, existingProfile)
if diff == "" {
Expand All @@ -271,7 +275,6 @@ func (s *AgentPoolSpec) Parameters(ctx context.Context, existing interface{}) (p
}

availabilityZones := azure.PtrSlice(&s.AvailabilityZones)
nodeTaints := azure.PtrSlice(&s.NodeTaints)
var sku *string
if s.SKU != "" {
sku = &s.SKU
Expand Down Expand Up @@ -402,3 +405,23 @@ func mergeSystemNodeLabels(capz, aks map[string]*string) map[string]*string {
}
return ret
}

// mergeSystemNodeTaints appends any kubernetes.azure.com-prefixed taints from the AKS taint set
// into the local capz taint set.
func mergeSystemNodeTaints(capz, aks []*string) []*string {
ret := capz
if ret == nil {
ret = make([]*string, 0)
}
// Look for taints returned from the AKS node pool API that begin with kubernetes.azure.com
for _, aksNodeTaint := range aks {
if azureutil.IsAzureSystemNodeLabelKey(*aksNodeTaint) {
ret = append(ret, aksNodeTaint)
}
}
// Preserve nil-ness of capz
if capz == nil && len(ret) == 0 {
ret = nil
}
return ret
}
39 changes: 39 additions & 0 deletions azure/services/agentpools/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"k8s.io/utils/ptr"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
)

func fakeAgentPool(changes ...func(*AgentPoolSpec)) AgentPoolSpec {
Expand Down Expand Up @@ -149,6 +150,12 @@ func sdkWithSpotMaxPrice(spotMaxPrice float32) func(*armcontainerservice.AgentPo
}
}

func sdkWithNodeTaints(nodeTaints []*string) func(*armcontainerservice.AgentPool) {
return func(pool *armcontainerservice.AgentPool) {
pool.Properties.NodeTaints = nodeTaints
}
}

func TestParameters(t *testing.T) {
testcases := []struct {
name string
Expand Down Expand Up @@ -357,6 +364,38 @@ func TestParameters(t *testing.T) {
expected: nil,
expectedError: nil,
},
{
name: "difference in non-system node taints with empty taints should trigger update",
spec: fakeAgentPool(
func(pool *AgentPoolSpec) {
pool.NodeTaints = nil
},
),
existing: sdkFakeAgentPool(
func(pool *armcontainerservice.AgentPool) {
pool.Properties.NodeTaints = []*string{ptr.To("fake-taint")}
},
sdkWithProvisioningState("Succeeded"),
),
expected: sdkFakeAgentPool(sdkWithNodeTaints(nil)),
expectedError: nil,
},
{
name: "difference in system node taints with empty taints shouldn't trigger update",
spec: fakeAgentPool(
func(pool *AgentPoolSpec) {
pool.NodeTaints = nil
},
),
existing: sdkFakeAgentPool(
func(pool *armcontainerservice.AgentPool) {
pool.Properties.NodeTaints = []*string{ptr.To(azureutil.AzureSystemNodeLabelPrefix + "-fake-taint")}
},
sdkWithProvisioningState("Succeeded"),
),
expected: nil,
expectedError: nil,
},
{
name: "parameters with an existing agent pool and update needed on node taints",
spec: fakeAgentPool(),
Expand Down

0 comments on commit fc44a4e

Please sign in to comment.