diff --git a/api/v1beta2/ibmpowervscluster_webhook.go b/api/v1beta2/ibmpowervscluster_webhook.go index 7ada37670..aa548a800 100644 --- a/api/v1beta2/ibmpowervscluster_webhook.go +++ b/api/v1beta2/ibmpowervscluster_webhook.go @@ -138,28 +138,21 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterLoadBalancerNames() (allErr } func (r *IBMPowerVSCluster) validateIBMPowerVSClusterVPCSubnetNames() (allErrs field.ErrorList) { - subnetFound := make(map[string]bool) + found := make(map[string]bool) for i, subnet := range r.Spec.VPCSubnets { if subnet.Name == nil { continue } - if subnetFound[*subnet.Name] { + if found[*subnet.Name] { allErrs = append(allErrs, field.Duplicate(field.NewPath("spec", fmt.Sprintf("vpcSubnets[%d]", i)), map[string]interface{}{"Name": *subnet.Name})) continue } - subnetFound[*subnet.Name] = true + found[*subnet.Name] = true } return allErrs } -func (r *IBMPowerVSCluster) validateIBMPowerVSClusterVPCSubnets() (allErrs field.ErrorList) { - if err := r.validateIBMPowerVSClusterVPCSubnetNames(); err != nil { - allErrs = append(allErrs, err...) - } - return allErrs -} - func (r *IBMPowerVSCluster) validateIBMPowerVSClusterTransitGateway() *field.Error { if r.Spec.Zone == nil && r.Spec.VPC == nil { return nil @@ -216,7 +209,7 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterCreateInfraPrereq() (allErr if r.Spec.ResourceGroup == nil { allErrs = append(allErrs, field.Invalid(field.NewPath("spec.resourceGroup"), r.Spec.ResourceGroup, "value of resource group is empty")) } - if err := r.validateIBMPowerVSClusterVPCSubnets(); err != nil { + if err := r.validateIBMPowerVSClusterVPCSubnetNames(); err != nil { allErrs = append(allErrs, err...) } diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go index df558c9fd..593e811ee 100644 --- a/cloud/scope/powervs_cluster.go +++ b/cloud/scope/powervs_cluster.go @@ -1111,13 +1111,12 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { 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. - - 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)), @@ -1171,11 +1170,7 @@ func (s *PowerVSClusterScope) ReconcileVPCSubnets() (bool, error) { } if subnet.Zone == nil { - if index < len(vpcZones) { - subnet.Zone = &vpcZones[index] - } else { - subnet.Zone = &vpcZones[index%len(vpcZones)] - } + subnet.Zone = &vpcZones[index%len(vpcZones)] } s.V(3).Info("Creating VPC subnet") subnetID, err = s.createVPCSubnet(subnet) diff --git a/cloud/scope/powervs_cluster_test.go b/cloud/scope/powervs_cluster_test.go index dbe11b452..9556ba185 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" @@ -3383,6 +3384,119 @@ func TestReconcileVPCSubnets(t *testing.T) { 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) @@ -3607,7 +3721,7 @@ func TestSetVPCSubnetStatus(t *testing.T) { clusterScope: PowerVSClusterScope{ IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{}, }, - resource: infrav1beta2.ResourceReference{ID: ptr.To("ID12")}, + resource: infrav1beta2.ResourceReference{ID: ptr.To("ID1")}, }, { name: "VPC subnet status is not nil",