Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved assertions & resource clean-up for E2E tests #311

Merged
merged 3 commits into from
Aug 4, 2023
Merged

Improved assertions & resource clean-up for E2E tests #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3c3fb1
Improved assertions & resource clean-up for E2E tests
rlymbur Aug 4, 2023
28540e9
Remove redundant assertion on Port Annotation on Service Export test
rlymbur Aug 4, 2023
be6ad9f
Remove Sleep Delay override on VPC Lattice client
rlymbur Aug 4, 2023
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
7 changes: 3 additions & 4 deletions pkg/aws/services/vpclattice.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ package services

import (
"context"
"github.com/golang/glog"
"os"

"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 {
Expand All @@ -35,7 +34,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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 20 retries?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VPC lattice cp api have pretty low per account throttling setting, controller and e2etest code can quite easily get "Too Many Request" error if we use default max retry

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these retries with exponential backoff? or whats the retry strategy?


glog.V(2).Infoln("Lattice Service EndPoint:", endpoint)

Expand Down
2 changes: 1 addition & 1 deletion test/pkg/test/elasticsearch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
31 changes: 25 additions & 6 deletions test/pkg/test/framework.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part looks good

})
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())
}
}

Expand All @@ -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{
Expand All @@ -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)
Expand Down
23 changes: 3 additions & 20 deletions test/suites/integration/srvexport_port_annotation_targets_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down Expand Up @@ -61,22 +61,12 @@ 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(
Expand All @@ -85,12 +75,5 @@ var _ = Describe("Port Annotations Targets", func() {
))
log.Println("Target:", target)
}

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)
})
})