chore: classify controller errors as terminal vs non-terminal

Author: ketanjani21

pkg/controllers/nodeclass/ami.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/karpenter/pkg/utils/pretty"
 
 	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	awserrors "github.com/aws/karpenter-provider-aws/pkg/errors"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/amifamily"
 )
 
@@ -55,6 +56,9 @@ func (a *AMI) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (reconc
 			nodeClass.StatusConditions(status.WithClock(a.clk)).SetFalse(v1.ConditionTypeAMIsReady, "UnsupportedAlias", err.Error())
 			return reconcile.Result{}, reconcile.TerminalError(fmt.Errorf("getting amis, %w", err))
 		}
+		if reason, msg, retryable := awserrors.ClassifyError(err); !retryable {
+			nodeClass.StatusConditions(status.WithClock(a.clk)).SetFalse(v1.ConditionTypeAMIsReady, reason, msg)
+		}
 		return reconcile.Result{}, fmt.Errorf("getting amis, %w", err)
 	}
 	if len(amis) == 0 {

pkg/controllers/nodeclass/capacityreservation.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/karpenter/pkg/utils/pretty"
 
 	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	awserrors "github.com/aws/karpenter-provider-aws/pkg/errors"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/capacityreservation"
 )
 
@@ -53,6 +54,9 @@ func NewCapacityReservationReconciler(clk clock.Clock, provider capacityreservat
 func (c *CapacityReservation) Reconcile(ctx context.Context, nc *v1.EC2NodeClass) (reconcile.Result, error) {
 	reservations, err := c.provider.List(ctx, nc.Spec.CapacityReservationSelectorTerms...)
 	if err != nil {
+		if reason, msg, retryable := awserrors.ClassifyError(err); !retryable {
+			nc.StatusConditions(status.WithClock(c.clk)).SetFalse(v1.ConditionTypeCapacityReservationsReady, reason, msg)
+		}
 		return reconcile.Result{}, fmt.Errorf("getting capacity reservations, %w", err)
 	}
 	if len(reservations) == 0 {

pkg/controllers/nodeclass/instanceprofile.go

@@ -71,6 +71,9 @@ func (ip *InstanceProfile) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeC
 			profile, err := ip.instanceProfileProvider.Get(ctx, nodeClass.Status.InstanceProfile)
 			if err != nil {
 				if !awserrors.IsNotFound(err) {
+					if reason, msg, retryable := awserrors.ClassifyError(err); !retryable {
+						nodeClass.StatusConditions(status.WithClock(ip.clk)).SetFalse(v1.ConditionTypeInstanceProfileReady, reason, msg)
+					}
 					return reconcile.Result{}, fmt.Errorf("getting instance profile %s, %w", nodeClass.Status.InstanceProfile, err)
 				}
 			} else if len(profile.Roles) > 0 {
@@ -94,6 +97,9 @@ func (ip *InstanceProfile) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeC
 				// role or remove the existing role. To prevent runaway instance profile creation, we'll attempt to delete the
 				// profile. We'll fail open here and rely on the garbage collector as a backstop.
 				_ = ip.instanceProfileProvider.Delete(ctx, newProfileName)
+				if reason, msg, retryable := awserrors.ClassifyError(err); !retryable {
+					nodeClass.StatusConditions(status.WithClock(ip.clk)).SetFalse(v1.ConditionTypeInstanceProfileReady, reason, msg)
+				}
 				return reconcile.Result{}, fmt.Errorf("creating instance profile, %w", err)
 			}
 			ip.recreationCache.SetDefault(generateCacheKey(nodeClass), newProfileName)

pkg/controllers/nodeclass/securitygroup.go

@@ -27,6 +27,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	awserrors "github.com/aws/karpenter-provider-aws/pkg/errors"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/securitygroup"
 )
 
@@ -45,6 +46,9 @@ func NewSecurityGroupReconciler(clk clock.Clock, securityGroupProvider securityg
 func (sg *SecurityGroup) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (reconcile.Result, error) {
 	securityGroups, err := sg.securityGroupProvider.List(ctx, nodeClass)
 	if err != nil {
+		if reason, msg, retryable := awserrors.ClassifyError(err); !retryable {
+			nodeClass.StatusConditions(status.WithClock(sg.clk)).SetFalse(v1.ConditionTypeSecurityGroupsReady, reason, msg)
+		}
 		return reconcile.Result{}, fmt.Errorf("getting security groups, %w", err)
 	}
 	if len(securityGroups) == 0 && len(nodeClass.Spec.SecurityGroupSelectorTerms) > 0 {

pkg/controllers/nodeclass/subnet.go

@@ -27,6 +27,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	awserrors "github.com/aws/karpenter-provider-aws/pkg/errors"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/subnet"
 )
 
@@ -45,6 +46,9 @@ func NewSubnetReconciler(clk clock.Clock, subnetProvider subnet.Provider) *Subne
 func (s *Subnet) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (reconcile.Result, error) {
 	subnets, err := s.subnetProvider.List(ctx, nodeClass)
 	if err != nil {
+		if reason, msg, retryable := awserrors.ClassifyError(err); !retryable {
+			nodeClass.StatusConditions(status.WithClock(s.clk)).SetFalse(v1.ConditionTypeSubnetsReady, reason, msg)
+		}
 		return reconcile.Result{}, fmt.Errorf("getting subnets, %w", err)
 	}
 	if len(subnets) == 0 {

pkg/errors/errors.go

@@ -15,6 +15,7 @@ limitations under the License.
 package errors
 
 import (
+	"errors"
 	"strings"
 
 	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
@@ -28,6 +29,10 @@ const (
 	RunInstancesInvalidParameterValueCode          = "InvalidParameterValue"
 	DryRunOperationErrorCode                       = "DryRunOperation"
 	UnauthorizedOperationErrorCode                 = "UnauthorizedOperation"
+	AccessDeniedErrorCode                          = "AccessDenied"
+	AccessDeniedExceptionErrorCode                 = "AccessDeniedException"
+	AuthFailureErrorCode                           = "AuthFailure"
+	LimitExceededErrorCode                         = "LimitExceeded"
 	RateLimitingErrorCode                          = "RequestLimitExceeded"
 	ServiceLinkedRoleCreationNotPermittedErrorCode = "AuthFailure.ServiceLinkedRoleCreationNotPermitted"
 	InsufficientFreeAddressesInSubnetErrorCode     = "InsufficientFreeAddressesInSubnet"
@@ -294,3 +299,33 @@ func ToReasonMessage(err error) (string, string) {
 	}
 	return "LaunchFailed", "Instance launch failed"
 }
+
+// ClassifyError inspects an AWS API error and, for known terminal codes,
+// returns a condition Reason, a user-facing Message, and retryable=false.
+// Callers use retryable=false as the signal to set a Ready-style status
+// condition to False (rather than leaving it Unknown for a retry). Codes
+// that are transient (or unrecognized) return retryable=true so the caller
+// keeps its existing retry behavior.
+//
+// Terminal codes surfaced here:
+//   - UnauthorizedOperation / AccessDenied / AccessDeniedException /
+//     AuthFailure — IAM misconfiguration; retrying without operator action
+//     will not succeed.
+//   - LimitExceeded — the account has hit an AWS service limit; requires a
+//     quota increase or resource cleanup to resolve.
+func ClassifyError(err error) (reason string, message string, retryable bool) {
+	if err == nil {
+		return "", "", true
+	}
+	var apiErr smithy.APIError
+	if !errors.As(err, &apiErr) {
+		return "", "", true
+	}
+	switch apiErr.ErrorCode() {
+	case UnauthorizedOperationErrorCode, AccessDeniedErrorCode, AccessDeniedExceptionErrorCode, AuthFailureErrorCode:
+		return "Unauthorized", apiErr.ErrorMessage(), false
+	case LimitExceededErrorCode:
+		return "LimitExceeded", apiErr.ErrorMessage(), false
+	}
+	return "", "", true
+}

pkg/errors/errors_test.go

@@ -0,0 +1,130 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package errors_test
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/aws/smithy-go"
+
+	awserrors "github.com/aws/karpenter-provider-aws/pkg/errors"
+)
+
+type apiErr struct {
+	code    string
+	message string
+}
+
+func (e *apiErr) Error() string                 { return fmt.Sprintf("%s: %s", e.code, e.message) }
+func (e *apiErr) ErrorCode() string             { return e.code }
+func (e *apiErr) ErrorMessage() string          { return e.message }
+func (e *apiErr) ErrorFault() smithy.ErrorFault { return smithy.FaultClient }
+
+func TestClassifyError(t *testing.T) {
+	tests := []struct {
+		name           string
+		err            error
+		wantReason     string
+		wantMessage    string
+		wantRetryable  bool
+	}{
+		{
+			name:          "nil error is retryable and returns empty classification",
+			err:           nil,
+			wantReason:    "",
+			wantMessage:   "",
+			wantRetryable: true,
+		},
+		{
+			name:          "non-AWS error falls through as retryable",
+			err:           errors.New("boom"),
+			wantReason:    "",
+			wantMessage:   "",
+			wantRetryable: true,
+		},
+		{
+			name:          "UnauthorizedOperation is terminal with Unauthorized reason",
+			err:           &apiErr{code: awserrors.UnauthorizedOperationErrorCode, message: "user is not authorized"},
+			wantReason:    "Unauthorized",
+			wantMessage:   "user is not authorized",
+			wantRetryable: false,
+		},
+		{
+			name:          "AccessDenied is terminal with Unauthorized reason",
+			err:           &apiErr{code: awserrors.AccessDeniedErrorCode, message: "explicit deny"},
+			wantReason:    "Unauthorized",
+			wantMessage:   "explicit deny",
+			wantRetryable: false,
+		},
+		{
+			name:          "AccessDeniedException (IAM shape) is terminal with Unauthorized reason",
+			err:           &apiErr{code: awserrors.AccessDeniedExceptionErrorCode, message: "iam explicit deny"},
+			wantReason:    "Unauthorized",
+			wantMessage:   "iam explicit deny",
+			wantRetryable: false,
+		},
+		{
+			name:          "AuthFailure is terminal with Unauthorized reason",
+			err:           &apiErr{code: awserrors.AuthFailureErrorCode, message: "auth failure"},
+			wantReason:    "Unauthorized",
+			wantMessage:   "auth failure",
+			wantRetryable: false,
+		},
+		{
+			name:          "LimitExceeded is terminal with LimitExceeded reason",
+			err:           &apiErr{code: awserrors.LimitExceededErrorCode, message: "instance profile limit reached"},
+			wantReason:    "LimitExceeded",
+			wantMessage:   "instance profile limit reached",
+			wantRetryable: false,
+		},
+		{
+			name:          "RequestLimitExceeded (rate limiting) is retryable",
+			err:           &apiErr{code: awserrors.RateLimitingErrorCode, message: "slow down"},
+			wantReason:    "",
+			wantMessage:   "",
+			wantRetryable: true,
+		},
+		{
+			name:          "InvalidParameterValue is retryable (transient/config error, not blanket-terminal)",
+			err:           &apiErr{code: awserrors.RunInstancesInvalidParameterValueCode, message: "invalid"},
+			wantReason:    "",
+			wantMessage:   "",
+			wantRetryable: true,
+		},
+		{
+			name:          "wrapped terminal error is still classified as terminal",
+			err:           fmt.Errorf("listing subnets: %w", &apiErr{code: awserrors.AccessDeniedErrorCode, message: "wrapped"}),
+			wantReason:    "Unauthorized",
+			wantMessage:   "wrapped",
+			wantRetryable: false,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			reason, message, retryable := awserrors.ClassifyError(tc.err)
+			if reason != tc.wantReason {
+				t.Errorf("reason: got %q, want %q", reason, tc.wantReason)
+			}
+			if message != tc.wantMessage {
+				t.Errorf("message: got %q, want %q", message, tc.wantMessage)
+			}
+			if retryable != tc.wantRetryable {
+				t.Errorf("retryable: got %v, want %v", retryable, tc.wantRetryable)
+			}
+		})
+	}
+}