cluster-api-provider-azure
cluster-api-provider-azure copied to clipboard
WIP AKS: enable IsVnetManaged with rate limiting and 429 retry
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR enables the IsVnetManaged() method for AKS scenarios, which results in actual GET calls against the Azure API to determine in real-time whether the expected capz owner tag (tag is applied one time during cluster create) is present on the Virtual Network.
This will address the subnet delete issue during custom VNET scenarios.
Additionally, in order to account for additional calls against the Azure APIs I've wired in a rate limiting + HTTP 429 retry solution (inspired by cloud-provider-azure).
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2484
Special notes for your reviewer:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
TODOs:
- [ ] squashed commits
- [ ] includes documentation
- [ ] adds unit tests
Release note:
Not sure if this is relevant, but should the commit message also be changed to be more relevant to what the PR is actually doing now? I'm not sure if that's being too picky in reviews...
Not sure if this is relevant, but should the commit message also be changed to be more relevant to what the PR is actually doing now? I'm not sure if that's being too picky in reviews...
Yes, will definitely change the title!
/retitle AKS: enable isVnetManaged, add caching
Yes, will definitely change the title!
can you please also amend the commit? That's what will show up in commit history
Let's also add a release note for this change
Additionally, in order to account for additional calls against the Azure APIs I've wired in a rate limiting + HTTP 429 retry solution (inspired by cloud-provider-azure).
Remove this from PR description?
Additionally, in order to account for additional calls against the Azure APIs I've wired in a rate limiting + HTTP 429 retry solution (inspired by cloud-provider-azure).
Remove this from PR description?
Done
/retest
@mtougeron @CecileRobertMichon this is ready for another (hopefully final? 🤞) review round
/approve
/cherry-pick release-1.4
@CecileRobertMichon: once the present PR merges, I will cherry-pick it on top of release-1.4 in a new PR and assign it to you.
In response to this:
/cherry-pick release-1.4
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/test-infra repository.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: CecileRobertMichon, mtougeron
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [CecileRobertMichon]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@CecileRobertMichon: new pull request created: #2600
In response to this:
/cherry-pick release-1.4
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/test-infra repository.