Skip to content

Commit

Permalink
Fix the delete subnet flow
Browse files Browse the repository at this point in the history
  • Loading branch information
@mkumatag
mkumatag committed Jan 16, 2023
1 parent bac7cf3 commit d863640
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 1 deletion.
26 changes: 25 additions & 1 deletion cloud/scope/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,35 @@ func (s *ClusterScope) ensureSubnetUnique(subnetName string) (*vpcv1.Subnet, err
func (s *ClusterScope) DeleteSubnet() error {
subnetID := *s.IBMVPCCluster.Status.Subnet.ID

// Lists the subnet available and compare before deleting to avoid any failure(404) later
if found, err := func() (bool, error) {
//TODO: Add paging mechanism and use that function everywhere
subnets, _, err := s.IBMVPCClient.ListSubnets(&vpcv1.ListSubnetsOptions{})
if err != nil {
return false, err
}

for _, subnet := range subnets.Subnets {
if *subnet.ID == subnetID {
return true, nil
}
}
return false, nil
}(); err != nil {
return err
} else if !found {
s.Logger.V(3).Info("No subnets found with ID", "Subnet ID", subnetID)
return nil
}

// get the pgw id for given subnet, so we can delete it later
getPGWOptions := &vpcv1.GetSubnetPublicGatewayOptions{}
getPGWOptions.SetID(subnetID)
pgw, _, err := s.IBMVPCClient.GetSubnetPublicGateway(getPGWOptions)
if pgw != nil && err == nil { // public gateway found
if err != nil {
return err
}
if pgw != nil { // public gateway found
// Unset the public gateway for subnet first
err = s.detachPublicGateway(subnetID, *pgw.ID)
if err != nil {
Expand Down
32 changes: 32 additions & 0 deletions cloud/scope/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,11 @@ func TestDeleteSubnet(t *testing.T) {
publicGateway := &vpcv1.PublicGateway{
ID: core.StringPtr("foo-public-gateway-id"),
}
subnet := &vpcv1.SubnetCollection{
Subnets: []vpcv1.Subnet{
{ID: core.StringPtr("foo-vpc-subnet-id")},
},
}

t.Run("Should delete subnet", func(t *testing.T) {
g := NewWithT(t)
Expand All @@ -591,6 +596,7 @@ func TestDeleteSubnet(t *testing.T) {
scope := setupClusterScope(clusterName, mockvpc)
scope.IBMVPCCluster.Spec = vpcCluster.Spec
scope.IBMVPCCluster.Status = vpcCluster.Status
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(publicGateway, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
mockvpc.EXPECT().DeletePublicGateway(gomock.AssignableToTypeOf(&vpcv1.DeletePublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
Expand All @@ -606,6 +612,7 @@ func TestDeleteSubnet(t *testing.T) {
scope := setupClusterScope(clusterName, mockvpc)
scope.IBMVPCCluster.Spec = vpcCluster.Spec
scope.IBMVPCCluster.Status = vpcCluster.Status
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(publicGateway, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, errors.New("Error when unsetting publicgateway for subnet"))
err := scope.DeleteSubnet()
Expand All @@ -619,6 +626,7 @@ func TestDeleteSubnet(t *testing.T) {
scope := setupClusterScope(clusterName, mockvpc)
scope.IBMVPCCluster.Spec = vpcCluster.Spec
scope.IBMVPCCluster.Status = vpcCluster.Status
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(publicGateway, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
mockvpc.EXPECT().DeletePublicGateway(gomock.AssignableToTypeOf(&vpcv1.DeletePublicGatewayOptions{})).Return(&core.DetailedResponse{}, errors.New("Error when deleting publicgateway for subnet"))
Expand All @@ -633,13 +641,37 @@ func TestDeleteSubnet(t *testing.T) {
scope := setupClusterScope(clusterName, mockvpc)
scope.IBMVPCCluster.Spec = vpcCluster.Spec
scope.IBMVPCCluster.Status = vpcCluster.Status
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(publicGateway, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
mockvpc.EXPECT().DeletePublicGateway(gomock.AssignableToTypeOf(&vpcv1.DeletePublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
mockvpc.EXPECT().DeleteSubnet(gomock.AssignableToTypeOf(&vpcv1.DeleteSubnetOptions{})).Return(&core.DetailedResponse{}, errors.New("Error when deleting subnet"))
err := scope.DeleteSubnet()
g.Expect(err).To(Not(BeNil()))
})

t.Run("Error listing subnets", func(t *testing.T) {
g := NewWithT(t)
mockController, mockvpc := setup(t)
t.Cleanup(mockController.Finish)
scope := setupClusterScope(clusterName, mockvpc)
scope.IBMVPCCluster.Spec = vpcCluster.Spec
scope.IBMVPCCluster.Status = vpcCluster.Status
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(nil, &core.DetailedResponse{}, errors.New("Error listing subnets"))
err := scope.DeleteSubnet()
g.Expect(err).To(Not(BeNil()))
})
t.Run("Subnet doesn't exist", func(t *testing.T) {
g := NewWithT(t)
mockController, mockvpc := setup(t)
t.Cleanup(mockController.Finish)
scope := setupClusterScope(clusterName, mockvpc)
scope.IBMVPCCluster.Spec = vpcCluster.Spec
scope.IBMVPCCluster.Status = vpcCluster.Status
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(&vpcv1.SubnetCollection{Subnets: []vpcv1.Subnet{}}, &core.DetailedResponse{}, nil)
err := scope.DeleteSubnet()
g.Expect(err).To(BeNil())
})
})
}

Expand Down
7 changes: 7 additions & 0 deletions controllers/ibmvpccluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
g.Expect(clusterScope.IBMVPCCluster.Finalizers).To(ContainElement(infrav1beta2.ClusterFinalizer))
})
getPGWOptions := &vpcv1.GetSubnetPublicGatewayOptions{ID: pointer.String("capi-subnet-id")}
subnet := &vpcv1.SubnetCollection{Subnets: []vpcv1.Subnet{{ID: core.StringPtr("capi-subnet-id")}}}
pgw := &vpcv1.PublicGateway{ID: pointer.String("capi-pgw-id")}
unsetPGWOptions := &vpcv1.UnsetSubnetPublicGatewayOptions{ID: pointer.String("capi-subnet-id")}
deleteSubnetOptions := &vpcv1.DeleteSubnetOptions{ID: pointer.String("capi-subnet-id")}
Expand All @@ -493,6 +494,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
setup(t)
t.Cleanup(teardown)
mockvpc.EXPECT().ListInstances(listVSIOpts).Return(instancelist, response, nil)
mockvpc.EXPECT().ListSubnets(&vpcv1.ListSubnetsOptions{}).Return(subnet, response, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(getPGWOptions).Return(pgw, response, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(unsetPGWOptions).Return(response, nil)
mockvpc.EXPECT().DeletePublicGateway(deletePGWOptions).Return(response, nil)
Expand All @@ -507,6 +509,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
setup(t)
t.Cleanup(teardown)
mockvpc.EXPECT().ListInstances(listVSIOpts).Return(instancelist, response, nil)
mockvpc.EXPECT().ListSubnets(&vpcv1.ListSubnetsOptions{}).Return(subnet, response, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(getPGWOptions).Return(pgw, response, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(unsetPGWOptions).Return(response, nil)
mockvpc.EXPECT().DeletePublicGateway(deletePGWOptions).Return(response, nil)
Expand All @@ -522,6 +525,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
setup(t)
t.Cleanup(teardown)
mockvpc.EXPECT().ListInstances(listVSIOpts).Return(instancelist, response, nil)
mockvpc.EXPECT().ListSubnets(&vpcv1.ListSubnetsOptions{}).Return(subnet, response, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(getPGWOptions).Return(pgw, response, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(unsetPGWOptions).Return(response, nil)
mockvpc.EXPECT().DeletePublicGateway(deletePGWOptions).Return(response, nil)
Expand All @@ -537,6 +541,7 @@ func TestIBMVPCClusterReconciler_delete(t *testing.T) {
setup(t)
t.Cleanup(teardown)
mockvpc.EXPECT().ListInstances(listVSIOpts).Return(instancelist, response, nil)
mockvpc.EXPECT().ListSubnets(&vpcv1.ListSubnetsOptions{}).Return(subnet, response, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(getPGWOptions).Return(pgw, response, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(unsetPGWOptions).Return(response, nil)
mockvpc.EXPECT().DeletePublicGateway(deletePGWOptions).Return(response, nil)
Expand Down Expand Up @@ -620,12 +625,14 @@ func TestIBMVPCClusterLBReconciler_delete(t *testing.T) {
g.Expect(clusterScope.IBMVPCCluster.Finalizers).To(ContainElement(infrav1beta2.ClusterFinalizer))
})
t.Run("Should successfully delete IBMVPCCluster and remove the finalizer", func(t *testing.T) {
subnet := &vpcv1.SubnetCollection{Subnets: []vpcv1.Subnet{{ID: core.StringPtr("capi-subnet-id")}}}
g := NewWithT(t)
mockController, mockvpc, clusterScope, reconciler := setup(t)
t.Cleanup(mockController.Finish)
pgw := &vpcv1.PublicGateway{ID: pointer.String("capi-pgw-id")}
mockvpc.EXPECT().ListInstances(gomock.AssignableToTypeOf(&vpcv1.ListInstancesOptions{})).Return(instancelist, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().ListLoadBalancers(gomock.AssignableToTypeOf(&vpcv1.ListLoadBalancersOptions{})).Return(&vpcv1.LoadBalancerCollection{}, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().ListSubnets(gomock.AssignableToTypeOf(&vpcv1.ListSubnetsOptions{})).Return(subnet, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().GetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.GetSubnetPublicGatewayOptions{})).Return(pgw, &core.DetailedResponse{}, nil)
mockvpc.EXPECT().UnsetSubnetPublicGateway(gomock.AssignableToTypeOf(&vpcv1.UnsetSubnetPublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
mockvpc.EXPECT().DeletePublicGateway(gomock.AssignableToTypeOf(&vpcv1.DeletePublicGatewayOptions{})).Return(&core.DetailedResponse{}, nil)
Expand Down

0 comments on commit d863640

Please sign in to comment.