feat: add azure zone list cache #4811
Conversation
Signed-off-by: tanujd11 <[email protected]>
k8s-ci-robot
added
cncf-cla: yes
|
Hi @tanujd11. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
size/L
provider/azure/cache.go
Outdated
| } | ||
| } | ||
|
|
||
| // Get method to retrieve the cached zones |
There was a problem hiding this comment.
comments should add more value than writing what you can read.
In this example you can add if it's a shallow copy or not for example.
There was a problem hiding this comment.
Have added more concrete lines. Thanks
provider/azure/cache.go
Outdated
| return z.zones | ||
| } | ||
|
|
||
| // Expired method to check if the cache has expired based on duration or if zones are empty |
There was a problem hiding this comment.
Why do we check for len(zones) would be interesting for the doc.
|
|
||
| // Expired method to check if the cache has expired based on duration or if zones are empty | ||
| func (z *zonesCache[T]) Expired() bool { | ||
| return len(z.zones) < 1 || time.Since(z.age) > z.duration |
There was a problem hiding this comment.
Why z.age and not time.Now() ?
There was a problem hiding this comment.
z.age is the time of last update. Basically the time passed since last update, if it is greater than the allowed duration, then we invalidate the cache. I am not sure what you mean how time.Now() will work here.
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
Signed-off-by: tanujd11 <[email protected]>
Signed-off-by: tanujd11 <[email protected]>
k8s-ci-robot
added
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: szuecs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
zoneListCache is introduced for Azure so that the azure ratelimiting can be countered with for this particular call. Although the ratelimit is higher for this particular call but in Azure there are some ratelimiters which are shared between various services for example this particular one comes in microsoft.network API provider. This has resulted in some API ratelimiting for this particular call for me even with a relatively lesser number of external DNS running(8). This particular call need not run on all the external DNS reconciliation anyways, it will improve the speed of the reconciliation as well.
Error is mentioned below:
Fixes #ISSUE
Checklist