Merge pull request #6 from alexander-demicev/cleanupimprove

Improve janitors cleanup logic

Author: alexander-demicev

action/action.go

@@ -43,6 +43,7 @@ func (a *action) Cleanup(ctx context.Context, input *Input) error {
 		{Service: elb.ServiceName, Run: a.cleanLoadBalancers},
 		{Service: ec2.ServiceName, Run: a.cleanSecurityGroups},
 		{Service: cloudformation.ServiceName, Run: a.cleanCfStacks},
+		{Service: ec2.ServiceName, Run: a.cleanVPCs},
 	}
 	inputRegions := strings.Split(input.Regions, ",")
 

action/cleanup_asg.go

@@ -16,9 +16,10 @@ func (a *action) cleanASGs(ctx context.Context, input *CleanupScope) error {
 		for _, asg := range page.AutoScalingGroups {
 			var ignore, markedForDeletion bool
 			for _, tag := range asg.Tags {
-				if *tag.Key == input.IgnoreTag {
+				switch *tag.Key {
+				case input.IgnoreTag:
 					ignore = true
-				} else if *tag.Key == DeletionTag {
+				case DeletionTag:
 					markedForDeletion = true
 				}
 			}

action/cleanup_cf.go

@@ -16,9 +16,10 @@ func (a *action) cleanCfStacks(ctx context.Context, input *CleanupScope) error {
 		for _, stack := range page.Stacks {
 			var ignore, markedForDeletion bool
 			for _, tag := range stack.Tags {
-				if *tag.Key == input.IgnoreTag {
+				switch *tag.Key {
+				case input.IgnoreTag:
 					ignore = true
-				} else if *tag.Key == DeletionTag {
+				case DeletionTag:
 					markedForDeletion = true
 				}
 			}
@@ -29,6 +30,12 @@ func (a *action) cleanCfStacks(ctx context.Context, input *CleanupScope) error {
 			}
 
 			if !markedForDeletion {
+				if !a.canUpdateStack(aws.StringValue(stack.StackStatus)) {
+					LogDebug("cloudformation stack %s is in state %s and cannot be updated, skipping",
+						*stack.StackName, aws.StringValue(stack.StackStatus))
+					continue
+				}
+
 				// NOTE: only mark for future deletion if we're not running in dry-mode
 				if a.commit {
 					LogDebug("cloudformation stack %s does not have deletion tag, marking for future deletion and skipping cleanup", *stack.StackName)
@@ -44,6 +51,10 @@ func (a *action) cleanCfStacks(ctx context.Context, input *CleanupScope) error {
 				cf.ResourceStatusDeleteInProgress:
 				LogDebug("cloudformation stack %s is already deleted/deleting, skipping cleanup", *stack.StackName)
 				continue
+			case cf.StackStatusDeleteFailed:
+				LogDebug("cloudformation stack %s is in DELETE_FAILED state, adding to delete list", *stack.StackName)
+				stacksToDelete = append(stacksToDelete, stack.StackName)
+				continue
 			}
 
 			LogDebug("adding cloudformation stack %s to delete list", *stack.StackName)
@@ -102,8 +113,27 @@ func (a *action) markCfStackForFutureDeletion(ctx context.Context, stack *cf.Sta
 func (a *action) deleteCfStack(ctx context.Context, stackName string, client *cf.CloudFormation) error {
 	Log("Deleting CloudFormation stack %s", stackName)
 
-	if _, err := client.DeleteStackWithContext(ctx, &cf.DeleteStackInput{StackName: &stackName}); err != nil {
-		return fmt.Errorf("failed to delete cloudformation stack %s: %w", stackName, err)
+	stacks, err := client.DescribeStacksWithContext(ctx, &cf.DescribeStacksInput{StackName: &stackName})
+	if err != nil {
+		return fmt.Errorf("failed to describe cloudformation stack %s: %w", stackName, err)
+	}
+
+	if len(stacks.Stacks) > 0 {
+		stackStatus := aws.StringValue(stacks.Stacks[0].StackStatus)
+		if stackStatus == cf.StackStatusDeleteFailed {
+			Log("Stack %s is in DELETE_FAILED state, attempting to continue deletion", stackName)
+
+			if _, err := client.DeleteStackWithContext(ctx, &cf.DeleteStackInput{
+				StackName:       &stackName,
+				RetainResources: []*string{},
+			}); err != nil {
+				return fmt.Errorf("failed to continue deletion of cloudformation stack %s: %w", stackName, err)
+			}
+		} else {
+			if _, err := client.DeleteStackWithContext(ctx, &cf.DeleteStackInput{StackName: &stackName}); err != nil {
+				return fmt.Errorf("failed to delete cloudformation stack %s: %w", stackName, err)
+			}
+		}
 	}
 
 	if err := client.WaitUntilStackDeleteCompleteWithContext(ctx, &cf.DescribeStacksInput{StackName: &stackName}); err != nil {
@@ -112,3 +142,23 @@ func (a *action) deleteCfStack(ctx context.Context, stackName string, client *cf
 
 	return nil
 }
+
+func (a *action) canUpdateStack(stackStatus string) bool {
+	nonUpdatableStates := []string{
+		cf.StackStatusCreateInProgress,
+		cf.StackStatusDeleteInProgress,
+		cf.StackStatusDeleteFailed,
+		cf.StackStatusDeleteComplete,
+		cf.StackStatusUpdateInProgress,
+		cf.StackStatusUpdateRollbackInProgress,
+		cf.StackStatusUpdateRollbackCompleteCleanupInProgress,
+		cf.StackStatusReviewInProgress,
+	}
+
+	for _, state := range nonUpdatableStates {
+		if stackStatus == state {
+			return false
+		}
+	}
+	return true
+}

action/cleanup_lb.go

@@ -22,9 +22,10 @@ func (a *action) cleanLoadBalancers(ctx context.Context, input *CleanupScope) er
 			var ignore, markedForDeletion bool
 			for _, tagDescription := range tags.TagDescriptions {
 				for _, tag := range tagDescription.Tags {
-					if *tag.Key == input.IgnoreTag {
+					switch *tag.Key {
+					case input.IgnoreTag:
 						ignore = true
-					} else if *tag.Key == DeletionTag {
+					case DeletionTag:
 						markedForDeletion = true
 					}
 				}

action/cleanup_sgs.go

@@ -3,6 +3,7 @@ package action
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -153,8 +154,59 @@ func (a *action) deleteSecurityGroupRules(ctx context.Context, sgId string, sgIn
 func (a *action) deleteSecurityGroup(ctx context.Context, sgId string, client *ec2.EC2) error {
 	Log("Deleting Security Group %s", sgId)
 
-	if _, err := client.DeleteSecurityGroupWithContext(ctx, &ec2.DeleteSecurityGroupInput{GroupId: &sgId}); err != nil {
-		return fmt.Errorf("failed to delete security group %s: %w", sgId, err)
+	maxRetries := 5
+	retryDelay := 30 * time.Second
+
+	for attempt := 1; attempt <= maxRetries; attempt++ {
+		if _, err := client.DeleteSecurityGroupWithContext(ctx, &ec2.DeleteSecurityGroupInput{GroupId: &sgId}); err != nil {
+			if attempt < maxRetries && a.isDependencyViolation(err) {
+				LogDebug("Security group %s has dependencies, retrying in %v (attempt %d/%d)", sgId, retryDelay, attempt, maxRetries)
+
+				if err := a.handleSecurityGroupDependencies(ctx, sgId, client); err != nil {
+					LogDebug("Failed to handle dependencies for security group %s: %v", sgId, err)
+				}
+
+				time.Sleep(retryDelay)
+				continue
+			}
+			return fmt.Errorf("failed to delete security group %s: %w", sgId, err)
+		}
+		break
+	}
+
+	return nil
+}
+
+func (a *action) isDependencyViolation(err error) bool {
+	if err == nil {
+		return false
+	}
+	errStr := err.Error()
+	return strings.Contains(errStr, "DependencyViolation") ||
+		strings.Contains(errStr, "has a dependent object")
+}
+
+func (a *action) handleSecurityGroupDependencies(ctx context.Context, sgId string, client *ec2.EC2) error {
+	eniResp, err := client.DescribeNetworkInterfacesWithContext(ctx, &ec2.DescribeNetworkInterfacesInput{
+		Filters: []*ec2.Filter{
+			{
+				Name:   aws.String("group-id"),
+				Values: []*string{aws.String(sgId)},
+			},
+		},
+	})
+	if err != nil {
+		return fmt.Errorf("failed to describe network interfaces for sg %s: %w", sgId, err)
+	}
+
+	for _, eni := range eniResp.NetworkInterfaces {
+		LogDebug("Security group %s is used by network interface %s (status: %s)",
+			sgId, aws.StringValue(eni.NetworkInterfaceId), aws.StringValue(eni.Status))
+
+		if aws.StringValue(eni.Status) == "available" {
+			LogDebug("Network interface %s is available but not being deleted automatically for safety",
+				aws.StringValue(eni.NetworkInterfaceId))
+		}
 	}
 
 	return nil