Merge pull request #5193 from s3rj1k/eks-vpce-deletion

✨ feat: Remove owned VPCEs when deleting VPC.
kubernetes-sigs · Nov 12, 2024 · df09e6c · df09e6c
2 parents 85759ce + 0c80965
commit df09e6c
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 21 deletions.
diff --git a/controllers/awscluster_controller_test.go b/controllers/awscluster_controller_test.go
@@ -1476,6 +1476,18 @@ func mockedDeleteVPCCallsForNonExistentVPC(m *mocks.MockEC2APIMockRecorder) {
 }
 
 func mockedDeleteVPCCalls(m *mocks.MockEC2APIMockRecorder) {
+	m.DescribeVpcEndpointsPages(gomock.Eq(&ec2.DescribeVpcEndpointsInput{
+		Filters: []*ec2.Filter{
+			{
+				Name:   aws.String("tag:sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
+				Values: []*string{aws.String("owned")},
+			},
+			{
+				Name:   aws.String("vpc-id"),
+				Values: []*string{aws.String("vpc-exists")},
+			},
+		},
+	}), gomock.Any()).Return(nil).AnyTimes()
 	m.DescribeSubnetsWithContext(context.TODO(), gomock.Eq(&ec2.DescribeSubnetsInput{
 		Filters: []*ec2.Filter{
 			{

diff --git a/pkg/cloud/services/network/vpc.go b/pkg/cloud/services/network/vpc.go
@@ -276,17 +276,8 @@ func (s *Service) deleteVPCEndpoints() error {
 		return nil
 	}
 
-	// Gather all services that might have been enabled.
-	services := sets.New[string]()
-	if s.scope.Bucket() != nil {
-		services.Insert(fmt.Sprintf("com.amazonaws.%s.s3", s.scope.Region()))
-	}
-	if services.Len() == 0 {
-		return nil
-	}
-
 	// Get all existing endpoints.
-	endpoints, err := s.describeVPCEndpoints()
+	endpoints, err := s.describeVPCEndpoints(filter.EC2.ClusterOwned(s.scope.Name()))
 	if err != nil {
 		return errors.Wrap(err, "failed to describe vpc endpoints")
 	}

diff --git a/test/e2e/data/e2e_eks_conf.yaml b/test/e2e/data/e2e_eks_conf.yaml
@@ -149,7 +149,7 @@ intervals:
   default/wait-machine-status: ["20m", "10s"]
   default/wait-infra-subnets: ["5m", "30s"]
   default/wait-control-plane-upgrade: ["35m", "30s"]
-  default/wait-addon-status: ["10m", "30s"]
+  default/wait-addon-status: ["30m", "30s"]
   default/wait-create-identity: ["1m", "10s"]
   default/wait-deployment-ready: ["5m", "10s"]
   default/wait-loadbalancer-ready: ["5m", "30s"]
diff --git a/test/e2e/shared/suite.go b/test/e2e/shared/suite.go
@@ -145,7 +145,7 @@ func Node1BeforeSuite(e2eCtx *E2EContext) []byte {
 				success = false
 			}
 			return success
-		}, 10*time.Minute, 5*time.Second).Should(BeTrue(), "Should've eventually succeeded creating an AWS CloudFormation stack")
+		}, 45*time.Minute, 30*time.Second).Should(BeTrue(), "Should've eventually succeeded creating an AWS CloudFormation stack")
 	}
 
 	ensureStackTags(e2eCtx.AWSSession, bootstrapTemplate.Spec.StackName, bootstrapTags)

diff --git a/test/e2e/suites/managed/eks_test.go b/test/e2e/suites/managed/eks_test.go
@@ -22,6 +22,7 @@ package managed
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -76,15 +77,22 @@ var _ = ginkgo.Describe("[managed] [general] EKS cluster tests", func() {
 		})
 
 		ginkgo.By("should set environment variables on the aws-node daemonset")
-		CheckAwsNodeEnvVarsSet(ctx, func() UpdateAwsNodeVersionSpecInput {
-			return UpdateAwsNodeVersionSpecInput{
-				E2EConfig:             e2eCtx.E2EConfig,
-				BootstrapClusterProxy: e2eCtx.Environment.BootstrapClusterProxy,
-				AWSSession:            e2eCtx.BootstrapUserAWSSession,
-				Namespace:             namespace,
-				ClusterName:           clusterName,
-			}
-		})
+		Eventually(func() error {
+			defer ginkgo.GinkgoRecover()
+			CheckAwsNodeEnvVarsSet(ctx, func() UpdateAwsNodeVersionSpecInput {
+				return UpdateAwsNodeVersionSpecInput{
+					E2EConfig:             e2eCtx.E2EConfig,
+					BootstrapClusterProxy: e2eCtx.Environment.BootstrapClusterProxy,
+					AWSSession:            e2eCtx.BootstrapUserAWSSession,
+					Namespace:             namespace,
+					ClusterName:           clusterName,
+				}
+			})
+			return nil
+		}).WithTimeout(5*time.Minute).WithPolling(10*time.Second).WithContext(ctx).Should(
+			Succeed(),
+			"Failed to verify AWS Node environment variables after 5 minutes of retries",
+		)
 
 		ginkgo.By("should have the VPC CNI installed")
 		CheckAddonExistsSpec(ctx, func() CheckAddonExistsSpecInput {