Skip to content

Commit 56a3c27

Browse files
mkumatagmkumatag
authored
Fix the delete subnet flow (#1033)
1 parent bac7cf3 commit 56a3c27

File tree

3 files changed

+64
-1
lines changed

3 files changed

+64
-1
lines changed

‎cloud/scope/cluster.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,11 +312,35 @@ func (s *ClusterScope) ensureSubnetUnique(subnetName string) (*vpcv1.Subnet, err
312312
func (s *ClusterScope) DeleteSubnet() error {
313313
subnetID := *s.IBMVPCCluster.Status.Subnet.ID
314314

315+
// Lists the subnet available and compare before deleting to avoid any failure(404) later
316+
if found, err := func() (bool, error) {
317+
//TODO: Add paging mechanism and use that function everywhere
318+
subnets, _, err := s.IBMVPCClient.ListSubnets(&vpcv1.ListSubnetsOptions{})
319+
if err != nil {
320+
return false, err
321+
}
322+
323+
for _, subnet := range subnets.Subnets {
324+
if *subnet.ID == subnetID {
325+
return true, nil
326+
}
327+
}
328+
return false, nil
329+
}(); err != nil {
330+
return err
331+
} else if !found {
332+
s.Logger.V(3).Info("No subnets found with ID", "Subnet ID", subnetID)
333+
return nil
334+
}
335+
315336
// get the pgw id for given subnet, so we can delete it later
316337
getPGWOptions := &vpcv1.GetSubnetPublicGatewayOptions{}
317338
getPGWOptions.SetID(subnetID)
318339
pgw, _, err := s.IBMVPCClient.GetSubnetPublicGateway(getPGWOptions)
319-
if pgw != nil && err == nil { // public gateway found
340+
if err != nil {
341+
return err
342+
}
343+
if pgw != nil { // public gateway found
320344
// Unset the public gateway for subnet first
321345
err = s.detachPublicGateway(subnetID, *pgw.ID)
322346
if err != nil {

‎cloud/scope/cluster_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,11 @@ func TestDeleteSubnet(t *testing.T) {
583583
publicGateway := &vpcv1.PublicGateway{
584584
ID: core.StringPtr("foo-public-gateway-id"),
585585
}
586+
subnet := &vpcv1.SubnetCollection{
587+
Subnets: []vpcv1.Subnet{
588+
{ID: core.StringPtr("foo-vpc-subnet-id")},
589+
},
590+
}
586591

587592
t.Run("Should delete subnet", func(t *testing.T) {
588593
g := NewWithT(t)
@@ -591,6 +596,7 @@ func TestDeleteSubnet(t *testing.T) {
591596
scope := setupClusterScope(clusterName, mockvpc)
592597
scope.IBMVPCCluster.Spec = vpcCluster.Spec
593598
scope.IBMVPCCluster.Status = vpcCluster.Status
599+
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
594600
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(publicGateway, &core.DetailedResponse{}, nil)
595601
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
596602
mockvpc.EXPECT().DeletePublicGateway(gomock.AssignableToTypeOf(&vpcv1.DeletePublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
@@ -606,6 +612,7 @@ func TestDeleteSubnet(t *testing.T) {
606612
scope := setupClusterScope(clusterName, mockvpc)
607613
scope.IBMVPCCluster.Spec = vpcCluster.Spec
608614
scope.IBMVPCCluster.Status = vpcCluster.Status
615+
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
609616
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(publicGateway, &core.DetailedResponse{}, nil)
610617
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, errors.New("Error when unsetting publicgateway for subnet"))
611618
err := scope.DeleteSubnet()
@@ -619,6 +626,7 @@ func TestDeleteSubnet(t *testing.T) {
619626
scope := setupClusterScope(clusterName, mockvpc)
620627
scope.IBMVPCCluster.Spec = vpcCluster.Spec
621628
scope.IBMVPCCluster.Status = vpcCluster.Status
629+
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
622630
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(publicGateway, &core.DetailedResponse{}, nil)
623631
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
624632
mockvpc.EXPECT().DeletePublicGateway(gomock.AssignableToTypeOf(&vpcv1.DeletePublicGatewayOptions{})).Return(&core.DetailedResponse{}, errors.New("Error when deleting publicgateway for subnet"))
@@ -633,13 +641,37 @@ func TestDeleteSubnet(t *testing.T) {
633641
scope := setupClusterScope(clusterName, mockvpc)
634642
scope.IBMVPCCluster.Spec = vpcCluster.Spec
635643
scope.IBMVPCCluster.Status = vpcCluster.Status
644+
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
636645
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(publicGateway, &core.DetailedResponse{}, nil)
637646
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
638647
mockvpc.EXPECT().DeletePublicGateway(gomock.AssignableToTypeOf(&vpcv1.DeletePublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
639648
mockvpc.EXPECT().DeleteSubnet(gomock.AssignableToTypeOf(&vpcv1.DeleteSubnetOptions{})).Return(&core.DetailedResponse{}, errors.New("Error when deleting subnet"))
640649
err := scope.DeleteSubnet()
641650
g.Expect(err).To(Not(BeNil()))
642651
})
652+
653+
t.Run("Error listing subnets", func(t *testing.T) {
654+
g := NewWithT(t)
655+
mockController, mockvpc := setup(t)
656+
t.Cleanup(mockController.Finish)
657+
scope := setupClusterScope(clusterName, mockvpc)
658+
scope.IBMVPCCluster.Spec = vpcCluster.Spec
659+
scope.IBMVPCCluster.Status = vpcCluster.Status
660+
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(nil, &core.DetailedResponse{}, errors.New("Error listing subnets"))
661+
err := scope.DeleteSubnet()
662+
g.Expect(err).To(Not(BeNil()))
663+
})
664+
t.Run("Subnet doesn't exist", func(t *testing.T) {
665+
g := NewWithT(t)
666+
mockController, mockvpc := setup(t)
667+
t.Cleanup(mockController.Finish)
668+
scope := setupClusterScope(clusterName, mockvpc)
669+
scope.IBMVPCCluster.Spec = vpcCluster.Spec
670+
scope.IBMVPCCluster.Status = vpcCluster.Status
671+
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(&vpcv1.SubnetCollection{Subnets: []vpcv1.Subnet{}}, &core.DetailedResponse{}, nil)
672+
err := scope.DeleteSubnet()
673+
g.Expect(err).To(BeNil())
674+
})
643675
})
644676
}
645677

‎controllers/ibmvpccluster_controller_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
483483
g.Expect(clusterScope.IBMVPCCluster.Finalizers).To(ContainElement(infrav1beta2.ClusterFinalizer))
484484
})
485485
getPGWOptions := &vpcv1.GetSubnetPublicGatewayOptions{ID: pointer.String("capi-subnet-id")}
486+
subnet := &vpcv1.SubnetCollection{Subnets: []vpcv1.Subnet{{ID: core.StringPtr("capi-subnet-id")}}}
486487
pgw := &vpcv1.PublicGateway{ID: pointer.String("capi-pgw-id")}
487488
unsetPGWOptions := &vpcv1.UnsetSubnetPublicGatewayOptions{ID: pointer.String("capi-subnet-id")}
488489
deleteSubnetOptions := &vpcv1.DeleteSubnetOptions{ID: pointer.String("capi-subnet-id")}
@@ -493,6 +494,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
493494
setup(t)
494495
t.Cleanup(teardown)
495496
mockvpc.EXPECT().ListInstances(listVSIOpts).Return(instancelist, response, nil)
497+
mockvpc.EXPECT().ListSubnets(&vpcv1.ListSubnetsOptions{}).Return(subnet, response, nil)
496498
mockvpc.EXPECT().GetSubnetPublicGateway(getPGWOptions).Return(pgw, response, nil)
497499
mockvpc.EXPECT().UnsetSubnetPublicGateway(unsetPGWOptions).Return(response, nil)
498500
mockvpc.EXPECT().DeletePublicGateway(deletePGWOptions).Return(response, nil)
@@ -507,6 +509,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
507509
setup(t)
508510
t.Cleanup(teardown)
509511
mockvpc.EXPECT().ListInstances(listVSIOpts).Return(instancelist, response, nil)
512+
mockvpc.EXPECT().ListSubnets(&vpcv1.ListSubnetsOptions{}).Return(subnet, response, nil)
510513
mockvpc.EXPECT().GetSubnetPublicGateway(getPGWOptions).Return(pgw, response, nil)
511514
mockvpc.EXPECT().UnsetSubnetPublicGateway(unsetPGWOptions).Return(response, nil)
512515
mockvpc.EXPECT().DeletePublicGateway(deletePGWOptions).Return(response, nil)
@@ -522,6 +525,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
522525
setup(t)
523526
t.Cleanup(teardown)
524527
mockvpc.EXPECT().ListInstances(listVSIOpts).Return(instancelist, response, nil)
528+
mockvpc.EXPECT().ListSubnets(&vpcv1.ListSubnetsOptions{}).Return(subnet, response, nil)
525529
mockvpc.EXPECT().GetSubnetPublicGateway(getPGWOptions).Return(pgw, response, nil)
526530
mockvpc.EXPECT().UnsetSubnetPublicGateway(unsetPGWOptions).Return(response, nil)
527531
mockvpc.EXPECT().DeletePublicGateway(deletePGWOptions).Return(response, nil)
@@ -537,6 +541,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
537541
setup(t)
538542
t.Cleanup(teardown)
539543
mockvpc.EXPECT().ListInstances(listVSIOpts).Return(instancelist, response, nil)
544+
mockvpc.EXPECT().ListSubnets(&vpcv1.ListSubnetsOptions{}).Return(subnet, response, nil)
540545
mockvpc.EXPECT().GetSubnetPublicGateway(getPGWOptions).Return(pgw, response, nil)
541546
mockvpc.EXPECT().UnsetSubnetPublicGateway(unsetPGWOptions).Return(response, nil)
542547
mockvpc.EXPECT().DeletePublicGateway(deletePGWOptions).Return(response, nil)
@@ -620,12 +625,14 @@ func TestIBMVPCClusterLBReconciler_delete(t *testing.T) {
620625
g.Expect(clusterScope.IBMVPCCluster.Finalizers).To(ContainElement(infrav1beta2.ClusterFinalizer))
621626
})
622627
t.Run("Should successfully delete IBMVPCCluster and remove the finalizer", func(t *testing.T) {
628+
subnet := &vpcv1.SubnetCollection{Subnets: []vpcv1.Subnet{{ID: core.StringPtr("capi-subnet-id")}}}
623629
g := NewWithT(t)
624630
mockController, mockvpc, clusterScope, reconciler := setup(t)
625631
t.Cleanup(mockController.Finish)
626632
pgw := &vpcv1.PublicGateway{ID: pointer.String("capi-pgw-id")}
627633
mockvpc.EXPECT().ListInstances(gomock.AssignableToTypeOf(&vpcv1.ListInstancesOptions{})).Return(instancelist, &core.DetailedResponse{}, nil)
628634
mockvpc.EXPECT().ListLoadBalancers(gomock.AssignableToTypeOf(&vpcv1.ListLoadBalancersOptions{})).Return(&vpcv1.LoadBalancerCollection{}, &core.DetailedResponse{}, nil)
635+
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
629636
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(pgw, &core.DetailedResponse{}, nil)
630637
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
631638
mockvpc.EXPECT().DeletePublicGateway(gomock.AssignableToTypeOf(&vpcv1.DeletePublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)

0 commit comments

Comments
 (0)