From 6a5e7248c54cbb62d0f2246af67994686cf991cc Mon Sep 17 00:00:00 2001 From: Jaime Melis Date: Sat, 27 Jun 2026 22:10:19 +0200 Subject: [PATCH] Pre-clean orphaned ENIs and security groups before VPC stack delete The e2e VPC CloudFormation stack delete consistently fails on the first attempt because ENIs and security groups left behind by hosted cluster teardown (LB controllers, CNI) haven't been fully released yet. The test retries up to 3 times with a 5-minute backoff, wasting ~19.5 minutes per CI run. Add a pre-cleanup step to DeleteVPC that actively removes orphaned ENIs and non-default security groups from the VPC before calling DeleteStack, so the first attempt succeeds. Co-Authored-By: Claude Opus 4.6 --- go.mod | 13 ++- go.sum | 26 +++-- internal/aws/ec2/cleanup.go | 146 ++++++++++++++++++++++++ internal/services/clustervpc/service.go | 29 ++++- 4 files changed, 191 insertions(+), 23 deletions(-) create mode 100644 internal/aws/ec2/cleanup.go diff --git a/go.mod b/go.mod index a127df1..dc9b2d0 100644 --- a/go.mod +++ b/go.mod @@ -4,15 +4,16 @@ go 1.25.8 require ( github.com/aws/aws-lambda-go v1.54.0 - github.com/aws/aws-sdk-go-v2 v1.41.11 + github.com/aws/aws-sdk-go-v2 v1.42.0 github.com/aws/aws-sdk-go-v2/config v1.32.22 github.com/aws/aws-sdk-go-v2/credentials v1.19.21 github.com/aws/aws-sdk-go-v2/service/cloudformation v1.71.15 + github.com/aws/aws-sdk-go-v2/service/ec2 v1.308.0 github.com/aws/aws-sdk-go-v2/service/ecr v1.58.2 github.com/aws/aws-sdk-go-v2/service/iam v1.54.2 github.com/aws/aws-sdk-go-v2/service/lambda v1.92.1 github.com/aws/aws-sdk-go-v2/service/sts v1.43.1 - github.com/aws/smithy-go v1.27.0 + github.com/aws/smithy-go v1.27.1 github.com/onsi/ginkgo/v2 v2.29.0 github.com/onsi/gomega v1.41.0 github.com/spf13/cobra v1.10.2 @@ -22,11 +23,11 @@ require ( github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.12 // indirect github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.27 // indirect - github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.27 // indirect - github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.27 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.29 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.29 // indirect github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.28 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.11 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.27 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.12 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.29 // indirect github.com/aws/aws-sdk-go-v2/service/signin v1.1.3 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.31.1 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.36.4 // indirect diff --git a/go.sum b/go.sum index 0abc5fd..2297d93 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1 github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/aws/aws-lambda-go v1.54.0 h1:EGYpdyRGF88xszqlGcBewz811mJeRS+maNlLZXFheII= github.com/aws/aws-lambda-go v1.54.0/go.mod h1:dpMpZgvWx5vuQJfBt0zqBha60q7Dd7RfgJv23DymV8A= -github.com/aws/aws-sdk-go-v2 v1.41.11 h1:9PRf7jyTMEUM6fuNRAJa2mO/skJfrF50rENJwf2LXqw= -github.com/aws/aws-sdk-go-v2 v1.41.11/go.mod h1:iiUX27gOXRuYaoeUVXhUpPwjJHzISfPAjjcuhUbLSVs= +github.com/aws/aws-sdk-go-v2 v1.42.0 h1:XvXMJTkFQtpBKIWZnmr9ZEOc2InWM2yldjXEJ/bymhA= +github.com/aws/aws-sdk-go-v2 v1.42.0/go.mod h1:27+ACypSLljLAEKsCYOmrjKh83vuTRkuAe9Uv/3A4bg= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.12 h1:oRtsqWgxbpeXrOlxOoQStx2M9WNbIkPq4C4Xn1or6bc= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.12/go.mod h1:Zg0Oe9qT+9wcezlm1a64wGJp2qZdRElVxo/seJf7jYU= github.com/aws/aws-sdk-go-v2/config v1.32.22 h1:Vfvp7+fYKsVCADcWOEllqEV47aIBXhNchvyDFu1B5fY= @@ -12,22 +12,24 @@ github.com/aws/aws-sdk-go-v2/credentials v1.19.21 h1:0+HscFXtNa4+3buV4IlG6v5lnOd github.com/aws/aws-sdk-go-v2/credentials v1.19.21/go.mod h1:UE8+9t5zudFwu5k5ShC1PKArVEdOkQQdCXIHQAVNUcU= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.27 h1:BEfN1sjtiKEdikRDxYkjZNE4tyvw/YbGWCbl3xDZgRw= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.27/go.mod h1:ISGSFNbOHRS+JV/17yStzRTPBUHHqF92kCpRLLyH3Nk= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.27 h1:8sPbKi1/KRHwl5oR3qN9mUXestCeHuaRutxylnr/eVY= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.27/go.mod h1:QV9IVIopJ1dpQUno0f9VYDUwOEjj8u0iEJ4JiZVre3Y= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.27 h1:9d8AoASQY9UwrOSmiJ7uSM0MGUPFhnenwSvpaFfat2c= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.27/go.mod h1:x0rldpsnUQaQIs4Rh+Vwm9Z/0vI6BxadGtsgJfZFb8s= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.29 h1:f3vKqSo13fhTYb+JEcXwXefZQE26I1FB5eTSniU67ko= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.29/go.mod h1:MzoLFUArKGpGD+ukmPiTPG1X5x4o6M2kq4v2dr1FiEc= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.29 h1:RdwIf/CuUsvJX3RgJagbOyotl/cxoLY4xviKuE7p2GY= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.29/go.mod h1:71wt8W2EgswdZy9Mf9KNnzxZ3TiZlv4caKghPktDOkA= github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.28 h1:eaS9vwQ5ym4Y9S6+G/K3d3lgZhxs9Sldcn/YS7cmdKY= github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.28/go.mod h1:oTdbDr+BMs7gAYrNpD0LDTyqQfv6yOYgTDv46+xbwFY= github.com/aws/aws-sdk-go-v2/service/cloudformation v1.71.15 h1:us9kmxnViaD2sbiIKXzSOGZ4c14C5aLGV+EbnccThF4= github.com/aws/aws-sdk-go-v2/service/cloudformation v1.71.15/go.mod h1:H2CEB5hS3tOB3f6/1x/6iuMpktmS6Wei9ylWTBMpGe8= +github.com/aws/aws-sdk-go-v2/service/ec2 v1.308.0 h1:xBP+yWpveXD/PxK7HRMcoG6yj1vdOjSahAg4qPomF+0= +github.com/aws/aws-sdk-go-v2/service/ec2 v1.308.0/go.mod h1:8mrDF7OtbuL0QpwP4YCvLuoOE4/5lL7D33MXgp069/Y= github.com/aws/aws-sdk-go-v2/service/ecr v1.58.2 h1:sSfN1WfNyYjV0CQC8moHP4uxgNXSUxDT0X2vXbcJSbc= github.com/aws/aws-sdk-go-v2/service/ecr v1.58.2/go.mod h1:CJsIIgsVqF1YRtS436i4RKrNFlo2LtQ7gORkdhKaj3k= github.com/aws/aws-sdk-go-v2/service/iam v1.54.2 h1:gJxdCZtnmAX9jF1RjBVJ6O2Bn4P2zLAQFXYmTBM+W5A= github.com/aws/aws-sdk-go-v2/service/iam v1.54.2/go.mod h1:GO5Zea33HKIxG12xqmrzTFgEZcok4jdpc4aM2TcHTfk= -github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.11 h1:rFSsqDfCMPAmG70JOsYqFZCHXkyatoGa1K4YEt/BggQ= -github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.11/go.mod h1:XG68qW+YLLFH0vnSDCou43Cgj5TeAG83O5NRSJgt04Y= -github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.27 h1:2/pUo42hhVmQcM21ttZoBOLHQymyUH8qEnZGTIuGBT8= -github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.27/go.mod h1:p7hwgbwompjCRNTdB3ytlldddNt1rDBgVVMqWEVG1II= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.12 h1:ZD2+BSw9vFsNlKYIasSNt3uDbjqqXIBcM13UJv/Lx2k= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.12/go.mod h1:Ms4zlcVBbXbiP7EVLhl+lgjvA/a7YphqQ3Ih3174EmI= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.29 h1:DRebniUGZ2MqiiIVmQJ04vIXr918hubdHMnarSLEWyU= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.29/go.mod h1:LfRkPCD8YHDM2E5eTkos2UpwYeZnBcVarTa8L59bJHA= github.com/aws/aws-sdk-go-v2/service/lambda v1.92.1 h1:WXCHC8eGCieCTxSJ0CsZc4yRK8PiUKPZm3un8L4Eu6I= github.com/aws/aws-sdk-go-v2/service/lambda v1.92.1/go.mod h1:RQmeW7HZrBCTGhwnmRTEw29G+Fhao0pkhrlwakALzbs= github.com/aws/aws-sdk-go-v2/service/signin v1.1.3 h1:t6U7sowMfOjTeZXtDOtgEJXsoJyX4MDag+sfWGwUM9M= @@ -38,8 +40,8 @@ github.com/aws/aws-sdk-go-v2/service/ssooidc v1.36.4 h1:p9+Fizo2sUB6wI5Yb3K5lmyk github.com/aws/aws-sdk-go-v2/service/ssooidc v1.36.4/go.mod h1:0x10Wy0dVS4Gn552xhHY5th2QdYpfJf44EsfyYGV194= github.com/aws/aws-sdk-go-v2/service/sts v1.43.1 h1:r/vUkpLilfCA3sxbRnkHbJejaoVHEdj4FEhv+Zva4DU= github.com/aws/aws-sdk-go-v2/service/sts v1.43.1/go.mod h1:t01JURC8Fe5M+7R1K0vzIZ2NT04HqvZR+FjlHrHDT2A= -github.com/aws/smithy-go v1.27.0 h1:ZoFioDKJxkSIW2otF9T0aPtNlUwhdVCcuZh/rzH9Hus= -github.com/aws/smithy-go v1.27.0/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= +github.com/aws/smithy-go v1.27.1 h1:4T340VFndXtADGF52gYa1POyL7s9E4Z1OeZ1hCscIw8= +github.com/aws/smithy-go v1.27.1/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/internal/aws/ec2/cleanup.go b/internal/aws/ec2/cleanup.go new file mode 100644 index 0000000..f8ad14d --- /dev/null +++ b/internal/aws/ec2/cleanup.go @@ -0,0 +1,146 @@ +package ec2 + +import ( + "context" + "fmt" + "log" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/ec2" + "github.com/aws/aws-sdk-go-v2/service/ec2/types" +) + +// CleanVPCForDeletion removes orphaned ENIs and non-default security groups +// from a VPC so that its CloudFormation stack can be deleted cleanly. +// Resources left behind by hosted cluster teardown (load balancer controllers, +// CNI plugin) cause CloudFormation delete-stack to fail on the first attempt. +func CleanVPCForDeletion(ctx context.Context, cfg aws.Config, vpcID string) error { + client := ec2.NewFromConfig(cfg) + + if err := deleteOrphanedENIs(ctx, client, vpcID); err != nil { + return fmt.Errorf("cleaning ENIs: %w", err) + } + + if err := deleteNonDefaultSecurityGroups(ctx, client, vpcID); err != nil { + return fmt.Errorf("cleaning security groups: %w", err) + } + + return nil +} + +func deleteOrphanedENIs(ctx context.Context, client *ec2.Client, vpcID string) error { + out, err := client.DescribeNetworkInterfaces(ctx, &ec2.DescribeNetworkInterfacesInput{ + Filters: []types.Filter{ + {Name: aws.String("vpc-id"), Values: []string{vpcID}}, + }, + }) + if err != nil { + return fmt.Errorf("describing ENIs: %w", err) + } + + for _, eni := range out.NetworkInterfaces { + eniID := aws.ToString(eni.NetworkInterfaceId) + + if eni.Attachment != nil && eni.Attachment.AttachmentId != nil { + // Only attempt detach for ENIs we can force-detach. + // ELB/Lambda-managed attachments will be cleaned up by their + // owning service; skip them to avoid API errors. + if eni.Attachment.DeleteOnTermination != nil && *eni.Attachment.DeleteOnTermination { + log.Printf(" skipping ENI %s (managed attachment, will be cleaned by owner)", eniID) + continue + } + + log.Printf(" detaching ENI %s", eniID) + _, err := client.DetachNetworkInterface(ctx, &ec2.DetachNetworkInterfaceInput{ + AttachmentId: eni.Attachment.AttachmentId, + Force: aws.Bool(true), + }) + if err != nil { + log.Printf(" warning: failed to detach ENI %s: %v (will still try delete)", eniID, err) + } else { + waitForENIAvailable(ctx, client, eniID) + } + } + + log.Printf(" deleting ENI %s", eniID) + _, err := client.DeleteNetworkInterface(ctx, &ec2.DeleteNetworkInterfaceInput{ + NetworkInterfaceId: aws.String(eniID), + }) + if err != nil { + log.Printf(" warning: failed to delete ENI %s: %v", eniID, err) + } + } + + return nil +} + +func waitForENIAvailable(ctx context.Context, client *ec2.Client, eniID string) { + for i := 0; i < 12; i++ { + time.Sleep(5 * time.Second) + out, err := client.DescribeNetworkInterfaces(ctx, &ec2.DescribeNetworkInterfacesInput{ + NetworkInterfaceIds: []string{eniID}, + }) + if err != nil { + return + } + if len(out.NetworkInterfaces) > 0 && out.NetworkInterfaces[0].Status == types.NetworkInterfaceStatusAvailable { + return + } + } +} + +func deleteNonDefaultSecurityGroups(ctx context.Context, client *ec2.Client, vpcID string) error { + out, err := client.DescribeSecurityGroups(ctx, &ec2.DescribeSecurityGroupsInput{ + Filters: []types.Filter{ + {Name: aws.String("vpc-id"), Values: []string{vpcID}}, + }, + }) + if err != nil { + return fmt.Errorf("describing security groups: %w", err) + } + + // First pass: revoke all rules so cross-references don't block deletion. + for _, sg := range out.SecurityGroups { + if aws.ToString(sg.GroupName) == "default" { + continue + } + sgID := aws.ToString(sg.GroupId) + + if len(sg.IpPermissions) > 0 { + _, err := client.RevokeSecurityGroupIngress(ctx, &ec2.RevokeSecurityGroupIngressInput{ + GroupId: aws.String(sgID), + IpPermissions: sg.IpPermissions, + }) + if err != nil { + log.Printf(" warning: failed to revoke ingress on SG %s: %v", sgID, err) + } + } + if len(sg.IpPermissionsEgress) > 0 { + _, err := client.RevokeSecurityGroupEgress(ctx, &ec2.RevokeSecurityGroupEgressInput{ + GroupId: aws.String(sgID), + IpPermissions: sg.IpPermissionsEgress, + }) + if err != nil { + log.Printf(" warning: failed to revoke egress on SG %s: %v", sgID, err) + } + } + } + + // Second pass: delete the security groups. + for _, sg := range out.SecurityGroups { + if aws.ToString(sg.GroupName) == "default" { + continue + } + sgID := aws.ToString(sg.GroupId) + log.Printf(" deleting security group %s (%s)", sgID, aws.ToString(sg.GroupName)) + _, err := client.DeleteSecurityGroup(ctx, &ec2.DeleteSecurityGroupInput{ + GroupId: aws.String(sgID), + }) + if err != nil { + log.Printf(" warning: failed to delete SG %s: %v", sgID, err) + } + } + + return nil +} diff --git a/internal/services/clustervpc/service.go b/internal/services/clustervpc/service.go index 5c247c9..2c7b2b6 100644 --- a/internal/services/clustervpc/service.go +++ b/internal/services/clustervpc/service.go @@ -4,11 +4,13 @@ import ( "context" "errors" "fmt" + "log" "time" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/cloudformation/types" "github.com/openshift-online/rosa-regional-platform-cli/internal/aws/cloudformation" + "github.com/openshift-online/rosa-regional-platform-cli/internal/aws/ec2" "github.com/openshift-online/rosa-regional-platform-cli/internal/cloudformation/templates" ) @@ -190,14 +192,31 @@ func updateVPC(ctx context.Context, cfnClient *cloudformation.Client, req *Creat }, nil } -// DeleteVPC deletes cluster VPC resources +// DeleteVPC deletes cluster VPC resources. It pre-cleans orphaned ENIs and +// security groups left behind by hosted cluster teardown so that the +// CloudFormation stack delete succeeds on the first attempt. func DeleteVPC(ctx context.Context, req *DeleteVPCRequest) error { - // Create CloudFormation client cfnClient := cloudformation.NewClient(req.AWSConfig) - - // Delete stack stackName := fmt.Sprintf("rosa-%s-vpc", req.ClusterName) - err := cfnClient.DeleteStack(ctx, stackName, 15*time.Minute) + + // Get the VPC ID from stack outputs so we can clean up orphaned resources. + outputs, err := cfnClient.GetStackOutputs(ctx, stackName) + if err != nil { + var notFound *cloudformation.StackNotFoundError + if errors.As(err, ¬Found) { + return nil + } + // Stack exists but we can't read outputs (e.g. rollback state). + // Fall through to DeleteStack without cleanup -- no worse than before. + log.Printf("warning: could not read stack outputs for %s: %v (skipping pre-cleanup)", stackName, err) + } else if vpcID := outputs.Outputs["VpcId"]; vpcID != "" { + log.Printf("pre-cleaning VPC %s before stack deletion", vpcID) + if cleanErr := ec2.CleanVPCForDeletion(ctx, req.AWSConfig, vpcID); cleanErr != nil { + log.Printf("warning: VPC pre-cleanup failed: %v (proceeding with stack delete)", cleanErr) + } + } + + err = cfnClient.DeleteStack(ctx, stackName, 15*time.Minute) if err != nil { var notFound *cloudformation.StackNotFoundError if errors.As(err, ¬Found) {