diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index e7ce26e90a..611876d046 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -32,10 +32,8 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/imageformat" "github.com/deckhouse/virtualization-controller/pkg/common/network" - "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/netmanager" "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" ) const ( @@ -279,8 +277,7 @@ func syncAttachedVMBDAHotplugVolumes( } if vd, ok := vdByName[ref.Name]; ok && vd != nil { - migrating, _ := conditions.GetCondition(vdcondition.MigratingType, vd.Status.Conditions) - if migrating.Status == metav1.ConditionTrue { + if isVolumeMigrating(vd) { continue } } @@ -400,6 +397,10 @@ func removeDisk(kvvm *KVVM, name string) { ) } +func isVolumeMigrating(vd *v1alpha2.VirtualDisk) bool { + return !vd.Status.MigrationState.StartTimestamp.IsZero() && vd.Status.MigrationState.EndTimestamp.IsZero() +} + func ApplyMigrationVolumes(kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdsByName map[string]*v1alpha2.VirtualDisk) error { bootOrder := uint(1) var updateVolumesStrategy *virtv1.UpdateVolumesStrategy = nil @@ -418,8 +419,7 @@ func ApplyMigrationVolumes(kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdsByName ma } var pvcName string - migrating, _ := conditions.GetCondition(vdcondition.MigratingType, vd.Status.Conditions) - if migrating.Status == metav1.ConditionTrue && conditions.IsLastUpdated(migrating, vd) && vd.Status.MigrationState.TargetPVC != "" { + if isVolumeMigrating(vd) && vd.Status.MigrationState.TargetPVC != "" { pvcName = vd.Status.MigrationState.TargetPVC updateVolumesStrategy = ptr.To(virtv1.UpdateVolumesStrategyMigration) } diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils_test.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils_test.go index 63ab83405c..8008127b2d 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils_test.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils_test.go @@ -84,8 +84,10 @@ var _ = Describe("syncAttachedVMBDAHotplugVolumes", func() { Reason: "MigrationReverted", }}, MigrationState: v1alpha2.VirtualDiskMigrationState{ - SourcePVC: sourcePVC, - TargetPVC: targetPVC, + SourcePVC: sourcePVC, + TargetPVC: targetPVC, + StartTimestamp: metav1.Now(), + EndTimestamp: metav1.Now(), }, }, } @@ -185,8 +187,9 @@ var _ = Describe("ApplyMigrationVolumes", func() { Reason: "Migrating", }}, MigrationState: v1alpha2.VirtualDiskMigrationState{ - SourcePVC: sourcePVC, - TargetPVC: targetPVC, + SourcePVC: sourcePVC, + TargetPVC: targetPVC, + StartTimestamp: metav1.Now(), }, }, } diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/migration.go b/images/virtualization-artifact/pkg/controller/vd/internal/migration.go index b6bcdae5ca..47992e08b2 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/migration.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/migration.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "log/slog" + "slices" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" @@ -649,6 +650,11 @@ func (h MigrationHandler) createTargetPersistentVolumeClaim(ctx context.Context, return nil, err } + // A previous migration's PVC may still be terminating when the next migration starts, remove it + pvcs = slices.DeleteFunc(pvcs, func(pvc corev1.PersistentVolumeClaim) bool { + return pvc.Name != sourcePVCName && pvc.DeletionTimestamp != nil + }) + if targetPVCName == sourcePVCName && targetPVCName != "" { logger.FromContext(ctx).Debug("Target PersistentVolumeClaim name matches source PersistentVolumeClaim; ignoring stale target name", slog.String("targetPVC", targetPVCName), diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go index dc972d4c9e..446dd0e4c1 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go @@ -23,7 +23,6 @@ import ( corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" @@ -37,12 +36,10 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/nodeaffinity" "github.com/deckhouse/virtualization-controller/pkg/common/object" commonvd "github.com/deckhouse/virtualization-controller/pkg/common/vd" - "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" "github.com/deckhouse/virtualization-controller/pkg/controller/powerstate" "github.com/deckhouse/virtualization-controller/pkg/controller/reconciler" "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" ) type VirtualMachineState interface { @@ -450,8 +447,7 @@ func (s *state) isVolumeMigrating(ctx context.Context, refs []blockDeviceRef) (b if vd == nil { continue } - migrating, _ := conditions.GetCondition(vdcondition.MigratingType, vd.Status.Conditions) - if migrating.Status == metav1.ConditionTrue { + if commonvd.IsMigrating(vd) { return true, nil } } @@ -539,10 +535,7 @@ func (s *state) resolveStorageClassName(ctx context.Context, kind v1alpha2.Block } // During migration, status.StorageClassName can still point to the old PVC's class. // Avoid using it for the target PVC; rely on target PVC fields instead. - migrating, _ := conditions.GetCondition(vdcondition.MigratingType, vd.Status.Conditions) - if migrating.Status == metav1.ConditionTrue && - conditions.IsLastUpdated(migrating, vd) && - vd.Status.MigrationState.TargetPVC == pvcName { + if commonvd.IsMigrating(vd) && vd.Status.MigrationState.TargetPVC == pvcName { return "", nil } return commonvd.ResolveStorageClassName(ctx, vd, nil) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go index bca3bd648a..d9368fe3a8 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go @@ -553,8 +553,9 @@ var _ = Describe("PVNodeAffinityTerms", func() { Reason: "Migrating", }} vd.Status.MigrationState = v1alpha2.VirtualDiskMigrationState{ - SourcePVC: "pvc-source", - TargetPVC: "pvc-target", + SourcePVC: "pvc-source", + TargetPVC: "pvc-target", + StartTimestamp: metav1.Now(), } pvcSource := makePVC("pvc-source", "pv-source") @@ -584,8 +585,9 @@ var _ = Describe("PVNodeAffinityTerms", func() { Reason: "Migrating", }} vd.Status.MigrationState = v1alpha2.VirtualDiskMigrationState{ - SourcePVC: "pvc-source", - TargetPVC: "pvc-target", + SourcePVC: "pvc-source", + TargetPVC: "pvc-target", + StartTimestamp: metav1.Now(), } pvcSource := makePVC("pvc-source", "pv-source") @@ -613,7 +615,38 @@ var _ = Describe("PVNodeAffinityTerms", func() { "affinity for unbound target PVC should use target storage class, not source VD status class") }) - It("should fall back to source PVC when migration condition is False (e.g. reverted)", func() { + It("should use target PVC's PV node affinity while in-flight even when the Migrating condition is False (WaitForTargetReady)", func() { + vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "local-disk"}) + + vd := makeVD("local-disk", "pvc-source") + vd.Generation = 1 + vd.Status.Conditions = []metav1.Condition{{ + Type: vdcondition.MigratingType.String(), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + Reason: "WaitForTargetReady", + }} + vd.Status.MigrationState = v1alpha2.VirtualDiskMigrationState{ + SourcePVC: "pvc-source", + TargetPVC: "pvc-target", + StartTimestamp: metav1.Now(), + } + + pvcSource := makePVC("pvc-source", "pv-source") + pvSource := makePV("pv-source", nodeAffinityTerm(node1)) + pvcTarget := makePVC("pvc-target", "pv-target") + pvTarget := makePV("pv-target", nodeAffinityTerm(node2)) + + s := buildState(vm, vd, pvcSource, pvSource, pvcTarget, pvTarget) + ctx := logger.ToContext(context.TODO(), slog.Default()) + terms, err := s.PVNodeAffinityTerms(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(terms).To(HaveLen(1)) + Expect(terms[0].MatchExpressions[0].Values).To(ConsistOf(node2), + "affinity must follow the in-flight migration target even while the Migrating condition is False") + }) + + It("should fall back to source PVC when migration has ended (reverted)", func() { vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "local-disk"}) vd := makeVD("local-disk", "pvc-source") @@ -625,8 +658,10 @@ var _ = Describe("PVNodeAffinityTerms", func() { Reason: "MigrationReverted", }} vd.Status.MigrationState = v1alpha2.VirtualDiskMigrationState{ - SourcePVC: "pvc-source", - TargetPVC: "pvc-target", + SourcePVC: "pvc-source", + TargetPVC: "pvc-target", + StartTimestamp: metav1.Now(), + EndTimestamp: metav1.Now(), } pvcSource := makePVC("pvc-source", "pv-source") @@ -655,8 +690,9 @@ var _ = Describe("PVNodeAffinityTerms", func() { Reason: "Migrating", }} vd.Status.MigrationState = v1alpha2.VirtualDiskMigrationState{ - SourcePVC: "pvc-source", - TargetPVC: "pvc-target", + SourcePVC: "pvc-source", + TargetPVC: "pvc-target", + StartTimestamp: metav1.Now(), } pvcSource := makePVC("pvc-source", "pv-source")