Add proposal for Power VS infra creation #1488

Karthik-K-N · 2023-11-09T17:27:23Z

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 #

Special notes for your reviewer:

/area provider/ibmcloud

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

Release note:

netlify · 2023-11-09T17:27:30Z

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

Name	Link
🔨 Latest commit	`b10cd0e`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/65c08006b6df7700084eef6a
😎 Deploy Preview	https://deploy-preview-1488--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.

mkumatag · 2023-11-09T17:44:35Z

@dharaneeshvrd please go through the flow diagram been published and see how we handle the hypershift scenario in this design

Karthik-K-N · 2023-11-10T02:26:09Z

@dharaneeshvrd please go through the flow diagram been published and see how we handle the hypershift scenario in this design

I think it should not affect the hypershift use case as I believe hypershift sets externally managed annotation which avoids ibmpowervscluster resource reconcilation. For info: #899

dharaneeshvrd · 2023-11-10T07:04:49Z

The main difference I am seeing from hypershift flow is that, load balancer will not get created as part of infra creation.
IMO no need to keep load balancer creation in infra as it's not required before nodes are created.

Other than that I have few comments that we need to keep in mind:

I can see PowerVS region is not in spec which means we need to rely on some map to fetch the respective region to create powervs go client, in hypershift case we are getting region from user and in doc we have given the link to the page where region, zone and vpc region mapping is explained with the expectation of user reads that and provides the proper input. Which is going to avoid any confusion in future if the mapping we are using is not up to date or has some changed info.
While handling user provided resources, its better to keep it that option for developer only, one sample scenario of handling user provided resource when user passes a transit gateway and vpc where the transit gateway already connected to other vpc instance or other powervs instance then it cannot be used. So the user must understand the use of this and provide the suitable resource to this option.
Also need to be mindful while cleaning up the cluster created with user provided resources that its skipped for deletion and if its attached to transit gateway then only detach it.

Will keep posting if something comes to my mind.

Karthik-K-N · 2023-11-11T03:11:32Z

The main difference I am seeing from hypershift flow is that, load balancer will not get created as part of infra creation. IMO no need to keep load balancer creation in infra as it's not required before nodes are created.

Other than that I have few comments that we need to keep in mind:

1. I can see PowerVS region is not in spec which means we need to rely on some map to fetch the respective region to create powervs go client, in hypershift case we are getting region from user and in doc we have given the link to the page where region, zone and vpc region mapping is explained with the expectation of user reads that and provides the proper input. Which is going to avoid any confusion in future if the mapping we are using is not up to date or has some changed info.

2. While handling user provided resources, its better to keep it that option for developer only, one sample scenario of handling user provided resource when user passes a transit gateway and vpc where the transit gateway already connected  to other vpc instance or other powervs instance then it cannot be used. So the user must understand the use of this and provide the suitable resource to this option.

3. Also need to be mindful while cleaning up the cluster created with user provided resources that its skipped for deletion and if its attached to transit gateway then only detach it.

Will keep posting if something comes to my mind.

I saw that region is being deprecated in options and thought it can be avoided and use only zone, Do you have any other reference where region is mandatory to use.
For user providing resources we can make a clear documentation of what can be used and what are the scenarios supported.
Regarding Loadbalancer creation, I think as per capi contracts its mandatory to set controlplane endpoints , if we intend to use LB we need to provision and update the fields.

docs/proposal/20231109-powervs-infra-creation.md

mkumatag · 2023-11-14T06:29:47Z

docs/proposal/20231109-powervs-infra-creation.md

+
+### Cluster creation workflow
+
+![img_4.png](../images/powervs-workflow.png)


Lets use tools like https://excalidraw.com which allows to save the spec file as well so that someone also can easily modify the content later

mkumatag · 2024-01-30T03:14:18Z

@Karthik-K-N lets merge this everything fixed, @Prajyot-Parab please review.

Karthik-K-N · 2024-01-30T04:24:26Z

@Karthik-K-N lets merge this everything fixed, @Prajyot-Parab please review.

sure, let me update the final api specs

Prajyot-Parab · 2024-01-30T05:46:08Z

docs/proposal/20231109-powervs-infra-creation.md

+
+	// vpcSubnet contains information about IBM Cloud VPC Subnet resources
+	// +optional
+	VPCSubnet *Subnet `json:"vpcSubnet,omitempty"`


Update

Suggested change

VPCSubnet *Subnet `json:"vpcSubnet,omitempty"`

VPCSubnets []Subnet `json:"vpcSubnets,omitempty"`

Prajyot-Parab · 2024-01-30T05:47:16Z

docs/proposal/20231109-powervs-infra-creation.md

+	// (https://coreos.github.io/ignition/) for bootstrapping (requires
+	// BootstrapFormatIgnition feature flag to be enabled).
+	// +optional
+	CosBucket *CosBucket `json:"cosBucket,omitempty"`


Suggested change

CosBucket *CosBucket `json:"cosBucket,omitempty"`

CosInstance *CosInstance `json:"cosInstance,omitempty"`

update the comments as well

Prajyot-Parab · 2024-01-30T05:48:28Z

docs/images/powervs-cluster-delete-workflow.png

vpcSubnet --> this should be list as per latest changes right? @Karthik-K-N

Prajyot-Parab · 2024-01-30T05:49:10Z

docs/proposal/20231109-powervs-infra-creation.md

+	Zone *string `json:"zone,omitempty"`
+}
+
+type CosBucket struct {


Update as per latest API changes.

I will update this by eod

Karthik-K-N

I have updated the proposal with new api spec , Please take a look

Prajyot-Parab · 2024-02-05T05:33:47Z

docs/proposal/20231109-powervs-infra-creation.md

@@ -0,0 +1,202 @@
+# Dynamically create infrastructure required for Power VS cluster


nit: Power VS --> PowerVS (Fix everywhere)

Prajyot-Parab · 2024-02-05T05:34:14Z

docs/proposal/20231109-powervs-infra-creation.md

+
+## Motivation
+Currently, inorder to create Power VS cluster using cluster api we need to create few resources in hand which includes
+1. Creating a Power VS workspace


Suggested change

1. Creating a Power VS workspace

1. Creating a PowerVS Workspace

Prajyot-Parab · 2024-02-05T05:36:09Z

docs/proposal/20231109-powervs-infra-creation.md

+Along the similar line today the cluster is accessible to end user via external ip and which is loadbalanced on controlplanes using kube-vip.
+
+## Goal
+1. Dynamically creating required cloud resources as a part of cluster creation.


Suggested change

1. Dynamically creating required cloud resources as a part of cluster creation.

1. Dynamically create the required cloud resources as part of the cluster creation process.

Prajyot-Parab · 2024-02-05T05:36:48Z

docs/proposal/20231109-powervs-infra-creation.md

+
+## Goal
+1. Dynamically creating required cloud resources as a part of cluster creation.
+2. Allowing users to access the cluster via loadbalacer.


Suggested change

2. Allowing users to access the cluster via loadbalacer.

2. Allow users to access the cluster via loadbalancer.

Prajyot-Parab · 2024-02-05T05:38:07Z

docs/proposal/20231109-powervs-infra-creation.md

+
+	// zone is the name of Power VS zone where the cluster will be created
+	// possible values can be found here https://cloud.ibm.com/docs/power-iaas?topic=power-iaas-creating-power-virtual-server.
+	// when omitted syd04 will be set as default zone.


Suggested change

// when omitted syd04 will be set as default zone.

// when omitted dal10 will be set as default zone.

(change in actual API file as well)

Prajyot-Parab · 2024-02-05T05:42:15Z

docs/proposal/20231109-powervs-infra-creation.md

+6. [VPC Loadbalancer](https://www.ibm.com/products/load-balancer)
+
+### Cluster creation workflow
+User is expected to set the annotation ```powervs.cluster.x-k8s.io/create-infra:true``` to IBMPowerVSCluser object make use of this feature. If unset the cluster creation goes through existing way.


Suggested change

User is expected to set the annotation ```powervs.cluster.x-k8s.io/create-infra:true``` to IBMPowerVSCluser object make use of this feature. If unset the cluster creation goes through existing way.

User is expected to set the annotation ```powervs.cluster.x-k8s.io/create-infra:true``` to IBMPowerVSCluser object to make use of this feature. If not set the cluster creation will proceed with existing way.

Prajyot-Parab · 2024-02-05T05:42:53Z

docs/proposal/20231109-powervs-infra-creation.md

+```
+
+### Following resources will be created
+1. [Power VS workspace](https://cloud.ibm.com/docs/power-iaas?topic=power-iaas-creating-power-virtual-server)


Suggested change

1. [Power VS workspace](https://cloud.ibm.com/docs/power-iaas?topic=power-iaas-creating-power-virtual-server)

1. [PowerVS Workspace](https://cloud.ibm.com/docs/power-iaas?topic=power-iaas-creating-power-virtual-server)

Prajyot-Parab · 2024-02-05T05:47:13Z

docs/proposal/20231109-powervs-infra-creation.md

+2. [Power VS Network](https://cloud.ibm.com/docs/power-iaas?topic=power-iaas-configuring-subnet) [DHCP service]
+3. [VPC](https://cloud.ibm.com/docs/vpc?topic=vpc-about-vpc)
+4. [VPC Subnet](https://cloud.ibm.com/docs/vpc?topic=vpc-about-networking-for-vpc)
+5. [Transit gateway](https://www.ibm.com/products/transit-gateway)


Suggested change

5. [Transit gateway](https://www.ibm.com/products/transit-gateway)

5. [Transit Gateway](https://www.ibm.com/products/transit-gateway)

Prajyot-Parab · 2024-02-05T05:51:11Z

docs/proposal/20231109-powervs-infra-creation.md

+# Dynamically create infrastructure required for Power VS cluster
+
+## Motivation
+Currently, inorder to create Power VS cluster using cluster api we need to create few resources in hand which includes


Suggested change

Currently, inorder to create Power VS cluster using cluster api we need to create few resources in hand which includes

Currently, inorder to create a PowerVS cluster using cluster api we need to create few resources as prerequisites which includes -

Prajyot-Parab · 2024-02-05T05:51:45Z

docs/proposal/20231109-powervs-infra-creation.md

+2. Creating a Power VS Network
+3. Creating a port on network
+
+as this involves some prerequisite work which is limiting true capabilities of cluster api.


Suggested change

as this involves some prerequisite work which is limiting true capabilities of cluster api.

As this involves some prerequisite work which is limiting the true capabilities of cluster api.

Karthik-K-N

@Prajyot-Parab Thanks for the suggestion, I have addressed all of them and they are ready for review again.

Prajyot-Parab

/lgtm

mkumatag

/lgtm

k8s-ci-robot · 2024-02-06T12:21:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Karthik-K-N, mkumatag, Prajyot-Parab

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~~ [mkumatag]

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 added area/provider/ibmcloud Issues or PRs related to ibmcloud provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 9, 2023

k8s-ci-robot requested a review from jichenjc November 9, 2023 17:27

k8s-ci-robot requested a review from wentao-zh November 9, 2023 17:27

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 9, 2023

Prajyot-Parab reviewed Nov 13, 2023

View reviewed changes

docs/proposal/20231109-powervs-infra-creation.md Outdated Show resolved Hide resolved

Karthik-K-N force-pushed the infra-proposal branch from c2d776d to 9d5df15 Compare November 14, 2023 04:40

mkumatag requested changes Nov 14, 2023

View reviewed changes

Karthik-K-N force-pushed the infra-proposal branch from 9d5df15 to d1ab4c1 Compare November 14, 2023 12:53

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 17, 2023

Karthik-K-N force-pushed the infra-proposal branch 2 times, most recently from 0d638a9 to e28884d Compare December 1, 2023 04:15

Karthik-K-N force-pushed the infra-proposal branch from e28884d to 1115fe0 Compare January 17, 2024 08:49

Prajyot-Parab suggested changes Jan 30, 2024

View reviewed changes

Karthik-K-N force-pushed the infra-proposal branch from 1115fe0 to 263f08e Compare January 31, 2024 05:12

Karthik-K-N commented Jan 31, 2024

View reviewed changes

Karthik-K-N requested a review from Prajyot-Parab February 1, 2024 17:17

This was referenced Feb 2, 2024

Handle create workflow of newly proposed Power VS cluster creation method #1599

Closed

Handle delete workflow of newly proposed Power VS cluster creation method #1600

Closed

Prajyot-Parab suggested changes Feb 5, 2024

View reviewed changes

Add proposal for Power VS infra creation

b10cd0e

Karthik-K-N force-pushed the infra-proposal branch from 6cd4eb4 to b10cd0e Compare February 5, 2024 06:28

Karthik-K-N commented Feb 5, 2024

View reviewed changes

Prajyot-Parab approved these changes Feb 5, 2024

View reviewed changes

k8s-ci-robot assigned Prajyot-Parab Feb 5, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 5, 2024

mkumatag approved these changes Feb 6, 2024

View reviewed changes

k8s-ci-robot assigned mkumatag Feb 6, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 6, 2024

k8s-ci-robot merged commit 362bc4f into kubernetes-sigs:main Feb 6, 2024
13 checks passed

Karthik-K-N mentioned this pull request Feb 9, 2024

Feature: PowerVS cluster creation with dynamic resource creation #1608

Merged

3 tasks

Prajyot-Parab mentioned this pull request Feb 9, 2024

EPIC - Introduce new API Specs and infra creation flow #1611

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add proposal for Power VS infra creation #1488

Add proposal for Power VS infra creation #1488

Karthik-K-N commented Nov 9, 2023

netlify bot commented Nov 9, 2023 •

edited

Loading

mkumatag commented Nov 9, 2023

Karthik-K-N commented Nov 10, 2023

dharaneeshvrd commented Nov 10, 2023

Karthik-K-N commented Nov 11, 2023

mkumatag Nov 14, 2023

mkumatag commented Jan 30, 2024

Karthik-K-N commented Jan 30, 2024

Prajyot-Parab Jan 30, 2024

Prajyot-Parab Jan 30, 2024

Prajyot-Parab Jan 30, 2024

Prajyot-Parab Jan 30, 2024

Karthik-K-N Jan 30, 2024

Karthik-K-N left a comment

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Prajyot-Parab Feb 5, 2024

Karthik-K-N left a comment

Prajyot-Parab left a comment

mkumatag left a comment

k8s-ci-robot commented Feb 6, 2024


		### Cluster creation workflow

		![img_4.png](../images/powervs-workflow.png)

	VPCSubnet *Subnet `json:"vpcSubnet,omitempty"`
	VPCSubnets []Subnet `json:"vpcSubnets,omitempty"`

	CosBucket *CosBucket `json:"cosBucket,omitempty"`
	CosInstance *CosInstance `json:"cosInstance,omitempty"`

		@@ -0,0 +1,202 @@
		# Dynamically create infrastructure required for Power VS cluster

	1. Creating a Power VS workspace
	1. Creating a PowerVS Workspace

	1. Dynamically creating required cloud resources as a part of cluster creation.
	1. Dynamically create the required cloud resources as part of the cluster creation process.

	2. Allowing users to access the cluster via loadbalacer.
	2. Allow users to access the cluster via loadbalancer.

	// when omitted syd04 will be set as default zone.
	// when omitted dal10 will be set as default zone.

	User is expected to set the annotation ```powervs.cluster.x-k8s.io/create-infra:true``` to IBMPowerVSCluser object make use of this feature. If unset the cluster creation goes through existing way.
	User is expected to set the annotation ```powervs.cluster.x-k8s.io/create-infra:true``` to IBMPowerVSCluser object to make use of this feature. If not set the cluster creation will proceed with existing way.

	1. [Power VS workspace](https://cloud.ibm.com/docs/power-iaas?topic=power-iaas-creating-power-virtual-server)
	1. [PowerVS Workspace](https://cloud.ibm.com/docs/power-iaas?topic=power-iaas-creating-power-virtual-server)

	5. [Transit gateway](https://www.ibm.com/products/transit-gateway)
	5. [Transit Gateway](https://www.ibm.com/products/transit-gateway)

	Currently, inorder to create Power VS cluster using cluster api we need to create few resources in hand which includes
	Currently, inorder to create a PowerVS cluster using cluster api we need to create few resources as prerequisites which includes -

	as this involves some prerequisite work which is limiting true capabilities of cluster api.
	As this involves some prerequisite work which is limiting the true capabilities of cluster api.

Add proposal for Power VS infra creation #1488

Add proposal for Power VS infra creation #1488

Conversation

Karthik-K-N commented Nov 9, 2023

netlify bot commented Nov 9, 2023 • edited Loading

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

mkumatag commented Nov 9, 2023

Karthik-K-N commented Nov 10, 2023

dharaneeshvrd commented Nov 10, 2023

Karthik-K-N commented Nov 11, 2023

Choose a reason for hiding this comment

mkumatag commented Jan 30, 2024

Karthik-K-N commented Jan 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karthik-K-N left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karthik-K-N left a comment

Choose a reason for hiding this comment

Prajyot-Parab left a comment

Choose a reason for hiding this comment

mkumatag left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 6, 2024

netlify bot commented Nov 9, 2023 •

edited

Loading