From fc81f947b723667559cfe5a357ec43c5137afbd1 Mon Sep 17 00:00:00 2001 From: Alexander Wels Date: Mon, 18 Nov 2024 09:09:46 -0600 Subject: [PATCH] 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 --- pkg/controller/directvolumemigration/task.go | 1 + pkg/controller/migmigration/storage.go | 48 ++++++++++ pkg/controller/migmigration/storage_test.go | 97 ++++++++++++++++++++ pkg/controller/migplan/pvlist.go | 12 ++- 4 files changed, 157 insertions(+), 1 deletion(-) diff --git a/pkg/controller/directvolumemigration/task.go b/pkg/controller/directvolumemigration/task.go index 27567093f..fa9537a72 100644 --- 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 index 0413381cc..330cdc2b7 100644 --- 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 index 57afaef6e..400157f45 100644 --- 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 index 8a4138efc..fb50d4ed9 100644 --- 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 }