diff --git a/pkg/destroy/vsphere/client.go b/pkg/destroy/vsphere/client.go index f7fc5f1652c..b60d5e2b328 100644 --- a/pkg/destroy/vsphere/client.go +++ b/pkg/destroy/vsphere/client.go @@ -404,12 +404,15 @@ func (c *Client) GetCnsVolumes(ctx context.Context, infraID string) ([]cnstypes. } cnsVolumes := make([]cnstypes.CnsVolume, 0, len(volumes.Volumes)) + var errs []error for _, v := range volumes.Volumes { // This must be called to retrieve the ClusterId result, err := c.cnsClient.QueryVolume(ctx, &cnstypes.CnsQueryFilter{VolumeIds: []cnstypes.CnsVolumeId{v.VolumeId}}) if err != nil { - return nil, err + c.logger.WithField("VolumeId", v.VolumeId.Id).Debug(err) + errs = append(errs, err) + continue } // Confirm that the cluster id matches the infraid @@ -419,7 +422,7 @@ func (c *Client) GetCnsVolumes(ctx context.Context, infraID string) ([]cnstypes. } } } - return cnsVolumes, nil + return cnsVolumes, utilerrors.NewAggregate(errs) } // DeleteCnsVolumes deletes the CNS volume. diff --git a/pkg/destroy/vsphere/vsphere.go b/pkg/destroy/vsphere/vsphere.go index f1fc7de914d..b9e7b5aa25f 100644 --- a/pkg/destroy/vsphere/vsphere.go +++ b/pkg/destroy/vsphere/vsphere.go @@ -265,25 +265,34 @@ func (o *ClusterUninstaller) deleteCnsVolumes(ctx context.Context) error { ctx, cancel := context.WithTimeout(ctx, time.Minute*30) defer cancel() + var errs []error for _, client := range o.clients { - o.Logger.WithField("vCenter", client.GetVCenterName()).Debug("Delete CNS Volumes") + clientLogger := o.Logger.WithField("vCenter", client.GetVCenterName()) + clientLogger.Debug("Delete CNS Volumes") cnsVolumes, err := client.GetCnsVolumes(ctx, o.InfraID) if err != nil { - return err + errs = append(errs, err) } + found := len(cnsVolumes) + deleted := 0 for _, cv := range cnsVolumes { cnsVolumeLogger := o.Logger.WithField("CNSVolume", cv.VolumeId.Id).WithField("vCenter", client.GetVCenterName()) cnsVolumeLogger.Debug("Destroying") - err := client.DeleteCnsVolumes(ctx, cv) - if err != nil { - return err + if err := client.DeleteCnsVolumes(ctx, cv); err != nil { + cnsVolumeLogger.Debug(err) + errs = append(errs, err) + } else { + deleted++ + cnsVolumeLogger.Info("Destroyed") } - cnsVolumeLogger.Info("Destroyed") + } + if found > 0 { + clientLogger.Infof("Deleted %d of %d CNS volumes", deleted, found) } } - return nil + return utilerrors.NewAggregate(errs) } func (o *ClusterUninstaller) destroyCluster(ctx context.Context) (bool, error) { diff --git a/pkg/destroy/vsphere/vsphere_test.go b/pkg/destroy/vsphere/vsphere_test.go index c3efc5c0125..cf84a524c36 100644 --- a/pkg/destroy/vsphere/vsphere_test.go +++ b/pkg/destroy/vsphere/vsphere_test.go @@ -723,4 +723,91 @@ func TestDeleteCnsVolumes(t *testing.T) { err := uninstaller.deleteCnsVolumes(context.TODO()) assert.Regexp(t, "some vsphere error", err) }) + + t.Run("Delete CNS volumes continues after failure", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + vsphereClient := mock.NewMockAPI(mockCtrl) + + vsphereClient.EXPECT(). + GetCnsVolumes(gomock.Any(), gomock.Eq(deleteFailsID)). + Return([]cnstypes.CnsVolume{failVolume, validVolume, failVolume}, nil). + Times(1) + vsphereClient.EXPECT(). + DeleteCnsVolumes(gomock.Any(), gomock.Eq(failVolume)). + Return(errors.New("some vsphere error deleting CNS volume")). + Times(2) + vsphereClient.EXPECT(). + DeleteCnsVolumes(gomock.Any(), gomock.Eq(validVolume)). + Return(nil). + Times(1) + vsphereClient.EXPECT(). + GetVCenterName(). + Return(""). + AnyTimes() + + metadata := newDefaultMetadata() + metadata.InfraID = deleteFailsID + uninstaller := newWithClient(nullLogger, &metadata, []API{vsphereClient}) + assert.NotNil(t, uninstaller) + err := uninstaller.deleteCnsVolumes(context.TODO()) + assert.Regexp(t, "some vsphere error deleting CNS volume", err) + }) + + t.Run("Delete CNS volumes processes all clients when first client fails", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + failClient := mock.NewMockAPI(mockCtrl) + successClient := mock.NewMockAPI(mockCtrl) + + failClient.EXPECT(). + GetCnsVolumes(gomock.Any(), gomock.Eq(infraID)). + Return(nil, errors.New("vcenter connection error")). + Times(1) + failClient.EXPECT(). + GetVCenterName(). + Return("fail-vcenter"). + AnyTimes() + + successClient.EXPECT(). + GetCnsVolumes(gomock.Any(), gomock.Eq(infraID)). + Return([]cnstypes.CnsVolume{validVolume}, nil). + Times(1) + successClient.EXPECT(). + DeleteCnsVolumes(gomock.Any(), gomock.Eq(validVolume)). + Return(nil). + Times(1) + successClient.EXPECT(). + GetVCenterName(). + Return("success-vcenter"). + AnyTimes() + + metadata := newDefaultMetadata() + uninstaller := newWithClient(nullLogger, &metadata, []API{failClient, successClient}) + assert.NotNil(t, uninstaller) + err := uninstaller.deleteCnsVolumes(context.TODO()) + assert.Regexp(t, "vcenter connection error", err) + }) + + t.Run("Delete CNS volumes processes partial results from GetCnsVolumes", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + vsphereClient := mock.NewMockAPI(mockCtrl) + + vsphereClient.EXPECT(). + GetCnsVolumes(gomock.Any(), gomock.Eq(infraID)). + Return([]cnstypes.CnsVolume{validVolume}, errors.New("partial query failure")). + Times(1) + vsphereClient.EXPECT(). + DeleteCnsVolumes(gomock.Any(), gomock.Eq(validVolume)). + Return(nil). + Times(1) + vsphereClient.EXPECT(). + GetVCenterName(). + Return(""). + AnyTimes() + + metadata := newDefaultMetadata() + uninstaller := newWithClient(nullLogger, &metadata, []API{vsphereClient}) + assert.NotNil(t, uninstaller) + err := uninstaller.deleteCnsVolumes(context.TODO()) + assert.Regexp(t, "partial query failure", err) + }) }