From c4171f21acc320fdfe17b812c98be908653134f7 Mon Sep 17 00:00:00 2001 From: Shilpa Gokul Date: Thu, 28 Nov 2024 17:16:58 +0530 Subject: [PATCH] Use multiple zones for multiple subnets (#1793) --- cloud/scope/powervs_cluster.go | 58 ++--- cloud/scope/powervs_cluster_test.go | 225 +++++++++++++----- .../ibmpowervscluster_controller_test.go | 7 +- pkg/cloud/services/vpc/mock/vpc_generated.go | 15 -- pkg/cloud/services/vpc/service.go | 44 ---- pkg/cloud/services/vpc/vpc.go | 1 - 6 files changed, 190 insertions(+), 160 deletions(-) diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go index 4ba01f6b6..44cd64124 100644 --- a/cloud/scope/powervs_cluster.go +++ b/cloud/scope/powervs_cluster.go @@ -74,7 +74,12 @@ var ( ) // powerEdgeRouter is identifier for PER. -const powerEdgeRouter = "power-edge-router" +const ( + powerEdgeRouter = "power-edge-router" + // vpcSubnetIPAddressCount is the total IP Addresses for the subnet. + // Support for custom address prefixes will be added at a later time. Currently, we use the ip count for subnet creation. + vpcSubnetIPAddressCount int64 = 256 +) // PowerVSClusterScopeParams defines the input parameters used to create a new PowerVSClusterScope. type PowerVSClusterScopeParams struct { @@ -1186,16 +1191,16 @@ func (s *PowerVSClusterScope) createVPC() (*string, error) { // ReconcileVPCSubnets reconciles VPC subnet. func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { subnets := make([]infrav1beta2.Subnet, 0) + vpcZones, err := regionUtil.VPCZonesForVPCRegion(*s.VPC().Region) + if err != nil { + return false, err + } + if len(vpcZones) == 0 { + return false, fmt.Errorf("failed to fetch VPC zones, no zone found for region %s", *s.VPC().Region) + } // check whether user has set the vpc subnets if len(s.IBMPowerVSCluster.Spec.VPCSubnets) == 0 { // if the user did not set any subnet, we try to create subnet in all the zones. - vpcZones, err := regionUtil.VPCZonesForVPCRegion(*s.VPC().Region) - if err != nil { - return false, err - } - if len(vpcZones) == 0 { - return false, fmt.Errorf("failed to fetch VPC zones, no zone found for region %s", *s.VPC().Region) - } for _, zone := range vpcZones { subnet := infrav1beta2.Subnet{ Name: ptr.To(fmt.Sprintf("%s-%s", *s.GetServiceName(infrav1beta2.ResourceTypeSubnet), zone)), @@ -1248,6 +1253,9 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { continue } + if subnet.Zone == nil { + subnet.Zone = &vpcZones[index%len(vpcZones)] + } s.V(3).Info("Creating VPC subnet") subnetID, err = s.createVPCSubnet(subnet) if err != nil { @@ -1256,7 +1264,10 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { } s.Info("Created VPC subnet", "subnetID", subnetID) s.SetVPCSubnetStatus(*subnet.Name, infrav1beta2.ResourceReference{ID: subnetID, ControllerCreated: ptr.To(true)}) - return true, nil + // Requeue only when the creation of all subnets has been triggered. + if index == len(subnets)-1 { + return true, nil + } } return false, nil } @@ -1282,42 +1293,25 @@ func (s *PowerVSClusterScope) createVPCSubnet(subnet infrav1beta2.Subnet) (*stri if resourceGroupID == "" { return nil, fmt.Errorf("failed to fetch resource group ID for resource group %v, ID is empty", s.ResourceGroup()) } - var zone string - if subnet.Zone != nil { - zone = *subnet.Zone - } else { - vpcZones, err := regionUtil.VPCZonesForVPCRegion(*s.VPC().Region) - if err != nil { - return nil, err - } - // TODO(karthik-k-n): Decide on using all zones or using one zone - if len(vpcZones) == 0 { - return nil, fmt.Errorf("failed to fetch VPC zones, error: %v", err) - } - zone = vpcZones[0] - } // create subnet vpcID := s.GetVPCID() if vpcID == nil { return nil, fmt.Errorf("VPC ID is empty") } - cidrBlock, err := s.IBMVPCClient.GetSubnetAddrPrefix(*vpcID, zone) - if err != nil { - return nil, err - } - ipVersion := "ipv4" + + ipVersion := vpcSubnetIPVersion4 options := &vpcv1.CreateSubnetOptions{} options.SetSubnetPrototype(&vpcv1.SubnetPrototype{ - IPVersion: &ipVersion, - Ipv4CIDRBlock: &cidrBlock, - Name: subnet.Name, + IPVersion: &ipVersion, + TotalIpv4AddressCount: ptr.To(vpcSubnetIPAddressCount), + Name: subnet.Name, VPC: &vpcv1.VPCIdentity{ ID: vpcID, }, Zone: &vpcv1.ZoneIdentity{ - Name: &zone, + Name: subnet.Zone, }, ResourceGroup: &vpcv1.ResourceGroupIdentity{ ID: &resourceGroupID, diff --git a/cloud/scope/powervs_cluster_test.go b/cloud/scope/powervs_cluster_test.go index bc3389118..8cb7e8f0e 100644 --- a/cloud/scope/powervs_cluster_test.go +++ b/cloud/scope/powervs_cluster_test.go @@ -27,6 +27,7 @@ import ( "github.com/IBM/ibm-cos-sdk-go/aws/awserr" "github.com/IBM/platform-services-go-sdk/resourcecontrollerv2" "github.com/IBM/vpc-go-sdk/vpcv1" + regionUtil "github.com/ppc64le-cloud/powervs-utils" "go.uber.org/mock/gomock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -3556,6 +3557,7 @@ func TestReconcileVPCSubnets(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, Spec: infrav1beta2.IBMPowerVSClusterSpec{ + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, VPCSubnets: []infrav1beta2.Subnet{{ID: ptr.To("subnet1ID"), Name: ptr.To("subnet1Name")}, {ID: ptr.To("subnet2ID"), Name: ptr.To("subnet2Name")}}}, }, } @@ -3571,8 +3573,122 @@ func TestReconcileVPCSubnets(t *testing.T) { g.Expect(clusterScope.IBMPowerVSCluster.Status.VPCSubnet[*subnet1Details.Name].ID).To(Equal(subnet1Details.ID)) g.Expect(clusterScope.IBMPowerVSCluster.Status.VPCSubnet[*subnet2Details.Name].ID).To(Equal(subnet2Details.ID)) g.Expect(clusterScope.IBMPowerVSCluster.Status.VPCSubnet[*subnet1Details.Name].ControllerCreated).To(BeNil()) + g.Expect(clusterScope.IBMPowerVSCluster.Status.VPCSubnet[*subnet2Details.Name].ControllerCreated).To(BeNil()) }) + t.Run("When more VPCSubnets are set in the spec than the available VPC zones with zone explicitly mentioned for few subnets", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + var subnetZone *string + + clusterScope := PowerVSClusterScope{ + IBMVPCClient: mockVPC, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + VPC: &infrav1beta2.ResourceReference{ID: ptr.To("vpcID")}, + }, + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")}, + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, + VPCSubnets: []infrav1beta2.Subnet{ + {Name: ptr.To("subnet1Name"), Zone: ptr.To("eu-de-2")}, + {Name: ptr.To("subnet2Name")}, + {Name: ptr.To("subnet3Name")}, + {Name: ptr.To("subnet4Name")}, + {Name: ptr.To("subnet5Name")}, + }}, + }, + } + + vpcZones, err := regionUtil.VPCZonesForVPCRegion(*clusterScope.IBMPowerVSCluster.Spec.VPC.Region) + g.Expect(err).To(BeNil()) + mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(nil, nil).Times(len(clusterScope.IBMPowerVSCluster.Spec.VPCSubnets)) + for i := 0; i < len(clusterScope.IBMPowerVSCluster.Spec.VPCSubnets); i++ { + subnet1Options := &vpcv1.CreateSubnetOptions{} + if clusterScope.IBMPowerVSCluster.Spec.VPCSubnets[i].Zone != nil { + subnetZone = clusterScope.IBMPowerVSCluster.Spec.VPCSubnets[i].Zone + } else { + subnetZone = &vpcZones[i%len(vpcZones)] + } + subnet1Options.SetSubnetPrototype(&vpcv1.SubnetPrototype{ + IPVersion: ptr.To("ipv4"), + TotalIpv4AddressCount: ptr.To(vpcSubnetIPAddressCount), + Name: clusterScope.IBMPowerVSCluster.Spec.VPCSubnets[i].Name, + VPC: &vpcv1.VPCIdentity{ + ID: clusterScope.IBMPowerVSCluster.Status.VPC.ID, + }, + Zone: &vpcv1.ZoneIdentity{ + Name: subnetZone, + }, + ResourceGroup: &vpcv1.ResourceGroupIdentity{ + ID: clusterScope.IBMPowerVSCluster.Spec.ResourceGroup.ID, + }, + }) + subnetDetails := &vpcv1.Subnet{ID: ptr.To(fmt.Sprintf("subnet%dID", i+1)), Name: ptr.To(fmt.Sprintf("subnet%dName", i+1))} + mockVPC.EXPECT().CreateSubnet(subnet1Options).Return(subnetDetails, nil, nil) + } + requeue, err := clusterScope.ReconcileVPCSubnets() + g.Expect(requeue).To(BeTrue()) + g.Expect(err).To(BeNil()) + for i := 1; i <= len(clusterScope.IBMPowerVSCluster.Spec.VPCSubnets); i++ { + g.Expect(*clusterScope.IBMPowerVSCluster.Status.VPCSubnet[fmt.Sprintf("subnet%dName", i)].ID).To(Equal(fmt.Sprintf("subnet%dID", i))) + g.Expect(*clusterScope.IBMPowerVSCluster.Status.VPCSubnet[fmt.Sprintf("subnet%dName", i)].ControllerCreated).To(BeTrue()) + } + }) + + t.Run("When VPCSubnets are set in the spec along with the zones", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMVPCClient: mockVPC, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + VPC: &infrav1beta2.ResourceReference{ID: ptr.To("vpcID")}, + }, + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")}, + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, + VPCSubnets: []infrav1beta2.Subnet{ + {Name: ptr.To("subnet1Name"), Zone: ptr.To("eu-de-1")}, + {Name: ptr.To("subnet2Name"), Zone: ptr.To("eu-de-2")}, + {Name: ptr.To("subnet3Name"), Zone: ptr.To("eu-de-3")}, + }}, + }, + } + + mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(nil, nil).Times(len(clusterScope.IBMPowerVSCluster.Spec.VPCSubnets)) + for i := 0; i < len(clusterScope.IBMPowerVSCluster.Spec.VPCSubnets); i++ { + subnet1Options := &vpcv1.CreateSubnetOptions{} + subnet1Options.SetSubnetPrototype(&vpcv1.SubnetPrototype{ + IPVersion: ptr.To("ipv4"), + TotalIpv4AddressCount: ptr.To(vpcSubnetIPAddressCount), + Name: clusterScope.IBMPowerVSCluster.Spec.VPCSubnets[i].Name, + VPC: &vpcv1.VPCIdentity{ + ID: clusterScope.IBMPowerVSCluster.Status.VPC.ID, + }, + Zone: &vpcv1.ZoneIdentity{ + Name: clusterScope.IBMPowerVSCluster.Spec.VPCSubnets[i].Zone, + }, + ResourceGroup: &vpcv1.ResourceGroupIdentity{ + ID: clusterScope.IBMPowerVSCluster.Spec.ResourceGroup.ID, + }, + }) + subnetDetails := &vpcv1.Subnet{ID: ptr.To(fmt.Sprintf("subnet%dID", i+1)), Name: ptr.To(fmt.Sprintf("subnet%dName", i+1))} + mockVPC.EXPECT().CreateSubnet(subnet1Options).Return(subnetDetails, nil, nil) + } + requeue, err := clusterScope.ReconcileVPCSubnets() + g.Expect(requeue).To(BeTrue()) + g.Expect(err).To(BeNil()) + for i := 1; i <= len(clusterScope.IBMPowerVSCluster.Spec.VPCSubnets); i++ { + g.Expect(*clusterScope.IBMPowerVSCluster.Status.VPCSubnet[fmt.Sprintf("subnet%dName", i)].ID).To(Equal(fmt.Sprintf("subnet%dID", i))) + g.Expect(*clusterScope.IBMPowerVSCluster.Status.VPCSubnet[fmt.Sprintf("subnet%dName", i)].ControllerCreated).To(BeTrue()) + } + }) t.Run("When VPCSubnets are not set in spec and subnet doesnot exist in cloud", func(t *testing.T) { g := NewWithT(t) setup(t) @@ -3591,13 +3707,41 @@ func TestReconcileVPCSubnets(t *testing.T) { }, } subnet1Details := &vpcv1.Subnet{ID: ptr.To("subnet1ID"), Name: ptr.To("ClusterName-vpcsubnet-eu-de-1")} + mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(nil, nil).Times(3) + mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(subnet1Details, nil, nil).Times(3) + requeue, err := clusterScope.ReconcileVPCSubnets() + g.Expect(requeue).To(BeTrue()) + g.Expect(err).To(BeNil()) + g.Expect(len(clusterScope.IBMPowerVSCluster.Status.VPCSubnet)).To(Equal(3)) + g.Expect(clusterScope.IBMPowerVSCluster.Status.VPCSubnet[*subnet1Details.Name].ID).To(Equal(subnet1Details.ID)) + g.Expect(*clusterScope.IBMPowerVSCluster.Status.VPCSubnet[*subnet1Details.Name].ControllerCreated).To(BeTrue()) + }) + + t.Run("When VPCSubnets are set in spec but zone is not specified", func(t *testing.T) { + g := NewWithT(t) + setup(t) + t.Cleanup(teardown) + + clusterScope := PowerVSClusterScope{ + IBMVPCClient: mockVPC, + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, + Status: infrav1beta2.IBMPowerVSClusterStatus{ + VPC: &infrav1beta2.ResourceReference{ID: ptr.To("vpcID")}, + }, + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")}, + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, + VPCSubnets: []infrav1beta2.Subnet{{Name: ptr.To("subnet1Name")}}}, + }, + } + subnet1Details := &vpcv1.Subnet{ID: ptr.To("subnet1ID"), Name: ptr.To("subnet1Name")} mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(nil, nil) - mockVPC.EXPECT().GetSubnetAddrPrefix(gomock.Any(), "eu-de-1").Return("10.240.20.0/18", nil) mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(subnet1Details, nil, nil) + requeue, err := clusterScope.ReconcileVPCSubnets() g.Expect(requeue).To(BeTrue()) g.Expect(err).To(BeNil()) - g.Expect(len(clusterScope.IBMPowerVSCluster.Status.VPCSubnet)).To(Equal(1)) g.Expect(clusterScope.IBMPowerVSCluster.Status.VPCSubnet[*subnet1Details.Name].ID).To(Equal(subnet1Details.ID)) g.Expect(*clusterScope.IBMPowerVSCluster.Status.VPCSubnet[*subnet1Details.Name].ControllerCreated).To(BeTrue()) }) @@ -3633,7 +3777,7 @@ func TestReconcileVPCSubnets(t *testing.T) { g.Expect(requeue).To(BeFalse()) g.Expect(err).ToNot(BeNil()) }) - t.Run("When subnetID and subnetName is nil", func(t *testing.T) { + t.Run("When subnetID and subnetName is nil and vpc subnet exists in cloud", func(t *testing.T) { g := NewWithT(t) setup(t) t.Cleanup(teardown) @@ -3643,7 +3787,9 @@ func TestReconcileVPCSubnets(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, Spec: infrav1beta2.IBMPowerVSClusterSpec{ - VPCSubnets: []infrav1beta2.Subnet{{}}}, + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, + VPCSubnets: []infrav1beta2.Subnet{{Zone: ptr.To("eu-de-1")}}, + }, Status: infrav1beta2.IBMPowerVSClusterStatus{ VPCSubnet: map[string]infrav1beta2.ResourceReference{ "subnet1Name": {ID: ptr.To("subnet1ID"), ControllerCreated: ptr.To(true)}, @@ -3652,7 +3798,7 @@ func TestReconcileVPCSubnets(t *testing.T) { } vpcSubnet1Name := fmt.Sprintf("%s-vpcsubnet-0", clusterScope.IBMPowerVSCluster.ObjectMeta.Name) subnet1Details := &vpcv1.Subnet{ID: ptr.To("subnet1ID"), Name: &vpcSubnet1Name} - mockVPC.EXPECT().GetVPCSubnetByName(vpcSubnet1Name).Return(subnet1Details, nil) + mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(subnet1Details, nil) requeue, err := clusterScope.ReconcileVPCSubnets() g.Expect(requeue).To(BeFalse()) @@ -3671,6 +3817,7 @@ func TestReconcileVPCSubnets(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, Spec: infrav1beta2.IBMPowerVSClusterSpec{ + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, VPCSubnets: []infrav1beta2.Subnet{ { ID: ptr.To("subnet1ID"), @@ -3696,6 +3843,7 @@ func TestReconcileVPCSubnets(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, Spec: infrav1beta2.IBMPowerVSClusterSpec{ + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, VPCSubnets: []infrav1beta2.Subnet{ { ID: ptr.To("subnet1ID"), @@ -3721,7 +3869,8 @@ func TestReconcileVPCSubnets(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, Spec: infrav1beta2.IBMPowerVSClusterSpec{ - VPCSubnets: []infrav1beta2.Subnet{{}}}, + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, + }, }, } mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(nil, fmt.Errorf("GetVPCSubnetByName returns error")) @@ -3740,7 +3889,8 @@ func TestReconcileVPCSubnets(t *testing.T) { IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}, Spec: infrav1beta2.IBMPowerVSClusterSpec{ - VPCSubnets: []infrav1beta2.Subnet{{}}}, + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-de")}, + }, }, } mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(nil, nil) @@ -3773,7 +3923,7 @@ func TestSetVPCSubnetStatus(t *testing.T) { Status: infrav1beta2.IBMPowerVSClusterStatus{ VPCSubnet: map[string]infrav1beta2.ResourceReference{ "subnet1Name": { - ControllerCreated: ptr.To(false), + ControllerCreated: ptr.To(true), }, }, }, @@ -3878,7 +4028,6 @@ func TestCreateVPCSubnet(t *testing.T) { subnet := infrav1beta2.Subnet{Name: ptr.To("ClusterName-vpcsubnet-eu-de-1"), Zone: ptr.To("eu-de-1")} subnet1Details := &vpcv1.Subnet{ID: ptr.To("subnet1ID"), Name: ptr.To("ClusterName-vpcsubnet-eu-de-1")} - mockVPC.EXPECT().GetSubnetAddrPrefix(*clusterScope.IBMPowerVSCluster.Status.VPC.ID, *subnet.Zone).Return("10.240.20.0/18", nil) mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(subnet1Details, nil, nil) subnetID, err := clusterScope.createVPCSubnet(subnet) g.Expect(subnetID).To(Equal(subnet1Details.ID)) @@ -3902,7 +4051,6 @@ func TestCreateVPCSubnet(t *testing.T) { subnet := infrav1beta2.Subnet{Name: ptr.To("ClusterName-vpcsubnet-eu-de-1")} subnet1Details := &vpcv1.Subnet{ID: ptr.To("subnet1ID"), Name: ptr.To("ClusterName-vpcsubnet-eu-de-1")} - mockVPC.EXPECT().GetSubnetAddrPrefix(*clusterScope.IBMPowerVSCluster.Status.VPC.ID, "eu-de-1").Return("10.240.20.0/18", nil) mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(subnet1Details, nil, nil) subnetID, err := clusterScope.createVPCSubnet(subnet) g.Expect(subnetID).To(Equal(subnet1Details.ID)) @@ -3932,30 +4080,13 @@ func TestCreateVPCSubnet(t *testing.T) { clusterScope := PowerVSClusterScope{ IBMVPCClient: mockVPC, IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")}}, Status: infrav1beta2.IBMPowerVSClusterStatus{}}, + Spec: infrav1beta2.IBMPowerVSClusterSpec{ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")}}}, } subnet := infrav1beta2.Subnet{Name: ptr.To("ClusterName-vpcsubnet-eu-de-1"), Zone: ptr.To("eu-de-1")} subnetID, err := clusterScope.createVPCSubnet(subnet) g.Expect(subnetID).To(BeNil()) g.Expect(err).ToNot(BeNil()) }) - t.Run("When GetSubnetAddrPrefix returns error", func(t *testing.T) { - g := NewWithT(t) - setup(t) - t.Cleanup(teardown) - - clusterScope := PowerVSClusterScope{ - IBMVPCClient: mockVPC, - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")}}, - Status: infrav1beta2.IBMPowerVSClusterStatus{VPC: &infrav1beta2.ResourceReference{ID: ptr.To("vpcID")}}}, - } - subnet := infrav1beta2.Subnet{Name: ptr.To("ClusterName-vpcsubnet-eu-de-1"), Zone: ptr.To("eu-de-1")} - mockVPC.EXPECT().GetSubnetAddrPrefix(gomock.Any(), gomock.Any()).Return("", fmt.Errorf("GetSubnetAddrPrefix returns error")) - subnetID, err := clusterScope.createVPCSubnet(subnet) - g.Expect(subnetID).To(BeNil()) - g.Expect(err).ToNot(BeNil()) - }) t.Run("When CreateSubnet returns error", func(t *testing.T) { g := NewWithT(t) @@ -3969,8 +4100,7 @@ func TestCreateVPCSubnet(t *testing.T) { Status: infrav1beta2.IBMPowerVSClusterStatus{VPC: &infrav1beta2.ResourceReference{ID: ptr.To("vpcID")}}}, } subnet := infrav1beta2.Subnet{Name: ptr.To("ClusterName-vpcsubnet-eu-de-1"), Zone: ptr.To("eu-de-1")} - mockVPC.EXPECT().GetSubnetAddrPrefix(gomock.Any(), gomock.Any()).Return("10.10.1.10/24", nil) - mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(nil, nil, fmt.Errorf("GetSubnetAddrPrefix returns error")) + mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(nil, nil, fmt.Errorf("error creating subnet")) subnetID, err := clusterScope.createVPCSubnet(subnet) g.Expect(subnetID).To(BeNil()) g.Expect(err).ToNot(BeNil()) @@ -3987,46 +4117,11 @@ func TestCreateVPCSubnet(t *testing.T) { Status: infrav1beta2.IBMPowerVSClusterStatus{VPC: &infrav1beta2.ResourceReference{ID: ptr.To("vpcID")}}}, } subnet := infrav1beta2.Subnet{Name: ptr.To("ClusterName-vpcsubnet-eu-de-1"), Zone: ptr.To("eu-de-1")} - mockVPC.EXPECT().GetSubnetAddrPrefix(gomock.Any(), gomock.Any()).Return("10.10.1.10/24", nil) mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(nil, nil, nil) subnetID, err := clusterScope.createVPCSubnet(subnet) g.Expect(subnetID).To(BeNil()) g.Expect(err).ToNot(BeNil()) }) - t.Run("When VPCZonesForVPCRegion returns error", func(t *testing.T) { - g := NewWithT(t) - setup(t) - t.Cleanup(teardown) - - clusterScope := PowerVSClusterScope{ - IBMVPCClient: mockVPC, - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")}, - VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("aadd")}}, - }, - } - subnet := infrav1beta2.Subnet{Name: ptr.To("ClusterName-vpcsubnet-eu-de-1")} - subnetID, err := clusterScope.createVPCSubnet(subnet) - g.Expect(subnetID).To(BeNil()) - g.Expect(err).ToNot(BeNil()) - }) - t.Run("When VPCZonesForVPCRegion returns zero zones for the region set in spec", func(t *testing.T) { - g := NewWithT(t) - setup(t) - t.Cleanup(teardown) - - clusterScope := PowerVSClusterScope{ - IBMVPCClient: mockVPC, - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ - Spec: infrav1beta2.IBMPowerVSClusterSpec{ResourceGroup: &infrav1beta2.IBMPowerVSResourceReference{ID: ptr.To("resourceGroupID")}, - VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("")}}, - }, - } - subnet := infrav1beta2.Subnet{Name: ptr.To("ClusterName-vpcsubnet-eu-de-1")} - subnetID, err := clusterScope.createVPCSubnet(subnet) - g.Expect(subnetID).To(BeNil()) - g.Expect(err).ToNot(BeNil()) - }) } func TestPowerVSDeleteLoadBalancer(t *testing.T) { var ( diff --git a/controllers/ibmpowervscluster_controller_test.go b/controllers/ibmpowervscluster_controller_test.go index e8f3ad9cd..72501c990 100644 --- a/controllers/ibmpowervscluster_controller_test.go +++ b/controllers/ibmpowervscluster_controller_test.go @@ -31,6 +31,7 @@ import ( tgapiv1 "github.com/IBM/networking-go-sdk/transitgatewayapisv1" "github.com/IBM/platform-services-go-sdk/resourcecontrollerv2" "github.com/IBM/vpc-go-sdk/vpcv1" + regionUtil "github.com/ppc64le-cloud/powervs-utils" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1378,11 +1379,11 @@ func TestReconcileVPCResources(t *testing.T) { }, }, } + vpcZones, _ := regionUtil.VPCZonesForVPCRegion("us-south") mockVPC := vpcmock.NewMockVpc(gomock.NewController(t)) mockVPC.EXPECT().GetVPC(gomock.Any()).Return(&vpcv1.VPC{Status: ptr.To("active")}, nil, nil) - mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(nil, nil) - mockVPC.EXPECT().GetSubnetAddrPrefix(gomock.Any(), gomock.Any()).Return("cidr", nil) - mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(&vpcv1.Subnet{Status: ptr.To("active")}, nil, nil) + mockVPC.EXPECT().GetVPCSubnetByName(gomock.Any()).Return(nil, nil).Times(len(vpcZones)) + mockVPC.EXPECT().CreateSubnet(gomock.Any()).Return(&vpcv1.Subnet{Status: ptr.To("active")}, nil, nil).Times(len(vpcZones)) clusterScope.IBMVPCClient = mockVPC return clusterScope }, diff --git a/pkg/cloud/services/vpc/mock/vpc_generated.go b/pkg/cloud/services/vpc/mock/vpc_generated.go index baa495664..f8225dd59 100644 --- a/pkg/cloud/services/vpc/mock/vpc_generated.go +++ b/pkg/cloud/services/vpc/mock/vpc_generated.go @@ -491,21 +491,6 @@ func (mr *MockVpcMockRecorder) GetSubnet(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubnet", reflect.TypeOf((*MockVpc)(nil).GetSubnet), arg0) } -// GetSubnetAddrPrefix mocks base method. -func (m *MockVpc) GetSubnetAddrPrefix(vpcID, zone string) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSubnetAddrPrefix", vpcID, zone) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetSubnetAddrPrefix indicates an expected call of GetSubnetAddrPrefix. -func (mr *MockVpcMockRecorder) GetSubnetAddrPrefix(vpcID, zone any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubnetAddrPrefix", reflect.TypeOf((*MockVpc)(nil).GetSubnetAddrPrefix), vpcID, zone) -} - // GetSubnetPublicGateway mocks base method. func (m *MockVpc) GetSubnetPublicGateway(options *vpcv1.GetSubnetPublicGatewayOptions) (*vpcv1.PublicGateway, *core.DetailedResponse, error) { m.ctrl.T.Helper() diff --git a/pkg/cloud/services/vpc/service.go b/pkg/cloud/services/vpc/service.go index 0bba96038..a1abce385 100644 --- a/pkg/cloud/services/vpc/service.go +++ b/pkg/cloud/services/vpc/service.go @@ -450,50 +450,6 @@ func (s *Service) GetLoadBalancerByName(loadBalancerName string) (*vpcv1.LoadBal return loadBalancer, nil } -// GetSubnetAddrPrefix returns subnets address prefix. -func (s *Service) GetSubnetAddrPrefix(vpcID, zone string) (string, error) { - var addrPrefix *vpcv1.AddressPrefix - f := func(start string) (bool, string, error) { - // check for existing vpcAddressPrefixes - listVPCAddressPrefixesOptions := &vpcv1.ListVPCAddressPrefixesOptions{ - VPCID: &vpcID, - } - if start != "" { - listVPCAddressPrefixesOptions.Start = &start - } - - vpcAddressPrefixesList, _, err := s.ListVPCAddressPrefixes(listVPCAddressPrefixesOptions) - if err != nil { - return false, "", err - } - - if vpcAddressPrefixesList == nil { - return false, "", fmt.Errorf("vpcAddressPrefix list returned is nil") - } - - for i, addressPrefix := range vpcAddressPrefixesList.AddressPrefixes { - if (*addressPrefix.Zone.Name) == zone { - addrPrefix = &vpcAddressPrefixesList.AddressPrefixes[i] - return true, "", nil - } - } - - if vpcAddressPrefixesList.Next != nil && *vpcAddressPrefixesList.Next.Href != "" { - return false, *vpcAddressPrefixesList.Next.Href, nil - } - return true, "", nil - } - - if err := utils.PagingHelper(f); err != nil { - return "", err - } - - if addrPrefix != nil { - return *addrPrefix.CIDR, nil - } - return "", fmt.Errorf("not found a valid CIDR for VPC %s in zone %s", vpcID, zone) -} - // CreateSecurityGroup creates a new security group. func (s *Service) CreateSecurityGroup(options *vpcv1.CreateSecurityGroupOptions) (*vpcv1.SecurityGroup, *core.DetailedResponse, error) { return s.vpcService.CreateSecurityGroup(options) diff --git a/pkg/cloud/services/vpc/vpc.go b/pkg/cloud/services/vpc/vpc.go index 88ee4a1fa..a15847859 100644 --- a/pkg/cloud/services/vpc/vpc.go +++ b/pkg/cloud/services/vpc/vpc.go @@ -64,7 +64,6 @@ type Vpc interface { GetVPCSubnetByName(subnetName string) (*vpcv1.Subnet, error) GetLoadBalancerPoolByName(loadBalancerID string, poolName string) (*vpcv1.LoadBalancerPool, error) GetLoadBalancerByName(loadBalancerName string) (*vpcv1.LoadBalancer, error) - GetSubnetAddrPrefix(vpcID, zone string) (string, error) CreateSecurityGroup(options *vpcv1.CreateSecurityGroupOptions) (*vpcv1.SecurityGroup, *core.DetailedResponse, error) DeleteSecurityGroup(options *vpcv1.DeleteSecurityGroupOptions) (*core.DetailedResponse, error) ListSecurityGroups(options *vpcv1.ListSecurityGroupsOptions) (*vpcv1.SecurityGroupCollection, *core.DetailedResponse, error)