From abc6f94c0a1f208b8a9223114e8bf5b07cf20b7a Mon Sep 17 00:00:00 2001 From: Dharaneeshwaran Ravichandran Date: Wed, 15 May 2024 14:52:57 +0530 Subject: [PATCH] Use local routing in transit gateway when PowerVS and VPC are from same region --- api/v1beta2/ibmpowervscluster_types.go | 5 +++ api/v1beta2/zz_generated.deepcopy.go | 5 +++ cloud/scope/powervs_cluster.go | 40 +++++++------------ ...e.cluster.x-k8s.io_ibmpowervsclusters.yaml | 6 +++ ...r.x-k8s.io_ibmpowervsclustertemplates.yaml | 6 +++ util/util.go | 33 +++++++++++++++ 6 files changed, 70 insertions(+), 25 deletions(-) diff --git a/api/v1beta2/ibmpowervscluster_types.go b/api/v1beta2/ibmpowervscluster_types.go index 65efbf5cd..bbee8922e 100644 --- a/api/v1beta2/ibmpowervscluster_types.go +++ b/api/v1beta2/ibmpowervscluster_types.go @@ -251,6 +251,11 @@ type TransitGateway struct { // id of resource. // +optional ID *string `json:"id,omitempty"` + // globalRouting indicates whether to set global routing true or not while creating the transit gateway. + // set this field to true only when PowerVS and VPC are from different regions, if they are same it's suggested to use local routing by setting the field to false. + // when the field is omitted, based on PowerVS region (region associated with IBMPowerVSCluster.Spec.Zone) and VPC region(IBMPowerVSCluster.Spec.VPC.Region) system will decide whether to enable globalRouting or not. + // +optional + GlobalRouting *bool `json:"globalRouting,omitempty"` } // VPCResourceReference is a reference to a specific VPC resource by ID or Name diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 20a61a028..5171d98b6 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -1518,6 +1518,11 @@ func (in *TransitGateway) DeepCopyInto(out *TransitGateway) { *out = new(string) **out = **in } + if in.GlobalRouting != nil { + in, out := &in.GlobalRouting, &out.GlobalRouting + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TransitGateway. diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go index b1a1160b0..330766eb2 100644 --- a/cloud/scope/powervs_cluster.go +++ b/cloud/scope/powervs_cluster.go @@ -1334,16 +1334,26 @@ func (s *PowerVSClusterScope) createTransitGateway() (*string, error) { return nil, fmt.Errorf("error getting resource group id for resource group %v, id is empty", s.ResourceGroup()) } - vpcRegion := s.getVPCRegion() - if vpcRegion == nil { - return nil, fmt.Errorf("failed to get vpc region") + location, globalRouting, err := genUtil.GetTransitGatewayLocationAndRouting(s.Zone(), s.VPC().Region) + if err != nil { + return nil, fmt.Errorf("failed to get transit gateway location and routing: %w", err) + } + + // throw error when user tries to use local routing where global routing is required. + // TODO: Add a webhook validation for below condition. + if s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting != nil && !*s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting && *globalRouting { + return nil, fmt.Errorf("failed to use local routing for transit gateway since powervs and vpc region used requires global routing") + } + // setting global routing to true when it is set by user. + if s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting != nil && *s.IBMPowerVSCluster.Spec.TransitGateway.GlobalRouting { + globalRouting = ptr.To(true) } tgName := s.GetServiceName(infrav1beta2.ResourceTypeTransitGateway) tg, _, err := s.TransitGatewayClient.CreateTransitGateway(&tgapiv1.CreateTransitGatewayOptions{ - Location: vpcRegion, + Location: location, Name: tgName, - Global: ptr.To(true), + Global: globalRouting, ResourceGroup: &tgapiv1.ResourceGroupIdentity{ID: ptr.To(resourceGroupID)}, }) if err != nil { @@ -1744,26 +1754,6 @@ func (s *PowerVSClusterScope) fetchResourceGroupID() (string, error) { return "", err } -// getVPCRegion returns region associated with VPC zone. -func (s *PowerVSClusterScope) getVPCRegion() *string { - if s.IBMPowerVSCluster.Spec.VPC != nil { - return s.IBMPowerVSCluster.Spec.VPC.Region - } - // if vpc region is not set try to fetch corresponding region from power vs zone - zone := s.Zone() - if zone == nil { - s.Info("powervs zone is not set") - return nil - } - region := endpoints.ConstructRegionFromZone(*zone) - vpcRegion, err := genUtil.VPCRegionForPowerVSRegion(region) - if err != nil { - s.Error(err, fmt.Sprintf("failed to fetch vpc region associated with powervs region %s", region)) - return nil - } - return &vpcRegion -} - // fetchVPCCRN returns VPC CRN. func (s *PowerVSClusterScope) fetchVPCCRN() (*string, error) { vpcID := s.GetVPCID() diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml index ad94420e4..36ad2d754 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml @@ -396,6 +396,12 @@ spec: when TransitGateway.ID is set, its expected that there exist a TransitGateway with ID or else system will give error. when TransitGateway.Name is set, system will first check for TransitGateway with Name, if not exist system will create new TransitGateway. properties: + globalRouting: + description: |- + globalRouting indicates whether to set global routing true or not while creating the transit gateway. + set this field to true only when PowerVS and VPC are from different regions, if they are same it's suggested to use local routing by setting the field to false. + when the field is omitted, based on PowerVS region (region associated with IBMPowerVSCluster.Spec.Zone) and VPC region(IBMPowerVSCluster.Spec.VPC.Region) system will decide whether to enable globalRouting or not. + type: boolean id: description: id of resource. type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml index fa43d3519..8d6198996 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml @@ -427,6 +427,12 @@ spec: when TransitGateway.ID is set, its expected that there exist a TransitGateway with ID or else system will give error. when TransitGateway.Name is set, system will first check for TransitGateway with Name, if not exist system will create new TransitGateway. properties: + globalRouting: + description: |- + globalRouting indicates whether to set global routing true or not while creating the transit gateway. + set this field to true only when PowerVS and VPC are from different regions, if they are same it's suggested to use local routing by setting the field to false. + when the field is omitted, based on PowerVS region (region associated with IBMPowerVSCluster.Spec.Zone) and VPC region(IBMPowerVSCluster.Spec.VPC.Region) system will decide whether to enable globalRouting or not. + type: boolean id: description: id of resource. type: string diff --git a/util/util.go b/util/util.go index deb85d576..6493bbdbe 100644 --- a/util/util.go +++ b/util/util.go @@ -21,6 +21,9 @@ import ( "fmt" "strconv" + "k8s.io/utils/ptr" + + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/endpoints" "sigs.k8s.io/controller-runtime/pkg/client" infrav1beta2 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2" @@ -197,3 +200,33 @@ func VPCZonesForPowerVSRegion(region string) ([]string, error) { } return nil, fmt.Errorf("VPC zones corresponding to a PowerVS region %s not found ", region) } + +// IsGlobalRoutingRequiredForTG returns true when powervs and vpc regions are different. +func IsGlobalRoutingRequiredForTG(powerVSRegion string, vpcRegion string) bool { + if r, ok := Regions[powerVSRegion]; ok && r.VPCRegion == vpcRegion { + return false + } + return true +} + +// GetTransitGatewayLocationAndRouting returns appropriate location and routing suitable for transit gateway. +// routing indicates whether to enable global routing on transit gateway or not. +// returns true when PowerVS and VPC region are not same otherwise false. +func GetTransitGatewayLocationAndRouting(powerVSZone *string, vpcRegion *string) (*string, *bool, error) { + if powerVSZone == nil { + return nil, nil, fmt.Errorf("powervs zone is not set") + } + powerVSRegion := endpoints.ConstructRegionFromZone(*powerVSZone) + + if vpcRegion != nil { + routing := IsGlobalRoutingRequiredForTG(powerVSRegion, *vpcRegion) + return vpcRegion, &routing, nil + } + location, err := VPCRegionForPowerVSRegion(powerVSRegion) + if err != nil { + return nil, nil, fmt.Errorf("failed to fetch vpc region associated with powervs region '%s': %w", powerVSRegion, err) + } + + // returning routing as true since VPC region is not set and used PowerVS region to calculate the transit gateway location. + return &location, ptr.To(true), nil +}