fix: (azure)machinepools: stop continuous reconciliation of resources by sorting ProviderIDs

Author: damdo

azure/scope/machinepool.go

@@ -21,6 +21,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
+	"slices"
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2"
@@ -357,6 +358,9 @@ func (m *MachinePoolScope) updateReplicasAndProviderIDs(ctx context.Context) err
 		providerIDs[i] = machine.Spec.ProviderID
 	}
 
+	// Sort providerIDs to ensure deterministic ordering to prevent continuous reconciliation.
+	slices.Sort(providerIDs)
+
 	m.AzureMachinePool.Status.Replicas = readyReplicas
 	m.AzureMachinePool.Spec.ProviderIDList = providerIDs
 	return nil

azure/scope/machinepool_test.go

@@ -647,7 +647,7 @@ func TestMachinePoolScope_updateReplicasAndProviderIDs(t *testing.T) {
 			Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, err error) {
 				g.Expect(err).NotTo(HaveOccurred())
 				g.Expect(amp.Status.Replicas).To(BeEquivalentTo(3))
-				g.Expect(amp.Spec.ProviderIDList).To(ConsistOf("azure://foo/ampm0", "azure://foo/ampm1", "azure://foo/ampm2"))
+				g.Expect(amp.Spec.ProviderIDList).To(Equal([]string{"azure://foo/ampm0", "azure://foo/ampm1", "azure://foo/ampm2"}))
 			},
 		},
 		{
@@ -680,6 +680,32 @@ func TestMachinePoolScope_updateReplicasAndProviderIDs(t *testing.T) {
 				g.Expect(amp.Status.Replicas).To(BeEquivalentTo(2))
 			},
 		},
+		{
+			Name: "providerIDList should be sorted for deterministic ordering",
+			Setup: func(cb *fake.ClientBuilder) {
+				// Create machines with provider IDs that would be unsorted if not explicitly sorted
+				machines := getReadyAzureMachinePoolMachines(3)
+				// Swap provider IDs to create non-alphabetical order: ampm2, ampm0, ampm1
+				machines[0].Spec.ProviderID = "azure://foo/ampm2"
+				machines[1].Spec.ProviderID = "azure://foo/ampm0"
+				machines[2].Spec.ProviderID = "azure://foo/ampm1"
+				for _, machine := range machines {
+					obj := machine
+					cb.WithObjects(&obj)
+				}
+			},
+			Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, err error) {
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(amp.Status.Replicas).To(BeEquivalentTo(3))
+				// Verify provider IDs are sorted alphabetically to ensure deterministic ordering
+				// and prevent continuous reconciliation due to order changes
+				g.Expect(amp.Spec.ProviderIDList).To(Equal([]string{
+					"azure://foo/ampm0",
+					"azure://foo/ampm1",
+					"azure://foo/ampm2",
+				}))
+			},
+		},
 	}
 
 	for _, c := range cases {

controllers/azuremanagedmachinepool_controller_test.go

@@ -61,8 +61,14 @@ func TestAzureManagedMachinePoolReconcile(t *testing.T) {
 			Setup: func(cb *fake.ClientBuilder, reconciler pausingReconciler, agentpools *mock_agentpools.MockAgentPoolScopeMockRecorder, nodelister *MockNodeListerMockRecorder) {
 				cluster, azManagedCluster, azManagedControlPlane, ammp, mp := newReadyAzureManagedMachinePoolCluster()
 				fakeAgentPoolSpec := fakeAgentPool()
-				providerIDs := []string{"azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156"}
+				// Provider IDs should be sorted alphabetically for deterministic ordering (100 < 12 < 156)
+				providerIDs := []string{
+					"azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/100",
+					"azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/12",
+					"azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
+				}
 				fakeVirtualMachineScaleSet := fakeVirtualMachineScaleSet()
+				// fakeVirtualMachineScaleSetVM returns VMs in unsorted order (156, 12, 100)
 				fakeVirtualMachineScaleSetVM := fakeVirtualMachineScaleSetVM()
 
 				reconciler.MockReconciler.EXPECT().Reconcile(gomock2.AContext()).Return(nil)
@@ -346,22 +352,26 @@ func fakeAgentPool(changes ...func(*agentpools.AgentPoolSpec)) agentpools.AgentP
 	return pool
 }
 
+// fakeVirtualMachineScaleSetVM returns VMs in non-sorted order (156, 12, 100) to verify
+// that provider IDs are sorted for deterministic ordering.
 func fakeVirtualMachineScaleSetVM() []armcompute.VirtualMachineScaleSetVM {
-	virtualMachineScaleSetVM := []armcompute.VirtualMachineScaleSetVM{
+	return []armcompute.VirtualMachineScaleSetVM{
 		{
-			InstanceID: ptr.To("0"),
+			InstanceID: ptr.To("2"),
 			ID:         ptr.To("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156"),
+			Name:       ptr.To("vm2"),
+		},
+		{
+			InstanceID: ptr.To("0"),
+			ID:         ptr.To("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/12"),
 			Name:       ptr.To("vm0"),
-			Zones:      []*string{ptr.To("zone0")},
-			Properties: &armcompute.VirtualMachineScaleSetVMProperties{
-				ProvisioningState: ptr.To("Succeeded"),
-				OSProfile: &armcompute.OSProfile{
-					ComputerName: ptr.To("instance-000000"),
-				},
-			},
+		},
+		{
+			InstanceID: ptr.To("1"),
+			ID:         ptr.To("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/100"),
+			Name:       ptr.To("vm1"),
 		},
 	}
-	return virtualMachineScaleSetVM
 }
 
 func fakeVirtualMachineScaleSet() []armcompute.VirtualMachineScaleSet {

controllers/azuremanagedmachinepool_reconciler.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"slices"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
@@ -164,6 +165,8 @@ func (s *azureManagedMachinePoolService) Reconcile(ctx context.Context) error {
 		providerIDs[i] = providerID
 	}
 
+	// Sort providerIDs to ensure deterministic ordering to prevent continuous reconciliation.
+	slices.Sort(providerIDs)
 	s.scope.SetAgentPoolProviderIDList(providerIDs)
 	s.scope.SetAgentPoolReplicas(int32(len(providerIDs)))
 	s.scope.SetAgentPoolReady(true)