feat(vmop): enable migration for VMs with hotplugged local disks

images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go

@@ -301,13 +301,18 @@ func ApplyMigrationVolumes(kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdsByName ma
 	bootOrder := uint(1)
 	var updateVolumesStrategy *virtv1.UpdateVolumesStrategy = nil
 
-	for _, bd := range vm.Spec.BlockDeviceRefs {
+	for _, bd := range vm.Status.BlockDeviceRefs {
 		if bd.Kind != v1alpha2.DiskDevice {
-			bootOrder++
+			if !bd.Hotplugged {
+				bootOrder++
+			}
 			continue
 		}
 
 		vd := vdsByName[bd.Name]
+		if vd == nil {
+			continue
+		}
 
 		var pvcName string
 		migrating, _ := conditions.GetCondition(vdcondition.MigratingType, vd.Status.Conditions)
@@ -320,14 +325,18 @@ func ApplyMigrationVolumes(kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdsByName ma
 		}
 
 		name := GenerateVDDiskName(bd.Name)
-		if err := kvvm.SetDisk(name, SetDiskOptions{
+		opts := SetDiskOptions{
 			PersistentVolumeClaim: pointer.GetPointer(pvcName),
 			Serial:                GenerateSerialFromObject(vd),
-			BootOrder:             bootOrder,
-		}); err != nil {
+			IsHotplugged:          bd.Hotplugged,
+		}
+		if !bd.Hotplugged {
+			opts.BootOrder = bootOrder
+			bootOrder++
+		}
+		if err := kvvm.SetDisk(name, opts); err != nil {
 			return err
 		}
-		bootOrder++
 	}
 
 	kvvm.SetUpdateVolumesStrategy(updateVolumesStrategy)

images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go

@@ -30,7 +30,6 @@ import (
 	"github.com/deckhouse/virtualization-controller/pkg/common/object"
 	commonvmop "github.com/deckhouse/virtualization-controller/pkg/common/vmop"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
-	"github.com/deckhouse/virtualization-controller/pkg/controller/indexer"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vmop/migration/internal/service"
 	genericservice "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/service"
 	"github.com/deckhouse/virtualization-controller/pkg/eventrecord"
@@ -189,23 +188,6 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *v1alpha2.VirtualMach
 		return reconcile.Result{}, nil
 	}
 
-	// 6.3 Fail if attached vmbdas are not shared.
-	attachedRWOHotplugDisks, err := h.areAnyRWOHotplugDisks(ctx, vm)
-	if err != nil {
-		return reconcile.Result{}, err
-	}
-	if attachedRWOHotplugDisks {
-		vmop.Status.Phase = v1alpha2.VMOPPhaseFailed
-		h.recorder.Event(vmop, corev1.EventTypeWarning, v1alpha2.ReasonErrVMOPFailed, "Hotplug disks are not shared. Cannot be migrated.")
-		conditions.SetCondition(
-			completedCond.
-				Reason(vmopcondition.ReasonHotplugDisksNotShared).
-				Status(metav1.ConditionFalse).
-				Message("Hotplug disks are not shared. Cannot be migrated."),
-			&vmop.Status.Conditions)
-		return reconcile.Result{}, nil
-	}
-
 	// 7. Check if the vm is migratable.
 	if !h.canExecute(vmop, vm) {
 		return reconcile.Result{}, nil
@@ -344,51 +326,6 @@ func (h LifecycleHandler) isKubeVirtMigrationRejectedDueToQuota(ctx context.Cont
 	return false, nil
 }
 
-func (h LifecycleHandler) areAnyRWOHotplugDisks(ctx context.Context, vm *v1alpha2.VirtualMachine) (bool, error) {
-	vmbdaList := &v1alpha2.VirtualMachineBlockDeviceAttachmentList{}
-	err := h.client.List(ctx, vmbdaList, client.InNamespace(vm.Namespace), &client.MatchingFields{
-		indexer.IndexFieldVMBDAByVM: vm.Name,
-	})
-	if err != nil {
-		return false, err
-	}
-
-	var vmbdas []*v1alpha2.VirtualMachineBlockDeviceAttachment
-	for _, vmbda := range vmbdaList.Items {
-		if vmbda.Spec.BlockDeviceRef.Kind != v1alpha2.VMBDAObjectRefKindVirtualDisk {
-			continue
-		}
-		vmbdas = append(vmbdas, &vmbda)
-	}
-
-	for _, vmbda := range vmbdas {
-		vd := &v1alpha2.VirtualDisk{}
-		err = h.client.Get(ctx, client.ObjectKey{Namespace: vmbda.Namespace, Name: vmbda.Spec.BlockDeviceRef.Name}, vd)
-		if err != nil {
-			return false, err
-		}
-
-		pvc := &corev1.PersistentVolumeClaim{}
-		err = h.client.Get(ctx, client.ObjectKey{Namespace: vd.Namespace, Name: vd.Status.Target.PersistentVolumeClaim}, pvc)
-		if err != nil {
-			return false, err
-		}
-
-		isRWX := false
-		for _, mode := range pvc.Status.AccessModes {
-			if mode == corev1.ReadWriteMany {
-				isRWX = true
-				break
-			}
-		}
-		if !isRWX {
-			return true, nil
-		}
-	}
-
-	return false, nil
-}
-
 func (h LifecycleHandler) otherMigrationsAreInProgress(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (bool, error) {
 	migList := &virtv1.VirtualMachineInstanceMigrationList{}
 	err := h.client.List(ctx, migList, client.InNamespace(vmop.GetNamespace()))

images/virtualization-artifact/pkg/controller/vmop/vmop_webhook.go

@@ -18,16 +18,11 @@ package vmop
 
 import (
 	"context"
-	"errors"
-	"fmt"
 
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/deckhouse/deckhouse/pkg/log"
-	"github.com/deckhouse/virtualization-controller/pkg/common/object"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/validator"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
 )
@@ -36,115 +31,7 @@ func NewValidator(c client.Client, log *log.Logger) admission.CustomValidator {
 	return validator.NewValidator[*v1alpha2.VirtualMachineOperation](log.
 		With("controller", "vmop-controller").
 		With("webhook", "validation"),
-	).WithCreateValidators(&deprecateMigrateValidator{}, NewLocalVirtualDiskValidator(c))
-}
-
-type LocalVirtualDiskValidator struct {
-	client client.Client
-}
-
-func NewLocalVirtualDiskValidator(client client.Client) *LocalVirtualDiskValidator {
-	return &LocalVirtualDiskValidator{client: client}
-}
-
-func (v *LocalVirtualDiskValidator) ValidateCreate(ctx context.Context, vmop *v1alpha2.VirtualMachineOperation) (admission.Warnings, error) {
-	if vmop.Spec.Type != v1alpha2.VMOPTypeEvict && vmop.Spec.Type != v1alpha2.VMOPTypeMigrate {
-		return nil, nil
-	}
-
-	vm, err := object.FetchObject(ctx, types.NamespacedName{
-		Namespace: vmop.Namespace,
-		Name:      vmop.Spec.VirtualMachine,
-	}, v.client, &v1alpha2.VirtualMachine{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to fetch virtual machine %s: %w", vmop.Spec.VirtualMachine, err)
-	}
-
-	if vm == nil {
-		return nil, nil
-	}
-
-	var hasHotplugs bool
-	var hasRWO bool
-
-	for _, bdRef := range vm.Status.BlockDeviceRefs {
-		if bdRef.Hotplugged {
-			hasHotplugs = true
-		}
-
-		switch bdRef.Kind {
-		case v1alpha2.VirtualDiskKind:
-			var vd *v1alpha2.VirtualDisk
-			vd, err = object.FetchObject(ctx, types.NamespacedName{
-				Namespace: vm.Namespace,
-				Name:      bdRef.Name,
-			}, v.client, &v1alpha2.VirtualDisk{})
-			if err != nil {
-				return nil, fmt.Errorf("failed to fetch virtual disk %s: %w", bdRef.Name, err)
-			}
-
-			if vd == nil || vd.Status.Target.PersistentVolumeClaim == "" {
-				return nil, nil
-			}
-
-			var isRWO bool
-			isRWO, err = v.isRWOPersistentVolumeClaim(ctx, vd.Status.Target.PersistentVolumeClaim, vm.Namespace)
-			if err != nil {
-				return nil, err
-			}
-
-			hasRWO = hasRWO || isRWO
-		case v1alpha2.VirtualImageKind:
-			var vi *v1alpha2.VirtualImage
-			vi, err = object.FetchObject(ctx, types.NamespacedName{
-				Namespace: vm.Namespace,
-				Name:      bdRef.Name,
-			}, v.client, &v1alpha2.VirtualImage{})
-			if err != nil {
-				return nil, fmt.Errorf("failed to fetch virtual image %s: %w", bdRef.Name, err)
-			}
-
-			if vi == nil || vi.Status.Target.PersistentVolumeClaim == "" {
-				return nil, nil
-			}
-
-			var isRWO bool
-			isRWO, err = v.isRWOPersistentVolumeClaim(ctx, vi.Status.Target.PersistentVolumeClaim, vm.Namespace)
-			if err != nil {
-				return nil, err
-			}
-
-			hasRWO = hasRWO || isRWO
-		}
-	}
-
-	if hasRWO && hasHotplugs {
-		return nil, errors.New("for now, migration of the rwo virtual disk is not allowed if the virtual machine has hot-plugged block devices")
-	}
-
-	return nil, nil
-}
-
-func (v *LocalVirtualDiskValidator) isRWOPersistentVolumeClaim(ctx context.Context, pvcName, pvcNamespace string) (bool, error) {
-	pvc, err := object.FetchObject(ctx, types.NamespacedName{
-		Namespace: pvcNamespace,
-		Name:      pvcName,
-	}, v.client, &corev1.PersistentVolumeClaim{})
-	if err != nil {
-		return false, fmt.Errorf("failed to fetch pvc %s: %w", pvcName, err)
-	}
-
-	if pvc == nil {
-		return false, nil
-	}
-
-	for _, mode := range pvc.Status.AccessModes {
-		if mode == corev1.ReadWriteOnce {
-			return true, nil
-		}
-	}
-
-	return false, nil
+	).WithCreateValidators(&deprecateMigrateValidator{})
 }
 
 type deprecateMigrateValidator struct{}

test/e2e/vm/volume_migration_local_disks.go

@@ -461,7 +461,7 @@ var _ = Describe("LocalVirtualDiskMigration", decoratorsForVolumeMigrations(), f
 		})
 	})
 
-	It("should be failed with RWO VMBDA", func() {
+	It("should succeed with hotplugged RWO disk", func() {
 		ns := f.Namespace().Name
 
 		vm, vds := localMigrationRootAndAdditionalBuild()