Network regex validation for powervs cluster

[carmal891]

What this PR does / why we need it:

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

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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: carmal891
Once this PR has been reviewed and has the lgtm label, please assign mkumatag for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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

[k8s-ci-robot]

Hi @carmal891. 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]

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

Name	Link
🔨 Latest commit	3aced86
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/6799b1bb2c3bd10008153d3f
😎 Deploy Preview	https://deploy-preview-2145--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.

[carmal891]

@Karthik-K-N As discussed, set the network regex validation only for powervs cluster for now. We can further triage the any additional changes required

[Amulyam24]

/ok-to-test

[Amulyam24]

@Karthik-K-N, I'm not quite sure if this validation makes sense in all scenarios.

IMO using regex is ideal when a resource exists but how will this validation help when DHCP server name is not set and it will end up creating a new DHCP server with name in format DHCPSERVER<dhcp_server_name>_Private later in the flow. Isn't the main aim to check if the network regex matches the format?

cc @dharaneeshvrd

[Karthik-K-N]

@Karthik-K-N, I'm not quite sure if this validation makes sense in all scenarios.

IMO using regex is ideal when a resource exists but how will this validation help when DHCP server name is not set and it will end up creating a new DHCP server with name in format DHCPSERVER<dhcp_server_name>_Private later in the flow. Isn't the main aim to check if the network regex matches the format?

cc @dharaneeshvrd

Correct, But currently there is no way to identify whether the network exist or not only from spec.

What we are trying to solve here is not allowing user to configure invalid network regex,

For example:

Say cluster name is powervs-cluster, spec.DHCPSserver not set , the network is set to RegEx say *.abc.$ and network does not exist,
The controller will create a network with DHCPSERVERpowervs-cluster_Private and set the network id in status, The next reconcilation continues by fetching network id from status.
The problem here is we are not at all caring what set in spec.Regex and if someone sees spec and actual resource created both are not at all related.

I think RegEx was introduced when we didn't what will be the DHCP created subnet name format. But now we have clear idea.

Should we support RegEx? or mark it as not supported?
@dharaneeshvrd @Prajyot-Parab whats your thought.

[dharaneeshvrd]

The same case will apply to network Name as well, when network name is given and network is not present, DHCP server that get's created will have either DHCP server name or cluster name not the network name.
I think we have to decide on the priority. We have already discussed about this issue here. Once we decide on the priority and what will happen in each case(created an issue for this here), we can allow regex too. And we can document it in API description I believe.
Since we are going to allow non DHCP network also, we have to consider that situation too before removing the support for regex IMO.

[Prajyot-Parab]

IMO

1. we should create new resources only if name is set

• If present fetch and use it
• If not present, create resource using that name

2. we should avoid creating new resources when either of ID or regex is set (Their sole purpose should be to retrieve existing resources if present and not create anything), just throw error if not present.
3. If all three are not set -> name, id, regex, then use default value set for name of that particular resource and use that for creation.

[Karthik-K-N]

IMO

1. we should create new resources only if name is set


* If present fetch and use it

* If not present, create resource using that name


2. we should avoid creating new resources when either of ID or regex is set (Their sole purpose should be to retrieve existing  resources if present and not create anything), just throw error if not present.

3. If all three are not set -> name, id, regex, then use default value set for name of that particular resource and use that for creation.

Yeah, I think this should be the behavior, We created #1726 to handle these scenarios.
@Amulyam24 whats your thought.

[Amulyam24]

IMO

1. we should create new resources only if name is set


* If present fetch and use it

* If not present, create resource using that name


2. we should avoid creating new resources when either of ID or regex is set (Their sole purpose should be to retrieve existing  resources if present and not create anything), just throw error if not present.

3. If all three are not set -> name, id, regex, then use default value set for name of that particular resource and use that for creation.

Yeah, I think this should be the behavior, We created #1726 to handle these scenarios. @Amulyam24 whats your thought.

I agree. This makes sense and seems like the best way to handle new and existing resources.

Regex should only be considered when the resource already exists.