From b3c3fb10df479ea0cb204cfa55925928acfdd959 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Fri, 4 Aug 2023 08:17:00 -0700 Subject: [PATCH 1/3] Improved assertions & resource clean-up for E2E tests --- pkg/aws/services/vpclattice.go | 3 +- test/pkg/test/elasticsearch.go | 2 +- test/pkg/test/framework.go | 31 +++++++++++++++---- .../srvexport_port_annotation_targets_test.go | 29 +++++++---------- 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/pkg/aws/services/vpclattice.go b/pkg/aws/services/vpclattice.go index 2fe4252c..4b171546 100644 --- a/pkg/aws/services/vpclattice.go +++ b/pkg/aws/services/vpclattice.go @@ -4,6 +4,7 @@ import ( "context" "github.com/golang/glog" "os" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" @@ -35,7 +36,7 @@ func NewDefaultLattice(sess *session.Session, region string) *defaultLattice { endpoint = latticeEndpoint } - latticeSess = vpclattice.New(sess, aws.NewConfig().WithRegion(region).WithEndpoint(endpoint)) + latticeSess = vpclattice.New(sess, aws.NewConfig().WithRegion(region).WithEndpoint(endpoint).WithMaxRetries(20).WithSleepDelay(func(t time.Duration) { time.Sleep(time.Millisecond * 500) })) glog.V(2).Infoln("Lattice Service EndPoint:", endpoint) diff --git a/test/pkg/test/elasticsearch.go b/test/pkg/test/elasticsearch.go index 43cb2174..15b4841c 100644 --- a/test/pkg/test/elasticsearch.go +++ b/test/pkg/test/elasticsearch.go @@ -19,7 +19,7 @@ type ElasticSearchOptions struct { MergeFromService []*v1.Service } -func (env *Framework) NewElasticeApp(options ElasticSearchOptions) (*appsv1.Deployment, *v1.Service) { +func (env *Framework) NewElasticApp(options ElasticSearchOptions) (*appsv1.Deployment, *v1.Service) { if options.Port == 0 { options.Port = 80 } diff --git a/test/pkg/test/framework.go b/test/pkg/test/framework.go index b7f2b5c6..8cd31526 100644 --- a/test/pkg/test/framework.go +++ b/test/pkg/test/framework.go @@ -144,8 +144,9 @@ func (env *Framework) ExpectToBeClean(ctx context.Context) { retrievedTargetGroups, _ := env.LatticeClient.ListTargetGroupsAsList(ctx, &vpclattice.ListTargetGroupsInput{}) for _, tg := range retrievedTargetGroups { - Logger(ctx).Infof("Found TargetGroup: %v, checking it whether it's created by current EKS Cluster", tg) + Logger(ctx).Infof("Found TargetGroup: %s, checking it whether it's created by current EKS Cluster", *tg.Id) if currentClusterVpcId != *tg.VpcIdentifier { + Logger(ctx).Infof("Target group VPC Id: %s, does not match current EKS Cluster VPC Id: %s", *tg.VpcIdentifier, currentClusterVpcId) //This tg is not created by current EKS Cluster, skip it continue } @@ -156,6 +157,7 @@ func (env *Framework) ExpectToBeClean(ctx context.Context) { Logger(ctx).Infof("Found Tags for tg %v tags: %v", *tg.Name, retrievedTags) tagValue, ok := retrievedTags.Tags[lattice.K8SParentRefTypeKey] if ok && *tagValue == lattice.K8SServiceExportType { + Logger(ctx).Infof("TargetGroup: %s was created by k8s controller, by a ServiceExport", *tg.Id) //This tg is created by k8s controller, by a ServiceExport, //ServiceExport still have a known targetGroup leaking issue, //so we temporarily skip to verify whether ServiceExport created TargetGroup is deleted or not @@ -220,9 +222,9 @@ func (env *Framework) EventuallyExpectNotFound(ctx context.Context, objects ...c Logger(ctx).Infof("Checking whether %s %s %s is not found", reflect.TypeOf(object), object.GetNamespace(), object.GetName()) g.Expect(errors.IsNotFound(env.Get(ctx, client.ObjectKeyFromObject(object), object))).To(BeTrue()) } - // Wait for 6 minutes at maximum just in case the k8sService deletion triggered targets draining time + // Wait for 7 minutes at maximum just in case the k8sService deletion triggered targets draining time // and httproute deletion need to wait for that targets draining time finish then it can return - }).WithTimeout(6 * time.Minute).WithOffset(1).Should(Succeed()) + }).WithTimeout(7 * time.Minute).WithOffset(1).Should(Succeed()) } func (env *Framework) EventuallyExpectNoneFound(ctx context.Context, objectList client.ObjectList) { @@ -459,7 +461,9 @@ func (env *Framework) DeleteAllFrameworkTracedVpcLatticeServices(ctx aws.Context _, err := env.LatticeClient.DeleteServiceNetworkServiceAssociationWithContext(ctx, &vpclattice.DeleteServiceNetworkServiceAssociationInput{ ServiceNetworkServiceAssociationIdentifier: snsaId, }) - Expect(err).ToNot(HaveOccurred()) + if err != nil { + Expect(err.(awserr.Error).Code()).To(Equal(vpclattice.ErrCodeResourceNotFoundException)) + } } Eventually(func(g Gomega) { @@ -493,6 +497,9 @@ func (env *Framework) DeleteAllFrameworkTracedTargetGroups(ctx aws.Context) { tgIds := lo.Map(filteredTgs, func(targetGroup *vpclattice.TargetGroupSummary, _ int) *string { return targetGroup.Id }) + + log.Println("Number of traced target groups to delete is:", len(tgIds)) + for _, tgId := range tgIds { targetSummaries, err := env.LatticeClient.ListTargetsAsList(ctx, &vpclattice.ListTargetsInput{ TargetGroupIdentifier: tgId, @@ -510,6 +517,18 @@ func (env *Framework) DeleteAllFrameworkTracedTargetGroups(ctx aws.Context) { TargetGroupIdentifier: tgId, Targets: targets, }) + } else { + Logger(ctx).Infof("Target group %s no longer has targets registered. Deleting now.", *tgId) + Eventually(func() bool { + _, err := env.LatticeClient.DeleteTargetGroup(&vpclattice.DeleteTargetGroupInput{ + TargetGroupIdentifier: tgId, + }) + if err != nil { + // Allow time for related service to be deleted prior + return err.(awserr.Error).Code() == vpclattice.ErrCodeResourceNotFoundException + } + return true + }).WithPolling(15 * time.Second).WithTimeout(2 * time.Minute).Should(BeTrue()) } } @@ -518,7 +537,7 @@ func (env *Framework) DeleteAllFrameworkTracedTargetGroups(ctx aws.Context) { //After initiating the DeregisterTargets call, the Targets will be in `draining` status for the next 5 minutes, //And VPC lattice backend will run a background job to completely delete the targets within 6 minutes at maximum in total. Eventually(func(g Gomega) { - log.Println("Trying to clear Target group", tgIdsThatNeedToWaitForDrainingTargetsToBeDeleted, " need to wait for draining targets to be deregistered") + log.Println("Trying to clear Target group", tgIdsThatNeedToWaitForDrainingTargetsToBeDeleted, "need to wait for draining targets to be deregistered") for _, tgId := range tgIdsThatNeedToWaitForDrainingTargetsToBeDeleted { _, err := env.LatticeClient.DeleteTargetGroupWithContext(ctx, &vpclattice.DeleteTargetGroupInput{ @@ -528,7 +547,7 @@ func (env *Framework) DeleteAllFrameworkTracedTargetGroups(ctx aws.Context) { g.Expect(err.(awserr.Error).Code()).To(Equal(vpclattice.ErrCodeResourceNotFoundException)) } } - }).WithTimeout(360 * time.Second).Should(Succeed()) + }).WithPolling(time.Minute).WithTimeout(7 * time.Minute).Should(Succeed()) } env.TestCasesCreatedServiceNames = make(map[string]bool) diff --git a/test/suites/integration/srvexport_port_annotation_targets_test.go b/test/suites/integration/srvexport_port_annotation_targets_test.go index 2750f21f..c93ece16 100644 --- a/test/suites/integration/srvexport_port_annotation_targets_test.go +++ b/test/suites/integration/srvexport_port_annotation_targets_test.go @@ -31,7 +31,7 @@ var _ = Describe("Port Annotations Targets", func() { BeforeEach(func() { gateway = testFramework.NewGateway("test-gateway", k8snamespace) - deployment, service = testFramework.NewElasticeApp(test.ElasticSearchOptions{ + deployment, service = testFramework.NewElasticApp(test.ElasticSearchOptions{ Name: "port-test", Namespace: k8snamespace, }) @@ -61,22 +61,13 @@ var _ = Describe("Port Annotations Targets", func() { AfterEach(func() { testFramework.CleanTestEnvironment(ctx) - testFramework.EventuallyExpectNotFound( - ctx, - gateway, - serviceExport, - serviceImport, - service, - deployment, - httpRoute, - ) }) - It("Port Annotaion on Service Export", func() { + It("Port Annotation on Service Export", func() { targets := testFramework.GetTargets(ctx, targetGroup, deployment) Expect(*targetGroup.Port).To(BeEquivalentTo(80)) - log.Println("Verifying Targets are only craeted for the port defined in Port Annotaion in ServiceExport") + log.Println("Verifying Targets are only created for the port defined in Port Annotation in ServiceExport") for _, target := range targets { Expect(*target.Port).To(BeEquivalentTo(service.Spec.Ports[0].Port)) Expect(*target.Status).To(Or( @@ -87,10 +78,14 @@ var _ = Describe("Port Annotations Targets", func() { } testFramework.ExpectDeleted(ctx, service) - Eventually(func(g Gomega) { - log.Println("Verifying Targets are only craeted for the port defined in Port Annotaion in ServiceExport") - targets := testFramework.GetTargets(ctx, targetGroup, deployment) - Expect(len(targets) == 0) - }).WithTimeout(5*time.Minute + 30*time.Second) + Eventually(func() int { + log.Println("Eventually deleting targets, getting registered targets") + retrievedTargets, err := testFramework.LatticeClient.ListTargetsAsList(ctx, &vpclattice.ListTargetsInput{ + TargetGroupIdentifier: targetGroup.Id, + }) + Expect(err).ToNot(HaveOccurred()) + log.Println("Number of targets registered is:", len(retrievedTargets)) + return len(retrievedTargets) + }).WithPolling(time.Minute).WithTimeout(7 * time.Minute).Should(BeZero()) }) }) From 28540e98e29c5dfd92c058e9f2d6b8245637a902 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Fri, 4 Aug 2023 09:54:57 -0700 Subject: [PATCH 2/3] Remove redundant assertion on Port Annotation on Service Export test --- .../srvexport_port_annotation_targets_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/test/suites/integration/srvexport_port_annotation_targets_test.go b/test/suites/integration/srvexport_port_annotation_targets_test.go index c93ece16..f563bc61 100644 --- a/test/suites/integration/srvexport_port_annotation_targets_test.go +++ b/test/suites/integration/srvexport_port_annotation_targets_test.go @@ -64,7 +64,6 @@ var _ = Describe("Port Annotations Targets", func() { }) It("Port Annotation on Service Export", func() { - targets := testFramework.GetTargets(ctx, targetGroup, deployment) Expect(*targetGroup.Port).To(BeEquivalentTo(80)) log.Println("Verifying Targets are only created for the port defined in Port Annotation in ServiceExport") @@ -76,16 +75,5 @@ var _ = Describe("Port Annotations Targets", func() { )) log.Println("Target:", target) } - - testFramework.ExpectDeleted(ctx, service) - Eventually(func() int { - log.Println("Eventually deleting targets, getting registered targets") - retrievedTargets, err := testFramework.LatticeClient.ListTargetsAsList(ctx, &vpclattice.ListTargetsInput{ - TargetGroupIdentifier: targetGroup.Id, - }) - Expect(err).ToNot(HaveOccurred()) - log.Println("Number of targets registered is:", len(retrievedTargets)) - return len(retrievedTargets) - }).WithPolling(time.Minute).WithTimeout(7 * time.Minute).Should(BeZero()) }) }) From be6ad9f188cc94b19f1a9972f907957d79059f88 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Fri, 4 Aug 2023 10:06:07 -0700 Subject: [PATCH 3/3] Remove Sleep Delay override on VPC Lattice client --- pkg/aws/services/vpclattice.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/aws/services/vpclattice.go b/pkg/aws/services/vpclattice.go index 4b171546..1acba627 100644 --- a/pkg/aws/services/vpclattice.go +++ b/pkg/aws/services/vpclattice.go @@ -2,14 +2,12 @@ package services import ( "context" - "github.com/golang/glog" - "os" - "time" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/vpclattice" "github.com/aws/aws-sdk-go/service/vpclattice/vpclatticeiface" + "github.com/golang/glog" + "os" ) type Lattice interface { @@ -36,7 +34,7 @@ func NewDefaultLattice(sess *session.Session, region string) *defaultLattice { endpoint = latticeEndpoint } - latticeSess = vpclattice.New(sess, aws.NewConfig().WithRegion(region).WithEndpoint(endpoint).WithMaxRetries(20).WithSleepDelay(func(t time.Duration) { time.Sleep(time.Millisecond * 500) })) + latticeSess = vpclattice.New(sess, aws.NewConfig().WithRegion(region).WithEndpoint(endpoint).WithMaxRetries(20)) glog.V(2).Infoln("Lattice Service EndPoint:", endpoint)