Skip to content

Commit

Permalink
Addressed review comments and added UT
Browse files Browse the repository at this point in the history
  • Loading branch information
@Shilpa-Gokul
Shilpa-Gokul committed Nov 27, 2024
1 parent bb47862 commit 02e1446
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 21 deletions.
15 changes: 4 additions & 11 deletions api/v1beta2/ibmpowervscluster_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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...)
}

Expand Down
13 changes: 4 additions & 9 deletions cloud/scope/powervs_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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)
Expand Down
116 changes: 115 additions & 1 deletion cloud/scope/powervs_cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 02e1446

Please sign in to comment.