kubernetes / autoscaler

Autoscaling components for Kubernetes
Apache License 2.0
7.99k stars 3.94k forks source link

Update Azure cluster-autoscaler e2e cluster template #6975

Closed nojnhuh closed 3 months ago

nojnhuh commented 3 months ago

What type of PR is this?

/kind bug /kind failing-test

What this PR does / why we need it:

This change adds a Secret stub to the CAPZ cluster template for cluster-autoscaler e2e tests on Azure to play nice with the changes made in https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/4946

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

nojnhuh commented 3 months ago

/retest

nojnhuh commented 3 months ago

/retest

nojnhuh commented 3 months ago

/retest

nojnhuh commented 3 months ago

testing one more time pointing to https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/4946

/test pull-cluster-autoscaler-e2e-azure

nojnhuh commented 3 months ago

This is passing now and ready for review.

/assign @jackfrancis @willie-yao /retitle Update Azure cluster-autoscaler e2e cluster template

jackfrancis commented 3 months ago

@nojnhuh I did notice this in the latest E2E run:

default cluster-autoscaler-azure-cluster-autoscaler-7c58b754fc-wt2m9 0/1 Error 0 18s 10.224.0.51 aks-pool1-36728867-vmss000001

Maybe you're already aware of that

nojnhuh commented 3 months ago

@nojnhuh I did notice this in the latest E2E run:

default cluster-autoscaler-azure-cluster-autoscaler-7c58b754fc-wt2m9 0/1 Error 0 18s 10.224.0.51 aks-pool1-36728867-vmss000001

Maybe you're already aware of that

Added a call to grab the logs in my CAPZ branch to see what's going on.

/test pull-cluster-autoscaler-e2e-azure

nojnhuh commented 3 months ago

/test pull-cluster-autoscaler-e2e-azure

nojnhuh commented 3 months ago

/test pull-cluster-autoscaler-e2e-azure

nojnhuh commented 3 months ago

/test pull-cluster-autoscaler-e2e-azure

(sorry for spam, keep seeing little things wrong in my CAPZ debugging changes, should be good enough now.)

nojnhuh commented 3 months ago

/test pull-cluster-autoscaler-e2e-azure

nojnhuh commented 3 months ago

I'd need to run this again with more debugging to confirm, but I imagine this issue I hit in the portal is the same as what's happening in the test, where our CI credentials don't have permission to create role assignments, even scoped to an individual resource group:

image
nojnhuh commented 3 months ago

I'd need to run this again with more debugging to confirm, but I imagine this issue I hit in the portal is the same as what's happening in the test, where our CI credentials don't have permission to create role assignments, even scoped to an individual resource group

@jackfrancis Are there any major concerns with giving the user-assigned identity an Owner role vs. Contributor? I think that would be one solution to this problem.

https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/52df930d5c636071091f7ba904b606bc095346c6/scripts/kind-with-registry.sh#L150

nojnhuh commented 3 months ago

Actually for real all green now, so this should be ready for review.

jackfrancis commented 3 months ago

/lgtm /approve

k8s-ci-robot commented 3 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, nojnhuh, willie-yao

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: - ~~[cluster-autoscaler/cloudprovider/azure/OWNERS](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/cloudprovider/azure/OWNERS)~~ [jackfrancis] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment