fix issue that pull image failed from a cross-subscription Azure Container Registry by SP access

[andyzhangx]

What this PR does / why we need it:
after granting sp access to azure ACR , pull image from ACR would fail, and after wait about 15-30min(or restart kubelet directly), pull image would succeed. Root cause is that servicePrincipalToken needs to be refreshed when doing registryClient.List, otherwise it will always return empty registry list. Pull image error would be like following:

Events:
  Type     Reason                 Age              From                               Message
  ----     ------                 ----             ----                               -------
  Warning  FailedScheduling       8m (x3 over 8m)  default-scheduler                  0/1 nodes are available: 1 Insufficient cpu.
  Normal   Scheduled              8m               default-scheduler                  Successfully assigned nginx-server-776564f79c-zhtjk to aks-nodepool1-20881069-0
  Normal   SuccessfulMountVolume  8m               kubelet, aks-nodepool1-20881069-0  MountVolume.SetUp succeeded for volume "default-token-4t7tk"
  Normal   SuccessfulMountVolume  8m               kubelet, aks-nodepool1-20881069-0  MountVolume.SetUp succeeded for volume "pvc-5c1f0521-739f-11e8-9b69-0a58ac1f09c2"
  Warning  Failed                 8m (x5 over 8m)  kubelet, aks-nodepool1-20881069-0  Error: ImagePullBackOff
  Normal   BackOff                8m (x5 over 8m)  kubelet, aks-nodepool1-20881069-0  Back-off pulling image "andyacr.azurecr.io/nginx-server:1.0.0"
  Warning  Failed                 8m (x2 over 8m)  kubelet, aks-nodepool1-20881069-0  Error: ErrImagePull
  Warning  Failed                 8m (x2 over 8m)  kubelet, aks-nodepool1-20881069-0  Failed to pull image "andyacr.azurecr.io/nginx-server:1.0.0": rpc error: code = Unknown desc = Error response from daemon: Get https://andyacr.azurecr.io/v2/nginx-server/manifests/1.0.0: unauthorized: authentication required

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 #65225

Special notes for your reviewer:
After discuss with dong, registryClient.List won't be necessary, instead we return {"*.azurecr.io", "*.azurecr.cn", "*.azurecr.de", "*.azurecr.us"} like aws, gce code logic, it will do the url matching.
I will cherry pick this PR to all supported version, every version has this issue.

Neet to mention that this PR would also make image pull from a cross-subscription ACR work after we grant SP access to an ACR in a different subs. Original code won’t work because it’s using the SubscriptionID of current AKS cluster, since ACR is in a different subs, registryClient.List will never get cross subs ACR repos.

Release note:

fix issue that pull image failed from a cross-subscription Azure Container Registry

/sig azure
/assign @feiskyer @khenidak @brendandburns @karataliu

[andyzhangx]

/kind bug

[karataliu]

This will call AAD API every time to get token every time 'Provide' is called.
Can it cache 'servicePrincipalToken', and 'registryClient', and only do a re-fetch if later 'registryClient.List' fails?

[feiskyer]

+1 to do this only when not getting the expected result.

Since the list is actually successful but without the expected result, you can add a check of the list, e.g. len(result) == 0?

[andyzhangx]

@karataliu @feiskyer The Provide func call happens when there is pull image requirement, and if pull image fails, it will try to rerun Provide by lookup func every 5 min, so not every kubelet will call this func only when there is pull image failure, related invocation code is here:

kubernetes/pkg/kubelet/kuberuntime/kuberuntime_image.go

Line 45 in 795b7da

creds, withCredentials := keyring.Lookup(repoToPull)

[karataliu]

Seems the config key can be a glob pattern, so we might just remove the list acr part.

kubernetes/pkg/credentialprovider/keyring.go

Line 184 in af60963

func urlsMatchStr(glob string, target string) (bool, error) {

[andyzhangx]

/hold

[andyzhangx]

/hold cancel

[andyzhangx]

@karataliu code changed, and test pass, PTAL

[andyzhangx]

note, we keep the original logic for UseManagedIdentityExtension, while for sp, we always return loginServer as *.azurecr.io, *.azurecr.cn and that would save ARM calls

[feiskyer]

+1 to the new method

[andyzhangx]

/test pull-kubernetes-integration
/test pull-kubernetes-e2e-kops-aws

[feiskyer]

/lgtm
/retest

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, feiskyer

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:

• pkg/credentialprovider/azure/OWNERS [andyzhangx,feiskyer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-github-robot]

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

[keyoke]

I'm sorry but maybe I am missing something but I'm not sure how the code solves the cross-sub acr access issue for MSI?
We are still using registryClient.List(ctx) will list this list registries across subscriptions? -

kubernetes/pkg/credentialprovider/azure/azure_credentials.go

Line 181 in 3741f02

if a.config.UseManagedIdentityExtension {

[andyzhangx]

@keyoke, this PR fix cross-sub acr access issue for sp, not MSI

[keyoke]

@andyzhangx - Ok thanks any plans for us to achieve the same with MSI?

[andyzhangx]

I filed another issue here: #67892, I will dig into the code and find a way. You could also comment if you have any better idea, thanks.