[WIP]: Use multiple zones in case of multiple subnets

[Shilpa-Gokul]

What this PR does / why we need it: Currently when there are multiple subnets specified, it uses the first available vpcZone for all subnets by default and as a result, there is a clash in the CIDR. Hence added logic to use the subsequent vpcZones for the subnets.

If there are multiple subnets provided than the available vpcZones (ex: when 4 subnets are provided but only 3 vpcZones are available in the region), in this case we return error as below

error validating input, total vpc subnets to be created cannot be more than the available vpc zones. subnets-4, 
vpczones-3

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

Special notes for your reviewer:

/area provider/ibmcloud

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

Release note:

[k8s-ci-robot]

Hi @Shilpa-Gokul. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

[netlify[bot]]

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Name	Link
🔨 Latest commit	b4bd92b9ce3cdbbfac9a03851abbb6efdd8fc1f8
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/6746d0f0f736e200086aa65a
😎 Deploy Preview	https://deploy-preview-1793--kubernetes-sigs-cluster-api-ibmcloud.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[dharaneeshvrd]

will there be any uses cases of user setting more subnets? like vpc region has 2 zones but user wants to create 4 subnets?

its possible, we can just round robin the zones once subnets created for all zones. i.e. consider the region has 2 zones, 10.241.0.0/24 is already used for subnet 1 for subnet 3, it automatically picks 10.241.1.0/24.

[Shilpa-Gokul]

Tested the following scenarios

Case 1: When user provides more subnets than the available vpczones and provides no zone information with each subnet (subnets are created in the available vpc zones in a round robin method)

vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet
  - name: capi-test-cluster-sg-vpcsubnet-1
  - name: capi-test-cluster-sg-vpcsubnet-2
  - name: capi-test-cluster-sg-vpcsubnet-3
  - name: capi-test-cluster-sg-vpcsubnet-4
  - name: capi-test-cluster-sg-vpcsubnet-5
  - name: capi-test-cluster-sg-vpcsubnet-6
  zone: mad02

Case 2: When user provides 2 subnets in the same zone to be created

 vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet-1
    zone: eu-es-2
  - name: capi-test-cluster-sg-vpcsubnet-2
    zone: eu-es-2

Case 3: User provides 1 subnet and the zone to be created in

 vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet-1
    zone: eu-es-2

Case 4: When user provides already created subnet

vpc:
  name: capi-test-cluster-sg-vpc-new
  region: eu-es
vpcSubnets:
- name: sn-20240723-01
zone: mad02

Log- [manager] I0723 12:15:38.121366 1 powervs_cluster.go:1103] "VPC subnet ID is set, fetching details" controller="ibmpowervscluster" controllerGroup="infrastructure.cluster.x-k8s.io" controllerKind="IBMPowerVSCluster" IBMPowerVSCluster="default/capi-test-cluster-sg" namespace="default" name="capi-test-cluster-sg" reconcileID="19a61637-59a3-4ea0-896f-6bb3a4616902" cluster="default/capi-test-cluster-sg" id="02w7-e123549e-4414-480c-b018-7017492889b3"

Case 5: When user provides subnets with no zone information

vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet-1
  - name: capi-test-cluster-sg-vpcsubnet-2
  zone: mad02

@Karthik-K-N @Amulyam24 Please check this and let me know if I have to test anything else

[Karthik-K-N]

Since you are on this please take a look at this as well https://github.com/kubernetes-sigs/cluster-api-provider-ibmcloud/issues/1779#issuecomment-2378449066

[Shilpa-Gokul]

/assign @Shilpa-Gokul

[Shilpa-Gokul]

@Karthik-K-N I have used TotalIpv4AddressCount instead of Ipv4CIDRBlock for creating vpc subnet to be in sync with the vpc code. Both seems to do the same job, just that its easy to understand the number of ip addresses a subnet will have with the TotalIpv4AddressCount rather than calculating it with Ipv4CIDRBlock.

Verified the following usecases with the latest change. Please check and let me know if I need to test any other scenarios.

Usecase 1- When the user provide more subnets than the available vpc zones

Input:

vpc:
    name: capi-test-cluster-sg-vpc
    region: eu-es
  vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet-1
  - name: capi-test-cluster-sg-vpcsubnet-2
  - name: capi-test-cluster-sg-vpcsubnet-3
  - name: capi-test-cluster-sg-vpcsubnet-4
  zone: mad02

Output:

I1028 13:36:03.854799      14 ibmpowervscluster_controller.go:191] "Reconciling VPC subnets" logger="vpc" controller="ibmpowervscluster" controllerGroup="infrastructure.cluster.x-k8s.io" controllerKind="IBMPowerVSCluster" IBMPowerVSCluster="default/capi-test-cluster-sg" namespace="default" name="capi-test-cluster-sg" reconcileID="48e3c68b-1e8f-4caf-bfd0-880455627211" cluster="default/capi-test-cluster-sg"
E1028 13:36:03.855044      14 ibmpowervscluster_controller.go:193] "failed to reconcile VPC subnets" err="error validating input, total vpc subnets to be created cannot be more than the available vpc zones. subnets-4, vpczones 3" logger="vpc" controller="ibmpowervscluster" controllerGroup="infrastructure.cluster.x-k8s.io" controllerKind="IBMPowerVSCluster" IBMPowerVSCluster="default/capi-test-cluster-sg" namespace="default" name="capi-test-cluster-sg" reconcileID="48e3c68b-1e8f-4caf-bfd0-880455627211" cluster="default/capi-test-cluster-sg"

Usecase 2- When user provides a single subnetName along with zone

Input:

vpc:
   name: capi-test-cluster-sg-vpc
   region: eu-es
 vpcSubnets:
 - name: capi-test-cluster-sg-vpcsubnet-1
   zone: eu-es-2
 zone: mad02

Output:

Usecase 3- When user provides subnets equal to the available vpc zones

Input:

vpc:
    name: capi-test-cluster-sg-vpc
    region: eu-es
  vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet-1
  - name: capi-test-cluster-sg-vpcsubnet-2
  - name: capi-test-cluster-sg-vpcsubnet-3
  zone: mad02

Output:

Usecase 4- When user provides no vpcsubnets, subnets are created in each available zone

Input:

vpc:
    name: capi-test-cluster-sg-vpc
    region: eu-es
  zone: mad02

Output:

[Shilpa-Gokul]

/retest-required

[k8s-ci-robot]

@Shilpa-Gokul: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest-required

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-sigs/prow repository.

[Karthik-K-N]

Thanks for the detailed explaination, For usecase1 can we use webhooks to validate?

[Shilpa-Gokul]

Thanks for the detailed explaination, For usecase1 can we use webhooks to validate?

Added the validation in the webhook itself, below is the error response

The IBMPowerVSCluster "capi-test-cluster-sg" is invalid: : Invalid value: "spec.vpcSubnets": eu-es vpc region supports only 3 subnets but 4 subnets were provided
shilpagokul@Shilpas-MacBook-Pro templates %

[Shilpa-Gokul]

@Karthik-K-N @Prajyot-Parab Addressed all changes suggested, could you please review this PR? Also the checks are in pending state for so many days, retesting is also not working. could you please help on that as well?

[Karthik-K-N]

/ok-to-test

[Karthik-K-N]

LGTM /assign @dharaneeshvrd

[dharaneeshvrd]

@Shilpa-Gokul what happens with this case? Case 2: When user provides 2 subnets in the same zone to be created I think we probably need to add a validation to not to allow dup zones, it would cause collision with ip range with current code I guess.

[Shilpa-Gokul]

Tested the following scenarios

Usecase 1- When the user provide more subnets than the available vpc zones

Input:

vpc:
    name: capi-test-cluster-sg-vpc
    region: eu-es
  vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet-1
  - name: capi-test-cluster-sg-vpcsubnet-2
  - name: capi-test-cluster-sg-vpcsubnet-3
  - name: capi-test-cluster-sg-vpcsubnet-4
  zone: mad02

Output:

cluster.cluster.x-k8s.io/capi-test-cluster-sg unchanged
The IBMPowerVSCluster "capi-test-cluster-sg" is invalid: : Invalid value: "spec.vpcSubnets": eu-es vpc region supports only 3 subnets but 4 subnets were provided

Usecase 2- When user provides 2 subnets to be created in the same zone

Input:

 vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet-1
    zone: eu-es-2
  - name: capi-test-cluster-sg-vpcsubnet-2
    zone: eu-es-2
  zone: mad02

Output:

cluster.cluster.x-k8s.io/capi-test-cluster-sg created
The IBMPowerVSCluster "capi-test-cluster-sg" is invalid: spec.vpcSubnets[1]: Duplicate value: map[string]interface {}{"Zone":"eu-es-2"}

Usecase 3- When user provides a single subnetName along with zone

Input:

vpc:
   name: capi-test-cluster-sg-vpc
   region: eu-es
 vpcSubnets:
 - name: capi-test-cluster-sg-vpcsubnet-1
   zone: eu-es-2
 zone: mad02

Output:

Usecase 4- When user provides subnets equal to the available vpc zones

Input:

vpc:
    name: capi-test-cluster-sg-vpc
    region: eu-es
  vpcSubnets:
  - name: capi-test-cluster-sg-vpcsubnet-1
  - name: capi-test-cluster-sg-vpcsubnet-2
  - name: capi-test-cluster-sg-vpcsubnet-3
  zone: mad02

Output:

Usecase 5- When user provides no vpcsubnets, subnets are created in each available zone

Input:

vpc:
    name: capi-test-cluster-sg-vpc
    region: eu-es
  zone: mad02

Output:

Usecase 6- When user provides already existing subnet

Input:

vpcSubnets:
  - name: eu-es-1-default-subnet
    zone: eu-es-1

Output:

I1118 07:02:17.889163      14 powervs_cluster.go:1167] "Found VPC subnet in IBM Cloud" controller="ibmpowervscluster" controllerGroup="infrastructure.cluster.x-k8s.io" controllerKind="IBMPowerVSCluster" IBMPowerVSCluster="default/capi-test-cluster-sg" namespace="default" name="capi-test-cluster-sg" reconcileID="ac49a2a8-5e0e-425e-ab48-406d43137677" cluster="default/capi-test-cluster-sg" subnetID="02w7-5e7172ea-8306-444f-b4e6-97acbf35f530"

[Shilpa-Gokul]

@Shilpa-Gokul what happens with this case? Case 2: When user provides 2 subnets in the same zone to be created I think we probably need to add a validation to not to allow dup zones, it would cause collision with ip range with current code I guess.

@dharaneeshvrd Have added validation to return error as shown below

cluster.cluster.x-k8s.io/capi-test-cluster-sg created
The IBMPowerVSCluster "capi-test-cluster-sg" is invalid: spec.vpcSubnets[1]: Duplicate value: map[string]interface {}{"Zone":"eu-es-2"}

[Shilpa-Gokul]

Currently when we use vpcSubnetIPAddressCount, when two subnets are to be created in the same zone, the subnets are created with the next available address prefix based on the vpcSubnetIPAddressCount. When no zones are mentioned, the subnets are assigned to the zones in round robin method. Taking one of the below examples for reference

- name: subnet-1
    zone: eu-es-2
  - name: subnet-2
  - name: subnet-3
  - name: subnet-4
  - name: subnet-5
    zone: eu-es-2
  - name: subnet-6
    zone: eu-es-1
  - name: subnet-7

The zone selection is done as below, considering index 0 -> Madrid 1, index 1 -> Madrid 2, index 2 -> Madrid 3

subnet 1 -> Madrid 2 (since zone is mentioned)
subnet 2 -> Madrid 2 (since index is 1)
subnet 3 -> Madrid 3 (since index is 2)
subnet 4 -> Madrid 1 (index % len(totalvpcZones)= 3%3= 0 so Madrid 1)
subnet 5 -> Madrid 2 (since zone is mentioned)
subnet 6 -> Madrid 1 (since zone is mentioned)
subnet 7 -> Madrid 1 (index % len(totalvpcZones)= 6%3= 0 so Madrid 1)

Tested the following scenarios.

Usecase 1: When user provides more subnets than the available VPC zones and also mentions zone for one subnet

vpcSubnets:
  - name: subnet-1
    zone: eu-es-2
  - name: subnet-2
  - name: subnet-3
  - name: subnet-4
  - name: subnet-5
zone: mad02

Usecase 2: When user provides subnets more than the available VPC zones and mentions zone randomly for few subnets

vpcSubnets:
- name: subnet-1
  zone: eu-es-2
- name: subnet-2
- name: subnet-3
- name: subnet-4
- name: subnet-5
  zone: eu-es-2
- name: subnet-6
  zone: eu-es-1
- name: subnet-7
zone: mad02

Usecase 3: When user provides subnets less than the available VPC zones along with zone specified for each subnet

vpcSubnets:
  - name: subnet-1
    zone: eu-es-2
  - name: subnet-2
    zone: eu-es-1
zone: mad02

Usecase 4: When user provides already existing subnet

 vpcSubnets:
  - name: eu-es-1-default-subnet
    zone: eu-es-1
 zone: mad02

I1125 13:27:02.827412 14 powervs_cluster.go:1167] "Found VPC subnet in IBM Cloud" controller="ibmpowervscluster" controllerGroup="infrastructure.cluster.x-k8s.io" controllerKind="IBMPowerVSCluster" IBMPowerVSCluster="default/capi-test-cluster-sg" namespace="default" name="capi-test-cluster-sg" reconcileID="20586ea8-2019-4770-97bd-81cd925624f4" cluster="default/capi-test-cluster-sg" subnetID="02w7-5e7172ea-8306-444f-b4e6-97acbf35f530"

Usecase 5: When user specifies subnets more than the available VPC zones with no zones mentioned

 vpcSubnets:
  - name: subnet1
  - name: subnet2
  - name: subnet3
  - name: subnet4
  - name: subnet5
  - name: subnet6
zone: mad02

Usecase 6: When user doesn't provide any subnets explicitly, a subnet is created in each available VPC zone

 vpc:
    name: capi-test-cluster-sg-vpc
    region: eu-es
  zone: mad02

[Shilpa-Gokul]

/test pull-cluster-api-provider-ibmcloud-coverage

[Prajyot-Parab]

/hold for @Amulyam24 review

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Amulyam24, Prajyot-Parab, Shilpa-Gokul

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~~ [Prajyot-Parab]

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

[mkumatag]

Thanks @Shilpa-Gokul for fixing this, it was a tricky one to solve.

[Shilpa-Gokul]

@mkumatag @Karthik-K-N @Amulyam24 @Prajyot-Parab @dharaneeshvrd Thanks to you all for helping me understand the codebase, different usecases and scenarios