Skip to content

Commit

Permalink
Merge pull request #786 from seanpang-vmware/deletevpc
Browse files Browse the repository at this point in the history 
Do not check cache when deleting unrealized vpc
  • Loading branch information
@seanpang-vmware
seanpang-vmware authored Sep 29, 2024
2 parents 7ee3892 + 41bfd55 commit c1ef61a
Show file tree
Hide file tree
Showing 2 changed files with 186 additions and 5 deletions.
14 changes: 9 additions & 5 deletions pkg/nsx/services/vpc/vpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,16 +204,13 @@ func (s *VPCService) ListVPC() []model.Vpc {
return vpcSet
}

// DeleteVPC will try to delete VPC resource from NSX.
func (s *VPCService) DeleteVPC(path string) error {
pathInfo, err := common.ParseVPCResourcePath(path)
if err != nil {
return err
}
vpcClient := s.NSXClient.VPCClient
vpc := s.VpcStore.GetByKey(pathInfo.VPCID)
if vpc == nil {
return nil
}

if err := vpcClient.Delete(pathInfo.OrgID, pathInfo.ProjectID, pathInfo.VPCID); err != nil {
err = nsxutil.TransNSXApiError(err)
Expand All @@ -223,6 +220,14 @@ func (s *VPCService) DeleteVPC(path string) error {
if lbs != nil {
s.LbsStore.Delete(lbs)
}

vpc := s.VpcStore.GetByKey(pathInfo.VPCID)
// When deleting vpc due to realization failure in VPC creation process. the VPC is created on NSX side,
// but not insert in to VPC store, in this condition, the vpc could not be found in vpc store.
if vpc == nil {
log.Info("VPC not found in vpc store, skip cleaning VPC store", "VPC", pathInfo.VPCID)
return nil
}
vpc.MarkedForDelete = &MarkedForDelete
if err := s.VpcStore.Apply(vpc); err != nil {
return err
Expand Down Expand Up @@ -725,7 +730,6 @@ func (s *VPCService) CreateOrUpdateVPC(obj *v1alpha1.NetworkInfo, nc *common.VPC
if realizestate.IsRealizeStateError(err) {
log.Error(err, "the created VPC is in error realization state, cleaning the resource", "VPC", *createdVpc.Id)
// delete the nsx vpc object and re-create it in the next loop
// TODO(gran) DeleteVPC will check VpcStore but new Vpc is not in store at this moment. Is it correct?
if err := s.DeleteVPC(*newVpc.Path); err != nil {
log.Error(err, "cleanup VPC failed", "VPC", *createdVpc.Id)
return nil, err
Expand Down
177 changes: 177 additions & 0 deletions pkg/nsx/services/vpc/vpc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ func createService(t *testing.T) (*VPCService, *gomock.Controller, *mocks.MockVp
BindingType: model.VpcBindingType(),
}}

lbsStore := &LBSStore{ResourceStore: common.ResourceStore{
Indexer: cache.NewIndexer(keyFunc, cache.Indexers{}),
BindingType: model.LBServiceBindingType(),
}}

service := &VPCService{
Service: common.Service{
Client: k8sClient,
Expand All @@ -75,6 +80,7 @@ func createService(t *testing.T) (*VPCService, *gomock.Controller, *mocks.MockVp
},
},
VpcStore: vpcStore,
LbsStore: lbsStore,
VPCNetworkConfigStore: VPCNetworkInfoStore{
VPCNetworkConfigMap: map[string]common.VPCNetworkConfigInfo{},
},
Expand Down Expand Up @@ -1172,3 +1178,174 @@ func TestVPCService_ValidateNetworkConfig(t *testing.T) {
})
}
}

func TestVPCService_DeleteVPC(t *testing.T) {
mockVpc := "mockVpc"
mockLb := "mockLb"
mockConnectityPath := fmt.Sprintf("/org/default/projects/proj1/vpcs/%s/connectivity", mockVpc)
mockLBKey := combineVPCIDAndLBSID(mockVpc, mockLb)
service, _, _ := createService(t)
fakeErr := errors.New("fake-errors")
tests := []struct {
name string
prepareFunc func(*testing.T, *VPCService) *gomonkey.Patches
LbsStore *LBSStore
path string
Lb *model.LBService
Vpc *model.Vpc
checkLBStore bool
checkVPCStore bool
wantErr bool
want error
}{
{
name: "parse vpc info error",
path: "/in/correct/path",
Lb: nil,
Vpc: nil,
want: nil,
wantErr: true,
checkLBStore: false,
checkVPCStore: false,
},
{
name: "delete vpc error",
prepareFunc: func(_ *testing.T, service *VPCService) (patches *gomonkey.Patches) {
patches = gomonkey.ApplyMethodSeq(reflect.TypeOf(service.NSXClient.VPCClient), "Delete", []gomonkey.OutputCell{{
Values: gomonkey.Params{
fakeErr,
},
Times: 1,
}})
return patches
},
path: "/orgs/default/projects/proj1/vpcs/mockVpc",
Lb: nil,
Vpc: nil,
wantErr: true,
want: nsxUtil.TransNSXApiError(fakeErr),
checkLBStore: false,
checkVPCStore: false,
},
{name: "lb in store but vpc not",
prepareFunc: func(_ *testing.T, service *VPCService) (patches *gomonkey.Patches) {
patches = gomonkey.ApplyMethodSeq(reflect.TypeOf(service.NSXClient.VPCClient), "Delete", []gomonkey.OutputCell{{
Values: gomonkey.Params{
nil,
},
Times: 1,
}})
return patches
},
path: "/orgs/default/projects/proj1/vpcs/mockVpc",
Lb: &model.LBService{
Id: &mockLb,
ConnectivityPath: &mockConnectityPath,
},
Vpc: nil,
wantErr: false,
want: nil,
checkLBStore: true,
checkVPCStore: false,
},
{name: "delete vpc store fail",
prepareFunc: func(_ *testing.T, service *VPCService) (patches *gomonkey.Patches) {
patches = gomonkey.ApplyMethodSeq(reflect.TypeOf(service.NSXClient.VPCClient), "Delete", []gomonkey.OutputCell{{
Values: gomonkey.Params{
nil,
},
Times: 1,
}})
patches.ApplyMethodSeq(reflect.TypeOf(service.VpcStore), "Apply", []gomonkey.OutputCell{{
Values: gomonkey.Params{
fakeErr,
},
Times: 1,
}})
return patches
},
Lb: &model.LBService{
Id: &mockLb,
ConnectivityPath: &mockConnectityPath,
},
Vpc: &model.Vpc{
Id: &mockVpc,
},
path: "/orgs/default/projects/proj1/vpcs/mockVpc",
want: fakeErr,
wantErr: true,
checkLBStore: true,
checkVPCStore: false,
},
{name: "happy pass",
prepareFunc: func(_ *testing.T, service *VPCService) (patches *gomonkey.Patches) {
patches = gomonkey.ApplyMethodSeq(reflect.TypeOf(service.NSXClient.VPCClient), "Delete", []gomonkey.OutputCell{{
Values: gomonkey.Params{
nil,
},
Times: 1,
}})
patches.ApplyMethodSeq(reflect.TypeOf(service.VpcStore), "Apply", []gomonkey.OutputCell{{
Values: gomonkey.Params{
nil,
},
Times: 1,
}})
return patches
},
Lb: &model.LBService{
Id: &mockLb,
ConnectivityPath: &mockConnectityPath,
},
Vpc: &model.Vpc{
Id: &mockVpc,
},
path: "/orgs/default/projects/proj1/vpcs/mockVpc",
want: nil,
wantErr: false,
checkLBStore: true,
checkVPCStore: true,
},
}
// We do not need to verify copylocks for test case.
//nolint: copylocks
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

if tt.prepareFunc != nil {
patches := tt.prepareFunc(t, service)
defer patches.Reset()
}

if tt.Lb != nil {
service.LbsStore.Add(tt.Lb)
}

if tt.Vpc != nil {
service.VpcStore.Add(tt.Vpc)
}

err := service.DeleteVPC(tt.path)
if (err != nil) != tt.wantErr {
t.Errorf("VPCService.DeleteVPC() error = %v, wantErr %v", err, tt.wantErr)
}

if tt.want != nil {
assert.Equal(t, err, tt.want)
}

if tt.checkLBStore {
//if prepare func executed, then lb store should be empty
lb := service.LbsStore.GetByKey(mockLBKey)
assert.Nil(t, lb)
}

if tt.checkVPCStore {
//if prepare func executed, then vpc store should be empty
lb := service.LbsStore.GetByKey(mockLBKey)
assert.Nil(t, lb)
}

})
}
}

0 comments on commit c1ef61a

Please sign in to comment.