Merge pull request #1032 from shysank/feature/1014

Handle terminal errors to prevent reconciliation failure loop

Author: k8s-ci-robot

cloud/errors.go

@@ -19,6 +19,7 @@ package azure
 import (
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/Azure/go-autorest/autorest"
 )
@@ -41,3 +42,66 @@ type VMDeletedError struct {
 func (vde VMDeletedError) Error() string {
 	return fmt.Sprintf("VM with provider id %q has been deleted", vde.ProviderID)
 }
+
+// ReconcileError represents an error that is not automatically recoverable
+// errorType indicates what type of action is required to recover. It can take two values
+// 1. `Transient` - Can be recovered through manual intervention, will be requeued after
+// 2. `Terminal` - Cannot be recovered, will not be requeued
+type ReconcileError struct {
+	error
+	errorType    ReconcileErrorType
+	requestAfter time.Duration
+}
+
+// ReconcileErrorType represents the type of a ReconcileError
+type ReconcileErrorType string
+
+const (
+	// TransientErrorType can be recovered, will be requeued after a configured time interval
+	TransientErrorType ReconcileErrorType = "Transient"
+	// TerminalErrorType cannot be recovered, will not be requeued
+	TerminalErrorType ReconcileErrorType = "Terminal"
+)
+
+// Error returns the error message for a ReconcileError
+func (t ReconcileError) Error() string {
+	var errStr string
+	if t.error != nil {
+		errStr = t.error.Error()
+	}
+	switch t.errorType {
+	case TransientErrorType:
+		return fmt.Sprintf("reconcile error occurred that can be recovered. Object will be requeued after %s "+
+			"The actual error is: %s", t.requestAfter.String(), errStr)
+	case TerminalErrorType:
+		return fmt.Sprintf("reconcile error occurred that cannot be recovered. Object will not be requeued. "+
+			"The actual error is: %s", errStr)
+	default:
+		return fmt.Sprintf("reconcile error occurred with unknown recovery type. The actual error is: %s", errStr)
+	}
+}
+
+// IsTransient returns if the ReconcileError is recoverable
+func (t ReconcileError) IsTransient() bool {
+	return t.errorType == TransientErrorType
+}
+
+// IsTerminal returns if the ReconcileError is recoverable
+func (t ReconcileError) IsTerminal() bool {
+	return t.errorType == TerminalErrorType
+}
+
+// RequeueAfter returns requestAfter value
+func (t ReconcileError) RequeueAfter() time.Duration {
+	return t.requestAfter
+}
+
+// WithTransientError wraps the error in a ReconcileError with errorType as `Transient`
+func WithTransientError(err error, requeueAfter time.Duration) ReconcileError {
+	return ReconcileError{error: err, errorType: TransientErrorType, requestAfter: requeueAfter}
+}
+
+// WithTerminalError wraps the error in a ReconcileError with errorType as `Terminal`
+func WithTerminalError(err error) ReconcileError {
+	return ReconcileError{error: err, errorType: TerminalErrorType}
+}

cloud/services/scalesets/scalesets.go

@@ -407,7 +407,7 @@ func getSecurityProfile(vmssSpec azure.ScaleSetSpec, sku resourceskus.SKU) (*com
 	}
 
 	if !sku.HasCapability(resourceskus.EncryptionAtHost) {
-		return nil, errors.Errorf("encryption at host is not supported for VM type %s", vmssSpec.Size)
+		return nil, azure.WithTerminalError(errors.Errorf("encryption at host is not supported for VM type %s", vmssSpec.Size))
 	}
 
 	return &compute.SecurityProfile{

cloud/services/scalesets/scalesets_test.go

@@ -973,7 +973,7 @@ func TestReconcileVMSS(t *testing.T) {
 		},
 		{
 			name:          "creating a vmss with encryption at host enabled for unsupported VM type fails",
-			expectedError: "encryption at host is not supported for VM type VM_SIZE",
+			expectedError: "reconcile error occurred that cannot be recovered. Object will not be requeued. The actual error is: encryption at host is not supported for VM type VM_SIZE",
 			expect: func(g *gomega.WithT, s *mock_scalesets.MockScaleSetScopeMockRecorder, m *mock_scalesets.MockClientMockRecorder) {
 				s.ScaleSetSpec().Return(azure.ScaleSetSpec{
 					Name:            "my-vmss",

cloud/services/virtualmachines/virtualmachines.go

@@ -458,7 +458,7 @@ func getSecurityProfile(vmSpec azure.VMSpec, sku resourceskus.SKU) (*compute.Sec
 	}
 
 	if !sku.HasCapability(resourceskus.EncryptionAtHost) {
-		return nil, errors.Errorf("encryption at host is not supported for VM type %s", vmSpec.Size)
+		return nil, azure.WithTerminalError(errors.Errorf("encryption at host is not supported for VM type %s", vmSpec.Size))
 	}
 
 	return &compute.SecurityProfile{

cloud/services/virtualmachines/virtualmachines_test.go

@@ -823,7 +823,7 @@ func TestReconcileVM(t *testing.T) {
 				m.Get(gomockinternal.AContext(), "my-rg", "my-vm").
 					Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
 			},
-			ExpectedError: "encryption at host is not supported for VM type Standard_D2v3",
+			ExpectedError: "reconcile error occurred that cannot be recovered. Object will not be requeued. The actual error is: encryption at host is not supported for VM type Standard_D2v3",
 			SetupSKUs: func(svc *Service) {
 				skus := []compute.ResourceSku{
 					{

controllers/azuremachine_controller.go

@@ -279,6 +279,25 @@ func (r *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineSco
 			return reconcile.Result{}, errors.Wrapf(err, "failed to reconcile AzureMachine")
 		}
 
+		// Handle transient and terminal errors
+		var reconcileError azure.ReconcileError
+		if errors.As(err, &reconcileError) {
+			r.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "ReconcileError", errors.Wrapf(err, "failed to reconcile AzureMachine").Error())
+			conditions.MarkFalse(machineScope.AzureMachine, infrav1.VMRunningCondition, infrav1.VMProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error())
+
+			if reconcileError.IsTerminal() {
+				machineScope.Error(err, "failed to reconcile AzureMachine", "name", machineScope.Name())
+				return reconcile.Result{}, nil
+			}
+
+			if reconcileError.IsTransient() {
+				machineScope.Error(err, "failed to reconcile AzureMachine", "name", machineScope.Name())
+				return reconcile.Result{RequeueAfter: reconcileError.RequeueAfter()}, nil
+			}
+
+			return reconcile.Result{}, errors.Wrapf(err, "failed to reconcile AzureMachine")
+		}
+
 		r.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "Error creating new AzureMachine", errors.Wrapf(err, "failed to reconcile AzureMachine").Error())
 		conditions.MarkFalse(machineScope.AzureMachine, infrav1.VMRunningCondition, infrav1.VMProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error())
 		return reconcile.Result{}, errors.Wrapf(err, "failed to reconcile AzureMachine")

exp/controllers/azuremachinepool_controller.go

@@ -43,6 +43,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3"
+	azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
 	"sigs.k8s.io/cluster-api-provider-azure/cloud/scope"
 	infracontroller "sigs.k8s.io/cluster-api-provider-azure/controllers"
 	infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha3"
@@ -258,6 +259,23 @@ func (r *AzureMachinePoolReconciler) reconcileNormal(ctx context.Context, machin
 
 	err := ams.Reconcile(ctx)
 	if err != nil {
+
+		// Handle transient and terminal errors
+		var reconcileError azure.ReconcileError
+		if errors.As(err, &reconcileError) {
+			if reconcileError.IsTerminal() {
+				machinePoolScope.Error(err, "failed to reconcile AzureMachinePool", "name", machinePoolScope.Name())
+				return reconcile.Result{}, nil
+			}
+
+			if reconcileError.IsTransient() {
+				machinePoolScope.Error(err, "failed to reconcile AzureMachinePool", "name", machinePoolScope.Name())
+				return reconcile.Result{RequeueAfter: reconcileError.RequeueAfter()}, nil
+			}
+
+			return reconcile.Result{}, errors.Wrapf(err, "failed to reconcile AzureMachinePool")
+		}
+
 		return reconcile.Result{}, err
 	}