kubernetes-sigs / cluster-api-provider-ibmcloud

Cluster API Provider for IBM Cloud
https://cluster-api-ibmcloud.sigs.k8s.io
Apache License 2.0
62 stars 84 forks source link

Vpc extended apis #1895

Closed cjschaef closed 3 months ago

cjschaef commented 4 months ago

Extending VPC related Cluster API's in order to provide additional VPC Infrastructure reconciliation support.

What this PR does / why we need it: introduces API's that will be used for extended VPC Cluster (Infrastructure) support

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:

/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:

add additional API's for extended VPC Cluster support
k8s-ci-robot commented 4 months ago

Hi @cjschaef. 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](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
netlify[bot] commented 4 months ago

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

Name Link
Latest commit 4012d7ccba8fb30b80b81f2417ee3e83b67ef3b8
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/66c60b58c96a7000085b9642
Deploy Preview https://deploy-preview-1895--kubernetes-sigs-cluster-api-ibmcloud.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

mkumatag commented 4 months ago

/cc @Karthik-K-N

mkumatag commented 4 months ago

/ok-to-test

Karthik-K-N commented 4 months ago

@dharaneeshvrd please take a look when you get some time.

mkumatag commented 4 months ago

/cc @dharaneeshvrd

cjschaef commented 3 months ago

Based on further investigation on requirements for Catalog Offerings, I have extended the ImageSpec to include a CatalogOffering resource now as well, to allow user to define whether an existing VPC Image is used, a Catalog Offering, or a new VPC Image gets created during Infrastructure reconciliation.

cjschaef commented 3 months ago

Rebased to resolve mock generation.

cjschaef commented 3 months ago

I dropped the Catalog Offering configuration for the Image definition, as I expect to handle it only within Machine API's and we may not have permission to validate a Catalog Offering, thus providing it in the Infrastructure is not useful.

dharaneeshvrd commented 3 months ago

It seems you need to regenerate the CRD def yamls, otherwise it LGTM!

cjschaef commented 3 months ago

@dharaneeshvrd Is there something specific I need to run to do that?

dharaneeshvrd commented 3 months ago

make generate would do that.

cjschaef commented 3 months ago

@dharaneeshvrd I have run that prior, I don't think it passes CI tests unless the CRD files are updated. Is there something specific missing, or perhaps something I need to add into a list to generate what appears to be missing?

dharaneeshvrd commented 3 months ago

@cjschaef Apologies for the confusion, it is updated properly. LGTM, @mkumatag please review!

cjschaef commented 3 months ago

@mkumatag I added some enum validation to some of the type fields, but I cannot validate locally to confirm things work as expected (issues with CRD resource creation). Please let me know if you see further issues.

mkumatag commented 3 months ago

@mkumatag I added some enum validation to some of the type fields, but I cannot validate locally to confirm things work as expected (issues with CRD resource creation). Please let me know if you see further issues.

Getting a proper message now,

k apply -f test/vpc_cluster.yaml 
The IBMVPCCluster "test-1" is invalid: 
* spec.controlPlaneLoadBalancer.backendPools[0].algorithm: Unsupported value: "invalid-algo": supported values: "least_connections", "round_robin", "weighted_round_robin"
* spec.controlPlaneLoadBalancer.backendPools[0].healthMonitor.type: Unsupported value: "invalidtype": supported values: "http", "https", "tcp"
* spec.controlPlaneLoadBalancer.backendPools[0].protocol: Unsupported value: "": supported values: "http", "https", "tcp", "udp"
(base) 
k8s-ci-robot commented 3 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cjschaef, mkumatag

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](https://github.com/kubernetes-sigs/cluster-api-provider-ibmcloud/blob/main/OWNERS)~~ [mkumatag] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment