[release-4.22] OCPBUGS-94774: vsphere - fix CNS volume destroy to aggregate errors

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.

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) {

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)
+	})
 }