diff --git a/cmd/manager/main.go b/cmd/manager/main.go index a768719..75d242f 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -96,6 +96,7 @@ func main() { metrics.Register(mgr.GetClient(), log, *namespace) // Setup the cloud provider + // Uses AWS SDK's built-in retry behavior cloudProvider, err := builder.BuildCloudProvider(*cloudProviderName, logger) if err != nil { log.Error(err, "Unable to build cloud provider") diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index 440ea22..5d56144 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -59,12 +59,12 @@ func verifyIfErrorOccurred(apiErr error, expectedMessage ...string) (bool, error } func verifyIfErrorOccurredWithDefaults(apiErr error, expectedMessage string) (bool, error) { - skip_errs := []string{ + skipErrs := []string{ // default errors we wanted to skip "is not in correct state", expectedMessage, } - return verifyIfErrorOccurred(apiErr, skip_errs...) + return verifyIfErrorOccurred(apiErr, skipErrs...) } type provider struct { @@ -134,7 +134,6 @@ func (p *provider) InstancesExist(providerIDs []string) (map[string]interface{}, InstanceIds: aws.StringSlice(instanceIDs), }, ) - if err != nil { return nil, err } diff --git a/pkg/cloudprovider/aws/aws_test.go b/pkg/cloudprovider/aws/aws_test.go index a92b37d..3c239b8 100644 --- a/pkg/cloudprovider/aws/aws_test.go +++ b/pkg/cloudprovider/aws/aws_test.go @@ -438,7 +438,7 @@ func TestInstance_OutOfDate(t *testing.T) { MixedInstancesPolicy: &autoscaling.MixedInstancesPolicy{ LaunchTemplate: &autoscaling.LaunchTemplate{ LaunchTemplateSpecification: &autoscaling.LaunchTemplateSpecification{ - Version: aws.String(launchTemplateLatestVersion), + Version: aws.String(launchTemplateLatestVersion), }, }, }, @@ -453,7 +453,7 @@ func TestInstance_OutOfDate(t *testing.T) { MixedInstancesPolicy: &autoscaling.MixedInstancesPolicy{ LaunchTemplate: &autoscaling.LaunchTemplate{ LaunchTemplateSpecification: &autoscaling.LaunchTemplateSpecification{ - Version: aws.String(launchTemplateLatestVersion), + Version: aws.String(launchTemplateLatestVersion), }, }, }, diff --git a/pkg/cloudprovider/aws/builder.go b/pkg/cloudprovider/aws/builder.go index 06a52af..5b54e56 100644 --- a/pkg/cloudprovider/aws/builder.go +++ b/pkg/cloudprovider/aws/builder.go @@ -13,7 +13,7 @@ import ( "github.com/go-logr/logr" ) -// NewCloudProvider returns a new AWS cloud provider +// NewCloudProvider returns a new AWS cloud provider using the AWS SDK's default retry behavior func NewCloudProvider(logger logr.Logger) (cloudprovider.CloudProvider, error) { sess, err := session.NewSession() if err != nil { @@ -21,7 +21,19 @@ func NewCloudProvider(logger logr.Logger) (cloudprovider.CloudProvider, error) { } var creds *credentials.Credentials - config := &aws.Config{Credentials: creds} + + // Configure AWS SDK with default retry logic + // The AWS SDK v1 automatically uses client.DefaultRetryer which handles: + // - Exponential backoff + // - Retries for throttling errors + // - Retries for transient network errors + // - Retries for 5xx server errors + config := &aws.Config{ + Credentials: creds, + // Use AWS SDK's default retry behavior (3 retries with exponential backoff) + // This is sufficient for most use cases + MaxRetries: aws.Int(3), + } ec2Service := ec2.New(sess, config) autoScalingService := autoscaling.New(sess, config) @@ -33,7 +45,7 @@ func NewCloudProvider(logger logr.Logger) (cloudprovider.CloudProvider, error) { } // Log the provider we used - credValue, err := autoScalingService.Client.Config.Credentials.Get() + credValue, err := config.Credentials.Get() if err != nil { return nil, err } diff --git a/pkg/cloudprovider/builder/builder.go b/pkg/cloudprovider/builder/builder.go index 13d8c44..83e4dbe 100644 --- a/pkg/cloudprovider/builder/builder.go +++ b/pkg/cloudprovider/builder/builder.go @@ -11,6 +11,7 @@ import ( type builderFunc func(logger logr.Logger) (cloudprovider.CloudProvider, error) // BuildCloudProvider returns a cloud provider based on the provided name +// Uses the AWS SDK's built-in retry behavior func BuildCloudProvider(name string, logger logr.Logger) (cloudprovider.CloudProvider, error) { buildFuncs := map[string]builderFunc{ aws.ProviderName: aws.NewCloudProvider, diff --git a/pkg/controller/cyclenoderequest/transitioner/checks.go b/pkg/controller/cyclenoderequest/transitioner/checks.go index 20c2a43..a6a9933 100644 --- a/pkg/controller/cyclenoderequest/transitioner/checks.go +++ b/pkg/controller/cyclenoderequest/transitioner/checks.go @@ -139,7 +139,7 @@ func (t *CycleNodeRequestTransitioner) makeRequest(httpMethod string, httpClient return 0, nil, err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() bytes, err := io.ReadAll(resp.Body) if err != nil { @@ -242,7 +242,7 @@ func (t *CycleNodeRequestTransitioner) performInitialHealthChecks(kubeNodes map[ // performCyclingHealthChecks before terminating an instance selected for termination. Cycling pauses // until all health checks pass for the new instance before terminating the old one func (t *CycleNodeRequestTransitioner) performCyclingHealthChecks(kubeNodes map[string]corev1.Node) (bool, error) { - var allHealthChecksPassed bool = true + allHealthChecksPassed := true // Find new instsances attached to the nodegroup and perform health checks on them // before terminating the old ones they are replacing @@ -376,7 +376,7 @@ func (t *CycleNodeRequestTransitioner) sendPreTerminationTrigger(node v1.CycleNo // It monitors the progress shutdown progress. Cyclops will wait until this endpoint returns the expected response before // proceeding to terminate the node. func (t *CycleNodeRequestTransitioner) performPreTerminationHealthChecks(node v1.CycleNodeRequestNode) (bool, error) { - var allHealthChecksPassed bool = true + allHealthChecksPassed := true nodeHash := getNodeHash(node) // Check that the trigger has already been send to the node before performing any health checks diff --git a/pkg/controller/cyclenoderequest/transitioner/errors.go b/pkg/controller/cyclenoderequest/transitioner/errors.go new file mode 100644 index 0000000..10eb1c5 --- /dev/null +++ b/pkg/controller/cyclenoderequest/transitioner/errors.go @@ -0,0 +1,68 @@ +package transitioner + +import ( + "net" + "strings" + + "github.com/aws/aws-sdk-go/aws/awserr" +) + +// isRetryableError determines if an error should trigger a requeue instead of moving to Healing phase +// Retryable errors typically include network timeouts and transient AWS service errors +func isRetryableError(err error) bool { + if err == nil { + return false + } + + // Check for network errors (timeouts, connection refused, etc.) + if netErr, ok := err.(net.Error); ok { + if netErr.Timeout() { + return true + } + } + + // Check error string for common network patterns + errStr := err.Error() + networkPatterns := []string{ + "i/o timeout", + "dial tcp", + "connection reset", + "connection refused", + "connection timeout", + "TLS handshake timeout", + "broken pipe", + "unexpected EOF", + } + + for _, pattern := range networkPatterns { + if strings.Contains(errStr, pattern) { + return true + } + } + + // Check for AWS SDK transient errors + if awsErr, ok := err.(awserr.Error); ok { + code := awsErr.Code() + // Common transient AWS error codes + transientCodes := map[string]bool{ + "Throttling": true, + "ThrottlingException": true, + "TooManyRequestsException": true, + "RequestLimitExceeded": true, + "SlowDown": true, + "ServiceUnavailable": true, + "ServiceUnavailableException": true, + "InternalFailure": true, + "InternalError": true, + "InternalServerError": true, + "RequestTimeout": true, + "RequestTimedOut": true, + } + + if transientCodes[code] { + return true + } + } + + return false +} diff --git a/pkg/controller/cyclenoderequest/transitioner/integration_test.go b/pkg/controller/cyclenoderequest/transitioner/integration_test.go new file mode 100644 index 0000000..6a3b577 --- /dev/null +++ b/pkg/controller/cyclenoderequest/transitioner/integration_test.go @@ -0,0 +1,467 @@ +package transitioner + +import ( + "errors" + "testing" + "time" + + v1 "github.com/atlassian-labs/cyclops/pkg/apis/atlassian/v1" + "github.com/atlassian-labs/cyclops/pkg/cloudprovider" + "github.com/atlassian-labs/cyclops/pkg/controller" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/go-logr/logr/testr" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// TestOriginalIssue_IOTimeoutCausesHealing reproduces the original bug +// BEFORE FIX: AWS i/o timeout → immediate Healing → Failed +// AFTER FIX: AWS i/o timeout → retry → requeue → eventual success +func TestOriginalIssue_IOTimeoutCausesHealing(t *testing.T) { + t.Run("BEFORE FIX: i/o timeout transitions to Healing immediately", func(t *testing.T) { + // This test documents the OLD behavior - it would fail with the fix applied + // Skip this test since we've fixed the issue + t.Skip("This test documents old behavior - now fixed") + + cnr := createTestCycleNodeRequest("test-cnr", v1.CycleNodeRequestPending) + node := createTestNode("test-node-1", "aws:///us-west-2a/i-1234567890abcdef0") + + // Mock cloud provider that always returns i/o timeout + mockCP := &mockCloudProviderAlwaysFails{ + error: errors.New("Post \"https://autoscaling.us-west-2.amazonaws.com/\": dial tcp 18.246.117.35:443: i/o timeout"), + } + + rm := createTestResourceManager(t, cnr, node, mockCP) + transitioner := NewCycleNodeRequestTransitioner(cnr, rm, Options{}) + + // Execute + _, err := transitioner.transitionPending() + + // OLD behavior: would transition to Healing + if cnr.Status.Phase != v1.CycleNodeRequestHealing { + t.Errorf("OLD behavior: Expected transition to Healing, got %s", cnr.Status.Phase) + } + if err == nil { + t.Errorf("OLD behavior: Expected error to be returned") + } + }) + + t.Run("AFTER FIX: i/o timeout requeues for retry", func(t *testing.T) { + cnr := createTestCycleNodeRequest("test-cnr", v1.CycleNodeRequestPending) + node := createTestNode("test-node-1", "aws:///us-west-2a/i-1234567890abcdef0") + + // Mock cloud provider that returns i/o timeout + mockCP := &mockCloudProviderAlwaysFails{ + error: errors.New("Post \"https://autoscaling.us-west-2.amazonaws.com/\": dial tcp 18.246.117.35:443: i/o timeout"), + } + + rm := createTestResourceManager(t, cnr, node, mockCP) + transitioner := NewCycleNodeRequestTransitioner(cnr, rm, Options{}) + + // Execute + result, err := transitioner.transitionPending() + + // NEW behavior: should requeue without error + if err != nil { + t.Errorf("NEW behavior: Expected no error (requeue instead), got: %v", err) + } + if !result.Requeue { + t.Errorf("NEW behavior: Expected Requeue=true") + } + if result.RequeueAfter == 0 { + t.Errorf("NEW behavior: Expected RequeueAfter > 0") + } + if cnr.Status.Phase != v1.CycleNodeRequestPending { + t.Errorf("NEW behavior: Expected to stay in Pending phase, got %s", cnr.Status.Phase) + } + + t.Logf("✓ FIX VERIFIED: i/o timeout now requeues after %v instead of failing", result.RequeueAfter) + }) +} + +// TestEndToEnd_TransientErrorRecovery demonstrates the complete recovery flow +func TestEndToEnd_TransientErrorRecovery(t *testing.T) { + scenarios := []struct { + name string + error error + failuresBeforeSuccess int + description string + }{ + { + name: "i/o timeout recovery (original issue)", + error: errors.New("Post \"https://autoscaling.us-west-2.amazonaws.com/\": dial tcp 18.246.117.35:443: i/o timeout"), + failuresBeforeSuccess: 3, + description: "Network timeout recovers after 3 attempts", + }, + { + name: "AWS throttling recovery", + error: awserr.New("Throttling", "Rate limit exceeded", nil), + failuresBeforeSuccess: 2, + description: "AWS rate limiting recovers after backoff", + }, + { + name: "Service unavailable recovery", + error: awserr.New("ServiceUnavailable", "Service temporarily unavailable", nil), + failuresBeforeSuccess: 4, + description: "AWS service outage recovers after retries", + }, + { + name: "Connection reset recovery", + error: errors.New("read tcp: connection reset by peer"), + failuresBeforeSuccess: 2, + description: "Connection reset recovers after retries", + }, + { + name: "TLS handshake timeout recovery", + error: errors.New("TLS handshake timeout"), + failuresBeforeSuccess: 2, + description: "TLS timeout recovers after retries", + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + cnr := createTestCycleNodeRequest("test-cnr", v1.CycleNodeRequestPending) + node := createTestNode("test-node-1", "aws:///us-west-2a/i-1234567890abcdef0") + + // Mock cloud provider that fails N times then succeeds + mockCP := &mockCloudProviderWithTransientErrors{ + failuresBeforeSuccess: scenario.failuresBeforeSuccess, + errorToReturn: scenario.error, + } + + rm := createTestResourceManager(t, cnr, node, mockCP) + + // Simulate controller reconciliation loop + maxAttempts := scenario.failuresBeforeSuccess + 5 + recovered := false + + for attempt := 1; attempt <= maxAttempts; attempt++ { + transitioner := NewCycleNodeRequestTransitioner(cnr, rm, Options{}) + result, err := transitioner.transitionPending() + + t.Logf("Attempt %d: Phase=%s, Requeue=%v, Error=%v", + attempt, cnr.Status.Phase, result.Requeue, err) + + // Check if we've recovered (moved to Initialised phase) + if cnr.Status.Phase == v1.CycleNodeRequestInitialised { + recovered = true + t.Logf("✓ RECOVERED: %s after %d attempts", scenario.description, attempt) + break + } + + // Should not transition to Healing/Failed for transient errors + if cnr.Status.Phase == v1.CycleNodeRequestHealing || cnr.Status.Phase == v1.CycleNodeRequestFailed { + t.Errorf("❌ FAILED: Transitioned to %s for transient error (old behavior)", cnr.Status.Phase) + break + } + + // Should stay in Pending and requeue + if cnr.Status.Phase != v1.CycleNodeRequestPending { + t.Errorf("Expected Pending phase during retries, got %s", cnr.Status.Phase) + break + } + } + + if !recovered { + t.Errorf("❌ FAILED: Did not recover after %d attempts", maxAttempts) + } + + // Verify cloud provider was called enough times + if mockCP.callCount <= scenario.failuresBeforeSuccess { + t.Errorf("Expected at least %d cloud provider calls, got %d", + scenario.failuresBeforeSuccess+1, mockCP.callCount) + } + }) + } +} + +// TestEndToEnd_PermanentErrorsStillFail verifies permanent errors still fail correctly +func TestEndToEnd_PermanentErrorsStillFail(t *testing.T) { + permanentErrors := []struct { + name string + error error + }{ + { + name: "AccessDenied", + error: awserr.New("AccessDenied", "Access denied", nil), + }, + { + name: "InvalidParameter", + error: awserr.New("ValidationError", "Invalid parameter", nil), + }, + { + name: "InvalidInstanceID", + error: awserr.New("InvalidInstanceID.NotFound", "Instance not found", nil), + }, + } + + for _, tc := range permanentErrors { + t.Run(tc.name, func(t *testing.T) { + cnr := createTestCycleNodeRequest("test-cnr", v1.CycleNodeRequestPending) + node := createTestNode("test-node-1", "aws:///us-west-2a/i-1234567890abcdef0") + + // Mock cloud provider that always returns permanent error + mockCP := &mockCloudProviderAlwaysFails{ + error: tc.error, + } + + rm := createTestResourceManager(t, cnr, node, mockCP) + transitioner := NewCycleNodeRequestTransitioner(cnr, rm, Options{}) + + // Execute + result, err := transitioner.transitionPending() + + // Permanent errors should transition to Healing (not requeue) + if cnr.Status.Phase != v1.CycleNodeRequestHealing { + t.Errorf("Expected Healing phase for permanent error, got %s", cnr.Status.Phase) + } + if err == nil { + t.Errorf("Expected error to be returned for permanent error") + } + if result.Requeue { + t.Errorf("Expected no requeue for permanent error") + } + + t.Logf("✓ Permanent error correctly transitioned to Healing: %s", tc.error) + }) + } +} + +// TestEndToEnd_EquilibriumTimeout verifies timeout still works +func TestEndToEnd_EquilibriumTimeout(t *testing.T) { + t.Run("Transitions to Healing after equilibrium timeout", func(t *testing.T) { + cnr := createTestCycleNodeRequest("test-cnr", v1.CycleNodeRequestPending) + node := createTestNode("test-node-1", "aws:///us-west-2a/i-1234567890abcdef0") + + // Set equilibrium wait to past the timeout + cnr.Status.EquilibriumWaitStarted = &metav1.Time{ + Time: time.Now().Add(-10 * time.Minute), // Past the 5 minute limit + } + + mockCP := &mockCloudProviderAlwaysFails{ + error: errors.New("i/o timeout"), // Transient error + } + + rm := createTestResourceManager(t, cnr, node, mockCP) + transitioner := NewCycleNodeRequestTransitioner(cnr, rm, Options{}) + + // Execute + _, err := transitioner.transitionPending() + + // Should transition to Healing due to timeout (not because of error type) + if cnr.Status.Phase != v1.CycleNodeRequestHealing { + t.Errorf("Expected Healing phase after equilibrium timeout, got %s", cnr.Status.Phase) + } + if err == nil { + t.Errorf("Expected error to be returned") + } + + t.Logf("✓ Equilibrium timeout still works correctly") + }) +} + +// TestEndToEnd_CompareBeforeAndAfter creates a side-by-side comparison +func TestEndToEnd_CompareBeforeAndAfter(t *testing.T) { + t.Run("Side-by-side comparison of behavior", func(t *testing.T) { + testError := errors.New("Post \"https://autoscaling.us-west-2.amazonaws.com/\": dial tcp 18.246.117.35:443: i/o timeout") + + t.Log("=== BEHAVIOR COMPARISON ===") + t.Log("") + t.Log("Scenario: AWS API returns i/o timeout during GetNodeGroups") + t.Log("") + + // Test current (fixed) behavior + t.Log("AFTER FIX:") + cnr := createTestCycleNodeRequest("test-cnr-after", v1.CycleNodeRequestPending) + node := createTestNode("test-node-1", "aws:///us-west-2a/i-1234567890abcdef0") + mockCP := &mockCloudProviderAlwaysFails{error: testError} + rm := createTestResourceManager(t, cnr, node, mockCP) + transitioner := NewCycleNodeRequestTransitioner(cnr, rm, Options{}) + + result, err := transitioner.transitionPending() + + t.Logf(" Phase: %s → %s", v1.CycleNodeRequestPending, cnr.Status.Phase) + t.Logf(" Error returned: %v", err) + t.Logf(" Requeue: %v", result.Requeue) + t.Logf(" RequeueAfter: %v", result.RequeueAfter) + t.Logf(" Result: Will retry automatically ✓") + t.Log("") + + // Verify new behavior + if cnr.Status.Phase != v1.CycleNodeRequestPending { + t.Errorf("Expected to stay in Pending phase, got %s", cnr.Status.Phase) + } + if err != nil { + t.Errorf("Expected no error (should requeue), got %v", err) + } + if !result.Requeue { + t.Errorf("Expected requeue for retry") + } + + t.Log("SUMMARY:") + t.Log(" ✓ Transient errors now trigger automatic retry") + t.Log(" ✓ CNR stays in Pending phase instead of transitioning to Healing") + t.Log(" ✓ Controller will automatically requeue and retry") + t.Log(" ✓ System can recover from intermittent network issues") + }) +} + +// Helper functions + +func createTestCycleNodeRequest(name string, phase v1.CycleNodeRequestPhase) *v1.CycleNodeRequest { + return &v1.CycleNodeRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "kube-system", + }, + Spec: v1.CycleNodeRequestSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "node-role": "worker", + }, + }, + NodeGroupsList: []string{"test-nodegroup"}, + CycleSettings: v1.CycleSettings{ + Method: "Drain", + Concurrency: 1, + }, + }, + Status: v1.CycleNodeRequestStatus{ + Phase: phase, + EquilibriumWaitStarted: &metav1.Time{ + Time: time.Now(), + }, + }, + } +} + +func createTestNode(name, providerID string) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + "node-role": "worker", + }, + }, + Spec: corev1.NodeSpec{ + ProviderID: providerID, + }, + } +} + +func createTestResourceManager(t *testing.T, cnr *v1.CycleNodeRequest, node *corev1.Node, cp cloudprovider.CloudProvider) *controller.ResourceManager { + scheme := runtime.NewScheme() + _ = v1.SchemeBuilder.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(cnr, node). + Build() + + return &controller.ResourceManager{ + Client: fakeClient, + CloudProvider: cp, + Logger: testr.NewWithOptions(t, testr.Options{Verbosity: 10}), + } +} + +// mockCloudProviderAlwaysFails always returns the configured error +type mockCloudProviderAlwaysFails struct { + error error +} + +func (m *mockCloudProviderAlwaysFails) GetNodeGroups(names []string) (cloudprovider.NodeGroups, error) { + return nil, m.error +} + +func (m *mockCloudProviderAlwaysFails) InstancesExist(providerIDs []string) (map[string]interface{}, error) { + return nil, m.error +} + +func (m *mockCloudProviderAlwaysFails) TerminateInstance(providerID string) error { + return m.error +} + +func (m *mockCloudProviderAlwaysFails) Name() string { + return "mock-aws-failing" +} + +// mockCloudProviderWithTransientErrors simulates AWS transient failures +type mockCloudProviderWithTransientErrors struct { + callCount int + failuresBeforeSuccess int + errorToReturn error +} + +func (m *mockCloudProviderWithTransientErrors) GetNodeGroups(names []string) (cloudprovider.NodeGroups, error) { + m.callCount++ + if m.callCount <= m.failuresBeforeSuccess { + return nil, m.errorToReturn + } + // Success after N failures + return &mockNodeGroups{}, nil +} + +func (m *mockCloudProviderWithTransientErrors) InstancesExist(providerIDs []string) (map[string]interface{}, error) { + return make(map[string]interface{}), nil +} + +func (m *mockCloudProviderWithTransientErrors) TerminateInstance(providerID string) error { + return nil +} + +func (m *mockCloudProviderWithTransientErrors) Name() string { + return "mock-aws" +} + +// mockNodeGroups is a simple mock implementation +type mockNodeGroups struct{} + +func (m *mockNodeGroups) Instances() map[string]cloudprovider.Instance { + return map[string]cloudprovider.Instance{ + "aws:///us-west-2a/i-1234567890abcdef0": &mockInstance{ + id: "i-1234567890abcdef0", + providerID: "aws:///us-west-2a/i-1234567890abcdef0", + }, + } +} + +func (m *mockNodeGroups) DetachInstance(providerID string) (bool, error) { + return false, nil +} + +func (m *mockNodeGroups) AttachInstance(providerID, nodeGroup string) (bool, error) { + return false, nil +} + +func (m *mockNodeGroups) ReadyInstances() map[string]cloudprovider.Instance { + return m.Instances() +} + +func (m *mockNodeGroups) NotReadyInstances() map[string]cloudprovider.Instance { + return make(map[string]cloudprovider.Instance) +} + +type mockInstance struct { + id string + providerID string +} + +func (m *mockInstance) ID() string { + return m.id +} + +func (m *mockInstance) OutOfDate() bool { + return false +} + +func (m *mockInstance) MatchesProviderID(providerID string) bool { + return m.providerID == providerID +} + +func (m *mockInstance) NodeGroupName() string { + return "test-nodegroup" +} diff --git a/pkg/controller/cyclenoderequest/transitioner/transitions.go b/pkg/controller/cyclenoderequest/transitioner/transitions.go index 2ce07a1..bfb39ea 100644 --- a/pkg/controller/cyclenoderequest/transitioner/transitions.go +++ b/pkg/controller/cyclenoderequest/transitioner/transitions.go @@ -75,6 +75,16 @@ func (t *CycleNodeRequestTransitioner) transitionPending() (reconcile.Result, er // should be all the nodes in each, regardless of it they exist in both. kubeNodes, nodeGroupInstances, err := t.findAllNodesForCycle() if err != nil { + // Check if this is a retryable error (network timeout, etc.) + if isRetryableError(err) { + t.rm.Logger.Info("Retryable error encountered, requeuing", "error", err.Error()) + // Requeue with backoff instead of transitioning to Healing + return reconcile.Result{ + Requeue: true, + RequeueAfter: requeueDuration, + }, nil + } + // Non-retryable error, transition to Healing return t.transitionToHealing(err) }