fix irrecoverable errors in async operations

Reconciliations involving asynchronous operations can fall into a loop
where an eventual "Failed" result can block future reconciliations from
making any further changes to that particular resource to fix the
problem.

This change sets `longRunningOperationStates` for agent pools on the
corresponding AzureManagedMachinePool instead of the
AzureManagedControlPlane, since changes to that resource were not being
persisted. It also only blocks starting new operations on the status of
existing operations of the same type. In-progress PUT operations will no
longer block new DELETEs and in-progress DELETEs will not block
in-progress PUTs.

In cases where polling a future from the Azure API would eventually
return both `isDone==true` and a non-nil error, a "failed checking if
the operation was complete" message would be logged and the error would
refer to the ultimate failure unrelated to polling the future itself.
This change treats all `isDone==true` polling checks as successful and
relies on the operation's error to be captured in the future's `Result`.
The future will always be deleted from the status when the operation is
done so it can be retried the next reconciliation if it failed.

Author: nojnhuh

azure/interfaces.go

@@ -84,8 +84,8 @@ type ClusterDescriber interface {
 // AsyncStatusUpdater is an interface used to keep track of long running operations in Status that has Conditions and Futures.
 type AsyncStatusUpdater interface {
 	SetLongRunningOperationState(*infrav1.Future)
-	GetLongRunningOperationState(string, string) *infrav1.Future
-	DeleteLongRunningOperationState(string, string)
+	GetLongRunningOperationState(string, string, string) *infrav1.Future
+	DeleteLongRunningOperationState(string, string, string)
 	UpdatePutStatus(clusterv1.ConditionType, string, error)
 	UpdateDeleteStatus(clusterv1.ConditionType, string, error)
 	UpdatePatchStatus(clusterv1.ConditionType, string, error)

azure/mock_azure/azure_mock.go

@@ -914,13 +914,13 @@ func (s *ClusterScope) SetLongRunningOperationState(future *infrav1.Future) {
 }
 
 // GetLongRunningOperationState will get the future on the AzureCluster status.
-func (s *ClusterScope) GetLongRunningOperationState(name, service string) *infrav1.Future {
-	return futures.Get(s.AzureCluster, name, service)
+func (s *ClusterScope) GetLongRunningOperationState(name, service, futureType string) *infrav1.Future {
+	return futures.Get(s.AzureCluster, name, service, futureType)
 }
 
 // DeleteLongRunningOperationState will delete the future from the AzureCluster status.
-func (s *ClusterScope) DeleteLongRunningOperationState(name, service string) {
-	futures.Delete(s.AzureCluster, name, service)
+func (s *ClusterScope) DeleteLongRunningOperationState(name, service, futureType string) {
+	futures.Delete(s.AzureCluster, name, service, futureType)
 }
 
 // UpdateDeleteStatus updates a condition on the AzureCluster status after a DELETE operation.

azure/scope/cluster.go

@@ -359,9 +359,9 @@ func (m *MachineScope) Subnet() infrav1.SubnetSpec {
 
 // AvailabilityZone returns the AzureMachine Availability Zone.
 // Priority for selecting the AZ is
-//   1) Machine.Spec.FailureDomain
-//   2) AzureMachine.Spec.FailureDomain (This is to support deprecated AZ)
-//   3) No AZ
+//  1. Machine.Spec.FailureDomain
+//  2. AzureMachine.Spec.FailureDomain (This is to support deprecated AZ)
+//  3. No AZ
 func (m *MachineScope) AvailabilityZone() string {
 	if m.Machine.Spec.FailureDomain != nil {
 		return *m.Machine.Spec.FailureDomain
@@ -687,13 +687,13 @@ func (m *MachineScope) SetLongRunningOperationState(future *infrav1.Future) {
 }
 
 // GetLongRunningOperationState will get the future on the AzureMachine status.
-func (m *MachineScope) GetLongRunningOperationState(name, service string) *infrav1.Future {
-	return futures.Get(m.AzureMachine, name, service)
+func (m *MachineScope) GetLongRunningOperationState(name, service, futureType string) *infrav1.Future {
+	return futures.Get(m.AzureMachine, name, service, futureType)
 }
 
 // DeleteLongRunningOperationState will delete the future from the AzureMachine status.
-func (m *MachineScope) DeleteLongRunningOperationState(name, service string) {
-	futures.Delete(m.AzureMachine, name, service)
+func (m *MachineScope) DeleteLongRunningOperationState(name, service, futureType string) {
+	futures.Delete(m.AzureMachine, name, service, futureType)
 }
 
 // UpdateDeleteStatus updates a condition on the AzureMachine status after a DELETE operation.

azure/scope/machine.go

@@ -297,7 +297,9 @@ func (m *MachinePoolScope) applyAzureMachinePoolMachines(ctx context.Context) er
 		return nil
 	}
 
-	if futures.Has(m.AzureMachinePool, m.Name(), ScalesetsServiceName) {
+	if futures.Has(m.AzureMachinePool, m.Name(), ScalesetsServiceName, infrav1.PatchFuture) ||
+		futures.Has(m.AzureMachinePool, m.Name(), ScalesetsServiceName, infrav1.PutFuture) ||
+		futures.Has(m.AzureMachinePool, m.Name(), ScalesetsServiceName, infrav1.DeleteFuture) {
 		log.V(4).Info("exiting early due an in-progress long running operation on the ScaleSet")
 		// exit early to be less greedy about delete
 		return nil
@@ -377,13 +379,13 @@ func (m *MachinePoolScope) SetLongRunningOperationState(future *infrav1.Future)
 }
 
 // GetLongRunningOperationState will get the future on the AzureMachinePool status.
-func (m *MachinePoolScope) GetLongRunningOperationState(name, service string) *infrav1.Future {
-	return futures.Get(m.AzureMachinePool, name, service)
+func (m *MachinePoolScope) GetLongRunningOperationState(name, service, futureType string) *infrav1.Future {
+	return futures.Get(m.AzureMachinePool, name, service, futureType)
 }
 
 // DeleteLongRunningOperationState will delete the future from the AzureMachinePool status.
-func (m *MachinePoolScope) DeleteLongRunningOperationState(name, service string) {
-	futures.Delete(m.AzureMachinePool, name, service)
+func (m *MachinePoolScope) DeleteLongRunningOperationState(name, service, futureType string) {
+	futures.Delete(m.AzureMachinePool, name, service, futureType)
 }
 
 // setProvisioningStateAndConditions sets the AzureMachinePool provisioning state and conditions.

azure/scope/machinepool.go

@@ -170,13 +170,13 @@ func (s *MachinePoolMachineScope) SetLongRunningOperationState(future *infrav1.F
 }
 
 // GetLongRunningOperationState will get the future on the AzureMachinePoolMachine status.
-func (s *MachinePoolMachineScope) GetLongRunningOperationState(name, service string) *infrav1.Future {
-	return futures.Get(s.AzureMachinePoolMachine, name, service)
+func (s *MachinePoolMachineScope) GetLongRunningOperationState(name, service, futureType string) *infrav1.Future {
+	return futures.Get(s.AzureMachinePoolMachine, name, service, futureType)
 }
 
 // DeleteLongRunningOperationState will delete the future from the AzureMachinePoolMachine status.
-func (s *MachinePoolMachineScope) DeleteLongRunningOperationState(name, service string) {
-	futures.Delete(s.AzureMachinePoolMachine, name, service)
+func (s *MachinePoolMachineScope) DeleteLongRunningOperationState(name, service, futureType string) {
+	futures.Delete(s.AzureMachinePoolMachine, name, service, futureType)
 }
 
 // UpdateDeleteStatus updates a condition on the AzureMachinePoolMachine status after a DELETE operation.

azure/scope/machinepoolmachine.go

@@ -561,13 +561,13 @@ func (s *ManagedControlPlaneScope) SetLongRunningOperationState(future *infrav1.
 }
 
 // GetLongRunningOperationState will get the future on the AzureManagedControlPlane status.
-func (s *ManagedControlPlaneScope) GetLongRunningOperationState(name, service string) *infrav1.Future {
-	return futures.Get(s.ControlPlane, name, service)
+func (s *ManagedControlPlaneScope) GetLongRunningOperationState(name, service, futureType string) *infrav1.Future {
+	return futures.Get(s.ControlPlane, name, service, futureType)
 }
 
 // DeleteLongRunningOperationState will delete the future from the AzureManagedControlPlane status.
-func (s *ManagedControlPlaneScope) DeleteLongRunningOperationState(name, service string) {
-	futures.Delete(s.ControlPlane, name, service)
+func (s *ManagedControlPlaneScope) DeleteLongRunningOperationState(name, service, futureType string) {
+	futures.Delete(s.ControlPlane, name, service, futureType)
 }
 
 // UpdateDeleteStatus updates a condition on the AzureManagedControlPlane status after a DELETE operation.

azure/scope/managedcontrolplane.go

@@ -221,20 +221,20 @@ func (s *ManagedMachinePoolScope) SetAgentPoolReady(ready bool) {
 	s.InfraMachinePool.Status.Ready = ready
 }
 
-// SetLongRunningOperationState will set the future on the AzureManagedControlPlane status to allow the resource to continue
+// SetLongRunningOperationState will set the future on the AzureManagedMachinePool status to allow the resource to continue
 // in the next reconciliation.
 func (s *ManagedMachinePoolScope) SetLongRunningOperationState(future *infrav1.Future) {
-	futures.Set(s.ControlPlane, future)
+	futures.Set(s.InfraMachinePool, future)
 }
 
-// GetLongRunningOperationState will get the future on the AzureManagedControlPlane status.
-func (s *ManagedMachinePoolScope) GetLongRunningOperationState(name, service string) *infrav1.Future {
-	return futures.Get(s.ControlPlane, name, service)
+// GetLongRunningOperationState will get the future on the AzureManagedMachinePool status.
+func (s *ManagedMachinePoolScope) GetLongRunningOperationState(name, service, futureType string) *infrav1.Future {
+	return futures.Get(s.InfraMachinePool, name, service, futureType)
 }
 
-// DeleteLongRunningOperationState will delete the future from the AzureManagedControlPlane status.
-func (s *ManagedMachinePoolScope) DeleteLongRunningOperationState(name, service string) {
-	futures.Delete(s.ControlPlane, name, service)
+// DeleteLongRunningOperationState will delete the future from the AzureManagedMachinePool status.
+func (s *ManagedMachinePoolScope) DeleteLongRunningOperationState(name, service, futureType string) {
+	futures.Delete(s.InfraMachinePool, name, service, futureType)
 }
 
 // UpdateDeleteStatus updates a condition on the AzureManagedControlPlane status after a DELETE operation.

azure/scope/managedmachinepool.go

@@ -131,12 +131,7 @@ func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAP
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "agentpools.azureClient.IsDone")
 	defer done()
 
-	isDone, err = future.DoneWithContext(ctx, ac.agentpools)
-	if err != nil {
-		return false, errors.Wrap(err, "failed checking if the operation was complete")
-	}
-
-	return isDone, nil
+	return future.DoneWithContext(ctx, ac.agentpools)
 }
 
 // Result fetches the result of a long-running operation future.