Label source PVCs with the migration plan UID (#1395)

After successful migration, label the source PVCs with the migration plan UID. This will allow us to easily identify which PVCs have been migrated and which have not. This can make it easier to remove the source PVCs after migration is complete. Signed-off-by: Alexander Wels <[email protected]>
migtools · Nov 18, 2024 · fc81f94 · fc81f94
1 parent c47fede
commit fc81f94
Show file tree

Hide file tree

Showing 4 changed files with 157 additions and 1 deletion.
diff --git a/pkg/controller/directvolumemigration/task.go b/pkg/controller/directvolumemigration/task.go
@@ -70,6 +70,7 @@ const (
 	DirectVolumeMigrationRsyncClient             = "rsync-client"
 	DirectVolumeMigrationStunnel                 = "stunnel"
 	MigratedByDirectVolumeMigration              = "migration.openshift.io/migrated-by-directvolumemigration" // (dvm UID)
+	MigrationSourceFor                           = "migration.openshift.io/source-for-directvolumemigration"  // (dvm UID)
 )
 
 // Itinerary names

diff --git a/pkg/controller/migmigration/storage.go b/pkg/controller/migmigration/storage.go
@@ -11,6 +11,7 @@ import (
 	"github.com/go-logr/logr"
 	liberr "github.com/konveyor/controller/pkg/error"
 	migapi "github.com/konveyor/mig-controller/pkg/apis/migration/v1alpha1"
+	"github.com/konveyor/mig-controller/pkg/controller/directvolumemigration"
 	dvmc "github.com/konveyor/mig-controller/pkg/controller/directvolumemigration"
 	ocappsv1 "github.com/openshift/api/apps/v1"
 	appsv1 "k8s.io/api/apps/v1"
@@ -138,6 +139,16 @@ func (t *Task) swapPVCReferences() (reasons []string, err error) {
 		reasons = append(reasons,
 			fmt.Sprintf("Failed updating PVC references on VirtualMachines [%s]", strings.Join(failedVirtualMachineSwaps, ",")))
 	}
+	sourceClient, err := t.getSourceClient()
+	if err != nil {
+		err = liberr.Wrap(err)
+		return
+	}
+	failedHandleSourceLabels := t.handleSourceLabels(sourceClient, mapping)
+	if len(failedHandleSourceLabels) > 0 {
+		reasons = append(reasons,
+			fmt.Sprintf("Failed updating labels on source PVCs [%s]", strings.Join(failedHandleSourceLabels, ",")))
+	}
 	return
 }
 
@@ -157,6 +168,43 @@ func (t *Task) getPVCNameMapping() pvcNameMapping {
 	return mapping
 }
 
+func (t *Task) handleSourceLabels(client k8sclient.Client, mapping pvcNameMapping) (failedPVCs []string) {
+	if t.rollback() {
+		return
+	}
+	for _, ns := range t.sourceNamespaces() {
+		list := v1.PersistentVolumeClaimList{}
+		options := k8sclient.InNamespace(ns)
+		err := client.List(
+			context.TODO(),
+			&list,
+			options)
+		if err != nil {
+			failedPVCs = append(failedPVCs, fmt.Sprintf("failed listing PVCs in namespace %s", ns))
+			continue
+		}
+		for _, pvc := range list.Items {
+			labels := pvc.Labels
+			if labels == nil {
+				labels = make(map[string]string)
+			}
+			if mapping.ExistsAsValue(pvc.Name) {
+				//Skip target PVCs if they are in the same namespace, ensure the label was not copied
+				//from the source PVC
+				delete(labels, directvolumemigration.MigrationSourceFor)
+			} else {
+				// Migration completed successfully, mark PVCs as migrated.
+				labels[directvolumemigration.MigrationSourceFor] = string(t.PlanResources.MigPlan.UID)
+			}
+			pvc.Labels = labels
+			if err := client.Update(context.TODO(), &pvc); err != nil {
+				failedPVCs = append(failedPVCs, fmt.Sprintf("failed to modify labels on PVC %s/%s", pvc.Namespace, pvc.Name))
+			}
+		}
+	}
+	return failedPVCs
+}
+
 func (t *Task) swapStatefulSetPVCRefs(client k8sclient.Client, mapping pvcNameMapping) (failedStatefulSets []string) {
 	for _, ns := range t.destinationNamespaces() {
 		list := appsv1.StatefulSetList{}

diff --git a/pkg/controller/migmigration/storage_test.go b/pkg/controller/migmigration/storage_test.go
@@ -7,11 +7,13 @@ import (
 
 	"github.com/go-logr/logr"
 	migapi "github.com/konveyor/mig-controller/pkg/apis/migration/v1alpha1"
+	"github.com/konveyor/mig-controller/pkg/controller/directvolumemigration"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	runtime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	virtv1 "kubevirt.io/api/core/v1"
@@ -388,6 +390,91 @@ func TestTask_swapVirtualMachinePVCRefs(t *testing.T) {
 	}
 }
 
+func TestTask_handleSourceLabels(t *testing.T) {
+	tests := []struct {
+		name                      string
+		pvcMapping                pvcNameMapping
+		client                    client.Client
+		rollback                  bool
+		namespaces                []string
+		expectedVolumesWithLabels []string
+	}{
+		{
+			name:       "Rollback, no PVCs, should do nothing",
+			client:     fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(),
+			namespaces: []string{"ns-0:ns-0", "ns-1:ns-1"},
+			rollback:   true,
+		},
+		{
+			name: "PVCs in wrong namespaces, should do nothing",
+			client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(
+				createPersistentVolumeClaim("pvc-0", "ns-2", nil),
+			).Build(),
+			namespaces: []string{"ns-0:ns-0", "ns-1:ns-1"},
+		},
+		{
+			name: "PVCs in namespaces, should add label",
+			client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(
+				createPersistentVolumeClaim("pvc-0", "ns-0", nil),
+				createPersistentVolumeClaim("pvc-1", "ns-2", nil),
+			).Build(),
+			namespaces:                []string{"ns-0:ns-0", "ns-1:ns-1"},
+			expectedVolumesWithLabels: []string{"pvc-0"},
+		},
+		{
+			name: "PVCs in namespaces, with mapping, should remove label",
+			client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(
+				createPersistentVolumeClaim("pvc-0", "ns-1", map[string]string{directvolumemigration.MigrationSourceFor: "mig-0"}),
+				createPersistentVolumeClaim("pvc-1", "ns-2", nil),
+			).Build(),
+			namespaces: []string{"ns-0:ns-0", "ns-1:ns-1"},
+			pvcMapping: pvcNameMapping{"ns-1/pvc-1": "pvc-0"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			task := &Task{
+				Owner: &migapi.MigMigration{
+					Spec: migapi.MigMigrationSpec{
+						Rollback: tt.rollback,
+					},
+				},
+				PlanResources: &migapi.PlanResources{
+					MigPlan: &migapi.MigPlan{
+						Spec: migapi.MigPlanSpec{
+							Namespaces: tt.namespaces,
+						},
+					},
+				},
+			}
+			task.handleSourceLabels(tt.client, tt.pvcMapping)
+			pvcs := &corev1.PersistentVolumeClaimList{}
+			if err := tt.client.List(context.TODO(), pvcs); err != nil {
+				t.Errorf("Error listing PVCs: %v", err)
+				t.FailNow()
+			}
+			foundPVCs := sets.NewString()
+			for _, pvc := range pvcs.Items {
+				if pvc.Labels != nil {
+					if _, ok := pvc.Labels[directvolumemigration.MigrationSourceFor]; ok {
+						foundPVCs.Insert(pvc.Name)
+					}
+				}
+			}
+			for _, expectedVolume := range tt.expectedVolumesWithLabels {
+				if !foundPVCs.Has(expectedVolume) {
+					t.Errorf("Expected PVC %s to have labels", expectedVolume)
+					t.FailNow()
+				}
+			}
+			if len(foundPVCs) != len(tt.expectedVolumesWithLabels) {
+				t.Errorf("Expected %d PVCs with labels, found %d", len(tt.expectedVolumesWithLabels), len(foundPVCs))
+				t.FailNow()
+			}
+		})
+	}
+}
+
 func createVirtualMachine(name, namespace string, volumes []virtv1.Volume) *virtv1.VirtualMachine {
 	return &virtv1.VirtualMachine{
 		ObjectMeta: metav1.ObjectMeta{
@@ -433,3 +520,13 @@ func createVirtualMachineWithAnnotation(name, namespace, key, value string, volu
 	vm.Annotations = map[string]string{key: value}
 	return vm
 }
+
+func createPersistentVolumeClaim(name, namespace string, labels map[string]string) *corev1.PersistentVolumeClaim {
+	return &corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+			Labels:    labels,
+		},
+	}
+}
diff --git a/pkg/controller/migplan/pvlist.go b/pkg/controller/migplan/pvlist.go
@@ -10,6 +10,7 @@ import (
 	liberr "github.com/konveyor/controller/pkg/error"
 	migapi "github.com/konveyor/mig-controller/pkg/apis/migration/v1alpha1"
 	"github.com/konveyor/mig-controller/pkg/compat"
+	"github.com/konveyor/mig-controller/pkg/controller/directvolumemigration"
 	migpods "github.com/konveyor/mig-controller/pkg/pods"
 	migref "github.com/konveyor/mig-controller/pkg/reference"
 	"github.com/opentracing/opentracing-go"
@@ -385,6 +386,15 @@ func (r *ReconcileMigPlan) getClaims(client compat.Client, plan *migapi.MigPlan)
 		return false
 	}
 
+	migrationSourceOtherPlan := func(pvc core.PersistentVolumeClaim) bool {
+		if planuid, exists := pvc.Labels[directvolumemigration.MigrationSourceFor]; exists {
+			if planuid != string(plan.UID) {
+				return true
+			}
+		}
+		return false
+	}
+
 	isStorageConversionPlan := isStorageConversionPlan(plan)
 
 	for _, pod := range runningPods.Items {
@@ -398,7 +408,7 @@ func (r *ReconcileMigPlan) getClaims(client compat.Client, plan *migapi.MigPlan)
 			continue
 		}
 
-		if isStorageConversionPlan && alreadyMigrated(pvc) {
+		if isStorageConversionPlan && (alreadyMigrated(pvc) || migrationSourceOtherPlan(pvc)) {
 			continue
 		}