-
Notifications
You must be signed in to change notification settings - Fork 590
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
feat(konnect): introduce backoff strategy for config updates #3989
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #3989 +/- ##
=======================================
- Coverage 59.5% 59.1% -0.5%
=======================================
Files 145 145
Lines 16125 16136 +11
=======================================
- Hits 9603 9540 -63
- Misses 5889 5964 +75
+ Partials 633 632 -1
☔ View full report in Codecov by Sentry. |
0e05c45
to
f43da1f
Compare
E2E (targeted) tests were started at https://github.com/Kong/kubernetes-ingress-controller/actions/runs/4951947486 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor comments
8db13b7
to
9d9d1cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nits. Otherwise 🚢
Co-authored-by: Patryk Małek <patryk.malek@konghq.com>
What this PR does / why we need it:
It introduces the concept of
UpdateBackoffStrategy
that can be used to decorate anyUpdateStrategy
with a custom backoff behavior. It implements such one (KonnectBackoffStrategy
) foradminapi.KonnectClient
and decorates itsUpdateStrategy
with that every time it's created.The reason for introducing this is to prevent KIC from issuing too many requests to Konnect APIs when the configuration it tries to push is broken due to client-side faults such as e.g. duplicate IDs of entities.
Konnect's backoff strategy is composed of two requirements that both have to be satisfied to perform an Update action:
Which issue this PR fixes:
Should fix #3959.
Special notes for your reviewer:
This implementation doesn't take into account
Retry-After
header that Konnect may return when we hit 429 as it wasn't possible to inspect headers on KIC level. We can consider extending thekong.APIError
with details for the specific status codes, allowing us to convey the retry-after value in that.Nevertheless, the backoff itself should save us from exceeding the API calls limits on Konnect side in the case of a faulty configuration.
PR Readiness Checklist:
Complete these before marking the PR as
ready to review
:CHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR