diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index 034d25714a..aeca27b35e 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/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) diff --git a/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go b/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go index 00584a401b..120ac8dd76 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/migration/internal/handler/lifecycle.go +++ b/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())) diff --git a/images/virtualization-artifact/pkg/controller/vmop/vmop_webhook.go b/images/virtualization-artifact/pkg/controller/vmop/vmop_webhook.go index be66f35f71..76116cfbb8 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/vmop_webhook.go +++ b/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{} diff --git a/test/e2e/vm/volume_migration_local_disks.go b/test/e2e/vm/volume_migration_local_disks.go index c1a32ae24a..46cc2df155 100644 --- a/test/e2e/vm/volume_migration_local_disks.go +++ b/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()