Add multi zone template for Cloud Provider Azure CI#6252
Conversation
Signed-off-by: William Yao <william2000yao@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6252 +/- ##
==========================================
+ Coverage 43.85% 46.25% +2.40%
==========================================
Files 289 289
Lines 25341 29951 +4610
==========================================
+ Hits 11113 13854 +2741
- Misses 13450 15319 +1869
Partials 778 778 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
This looks good, but I have two comments:
- it assumes 3 availability zones, which isn't true for all Azure regions. So it's on the consumer (cloud-provider-azure) to deploy this only in comptible regions. Just sayin'.
- It's not used by CAPZ itself. So as we've seen before (with the Windows templates in particular), it can rust and become broken if downstream consumers don't actively use it and effectively help us maintain it.
It sounds like you're aware of these tradeoffs and decided it was still preferable to have it in CAPZ, correct?
|
@mboersma Thanks for the review! I did consider both of these. I verified that all the locations we use in CI had three availability zones. Also, the template is based mostly on the machine pool ci version template with just the availability zone patched in, so there shouldn't be issues unless more needs to be added. Cloud provider azure also uses the CAPZ template for |
mboersma
left a comment
There was a problem hiding this comment.
/lgtm
/assign @jackfrancis
|
LGTM label has been added. DetailsGit tree hash: f9e50c90f88bdc1dfebb30963259c9e4bd5aedb9 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mboersma The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
This PR adds a newer version of the multi zone vmss template from cloud provider azure with improvements in an attempt to fix the test. It was removed in kubernetes/test-infra#36242 for being broken
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 kubernetes-sigs/cloud-provider-azure#9863
Special notes for your reviewer:
TODOs:
Release note: