cluster-api-provider-ibmcloud icon indicating copy to clipboard operation
cluster-api-provider-ibmcloud copied to clipboard

Plan to handle exsiting TransitGateway and its connections

Open Karthik-K-N opened this issue 1 year ago • 6 comments

/kind feature /area provider/ibmcloud

Describe the solution you'd like [A clear and concise description of what you want to happen.]

Currently when TransitGateway details are not set the controller will automatically creates a TransitGateway and necessary connections.

Also we support user to pass existing TransitGateway and which already has attached connections to it.

We dont support case where a TransitGatewa exist and which does not have any connections and user is expecting controller to attach the connections.

Anything else you would like to add: [Miscellaneous information that will assist in solving the issue.]

Goal of this issue is to decide on

  1. should we support this if not what are the alternatives
  2. Consequence of supporting it.

Karthik-K-N avatar Jul 22 '24 04:07 Karthik-K-N

I expect the use cases to be the following: Neither VPC nor TG are pre-existing. TG is pre-existing, VPC is not. VPC is pre-existing, TG is not. Both VPC and TG are pre-existing. #1 is already handled. #2 is new for IPI. I expect the provider to create both connections. #3 is not new. #4 is new for IPI. For consistency, I expect the provider to create both connections. So for every case, I expect the provider to create both connections.

hamzy avatar Jul 22 '24 13:07 hamzy

Need changes in API status to store the status of the vpc and powervs connections separately to use them to take decision while deleting them.

+++ b/api/v1beta2/ibmpowervscluster_types.go
@@ -181,6 +181,19 @@ type ResourceReference struct {
        ControllerCreated *bool `json:"controllerCreated,omitempty"`
 }
 
+// TransitGatewayStatus defines the status of transit gateway as well as it's connection's status.
+type TransitGatewayStatus struct {
+       // id represents the id of the resource.
+       ID *string `json:"id,omitempty"`
+       // +kubebuilder:default=false
+       // controllerCreated indicates whether the resource is created by the controller.
+       ControllerCreated *bool `json:"controllerCreated,omitempty"`
+       // vpcConnection defines the vpc connection status in transit gateway.
+       VPCConnection *ResourceReference `json:"vpcConnection,omitempty"`
+       // powerVSConnection defines the powervs connection status in transit gateway.
+       PowerVSConnection *ResourceReference `json:"powerVSConnection,omitempty"`
+}
+
 // IBMPowerVSClusterStatus defines the observed state of IBMPowerVSCluster.
 type IBMPowerVSClusterStatus struct {
        // ready is true when the provider resource is ready.
@@ -209,7 +222,7 @@ type IBMPowerVSClusterStatus struct {
        VPCSecurityGroups map[string]VPCSecurityGroupStatus `json:"vpcSecurityGroups,omitempty"`
 
        // transitGateway is reference to IBM Cloud TransitGateway.
-       TransitGateway *ResourceReference `json:"transitGateway,omitempty"`
+       TransitGateway *TransitGatewayStatus `json:"transitGateway,omitempty"`
 
        // cosInstance is reference to IBM Cloud COS Instance resource.
        COSInstance *ResourceReference `json:"cosInstance,omitempty"

@mkumatag @Karthik-K-N Hope this API status change is ok and I can continue with the development. Let me know if you have different thoughts on this!

dharaneeshvrd avatar Jul 23 '24 11:07 dharaneeshvrd

IBMPowerVSClusterStatus

Looks good to me, I think there is no need to do any spec changes right? I hope we should not allow user to create any connection with specific name and that requirement wont come.

Karthik-K-N avatar Jul 23 '24 12:07 Karthik-K-N

@mkumatag @Karthik-K-N Hope this API status change is ok and I can continue with the development. Let me know if you have different thoughts on this!

I see a change here in the status, we need to be little cautious here..

mkumatag avatar Jul 23 '24 13:07 mkumatag

I see a change here in the status, we need to be little cautious here..

Sure, but still proceed, right?

hamzy avatar Jul 23 '24 17:07 hamzy

I hope we should not allow user to create any connection with specific name and that requirement wont come.

Yes Let's not do that, it will add more complications!

dharaneeshvrd avatar Jul 24 '24 03:07 dharaneeshvrd