Issues when upgrading Controller to a version that include the backendPool.name feature

[primeroz]

/kind bug

What steps did you take and what happened:
When upgrading controller to a version that include backendPool.name there is an issue with AzureCluster resources that were created before and did not include the required field

• there is a webhook that will add the required fields backendPool.name when the resource is Created / Updated
• There is conversion code from old version of the api that will also handle this
• when upgrading the controller any exisiting Azurecluster will not be updated by the webhook ( since no event happened ) and the requests to Azure will fail with

message: 'loadbalancers failed to create or update. err: failed to update resource
      glippy/glippy-public-lb (service: loadbalancers): network.LoadBalancersClient#CreateOrUpdate:
      Failure sending request: StatusCode=400 -- Original Error: Code="InvalidResourceName"
      Message="Resource name  is invalid. The name can be up to 80 characters long.
      It must begin with a word character, and it must end with a word character or
      with ''_''. The name may contain word characters or ''.'', ''-'', ''_''." Details=[]'

• this is because the request must contain a name for the backend pool that is actually ""

Adding the fields to the AzureClusters CR fixes the issue and everything is clean

What did you expect to happen:
Upgrade would not require to modify the CRs , ideally the code should be backward compatible

Anything else you would like to add:
Discussion in slack https://kubernetes.slack.com/archives/CEX9HENG7/p1678370754712279

Environment:

• cluster-api-provider-azure version: upgraded from 1.6.1 to 1.7.0 ( and 1.7.2 and 1.8.0 )
• Kubernetes version: (use kubectl version): 1.24.11
• OS (e.g. from /etc/os-release):

[CecileRobertMichon]

/milestone v1.8

[CecileRobertMichon]

/help

[k8s-ci-robot]

@CecileRobertMichon:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

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.

[sonasingh46]

/assign @sonasingh46

[sonasingh46]

Well here is what is happening:

• There exists AzureCluster that has an empty ("") backend pool name on prior versions that does not support customising backend pool name i.e prior to this change
• Now, the capz controller gets upgraded to this new change.
• The webhook does not execute on read calls failing to execute the defaulting logic causing errors as empty LB backend pool name is not acceptable by Azure API.
• So, if anyone just trigger an update on the AzureCluster object ( e.g. adding labels on AzureCluster object ) -- it fixes the error coz now webhook gets triggered due to a write operation.

What is the fix?

• CAPZ controller has a defensive logic that whenever it encounters an empty LB backend pool name, it just assigns a backend pool name by using the previous name generator function and not the new one to keep the default behaviour.
• For the greenfield AzureCluster, the behaviour is expected i.e if explicitly specified on AzureCluster the backend pool name is generate as according to the new generator function.
• And once folks have upgraded, in the next release we can remove this code.