Update subnet CIDRs when reconciling existing vnet

[CecileRobertMichon]

What type of PR is this?
/kind bug

What this PR does / why we need it: This fixes an edge case scenario where the user specifies an existing BYO vnet along with subnets without specifying CIDR ranges. CAPZ will fill out the details in the controller when doing a GET of the existing vnet/subnet. However, in the case that the controller returns in between those reconciling the vnet and the subnets, which it often does as we reconcile resources async, it will attempt to patch the object with the updated vnet CIDR but the wrong defaulted subnet CIDRs, which results in a validation error similar to: "validation.azurecluster.infrastructure.cluster.x-k8s.io" denied the request: AzureCluster.infrastructure.cluster.x-k8s.io "efreed-workload1" is invalid: [spec.networkSpec.subnets[0].cidrBlocks: Invalid value: "10.0.0.0/16": subnet CIDR not in vnet address space: [10.22.128.0/22], .

This updates the reconciler code to update both vnet and subnet CIDR ranges together when an existing VNet with subnets matching the defined subnet names is found.

Thanks @evanfreed for reporting the issue.

slack thread: https://kubernetes.slack.com/archives/CEX9HENG7/p1653600096205839

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 #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

• squashed commits
• includes documentation
• adds unit tests

Release note:

Update subnet CIDRs when reconciling existing vnet

[CecileRobertMichon]

/hold for confirmation from @evanfreed that this fixes the issue

[evanfreed]

@CecileRobertMichon I can confirm your fix seems to work!

Example spec used

apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AzureCluster
metadata:
  name: ${CLUSTER_NAME}
  namespace: default
spec:
  location: ${AZURE_LOCATION}
  networkSpec:
    vnet:
      resourceGroup: ${AZURE_RESOURCE_GROUP}
      name: ${AZURE_VNET_NAME}
    subnets:
      - name: control_plane
        role: control-plane
        #cidrBlocks:
          #- 10.22.128.128/25
      - name: worker_nodes
        role: node
        #cidrBlocks:
          #- 10.22.130.0/23
  resourceGroup: ${AZURE_RESOURCE_GROUP}

The machine provisioned in the pre-created CIDR range that we self manage without needing to define it in the spec 🎉 thanks for the help!

/lgtm

[k8s-ci-robot]

@evanfreed: changing LGTM is restricted to collaborators

In response to this:

@CecileRobertMichon I can confirm your fix seems to work!

Example spec used

apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AzureCluster
metadata:
 name: ${CLUSTER_NAME}
 namespace: default
spec:
 location: ${AZURE_LOCATION}
 networkSpec:
   vnet:
     resourceGroup: ${AZURE_RESOURCE_GROUP}
     name: ${AZURE_VNET_NAME}
   subnets:
     - name: control_plane
       role: control-plane
       #cidrBlocks:
         #- 10.22.128.128/25
     - name: worker_nodes
       role: node
       #cidrBlocks:
         #- 10.22.130.0/23
 resourceGroup: ${AZURE_RESOURCE_GROUP}

The machine provisioned in the pre-created CIDR range that we self manage without needing to define it in the spec 🎉 thanks for the help!

/lgtm

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.

[jackfrancis]

/retest

[jackfrancis]

wow gfmt is picky

[jackfrancis]

/lgtm

[jackfrancis]

/assign @mboersma

for final review and approval/merge

[CecileRobertMichon]

/hold cancel

[CecileRobertMichon]

2022-06-02T15:30:11.515802566Z stderr F E0602 15:30:11.515601 1 controller.go:317] controller/azuremachine "msg"="Reconciler error" "error"="failed to reconcile AzureMachine: failed to reconcile AzureMachine service virtualmachine: failed checking if the operation was complete: failed checking if the operation was complete: Code="ZonalAllocationFailed" Message="Allocation failed. We do not have sufficient capacity for the requested VM size in this zone. Read more about improving likelihood of allocation success at http://aka.ms/allocation-guidance\"" "name"="capz-pbacet-control-plane-dljw7" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine"

/retest

[CecileRobertMichon]

/retest

[k8s-ci-robot]

@CecileRobertMichon: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-azure-apidiff	29c9dc5	link	false	/test pull-cluster-api-provider-azure-apidiff

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.

[Jont828]

/lgtm

[CecileRobertMichon]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

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:

• OWNERS [CecileRobertMichon]

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