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 2, 2023
1 parent 337e5d5 commit f59ad10
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
27 changes: 26 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,18 +261,22 @@ 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 == "" {
// agent pool is up to date, nothing to do
log.V(4).Info("no changes found between user-updated spec and existing spec")
fmt.Printf("no changes found between user-updated spec and existing spec\n")
return nil, nil
}
log.V(4).Info("found a diff between the desired spec and the existing agentpool", "difference", diff)
fmt.Printf("found a diff between the desired spec and the existing agentpool: %s\n", diff)
}

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 +407,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
}
16 changes: 16 additions & 0 deletions azure/services/agentpools/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,22 @@ func TestParameters(t *testing.T) {
expected: 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("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 f59ad10

Please sign in to comment.