From 79691183f179a1fee8b38704f573a8878132e5b2 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Tue, 4 Feb 2025 11:55:30 -0800 Subject: [PATCH 1/4] Increase timeouts to address flaky E2E tests --- test/suites/integration/access_log_policy_test.go | 3 +++ test/suites/integration/grpcroute_test.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/test/suites/integration/access_log_policy_test.go b/test/suites/integration/access_log_policy_test.go index 64245b72..39385504 100644 --- a/test/suites/integration/access_log_policy_test.go +++ b/test/suites/integration/access_log_policy_test.go @@ -86,6 +86,9 @@ var _ = Describe("Access Log Policy", Ordered, func() { ) BeforeAll(func() { + SetDefaultEventuallyTimeout(5 * time.Minute) + SetDefaultEventuallyPollingInterval(10 * time.Second) + awsResourceName = awsResourceNamePrefix + utils.RandomAlphaString(10) tags := testFramework.NewTestTags(testSuite) diff --git a/test/suites/integration/grpcroute_test.go b/test/suites/integration/grpcroute_test.go index 67562c6d..b5a70c08 100644 --- a/test/suites/integration/grpcroute_test.go +++ b/test/suites/integration/grpcroute_test.go @@ -70,7 +70,7 @@ var _ = Describe("GRPCRoute test", Ordered, func() { g.Expect(len(rules)).To(Equal(1)) g.Expect(*rules[0].Match.HttpMatch.Method).To(Equal("POST")) g.Expect(*rules[0].Match.HttpMatch.PathMatch.Match.Prefix).To(Equal("/")) - }).Within(30 * time.Second).Should(Succeed()) + }).Within(1 * time.Minute).Should(Succeed()) }) Context("Traffic test: client pod (grpcurl-runner) can send request to all services/methods of grpcBinService", func() { From 94b4becfabc9734ba5e96702156edfb3bd5dcf77 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Tue, 4 Feb 2025 16:06:42 -0800 Subject: [PATCH 2/4] Added custom retryer for RAM tests --- pkg/aws/retry/custom_retryer.go | 39 +++++++++++++++++++++++ pkg/aws/retry/custom_retryer_test.go | 35 ++++++++++++++++++++ test/suites/integration/ram_share_test.go | 10 ++++-- 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 pkg/aws/retry/custom_retryer.go create mode 100644 pkg/aws/retry/custom_retryer_test.go diff --git a/pkg/aws/retry/custom_retryer.go b/pkg/aws/retry/custom_retryer.go new file mode 100644 index 00000000..f6c62947 --- /dev/null +++ b/pkg/aws/retry/custom_retryer.go @@ -0,0 +1,39 @@ +package retry + +import ( + "github.com/aws/aws-sdk-go/aws/request" + "math" + "math/rand" + "time" +) + +const ( + BackoffMultiplier = 100 +) + +func WithMaxRetries(maxRetries int) request.Option { + return func(r *request.Request) { + r.Retryer = &CustomRetryer{ + Retryer: r.Retryer, + numMaxRetries: maxRetries, + } + } +} + +type CustomRetryer struct { + request.Retryer + numMaxRetries int +} + +func (c *CustomRetryer) MaxRetries() int { + return c.numMaxRetries +} + +func (c *CustomRetryer) RetryRules(req *request.Request) time.Duration { + if req.RetryCount < c.numMaxRetries { + backoff := time.Duration((math.Pow(2, float64(req.RetryCount)))*BackoffMultiplier) * time.Millisecond + jitter := time.Duration(float64(backoff) * (0.1*rand.Float64() - 0.05)) + return backoff + jitter + } + return 0 +} diff --git a/pkg/aws/retry/custom_retryer_test.go b/pkg/aws/retry/custom_retryer_test.go new file mode 100644 index 00000000..7517b7cf --- /dev/null +++ b/pkg/aws/retry/custom_retryer_test.go @@ -0,0 +1,35 @@ +package retry + +import ( + "github.com/aws/aws-sdk-go/aws/request" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestWithMaxRetries(t *testing.T) { + type args struct { + maxRetries int + } + tests := []struct { + name string + args args + wantMaxRetries int + }{ + { + name: "normal case", + args: args{ + maxRetries: 10, + }, + wantMaxRetries: 10, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + option := WithMaxRetries(tt.args.maxRetries) + r := &request.Request{} + option(r) + gotMaxRetries := r.MaxRetries() + assert.Equal(t, tt.wantMaxRetries, gotMaxRetries) + }) + } +} diff --git a/test/suites/integration/ram_share_test.go b/test/suites/integration/ram_share_test.go index 6f0b06a5..aaa98ede 100644 --- a/test/suites/integration/ram_share_test.go +++ b/test/suites/integration/ram_share_test.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" + "github.com/aws/aws-application-networking-k8s/pkg/aws/retry" "github.com/aws/aws-application-networking-k8s/pkg/config" "github.com/aws/aws-application-networking-k8s/pkg/utils" ) @@ -50,7 +51,11 @@ var _ = Describe("RAM Share", Ordered, func() { BeforeAll(func() { secondaryTestRoleArn = os.Getenv("SECONDARY_ACCOUNT_TEST_ROLE_ARN") - primarySess := session.Must(session.NewSession(&aws.Config{Region: aws.String(config.Region)})) + primarySess := session.Must(session.NewSession(&aws.Config{ + Region: aws.String(config.Region), + Retryer: retry.WithMaxRetries(3), + })) + stsClient := sts.New(primarySess) assumeRoleInput := &sts.AssumeRoleInput{ RoleArn: aws.String(secondaryTestRoleArn), @@ -63,7 +68,8 @@ var _ = Describe("RAM Share", Ordered, func() { creds := assumeRoleResult.Credentials secondarySess := session.Must(session.NewSession(&aws.Config{ - Region: aws.String(config.Region), + Region: aws.String(config.Region), + Retryer: retry.WithMaxRetries(3), Credentials: credentials.NewStaticCredentials( *creds.AccessKeyId, *creds.SecretAccessKey, From e80b6fdb22c38ae1c3df8062ff7a5690e922f015 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Wed, 5 Feb 2025 15:49:59 -0800 Subject: [PATCH 3/4] Use DefaultRetryer --- pkg/aws/retry/custom_retryer.go | 39 ----------------------- pkg/aws/retry/custom_retryer_test.go | 35 -------------------- test/suites/integration/ram_share_test.go | 14 ++++++-- 3 files changed, 11 insertions(+), 77 deletions(-) delete mode 100644 pkg/aws/retry/custom_retryer.go delete mode 100644 pkg/aws/retry/custom_retryer_test.go diff --git a/pkg/aws/retry/custom_retryer.go b/pkg/aws/retry/custom_retryer.go deleted file mode 100644 index f6c62947..00000000 --- a/pkg/aws/retry/custom_retryer.go +++ /dev/null @@ -1,39 +0,0 @@ -package retry - -import ( - "github.com/aws/aws-sdk-go/aws/request" - "math" - "math/rand" - "time" -) - -const ( - BackoffMultiplier = 100 -) - -func WithMaxRetries(maxRetries int) request.Option { - return func(r *request.Request) { - r.Retryer = &CustomRetryer{ - Retryer: r.Retryer, - numMaxRetries: maxRetries, - } - } -} - -type CustomRetryer struct { - request.Retryer - numMaxRetries int -} - -func (c *CustomRetryer) MaxRetries() int { - return c.numMaxRetries -} - -func (c *CustomRetryer) RetryRules(req *request.Request) time.Duration { - if req.RetryCount < c.numMaxRetries { - backoff := time.Duration((math.Pow(2, float64(req.RetryCount)))*BackoffMultiplier) * time.Millisecond - jitter := time.Duration(float64(backoff) * (0.1*rand.Float64() - 0.05)) - return backoff + jitter - } - return 0 -} diff --git a/pkg/aws/retry/custom_retryer_test.go b/pkg/aws/retry/custom_retryer_test.go deleted file mode 100644 index 7517b7cf..00000000 --- a/pkg/aws/retry/custom_retryer_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package retry - -import ( - "github.com/aws/aws-sdk-go/aws/request" - "github.com/stretchr/testify/assert" - "testing" -) - -func TestWithMaxRetries(t *testing.T) { - type args struct { - maxRetries int - } - tests := []struct { - name string - args args - wantMaxRetries int - }{ - { - name: "normal case", - args: args{ - maxRetries: 10, - }, - wantMaxRetries: 10, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - option := WithMaxRetries(tt.args.maxRetries) - r := &request.Request{} - option(r) - gotMaxRetries := r.MaxRetries() - assert.Equal(t, tt.wantMaxRetries, gotMaxRetries) - }) - } -} diff --git a/test/suites/integration/ram_share_test.go b/test/suites/integration/ram_share_test.go index aaa98ede..8ef424fb 100644 --- a/test/suites/integration/ram_share_test.go +++ b/test/suites/integration/ram_share_test.go @@ -2,6 +2,7 @@ package integration import ( "fmt" + awsClient "github.com/aws/aws-sdk-go/aws/client" "os" "strings" "time" @@ -22,7 +23,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" - "github.com/aws/aws-application-networking-k8s/pkg/aws/retry" "github.com/aws/aws-application-networking-k8s/pkg/config" "github.com/aws/aws-application-networking-k8s/pkg/utils" ) @@ -51,9 +51,17 @@ var _ = Describe("RAM Share", Ordered, func() { BeforeAll(func() { secondaryTestRoleArn = os.Getenv("SECONDARY_ACCOUNT_TEST_ROLE_ARN") + retryer := awsClient.DefaultRetryer{ + MinThrottleDelay: 1 * time.Second, + MinRetryDelay: 1 * time.Second, + MaxThrottleDelay: 5 * time.Second, + MaxRetryDelay: 5 * time.Second, + NumMaxRetries: 3, + } + primarySess := session.Must(session.NewSession(&aws.Config{ Region: aws.String(config.Region), - Retryer: retry.WithMaxRetries(3), + Retryer: retryer, })) stsClient := sts.New(primarySess) @@ -69,7 +77,7 @@ var _ = Describe("RAM Share", Ordered, func() { secondarySess := session.Must(session.NewSession(&aws.Config{ Region: aws.String(config.Region), - Retryer: retry.WithMaxRetries(3), + Retryer: retryer, Credentials: credentials.NewStaticCredentials( *creds.AccessKeyId, *creds.SecretAccessKey, From 0e11187a6e14052209631360a7aebdff448e8dd4 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Thu, 6 Feb 2025 09:34:36 -0800 Subject: [PATCH 4/4] Remove 3 maximum retry override --- test/suites/integration/ram_share_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/test/suites/integration/ram_share_test.go b/test/suites/integration/ram_share_test.go index 8ef424fb..03006689 100644 --- a/test/suites/integration/ram_share_test.go +++ b/test/suites/integration/ram_share_test.go @@ -56,7 +56,6 @@ var _ = Describe("RAM Share", Ordered, func() { MinRetryDelay: 1 * time.Second, MaxThrottleDelay: 5 * time.Second, MaxRetryDelay: 5 * time.Second, - NumMaxRetries: 3, } primarySess := session.Must(session.NewSession(&aws.Config{