From 21cc91ae4a315a9bbdf5251cea9731c832ff6c55 Mon Sep 17 00:00:00 2001
From: Arik Hadas <ahadas@redhat.com>
Date: Thu, 19 Oct 2023 10:39:55 +0300
Subject: [PATCH] fix error handling in DeletePVCs

To return an error when we can't get a prime PVC rather than continue to
the next PVC.

Also refactored the code a bit to reduce cognitive complexity.

Signed-off-by: Arik Hadas <ahadas@redhat.com>
---
 pkg/controller/plan/kubevirt.go  | 20 +++++++-------------
 pkg/controller/plan/migration.go | 12 ++++++------
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/pkg/controller/plan/kubevirt.go b/pkg/controller/plan/kubevirt.go
index 4dfc6704a..0e0d01b00 100644
--- a/pkg/controller/plan/kubevirt.go
+++ b/pkg/controller/plan/kubevirt.go
@@ -937,27 +937,21 @@ func (r *KubeVirt) DeletePVCs(vm *plan.VMStatus) (err error) {
 	for _, pvc := range pvcs {
 		primePVC := core.PersistentVolumeClaim{}
 		err = r.Destination.Client.Get(context.TODO(), client.ObjectKey{Namespace: r.Plan.Spec.TargetNamespace, Name: fmt.Sprintf("prime-%s", string(pvc.UID))}, &primePVC)
-		if err != nil {
-			if k8serr.IsNotFound(err) {
-				err = nil
-			}
-			continue
+		if err != nil && !k8serr.IsNotFound(err) {
+			return
 		}
+		err = nil
 		// Best effort deletion
 		err = r.DeleteObject(&primePVC, vm, "Deleted prime PVC.", "pvc")
-		if err != nil && k8serr.IsNotFound(err) {
-			err = nil
-		}
-		if err != nil {
+		if err != nil && !k8serr.IsNotFound(err) {
 			return
 		}
+		err = nil
 		err = r.DeleteObject(&pvc, vm, "Deleted PVC.", "pvc")
-		if err != nil && k8serr.IsNotFound(err) {
-			err = nil
-		}
-		if err != nil {
+		if err != nil && !k8serr.IsNotFound(err) {
 			return
 		}
+		err = nil
 		pvcCopy := pvc.DeepCopy()
 		pvc.Finalizers = nil
 		patch := client.MergeFrom(pvcCopy)
diff --git a/pkg/controller/plan/migration.go b/pkg/controller/plan/migration.go
index f674433f0..33bae589b 100644
--- a/pkg/controller/plan/migration.go
+++ b/pkg/controller/plan/migration.go
@@ -424,7 +424,9 @@ func (r *Migration) Cancel() (err error) {
 }
 
 func (r *Migration) cleanUpPopulatorPVCs(vm *plan.VMStatus) (err error) {
-	err = r.kubevirt.DeletePVCs(vm)
+	if r.builder.SupportsVolumePopulators() {
+		err = r.kubevirt.DeletePVCs(vm)
+	}
 	return
 }
 
@@ -435,11 +437,9 @@ func (r *Migration) cleanup(vm *plan.VMStatus) (err error) {
 		if err != nil {
 			return
 		}
-		if r.builder.SupportsVolumePopulators() {
-			err = r.cleanUpPopulatorPVCs(vm)
-			if err != nil {
-				return
-			}
+		err = r.cleanUpPopulatorPVCs(vm)
+		if err != nil {
+			return
 		}
 	}
 	err = r.deleteImporterPods(vm)