cleanup: refactor Azure cache and remove redundant API calls

[CecileRobertMichon]

This PR cleans up the Azure cloud provider cache to optimize API calls and facilitate further improvements.

Currently, multiple Azure VMSS List calls are made throughout the code, such as here and here. In addition, agent_pools and aks providers both have their own code to list VMs.

This centralizes all VM and VMSS List() API calls in one central cache which refreshes every minute, which means we should see 60 VM list and 60 VMSS list API calls per hour, regardless of # of agent pools/scale sets. The other calls for VMSS are 1) capacity increase/decrease for scale ups and scale downs, and 2) VMSS VM list (which lists all the instances in a scale set). Number 2) is still quite costly and increases linearly with the number of scale sets. I'd like to try and tackle it next once this PR merges. For agent pools, the only other calls are Deletes and Creates to add/remove VMs.

Here is some initial data I gathered. Both of these clusters have 10 scale sets. The first one is running the latest cluster-autoscaler release, and the second is running with an image built from this PR's code. Chart shows number of API calls per 5 minutes:

With 20 scale sets when autoscaler is idle (no workloads running):
Before (54-56 calls per 5 minutes):

After (50 calls per 5 minutes):

[CecileRobertMichon]

/cc @khenidak @marwanad @feiskyer

[CecileRobertMichon]

@feiskyer @nilo19 please review

[feiskyer]

LGTM in general

@marwanad since the changes here are not small, could you run an e2e test with the patch?

[feiskyer]

Please fix the unit test failures:

# k8s.io/autoscaler/cluster-autoscaler/cloudprovider/azure [k8s.io/autoscaler/cluster-autoscaler/cloudprovider/azure.test]
cloudprovider/azure/azure_cache_test.go:31:26: not enough arguments in call to newAzureCache
	have (*azClient, string)
	want (*azClient, time.Duration, string, string)
cloudprovider/azure/azure_cache_test.go:49:26: not enough arguments in call to newAzureCache
	have (*azClient, string)
	want (*azClient, time.Duration, string, string)
cloudprovider/azure/azure_cache_test.go:60:26: not enough arguments in call to newAzureCache
	have (*azClient, string)
	want (*azClient, time.Duration, string, string)
cloudprovider/azure/azure_cloud_provider_test.go:71:31: not enough arguments in call to newAzureCache
	have (*azClient, string)
	want (*azClient, time.Duration, string, string)
cloudprovider/azure/azure_scale_set_test.go:44:22: undefined: defaultVmssSizeRefreshPeriod

[nilo19]

/lgtm

[nilo19]

the unit test still fails

[marwanad]

what is this channel used for?

[CecileRobertMichon]

I'm not entirely sure, it was there before https://github.com/kubernetes/autoscaler/pull/3717/files#diff-63481ca096e322d8f48e57a4b21089a30481cc3798e3dc123a1b4ba0938ceb1dL37

[marwanad]

Were the before and after case running with a different TTL? So, before was run with 15 seconds and the after case is this PR (which has a TTL of 60 seconds)

[CecileRobertMichon]

Were the before and after case running with a different TTL? So, before was run with 15 seconds and the after case is this PR (which has a TTL of 60 seconds)

Before there were 4 caches:

• vmInstancesRefreshPeriod (list agent pool VMs): TTL 5 minutes
• vmssSizeRefreshPeriod (list scale sets): TTL 15 seconds
• vmssInstancesRefreshPeriod (list scale set VMs): TTL 5 minutes
• manager asgCache (also calling list scale sets): TTL 1 minute

Now there is:

• manager azureCache (lists VMs or VMSS depending on config): TTL 1 minute
• vmssInstancesRefreshPeriod (list scale set VMs): TTL 5 minutes

So yes, one of the TTLs which was at 15s to list scale sets in the scale set cache changed from 15s to 1 minute by default. The VmssCacheTTL config value is still honored though and will override that 1 minute refresh interval if set.

[CecileRobertMichon]

@marwanad were you able to run the e2e test that @feiskyer mentioned above? I can also help with that if you show me how to do it.

[marwanad]

will have a soak cluster over the weekend and take another pass at the PR.

[marwanad]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, marwanad

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 [marwanad]

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