enable per-sub msi client #4712
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
cncf-cla: yes
jackfrancis
force-pushed
the
msi-per-sub-client
branch
from
4a1621e to
dfd7fde
Compare
|
Thanks Jack for the quick fix. The change looks good to me. Could you apply the same fix to the below places as well?
|
jackfrancis
force-pushed
the
msi-per-sub-client
branch
from
dfd7fde to
c2d5e10
Compare
k8s-ci-robot
added
size/M
|
@zmyzheng additional controller flows updated as well I'd like some comments from @nojnhuh and @willie-yao if this approach is sustainable (and if it's correct to call this a bug; IMO if the documentation suggests you can use any MSI and not just one that is in the same sub, I think it's a bug) |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 91121dc208f79c38033b9c698d79835b4934e436
|
jackfrancis
force-pushed
the
msi-per-sub-client
branch
from
c2d5e10 to
87dab2a
Compare
k8s-ci-robot
removed
the
lgtm
Signed-off-by: Jack Francis <[email protected]>
jackfrancis
force-pushed
the
msi-per-sub-client
branch
from
87dab2a to
a22c64b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4712 +/- ##
==========================================
- Coverage 62.73% 62.61% -0.12%
==========================================
Files 192 192
Lines 15641 15673 +32
==========================================
+ Hits 9812 9814 +2
- Misses 5146 5172 +26
- Partials 683 687 +4 ☔ View full report in Codecov by Sentry. |
|
/test ls |
|
@jackfrancis: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
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. |
|
/test pull-cluster-api-provider-azure-apiversion-upgrade |
|
/retest |
1 similar comment
|
/retest |
|
@jackfrancis: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
|
@nojnhuh this is ready for a final review |
There was a problem hiding this comment.
/lgtm
/approve
/hold if we want to make sure the optional tests pass before merging this. @jackfrancis feel free to remove if we determine the issue is with those tests and not these changes.
k8s-ci-robot
added
do-not-merge/hold
|
LGTM label has been added. Git tree hash: a323885a0d90cb0baf808a5770d40e2600f1af54
|
|
/cherry-pick release-1.13 ^ @jackfrancis I think we should backport this? |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nojnhuh 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 |
k8s-ci-robot
added
the
approved
|
@nojnhuh: once the present PR merges, I will cherry-pick it on top of release-1.13 in a new PR and assign it to you. In response to this:
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. |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
@nojnhuh: new pull request created: #4716 In response to this:
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. |
|
@nojnhuh: new pull request created: #4717 In response to this:
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. |
|
/cherry-pick release-1.14 |
|
@nojnhuh: new pull request could not be created: failed to create pull request against kubernetes-sigs/cluster-api-provider-azure#release-1.14 from head k8s-infra-cherrypick-robot:cherry-pick-4712-to-release-1.14: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for k8s-infra-cherrypick-robot:cherry-pick-4712-to-release-1.14."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request"} In response to this:
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. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR fixes a shortcoming where we assume that all MSI operate against a common subscription ID (inherited from the MSI attached to the workload cluster VMs).
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 #4711
Special notes for your reviewer:
TODOs:
Release note: