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.

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

fix: creating a retryable kong client for tests and retry logic in konnect authentication for CI rate-limit errors #1381

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

Prashansa-K
Copy link
Contributor

@Prashansa-K Prashansa-K commented Aug 27, 2024

  • chore: creating a retryable kong client for tests to account for rate-limit errors
  • fix: added retry and backoff logic in Konnect authentication layer to account for rate-limit errors during authentication.

Fixes #1364

@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Project coverage is 22.42%. Comparing base (e44d6c4) to head (2125626).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
cmd/common_konnect.go 0.00% 13 Missing ⚠️
tests/integration/test_utils.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1381   +/-   ##
=======================================
  Coverage   22.42%   22.42%           
=======================================
  Files          54       54           
  Lines        4517     4530   +13     
=======================================
+ Hits         1013     1016    +3     
- Misses       3404     3415   +11     
+ Partials      100       99    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GGabriele
Copy link
Collaborator

Don't we already have retries supported in our client? https:/Kong/go-database-reconciler/blob/25ccc04e6c42e1b447070391e3938913deaed9fb/pkg/utils/types.go#L182

@Prashansa-K
Copy link
Contributor Author

Prashansa-K commented Sep 3, 2024

Don't we already have retries supported in our client? https:/Kong/go-database-reconciler/blob/25ccc04e6c42e1b447070391e3938913deaed9fb/pkg/utils/types.go#L182

Yeah the client does have it, but the authentication layer still returns 429. The Konnect's login_service uses a normal http request here and is not retried. Does the client retry it by default?
I added my own retry layer but it still fails with 429 with 5 retries. Trying with an increased backoff period.

Screenshot 2024-09-03 at 10 46 59 AM

@Prashansa-K
Copy link
Contributor Author

Works fine with an increased backoff period.
I tried the pipeline run thrice and it succeeds each time.
Screenshot 2024-09-03 at 10 51 55 AM

@Prashansa-K Prashansa-K changed the title chore: creating a retryable kong client for tests to account for rate-limit errors fix: creating a retryable kong client for tests and retry logic in konnect authentication for CI rate-limit errors Sep 3, 2024
@Prashansa-K Prashansa-K merged commit 18831e9 into main Sep 4, 2024
19 checks passed
@Prashansa-K Prashansa-K deleted the fix/test-fix-retry-strategy branch September 4, 2024 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling Konnect API rate limiting
3 participants