Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
5 changes: 2 additions & 3 deletions pkg/cloudprovider/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -134,7 +134,6 @@ func (p *provider) InstancesExist(providerIDs []string) (map[string]interface{},
InstanceIds: aws.StringSlice(instanceIDs),
},
)

if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cloudprovider/aws/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
},
Expand All @@ -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),
},
},
},
Expand Down
18 changes: 15 additions & 3 deletions pkg/cloudprovider/aws/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,27 @@ 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 {
return nil, err
}

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)
Expand All @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions pkg/cloudprovider/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/cyclenoderequest/transitioner/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
68 changes: 68 additions & 0 deletions pkg/controller/cyclenoderequest/transitioner/errors.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading
Loading