From 7c4df8583eabbaec385fcdcec71f427ddf15431b Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Fri, 27 Feb 2026 14:14:45 +0300 Subject: [PATCH 01/28] wip Signed-off-by: Daniil Loktev --- api/core/v1alpha2/block_device.go | 5 + api/core/v1alpha2/zz_generated.deepcopy.go | 9 +- crds/doc-ru-virtualmachines.yaml | 8 + crds/virtualmachines.yaml | 13 +- .../cmd/virtualization-controller/main.go | 2 +- .../pkg/controller/kvbuilder/kvvm_utils.go | 64 ++++-- .../vm/internal/block_device_handler.go | 32 +-- .../vm/internal/block_device_status.go | 92 +++++--- .../controller/vm/internal/hotplug_handler.go | 208 ++++++++++++++++++ .../pkg/controller/vm/internal/sync_kvvm.go | 50 ++++- .../validators/block_device_refs_validator.go | 36 ++- .../pkg/controller/vm/vm_controller.go | 4 + .../vmchange/comparator_block_devices.go | 7 +- .../pkg/controller/vmchange/compare_test.go | 16 +- .../pkg/controller/vmchange/spec_changes.go | 13 ++ 15 files changed, 455 insertions(+), 104 deletions(-) create mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go diff --git a/api/core/v1alpha2/block_device.go b/api/core/v1alpha2/block_device.go index 4e7eb743e4..926bc90e23 100644 --- a/api/core/v1alpha2/block_device.go +++ b/api/core/v1alpha2/block_device.go @@ -20,6 +20,9 @@ type BlockDeviceSpecRef struct { Kind BlockDeviceKind `json:"kind"` // The name of attached resource. Name string `json:"name"` + // Boot priority for the block device. Smaller value = higher priority. + // +optional + BootOrder *int `json:"bootOrder,omitempty"` } type BlockDeviceStatusRef struct { @@ -35,6 +38,8 @@ type BlockDeviceStatusRef struct { Target string `json:"target,omitempty"` // Block device is attached via hot plug connection. Hotplugged bool `json:"hotplugged,omitempty"` + // Computed boot order of the block device. + BootOrder int `json:"bootOrder"` // The name of the `VirtualMachineBlockDeviceAttachment` resource that defines hot plug disk connection to the virtual machine. VirtualMachineBlockDeviceAttachmentName string `json:"virtualMachineBlockDeviceAttachmentName,omitempty"` } diff --git a/api/core/v1alpha2/zz_generated.deepcopy.go b/api/core/v1alpha2/zz_generated.deepcopy.go index c64f428875..11d64d5eda 100644 --- a/api/core/v1alpha2/zz_generated.deepcopy.go +++ b/api/core/v1alpha2/zz_generated.deepcopy.go @@ -48,6 +48,11 @@ func (in *AttachedVirtualMachine) DeepCopy() *AttachedVirtualMachine { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BlockDeviceSpecRef) DeepCopyInto(out *BlockDeviceSpecRef) { *out = *in + if in.BootOrder != nil { + in, out := &in.BootOrder, &out.BootOrder + *out = new(int) + **out = **in + } return } @@ -3169,7 +3174,9 @@ func (in *VirtualMachineSpec) DeepCopyInto(out *VirtualMachineSpec) { if in.BlockDeviceRefs != nil { in, out := &in.BlockDeviceRefs, &out.BlockDeviceRefs *out = make([]BlockDeviceSpecRef, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.Provisioning != nil { in, out := &in.Provisioning, &out.Provisioning diff --git a/crds/doc-ru-virtualmachines.yaml b/crds/doc-ru-virtualmachines.yaml index 1979a58ba6..8b1bf48507 100644 --- a/crds/doc-ru-virtualmachines.yaml +++ b/crds/doc-ru-virtualmachines.yaml @@ -367,6 +367,11 @@ spec: name: description: | Имя ресурса заданного типа. + bootOrder: + description: | + Приоритет загрузки блочного устройства. Меньшее значение означает более высокий приоритет. + Если не задано ни для одного устройства, порядок загрузки определяется позицией в списке (начиная с 0). + Если задано хотя бы для одного устройства, явные значения определяют приоритет загрузки. bootloader: description: | Загрузчик для ВМ: @@ -588,6 +593,9 @@ spec: size: description: | Размер подключённого блочного устройства. + bootOrder: + description: | + Вычисленный порядок загрузки блочного устройства. target: description: | Название подключённого блочного устройства. diff --git a/crds/virtualmachines.yaml b/crds/virtualmachines.yaml index c8234cf73f..2a5853b61d 100644 --- a/crds/virtualmachines.yaml +++ b/crds/virtualmachines.yaml @@ -956,6 +956,13 @@ spec: type: string description: | Name of the attached resource. + bootOrder: + type: integer + minimum: 0 + description: | + Boot priority for the block device. Smaller value means higher boot priority. + If not set for any device, boot order follows the position in the list (0-based). + If set for at least one device, explicit values determine boot priority. liveMigrationPolicy: type: string @@ -1054,7 +1061,7 @@ spec: List of block devices attached to the VM. items: type: object - required: ["kind", "name", "size", "attached"] + required: ["kind", "name", "size", "attached", "bootOrder"] properties: attached: type: boolean @@ -1083,6 +1090,10 @@ spec: type: string description: | Size of the attached block device. + bootOrder: + type: integer + description: | + Computed boot order of the block device. virtualMachineBlockDeviceAttachmentName: type: string description: | diff --git a/images/virtualization-artifact/cmd/virtualization-controller/main.go b/images/virtualization-artifact/cmd/virtualization-controller/main.go index bee96df3e9..0bf17d61e7 100644 --- a/images/virtualization-artifact/cmd/virtualization-controller/main.go +++ b/images/virtualization-artifact/cmd/virtualization-controller/main.go @@ -342,7 +342,7 @@ func main() { } vmLogger := logger.NewControllerLogger(vm.ControllerName, logLevel, logOutput, logDebugVerbosity, logDebugControllerList) - if err = vm.SetupController(ctx, mgr, vmLogger, dvcrSettings, firmwareImage); err != nil { + if err = vm.SetupController(ctx, mgr, vmLogger, dvcrSettings, firmwareImage, virtClient, controllerNamespace); err != nil { log.Error(err.Error()) os.Exit(1) } diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index 284c21eaa1..2c05296084 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -125,9 +125,13 @@ func ApplyVirtualMachineSpec( return err } + existingNonHotplug := make(map[string]struct{}) hotpluggedDevices := make([]HotPlugDeviceSettings, 0) for _, volume := range kvvm.Resource.Spec.Template.Spec.Volumes { + isHotpluggable := false + if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.Hotpluggable { + isHotpluggable = true hotpluggedDevices = append(hotpluggedDevices, HotPlugDeviceSettings{ VolumeName: volume.Name, PVCName: volume.PersistentVolumeClaim.ClaimName, @@ -135,22 +139,57 @@ func ApplyVirtualMachineSpec( } if volume.ContainerDisk != nil && volume.ContainerDisk.Hotpluggable { + isHotpluggable = true hotpluggedDevices = append(hotpluggedDevices, HotPlugDeviceSettings{ VolumeName: volume.Name, Image: volume.ContainerDisk.Image, }) } + + if !isHotpluggable { + existingNonHotplug[volume.Name] = struct{}{} + } } kvvm.ClearDisks() - bootOrder := uint(1) + hasExplicitBootOrder := false for _, bd := range vm.Spec.BlockDeviceRefs { - // bootOrder starts from 1. + if bd.BootOrder != nil { + hasExplicitBootOrder = true + break + } + } + + isExistingKVVM := len(existingNonHotplug) > 0 || len(hotpluggedDevices) > 0 + + for i, bd := range vm.Spec.BlockDeviceRefs { + var kvBootOrder uint + if hasExplicitBootOrder { + if bd.BootOrder != nil { + kvBootOrder = uint(*bd.BootOrder) + 1 + } + } else { + kvBootOrder = uint(i) + 1 + } + + var volumeName string switch bd.Kind { case v1alpha2.ImageDevice: - // Attach ephemeral disk for storage: Kubernetes. - // Attach containerDisk for storage: ContainerRegistry (i.e. image from DVCR). + volumeName = GenerateVIDiskName(bd.Name) + case v1alpha2.ClusterImageDevice: + volumeName = GenerateCVIDiskName(bd.Name) + case v1alpha2.DiskDevice: + volumeName = GenerateVDDiskName(bd.Name) + } + + if isExistingKVVM { + if _, exists := existingNonHotplug[volumeName]; !exists { + continue + } + } + switch bd.Kind { + case v1alpha2.ImageDevice: vi, ok := viByName[bd.Name] if !ok || vi == nil { return fmt.Errorf("unexpected error: virtual image %q should exist in the cluster; please recreate it", bd.Name) @@ -160,12 +199,11 @@ func ApplyVirtualMachineSpec( switch vi.Spec.Storage { case v1alpha2.StorageKubernetes, v1alpha2.StoragePersistentVolumeClaim: - // Attach PVC as ephemeral volume: its data will be restored to initial state on VM restart. if err := kvvm.SetDisk(name, SetDiskOptions{ PersistentVolumeClaim: pointer.GetPointer(vi.Status.Target.PersistentVolumeClaim), IsEphemeral: true, Serial: GenerateSerialFromObject(vi), - BootOrder: bootOrder, + BootOrder: kvBootOrder, }); err != nil { return err } @@ -174,18 +212,15 @@ func ApplyVirtualMachineSpec( ContainerDisk: pointer.GetPointer(vi.Status.Target.RegistryURL), IsCdrom: imageformat.IsISO(vi.Status.Format), Serial: GenerateSerialFromObject(vi), - BootOrder: bootOrder, + BootOrder: kvBootOrder, }); err != nil { return err } default: return fmt.Errorf("unexpected storage type %q for vi %s. %w", vi.Spec.Storage, vi.Name, common.ErrUnknownType) } - bootOrder++ case v1alpha2.ClusterImageDevice: - // ClusterVirtualImage is attached as containerDisk. - cvi, ok := cviByName[bd.Name] if !ok || cvi == nil { return fmt.Errorf("unexpected error: cluster virtual image %q should exist in the cluster; please recreate it", bd.Name) @@ -196,22 +231,18 @@ func ApplyVirtualMachineSpec( ContainerDisk: pointer.GetPointer(cvi.Status.Target.RegistryURL), IsCdrom: imageformat.IsISO(cvi.Status.Format), Serial: GenerateSerialFromObject(cvi), - BootOrder: bootOrder, + BootOrder: kvBootOrder, }); err != nil { return err } - bootOrder++ case v1alpha2.DiskDevice: - // VirtualDisk is attached as a regular disk. - vd, ok := vdByName[bd.Name] if !ok || vd == nil { return fmt.Errorf("unexpected error: virtual disk %q should exist in the cluster; please recreate it", bd.Name) } pvcName := vd.Status.Target.PersistentVolumeClaim - // VirtualDisk doesn't have pvc yet: wait for pvc and reconcile again. if pvcName == "" { continue } @@ -220,11 +251,10 @@ func ApplyVirtualMachineSpec( if err := kvvm.SetDisk(name, SetDiskOptions{ PersistentVolumeClaim: pointer.GetPointer(pvcName), Serial: GenerateSerialFromObject(vd), - BootOrder: bootOrder, + BootOrder: kvBootOrder, }); err != nil { return err } - bootOrder++ default: return fmt.Errorf("unknown block device kind %q. %w", bd.Kind, common.ErrUnknownType) } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_handler.go index de3166476d..ec57ea56b2 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_handler.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" "log/slog" - "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -196,35 +195,8 @@ func (h *BlockDeviceHandler) Name() string { return nameBlockDeviceHandler } -func (h *BlockDeviceHandler) getBlockDeviceWarnings(ctx context.Context, s state.VirtualMachineState) (string, error) { - vmbdasByBlockDevice, err := s.VirtualMachineBlockDeviceAttachments(ctx) - if err != nil { - return "", err - } - - var conflictedRefs []string - - for _, bdSpecRef := range s.VirtualMachine().Current().Spec.BlockDeviceRefs { - // It is a precaution to not apply changes in spec.blockDeviceRefs if disk is already - // hotplugged using the VMBDA resource. - // spec check is done by VirtualDisk status - // the reverse check is done by the vmbda-controller. - _, conflict := vmbdasByBlockDevice[v1alpha2.VMBDAObjectRef{ - Kind: v1alpha2.VMBDAObjectRefKind(bdSpecRef.Kind), - Name: bdSpecRef.Name, - }] - if conflict { - conflictedRefs = append(conflictedRefs, fmt.Sprintf("%s/%s", strings.ToLower(string(bdSpecRef.Kind)), bdSpecRef.Name)) - continue - } - } - - var warning string - if len(conflictedRefs) > 0 { - warning = fmt.Sprintf("spec.blockDeviceRefs field contains block devices to hotplug (%s): unplug or remove them from spec to continue.", strings.Join(conflictedRefs, ", ")) - } - - return warning, nil +func (h *BlockDeviceHandler) getBlockDeviceWarnings(_ context.Context, _ state.VirtualMachineState) (string, error) { + return "", nil } // setFinalizersOnBlockDevices sets protection finalizers on CVMI and VMD attached to the VM. diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go index b92db5c0ff..6a99305796 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go @@ -33,21 +33,51 @@ type nameKindKey struct { name string } -// getBlockDeviceStatusRefs returns block device refs to populate .status.blockDeviceRefs of the virtual machine. -// If kvvm is present, this method will reflect all volumes with prefixes (vi,vd, or cvi) into the slice of `BlockDeviceStatusRef`. -// Block devices from the virtual machine specification will be added to the resulting slice if they have not been included in the previous step. +func computeBootOrders(bdRefs []v1alpha2.BlockDeviceSpecRef) map[nameKindKey]int { + hasExplicit := false + for _, bd := range bdRefs { + if bd.BootOrder != nil { + hasExplicit = true + break + } + } + + result := make(map[nameKindKey]int, len(bdRefs)) + for i, bd := range bdRefs { + key := nameKindKey{kind: bd.Kind, name: bd.Name} + if hasExplicit { + if bd.BootOrder != nil { + result[key] = *bd.BootOrder + } else { + result[key] = -1 + } + } else { + result[key] = i + } + } + return result +} + func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s state.VirtualMachineState) ([]v1alpha2.BlockDeviceStatusRef, error) { kvvm, err := s.KVVM(ctx) if err != nil { return nil, err } + specRefs := s.VirtualMachine().Current().Spec.BlockDeviceRefs + bootOrders := computeBootOrders(specRefs) + + specDevices := make(map[nameKindKey]struct{}, len(specRefs)) + for _, bd := range specRefs { + specDevices[nameKindKey{kind: bd.Kind, name: bd.Name}] = struct{}{} + } + var refs []v1alpha2.BlockDeviceStatusRef - // 1. There is no kvvm yet: populate block device refs with the spec. if kvvm == nil { - for _, specBlockDeviceRef := range s.VirtualMachine().Current().Spec.BlockDeviceRefs { - ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name) + for _, specBlockDeviceRef := range specRefs { + key := nameKindKey{kind: specBlockDeviceRef.Kind, name: specBlockDeviceRef.Name} + ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name, bootOrders[key]) ref.Size, err = h.getBlockDeviceRefSize(ctx, ref, s) if err != nil { return nil, err @@ -77,16 +107,19 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta attachedBlockDeviceRefs := make(map[nameKindKey]struct{}) - // 2. The kvvm already exists: populate block device refs with the kvvm volumes. for _, volume := range kvvm.Spec.Template.Spec.Volumes { bdName, kind := kvbuilder.GetOriginalDiskName(volume.Name) if kind == "" { - // Reflect only vi, vd, or cvi block devices in status. - // This is neither of them, so skip. continue } - ref := h.getBlockDeviceStatusRef(kind, bdName) + key := nameKindKey{kind: kind, name: bdName} + bo, hasBo := bootOrders[key] + if !hasBo { + bo = -1 + } + + ref := h.getBlockDeviceStatusRef(kind, bdName, bo) ref.Target, ref.Attached = h.getBlockDeviceTarget(volume, kvvmiVolumeStatusByName) ref.Size, err = h.getBlockDeviceRefSize(ctx, ref, s) if err != nil { @@ -95,30 +128,26 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta ref.Hotplugged = h.isHotplugged(volume, kvvmiVolumeStatusByName) if ref.Hotplugged { - ref.VirtualMachineBlockDeviceAttachmentName, err = h.getBlockDeviceAttachmentName(ctx, kind, bdName, s) - if err != nil { - return nil, err + _, isSpecDevice := specDevices[key] + if !isSpecDevice { + ref.VirtualMachineBlockDeviceAttachmentName, err = h.getBlockDeviceAttachmentName(ctx, kind, bdName, s) + if err != nil { + return nil, err + } } } refs = append(refs, ref) - attachedBlockDeviceRefs[nameKindKey{ - kind: ref.Kind, - name: ref.Name, - }] = struct{}{} + attachedBlockDeviceRefs[key] = struct{}{} } - // 3. The kvvm may be missing some block devices from the spec; they need to be added as well. - for _, specBlockDeviceRef := range s.VirtualMachine().Current().Spec.BlockDeviceRefs { - _, ok := attachedBlockDeviceRefs[nameKindKey{ - kind: specBlockDeviceRef.Kind, - name: specBlockDeviceRef.Name, - }] - if ok { + for _, specBlockDeviceRef := range specRefs { + key := nameKindKey{kind: specBlockDeviceRef.Kind, name: specBlockDeviceRef.Name} + if _, ok := attachedBlockDeviceRefs[key]; ok { continue } - ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name) + ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name, bootOrders[key]) ref.Size, err = h.getBlockDeviceRefSize(ctx, ref, s) if err != nil { return nil, err @@ -129,10 +158,11 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta return refs, nil } -func (h *BlockDeviceHandler) getBlockDeviceStatusRef(kind v1alpha2.BlockDeviceKind, name string) v1alpha2.BlockDeviceStatusRef { +func (h *BlockDeviceHandler) getBlockDeviceStatusRef(kind v1alpha2.BlockDeviceKind, name string, bootOrder int) v1alpha2.BlockDeviceStatusRef { return v1alpha2.BlockDeviceStatusRef{ - Kind: kind, - Name: name, + Kind: kind, + Name: name, + BootOrder: bootOrder, } } @@ -189,20 +219,14 @@ func (h *BlockDeviceHandler) getBlockDeviceTarget(volume virtv1.Volume, kvvmiVol func (h *BlockDeviceHandler) isHotplugged(volume virtv1.Volume, kvvmiVolumeStatusByName map[string]virtv1.VolumeStatus) bool { switch { - // 1. If kvvmi has volume status with hotplugVolume reference then it's 100% hot-plugged volume. case kvvmiVolumeStatusByName[volume.Name].HotplugVolume != nil: return true - - // 2. If kvvm has volume with hot-pluggable pvc reference then it's 100% hot-plugged volume. case volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.Hotpluggable: return true - - // 3. If kvvm has volume with hot-pluggable disk reference then it's 100% hot-plugged volume. case volume.ContainerDisk != nil && volume.ContainerDisk.Hotpluggable: return true } - // 4. Is not hot-plugged. return false } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go new file mode 100644 index 0000000000..aafd444809 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go @@ -0,0 +1,208 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package internal + +import ( + "context" + "fmt" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + virtv1 "kubevirt.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" + "github.com/deckhouse/virtualization-controller/pkg/logger" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" +) + +const nameHotplugHandler = "HotplugHandler" + +type hotplugService interface { + HotPlugDisk(ctx context.Context, ad *service.AttachmentDisk, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) error + UnplugDisk(ctx context.Context, kvvm *virtv1.VirtualMachine, diskName string) error +} + +func NewHotplugHandler(svc hotplugService) *HotplugHandler { + return &HotplugHandler{svc: svc} +} + +type HotplugHandler struct { + svc hotplugService +} + +func (h *HotplugHandler) Handle(ctx context.Context, s state.VirtualMachineState) (reconcile.Result, error) { + log := logger.FromContext(ctx).With(logger.SlogHandler(nameHotplugHandler)) + + if s.VirtualMachine().IsEmpty() { + return reconcile.Result{}, nil + } + + current := s.VirtualMachine().Current() + + if isDeletion(current) { + return reconcile.Result{}, nil + } + + kvvmi, err := s.KVVMI(ctx) + if err != nil { + return reconcile.Result{}, err + } + if kvvmi == nil { + return reconcile.Result{}, nil + } + + if current.Status.Phase == v1alpha2.MachineMigrating { + log.Info("VM is migrating, skip hotplug") + return reconcile.Result{RequeueAfter: 30 * time.Second}, nil + } + + bdReady, ok := conditions.GetCondition(vmcondition.TypeBlockDevicesReady, current.Status.Conditions) + if ok && bdReady.Status != metav1.ConditionTrue { + return reconcile.Result{}, nil + } + + kvvm, err := s.KVVM(ctx) + if err != nil || kvvm == nil { + return reconcile.Result{}, err + } + + specDevices := make(map[nameKindKey]v1alpha2.BlockDeviceSpecRef) + for _, bd := range current.Spec.BlockDeviceRefs { + specDevices[nameKindKey{kind: bd.Kind, name: bd.Name}] = bd + } + + type kvvmVolume struct { + name string + hotpluggable bool + } + kvvmDevices := make(map[nameKindKey]kvvmVolume) + if kvvm.Spec.Template != nil { + for _, vol := range kvvm.Spec.Template.Spec.Volumes { + name, kind := kvbuilder.GetOriginalDiskName(vol.Name) + if kind == "" { + continue + } + hp := (vol.PersistentVolumeClaim != nil && vol.PersistentVolumeClaim.Hotpluggable) || + (vol.ContainerDisk != nil && vol.ContainerDisk.Hotpluggable) + kvvmDevices[nameKindKey{kind: kind, name: name}] = kvvmVolume{name: vol.Name, hotpluggable: hp} + } + } + + pendingRequests := make(map[string]struct{}) + for _, vr := range kvvm.Status.VolumeRequests { + if vr.AddVolumeOptions != nil { + pendingRequests[vr.AddVolumeOptions.Name] = struct{}{} + } + if vr.RemoveVolumeOptions != nil { + pendingRequests[vr.RemoveVolumeOptions.Name] = struct{}{} + } + } + + bdState := NewBlockDeviceState(s) + if err = bdState.Reload(ctx); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to reload block device state: %w", err) + } + + var errs []error + + for key := range specDevices { + if _, attached := kvvmDevices[key]; attached { + continue + } + + var volumeName string + switch key.kind { + case v1alpha2.DiskDevice: + volumeName = kvbuilder.GenerateVDDiskName(key.name) + case v1alpha2.ImageDevice: + volumeName = kvbuilder.GenerateVIDiskName(key.name) + case v1alpha2.ClusterImageDevice: + volumeName = kvbuilder.GenerateCVIDiskName(key.name) + } + + if _, pending := pendingRequests[volumeName]; pending { + continue + } + + ad := h.buildAttachmentDisk(key, bdState) + if ad == nil { + log.Info("Block device not ready for hotplug", "kind", key.kind, "name", key.name) + continue + } + + if err = h.svc.HotPlugDisk(ctx, ad, current, kvvm); err != nil { + errs = append(errs, fmt.Errorf("hotplug %s/%s: %w", key.kind, key.name, err)) + } + } + + for key, vol := range kvvmDevices { + if _, inSpec := specDevices[key]; inSpec { + continue + } + + if !vol.hotpluggable { + continue + } + + if _, pending := pendingRequests[vol.name]; pending { + continue + } + + if err = h.svc.UnplugDisk(ctx, kvvm, vol.name); err != nil { + errs = append(errs, fmt.Errorf("unplug %s/%s: %w", key.kind, key.name, err)) + } + } + + if len(errs) > 0 { + return reconcile.Result{}, fmt.Errorf("hotplug errors: %v", errs) + } + + return reconcile.Result{}, nil +} + +func (h *HotplugHandler) Name() string { + return nameHotplugHandler +} + +func (h *HotplugHandler) buildAttachmentDisk(key nameKindKey, bdState BlockDevicesState) *service.AttachmentDisk { + switch key.kind { + case v1alpha2.DiskDevice: + vd, ok := bdState.VDByName[key.name] + if !ok || vd == nil || vd.Status.Target.PersistentVolumeClaim == "" { + return nil + } + return service.NewAttachmentDiskFromVirtualDisk(vd) + case v1alpha2.ImageDevice: + vi, ok := bdState.VIByName[key.name] + if !ok || vi == nil { + return nil + } + return service.NewAttachmentDiskFromVirtualImage(vi) + case v1alpha2.ClusterImageDevice: + cvi, ok := bdState.CVIByName[key.name] + if !ok || cvi == nil { + return nil + } + return service.NewAttachmentDiskFromClusterVirtualImage(cvi) + } + return nil +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 370ee737a6..2c15a71537 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -646,7 +646,6 @@ func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.Virt switch action { case vmchange.ActionRestart: - // Update KVVM spec according the current VM spec. if err = h.updateKVVM(ctx, s); err != nil { return fmt.Errorf("update virtual machine instance with new spec: %w", err) } @@ -659,8 +658,18 @@ func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.Virt h.recorder.Event(current, corev1.EventTypeNormal, v1alpha2.ReasonVMChangesApplied, message) log.Debug(message, "vm.name", current.GetName(), "changes", changes) - if err := h.updateKVVM(ctx, s); err != nil { - return fmt.Errorf("unable to update KVVM using new VM spec: %w", err) + if changes.IsBlockDeviceRefsOnly() && kvvmi != nil { + class, err := s.Class(ctx) + if err != nil { + return fmt.Errorf("failed to get vmclass: %w", err) + } + if err = h.patchKVVMLastAppliedSpec(ctx, current, kvvm, class); err != nil { + return fmt.Errorf("unable to patch last-applied-spec on KVVM: %w", err) + } + } else { + if err := h.updateKVVM(ctx, s); err != nil { + return fmt.Errorf("unable to update KVVM using new VM spec: %w", err) + } } case vmchange.ActionNone: @@ -688,6 +697,8 @@ func (h *SyncKvvmHandler) updateKVVMLastAppliedSpec( return nil } + log := logger.FromContext(ctx) + err := kvbuilder.SetLastAppliedSpec(kvvm, vm) if err != nil { return fmt.Errorf("set vm last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err) @@ -701,12 +712,43 @@ func (h *SyncKvvmHandler) updateKVVMLastAppliedSpec( return fmt.Errorf("unable to update KubeVirt VM '%s': %w", kvvm.GetName(), err) } - log := logger.FromContext(ctx) log.Info("Update last applied spec on KubeVirt VM done", "name", kvvm.GetName()) return nil } +// patchKVVMLastAppliedSpec updates only the last-applied-spec annotations on KubeVirt VirtualMachine +// using a merge patch. Unlike updateKVVMLastAppliedSpec, this does not send the full KVVM object, +// so it won't accidentally overwrite spec changes made by KubeVirt (e.g., from AddVolume processing). +func (h *SyncKvvmHandler) patchKVVMLastAppliedSpec( + ctx context.Context, + vm *v1alpha2.VirtualMachine, + kvvm *virtv1.VirtualMachine, + class *v1alpha2.VirtualMachineClass, +) error { + if vm == nil || kvvm == nil { + return nil + } + + patch := client.MergeFrom(kvvm.DeepCopy()) + + if err := kvbuilder.SetLastAppliedSpec(kvvm, vm); err != nil { + return fmt.Errorf("set vm last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err) + } + if err := kvbuilder.SetLastAppliedClassSpec(kvvm, class); err != nil { + return fmt.Errorf("set vmclass last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err) + } + + if err := h.client.Patch(ctx, kvvm, patch); err != nil { + return fmt.Errorf("unable to patch KubeVirt VM '%s': %w", kvvm.GetName(), err) + } + + log := logger.FromContext(ctx) + log.Info("Patch last applied spec on KubeVirt VM done", "name", kvvm.GetName()) + + return nil +} + func (h *SyncKvvmHandler) isVMUnschedulable( vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine, diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/block_device_refs_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/block_device_refs_validator.go index f578aef5cc..1dc843fb86 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/block_device_refs_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/block_device_refs_validator.go @@ -26,6 +26,11 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2" ) +type blockDeviceKey struct { + Kind v1alpha2.BlockDeviceKind + Name string +} + type BlockDeviceSpecRefsValidator struct{} func NewBlockDeviceSpecRefsValidator() *BlockDeviceSpecRefsValidator { @@ -33,8 +38,11 @@ func NewBlockDeviceSpecRefsValidator() *BlockDeviceSpecRefsValidator { } func (v *BlockDeviceSpecRefsValidator) validate(vm *v1alpha2.VirtualMachine) error { - err := v.noDoubles(vm) - if err != nil { + if err := v.noDoubles(vm); err != nil { + return err + } + + if err := v.validateBootOrder(vm); err != nil { return err } @@ -68,15 +76,33 @@ func (v *BlockDeviceSpecRefsValidator) ValidateUpdate(_ context.Context, _, newV } func (v *BlockDeviceSpecRefsValidator) noDoubles(vm *v1alpha2.VirtualMachine) error { - blockDevicesByRef := make(map[v1alpha2.BlockDeviceSpecRef]struct{}, len(vm.Spec.BlockDeviceRefs)) + seen := make(map[blockDeviceKey]struct{}, len(vm.Spec.BlockDeviceRefs)) for _, bdRef := range vm.Spec.BlockDeviceRefs { - if _, ok := blockDevicesByRef[bdRef]; ok { + key := blockDeviceKey{Kind: bdRef.Kind, Name: bdRef.Name} + if _, ok := seen[key]; ok { return fmt.Errorf("cannot specify the same block device reference more than once: %s with name %q has a duplicate reference", bdRef.Kind, bdRef.Name) } - blockDevicesByRef[bdRef] = struct{}{} + seen[key] = struct{}{} } return nil } + +func (v *BlockDeviceSpecRefsValidator) validateBootOrder(vm *v1alpha2.VirtualMachine) error { + seen := make(map[int]string) + for _, bdRef := range vm.Spec.BlockDeviceRefs { + if bdRef.BootOrder == nil { + continue + } + if *bdRef.BootOrder < 0 { + return fmt.Errorf("bootOrder must be >= 0, got %d for %s %q", *bdRef.BootOrder, bdRef.Kind, bdRef.Name) + } + if prev, exists := seen[*bdRef.BootOrder]; exists { + return fmt.Errorf("duplicate bootOrder %d: already used by %s, conflicts with %s %q", *bdRef.BootOrder, prev, bdRef.Kind, bdRef.Name) + } + seen[*bdRef.BootOrder] = fmt.Sprintf("%s/%s", bdRef.Kind, bdRef.Name) + } + return nil +} diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go index 1cd2ad4433..551c388d44 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go @@ -49,12 +49,15 @@ func SetupController( log *log.Logger, dvcrSettings *dvcr.Settings, firmwareImage string, + virtClient service.VirtClient, + controllerNamespace string, ) error { recorder := eventrecord.NewEventRecorderLogger(mgr, ControllerName) mgrCache := mgr.GetCache() client := mgr.GetClient() blockDeviceService := service.NewBlockDeviceService(client) vmClassService := service.NewVirtualMachineClassService(client) + attachmentService := service.NewAttachmentService(client, virtClient, controllerNamespace) migrateVolumesService := vmservice.NewMigrationVolumesService(client, internal.MakeKVVMFromVMSpec, 10*time.Second) @@ -73,6 +76,7 @@ func SetupController( internal.NewSizePolicyHandler(), internal.NewNetworkInterfaceHandler(featuregates.Default()), internal.NewSyncKvvmHandler(dvcrSettings, client, recorder, migrateVolumesService), + internal.NewHotplugHandler(attachmentService), internal.NewSyncPowerStateHandler(client, recorder), internal.NewSyncMetadataHandler(client), internal.NewLifeCycleHandler(client, recorder), diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go index 9eff82f650..0cae958499 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go @@ -81,6 +81,7 @@ func compareBlockDevices(current, desired *v1alpha2.VirtualMachineSpec) []FieldC switch { case isAdded && isRemoved: // Compact add+remove for the same index into one replace. + // A different device at the same index requires restart. changes = append(changes, FieldChange{ Operation: ChangeReplace, Path: itemPath, @@ -93,14 +94,14 @@ func compareBlockDevices(current, desired *v1alpha2.VirtualMachineSpec) []FieldC Operation: ChangeAdd, Path: itemPath, DesiredValue: desired.BlockDeviceRefs[idx], - ActionRequired: ActionRestart, + ActionRequired: ActionApplyImmediate, }) case isRemoved: changes = append(changes, FieldChange{ Operation: ChangeRemove, Path: itemPath, CurrentValue: current.BlockDeviceRefs[idx], - ActionRequired: ActionRestart, + ActionRequired: ActionApplyImmediate, }) case isSwapped: changes = append(changes, FieldChange{ @@ -108,7 +109,7 @@ func compareBlockDevices(current, desired *v1alpha2.VirtualMachineSpec) []FieldC Path: itemPath, CurrentValue: current.BlockDeviceRefs[idx], DesiredValue: desired.BlockDeviceRefs[idx], - ActionRequired: ActionRestart, + ActionRequired: ActionApplyImmediate, }) } } diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go index 7d3951b293..74212d554b 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go @@ -156,7 +156,7 @@ blockDeviceRefs: ), }, { - "restart on blockDeviceRefs add disk", + "apply immediate on blockDeviceRefs add disk", ` blockDeviceRefs: - kind: VirtualImage @@ -170,12 +170,12 @@ blockDeviceRefs: name: linux `, assertChanges( - actionRequired(ActionRestart), + actionRequired(ActionApplyImmediate), requirePathOperation("blockDeviceRefs.0", ChangeAdd), ), }, { - "restart on blockDeviceRefs remove disk", + "apply immediate on blockDeviceRefs remove disk", ` blockDeviceRefs: - kind: VirtualDisk @@ -189,12 +189,12 @@ blockDeviceRefs: name: linux `, assertChanges( - actionRequired(ActionRestart), + actionRequired(ActionApplyImmediate), requirePathOperation("blockDeviceRefs.0", ChangeRemove), ), }, { - "restart on blockDeviceRefs change order", + "apply immediate on blockDeviceRefs change order", ` blockDeviceRefs: - kind: VirtualImage @@ -210,13 +210,13 @@ blockDeviceRefs: name: linux `, assertChanges( - actionRequired(ActionRestart), + actionRequired(ActionApplyImmediate), requirePathOperation("blockDeviceRefs.0", ChangeReplace), requirePathOperation("blockDeviceRefs.1", ChangeReplace), ), }, { - "restart on blockDeviceRefs change order :: bigger", + "apply immediate on blockDeviceRefs change order :: bigger", ` blockDeviceRefs: - kind: VirtualImage @@ -245,7 +245,7 @@ blockDeviceRefs: name: linux `, assertChanges( - actionRequired(ActionRestart), + actionRequired(ActionApplyImmediate), requirePathOperation("blockDeviceRefs.0", ChangeReplace), requirePathOperation("blockDeviceRefs.1", ChangeReplace), requirePathOperation("blockDeviceRefs.4", ChangeReplace), diff --git a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go index 857b1c1312..88bf5ad61e 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "sort" + "strings" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "sigs.k8s.io/yaml" @@ -266,6 +267,18 @@ func (s *SpecChanges) IsDisruptive() bool { return s.ActionType() == ActionRestart } +func (s *SpecChanges) IsBlockDeviceRefsOnly() bool { + if s.IsEmpty() { + return false + } + for _, change := range s.changes { + if !strings.HasPrefix(change.Path, BlockDevicesPath) { + return false + } + } + return true +} + func (s *SpecChanges) ToJSON() string { // Sort by path. sort.SliceStable(s.changes, func(i, j int) bool { From ddf6e44263c065876e429c75779fab29378f5176 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Fri, 27 Feb 2026 19:51:21 +0300 Subject: [PATCH 02/28] wip Signed-off-by: Daniil Loktev --- api/core/v1alpha2/block_device.go | 5 +- crds/doc-ru-virtualmachines.yaml | 7 +- crds/virtualmachines.yaml | 12 +- .../pkg/controller/kvbuilder/kvvm_utils.go | 298 +++++++----------- .../vm/internal/block_device_handler.go | 32 +- .../vm/internal/block_device_status.go | 44 +-- .../controller/vm/internal/hotplug_handler.go | 160 +++++----- .../pkg/controller/vm/internal/sync_kvvm.go | 8 +- .../validators/block_device_refs_validator.go | 4 +- 9 files changed, 233 insertions(+), 337 deletions(-) diff --git a/api/core/v1alpha2/block_device.go b/api/core/v1alpha2/block_device.go index 926bc90e23..10dae40deb 100644 --- a/api/core/v1alpha2/block_device.go +++ b/api/core/v1alpha2/block_device.go @@ -20,8 +20,9 @@ type BlockDeviceSpecRef struct { Kind BlockDeviceKind `json:"kind"` // The name of attached resource. Name string `json:"name"` - // Boot priority for the block device. Smaller value = higher priority. + // Boot priority for the block device. 1-based: 1 = first to boot. Smaller value = higher priority. // +optional + // +kubebuilder:validation:Minimum=1 BootOrder *int `json:"bootOrder,omitempty"` } @@ -38,8 +39,6 @@ type BlockDeviceStatusRef struct { Target string `json:"target,omitempty"` // Block device is attached via hot plug connection. Hotplugged bool `json:"hotplugged,omitempty"` - // Computed boot order of the block device. - BootOrder int `json:"bootOrder"` // The name of the `VirtualMachineBlockDeviceAttachment` resource that defines hot plug disk connection to the virtual machine. VirtualMachineBlockDeviceAttachmentName string `json:"virtualMachineBlockDeviceAttachmentName,omitempty"` } diff --git a/crds/doc-ru-virtualmachines.yaml b/crds/doc-ru-virtualmachines.yaml index 4cf0598957..78a7d423c7 100644 --- a/crds/doc-ru-virtualmachines.yaml +++ b/crds/doc-ru-virtualmachines.yaml @@ -369,8 +369,8 @@ spec: Имя ресурса заданного типа. bootOrder: description: | - Приоритет загрузки блочного устройства. Меньшее значение означает более высокий приоритет. - Если не задано ни для одного устройства, порядок загрузки определяется позицией в списке (начиная с 0). + Приоритет загрузки блочного устройства. Нумерация с 1: 1 = загружается первым. Меньшее значение означает более высокий приоритет. + Если не задано ни для одного устройства, порядок загрузки определяется позицией в списке (начиная с 1). Если задано хотя бы для одного устройства, явные значения определяют приоритет загрузки. bootloader: description: | @@ -604,9 +604,6 @@ spec: size: description: | Размер подключённого блочного устройства. - bootOrder: - description: | - Вычисленный порядок загрузки блочного устройства. target: description: | Название подключённого блочного устройства. diff --git a/crds/virtualmachines.yaml b/crds/virtualmachines.yaml index fdb6bb5a1e..a34e8e1e4e 100644 --- a/crds/virtualmachines.yaml +++ b/crds/virtualmachines.yaml @@ -958,10 +958,10 @@ spec: Name of the attached resource. bootOrder: type: integer - minimum: 0 + minimum: 1 description: | - Boot priority for the block device. Smaller value means higher boot priority. - If not set for any device, boot order follows the position in the list (0-based). + Boot priority for the block device. 1-based: 1 = first to boot. Smaller value means higher boot priority. + If not set for any device, boot order follows the position in the list (starting from 1). If set for at least one device, explicit values determine boot priority. liveMigrationPolicy: @@ -1079,7 +1079,7 @@ spec: List of block devices attached to the VM. items: type: object - required: ["kind", "name", "size", "attached", "bootOrder"] + required: ["kind", "name", "size", "attached"] properties: attached: type: boolean @@ -1108,10 +1108,6 @@ spec: type: string description: | Size of the attached block device. - bootOrder: - type: integer - description: | - Computed boot order of the block device. virtualMachineBlockDeviceAttachmentName: type: string description: | diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index 2c05296084..3248b60578 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -20,14 +20,9 @@ import ( "crypto/md5" "encoding/hex" "fmt" + "slices" "strings" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/utils/ptr" - virtv1 "kubevirt.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/deckhouse/virtualization-controller/pkg/common" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/imageformat" @@ -37,6 +32,10 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/netmanager" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/ptr" + virtv1 "kubevirt.io/api/core/v1" ) const ( @@ -57,6 +56,18 @@ func GenerateCVIDiskName(name string) string { return CVIDiskPrefix + name } +func GenerateDiskName(kind v1alpha2.BlockDeviceKind, name string) string { + switch kind { + case v1alpha2.DiskDevice: + return VDDiskPrefix + name + case v1alpha2.ImageDevice: + return VIDiskPrefix + name + case v1alpha2.ClusterImageDevice: + return CVIDiskPrefix + name + } + return "" +} + func GetOriginalDiskName(prefixedName string) (string, v1alpha2.BlockDeviceKind) { switch { case strings.HasPrefix(prefixedName, VDDiskPrefix): @@ -81,19 +92,11 @@ func GenerateSerial(input string) string { return hex.EncodeToString(hashInBytes) } -type HotPlugDeviceSettings struct { - VolumeName string - PVCName string - Image string - DataVolumeName string -} - func ApplyVirtualMachineSpec( kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdByName map[string]*v1alpha2.VirtualDisk, viByName map[string]*v1alpha2.VirtualImage, cviByName map[string]*v1alpha2.ClusterVirtualImage, - vmbdas map[v1alpha2.VMBDAObjectRef][]*v1alpha2.VirtualMachineBlockDeviceAttachment, class *v1alpha2.VirtualMachineClass, ipAddress string, networkSpec network.InterfaceSpecList, @@ -125,33 +128,37 @@ func ApplyVirtualMachineSpec( return err } - existingNonHotplug := make(map[string]struct{}) - hotpluggedDevices := make([]HotPlugDeviceSettings, 0) - for _, volume := range kvvm.Resource.Spec.Template.Spec.Volumes { - isHotpluggable := false - - if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.Hotpluggable { - isHotpluggable = true - hotpluggedDevices = append(hotpluggedDevices, HotPlugDeviceSettings{ - VolumeName: volume.Name, - PVCName: volume.PersistentVolumeClaim.ClaimName, - }) - } + if err := applyBlockDeviceRefs(kvvm, vm, vdByName, viByName, cviByName); err != nil { + return err + } - if volume.ContainerDisk != nil && volume.ContainerDisk.Hotpluggable { - isHotpluggable = true - hotpluggedDevices = append(hotpluggedDevices, HotPlugDeviceSettings{ - VolumeName: volume.Name, - Image: volume.ContainerDisk.Image, - }) - } + if err := kvvm.SetProvisioning(vm.Spec.Provisioning); err != nil { + return err + } - if !isHotpluggable { - existingNonHotplug[volume.Name] = struct{}{} - } + kvvm.SetOwnerRef(vm, schema.GroupVersionKind{ + Group: v1alpha2.SchemeGroupVersion.Group, + Version: v1alpha2.SchemeGroupVersion.Version, + Kind: "VirtualMachine", + }) + kvvm.AddFinalizer(v1alpha2.FinalizerKVVMProtection) + + if ipAddress != "" { + kvvm.SetKVVMIAnnotation(netmanager.AnnoIPAddressCNIRequest, ipAddress) } - kvvm.ClearDisks() + kvvm.SetKVVMIAnnotation(virtv1.AllowPodBridgeNetworkLiveMigrationAnnotation, "true") + kvvm.SetKVVMILabel(annotations.SkipPodSecurityStandardsCheckLabel, "true") + + return setNetworksAnnotation(kvvm, networkSpec) +} + +func applyBlockDeviceRefs( + kvvm *KVVM, vm *v1alpha2.VirtualMachine, + vdByName map[string]*v1alpha2.VirtualDisk, + viByName map[string]*v1alpha2.VirtualImage, + cviByName map[string]*v1alpha2.ClusterVirtualImage, +) error { hasExplicitBootOrder := false for _, bd := range vm.Spec.BlockDeviceRefs { if bd.BootOrder != nil { @@ -160,180 +167,89 @@ func ApplyVirtualMachineSpec( } } - isExistingKVVM := len(existingNonHotplug) > 0 || len(hotpluggedDevices) > 0 - + kvvmVolumes := kvvm.Resource.Spec.Template.Spec.Volumes for i, bd := range vm.Spec.BlockDeviceRefs { + if len(kvvmVolumes) > 0 && !slices.ContainsFunc(kvvmVolumes, func(v virtv1.Volume) bool { return v.Name == GenerateDiskName(bd.Kind, bd.Name) }) { + continue + } + var kvBootOrder uint if hasExplicitBootOrder { if bd.BootOrder != nil { - kvBootOrder = uint(*bd.BootOrder) + 1 + kvBootOrder = uint(*bd.BootOrder) } } else { kvBootOrder = uint(i) + 1 } - var volumeName string - switch bd.Kind { - case v1alpha2.ImageDevice: - volumeName = GenerateVIDiskName(bd.Name) - case v1alpha2.ClusterImageDevice: - volumeName = GenerateCVIDiskName(bd.Name) - case v1alpha2.DiskDevice: - volumeName = GenerateVDDiskName(bd.Name) - } - - if isExistingKVVM { - if _, exists := existingNonHotplug[volumeName]; !exists { - continue - } - } - - switch bd.Kind { - case v1alpha2.ImageDevice: - vi, ok := viByName[bd.Name] - if !ok || vi == nil { - return fmt.Errorf("unexpected error: virtual image %q should exist in the cluster; please recreate it", bd.Name) - } - - name := GenerateVIDiskName(bd.Name) - switch vi.Spec.Storage { - case v1alpha2.StorageKubernetes, - v1alpha2.StoragePersistentVolumeClaim: - if err := kvvm.SetDisk(name, SetDiskOptions{ - PersistentVolumeClaim: pointer.GetPointer(vi.Status.Target.PersistentVolumeClaim), - IsEphemeral: true, - Serial: GenerateSerialFromObject(vi), - BootOrder: kvBootOrder, - }); err != nil { - return err - } - case v1alpha2.StorageContainerRegistry: - if err := kvvm.SetDisk(name, SetDiskOptions{ - ContainerDisk: pointer.GetPointer(vi.Status.Target.RegistryURL), - IsCdrom: imageformat.IsISO(vi.Status.Format), - Serial: GenerateSerialFromObject(vi), - BootOrder: kvBootOrder, - }); err != nil { - return err - } - default: - return fmt.Errorf("unexpected storage type %q for vi %s. %w", vi.Spec.Storage, vi.Name, common.ErrUnknownType) - } - - case v1alpha2.ClusterImageDevice: - cvi, ok := cviByName[bd.Name] - if !ok || cvi == nil { - return fmt.Errorf("unexpected error: cluster virtual image %q should exist in the cluster; please recreate it", bd.Name) - } - - name := GenerateCVIDiskName(bd.Name) - if err := kvvm.SetDisk(name, SetDiskOptions{ - ContainerDisk: pointer.GetPointer(cvi.Status.Target.RegistryURL), - IsCdrom: imageformat.IsISO(cvi.Status.Format), - Serial: GenerateSerialFromObject(cvi), - BootOrder: kvBootOrder, - }); err != nil { - return err - } - - case v1alpha2.DiskDevice: - vd, ok := vdByName[bd.Name] - if !ok || vd == nil { - return fmt.Errorf("unexpected error: virtual disk %q should exist in the cluster; please recreate it", bd.Name) - } - - pvcName := vd.Status.Target.PersistentVolumeClaim - if pvcName == "" { - continue - } - - name := GenerateVDDiskName(bd.Name) - if err := kvvm.SetDisk(name, SetDiskOptions{ - PersistentVolumeClaim: pointer.GetPointer(pvcName), - Serial: GenerateSerialFromObject(vd), - BootOrder: kvBootOrder, - }); err != nil { - return err - } - default: - return fmt.Errorf("unknown block device kind %q. %w", bd.Kind, common.ErrUnknownType) + if err := setBlockDeviceDisk(kvvm, bd, kvBootOrder, vdByName, viByName, cviByName); err != nil { + return err } } - if err := kvvm.SetProvisioning(vm.Spec.Provisioning); err != nil { - return err - } - - for _, device := range hotpluggedDevices { - name, kind := GetOriginalDiskName(device.VolumeName) - - var obj client.Object - var exists bool + return nil +} - switch kind { - case v1alpha2.ImageDevice: - obj, exists = viByName[name] - case v1alpha2.ClusterImageDevice: - obj, exists = cviByName[name] - case v1alpha2.DiskDevice: - obj, exists = vdByName[name] +func setBlockDeviceDisk( + kvvm *KVVM, bd v1alpha2.BlockDeviceSpecRef, bootOrder uint, + vdByName map[string]*v1alpha2.VirtualDisk, + viByName map[string]*v1alpha2.VirtualImage, + cviByName map[string]*v1alpha2.ClusterVirtualImage, +) error { + switch bd.Kind { + case v1alpha2.ImageDevice: + vi, ok := viByName[bd.Name] + if !ok || vi == nil { + return fmt.Errorf("unexpected error: virtual image %q should exist in the cluster; please recreate it", bd.Name) + } + opts := SetDiskOptions{ + Serial: GenerateSerialFromObject(vi), + BootOrder: bootOrder, + IsHotplugged: true, + } + switch vi.Spec.Storage { + case v1alpha2.StorageKubernetes, v1alpha2.StoragePersistentVolumeClaim: + opts.PersistentVolumeClaim = pointer.GetPointer(vi.Status.Target.PersistentVolumeClaim) + opts.IsEphemeral = true + case v1alpha2.StorageContainerRegistry: + opts.ContainerDisk = pointer.GetPointer(vi.Status.Target.RegistryURL) + opts.IsCdrom = imageformat.IsISO(vi.Status.Format) default: - return fmt.Errorf("unknown block device kind %q. %w", kind, common.ErrUnknownType) + return fmt.Errorf("unexpected storage type %q for vi %s. %w", vi.Spec.Storage, vi.Name, common.ErrUnknownType) } + return kvvm.SetDisk(GenerateVIDiskName(bd.Name), opts) - if !exists || obj == nil || obj.GetUID() == "" { - continue + case v1alpha2.ClusterImageDevice: + cvi, ok := cviByName[bd.Name] + if !ok || cvi == nil { + return fmt.Errorf("unexpected error: cluster virtual image %q should exist in the cluster; please recreate it", bd.Name) } - - switch { - case device.PVCName != "": - pvcName := device.PVCName - if kind == v1alpha2.DiskDevice { - if vd := vdByName[name]; vd != nil && vd.Status.Target.PersistentVolumeClaim != "" { - pvcName = vd.Status.Target.PersistentVolumeClaim - } - } - if err := kvvm.SetDisk(device.VolumeName, SetDiskOptions{ - PersistentVolumeClaim: pointer.GetPointer(pvcName), - IsHotplugged: true, - Serial: GenerateSerialFromObject(obj), - }); err != nil { - return err - } - case device.Image != "": - if err := kvvm.SetDisk(device.VolumeName, SetDiskOptions{ - ContainerDisk: pointer.GetPointer(device.Image), - IsHotplugged: true, - Serial: GenerateSerialFromObject(obj), - }); err != nil { - return err - } + return kvvm.SetDisk(GenerateCVIDiskName(bd.Name), SetDiskOptions{ + ContainerDisk: pointer.GetPointer(cvi.Status.Target.RegistryURL), + IsCdrom: imageformat.IsISO(cvi.Status.Format), + Serial: GenerateSerialFromObject(cvi), + BootOrder: bootOrder, + IsHotplugged: true, + }) + + case v1alpha2.DiskDevice: + vd, ok := vdByName[bd.Name] + if !ok || vd == nil { + return fmt.Errorf("unexpected error: virtual disk %q should exist in the cluster; please recreate it", bd.Name) } - } - - kvvm.SetOwnerRef(vm, schema.GroupVersionKind{ - Group: v1alpha2.SchemeGroupVersion.Group, - Version: v1alpha2.SchemeGroupVersion.Version, - Kind: "VirtualMachine", - }) - kvvm.AddFinalizer(v1alpha2.FinalizerKVVMProtection) - - if ipAddress != "" { - // Set ip address cni request annotation. - kvvm.SetKVVMIAnnotation(netmanager.AnnoIPAddressCNIRequest, ipAddress) - } - - // Set live migration annotation. - kvvm.SetKVVMIAnnotation(virtv1.AllowPodBridgeNetworkLiveMigrationAnnotation, "true") - // Set label to skip the check for PodSecurityStandards to avoid irrelevant alerts related to a privileged virtual machine pod. - kvvm.SetKVVMILabel(annotations.SkipPodSecurityStandardsCheckLabel, "true") + if vd.Status.Target.PersistentVolumeClaim == "" { + return nil + } + return kvvm.SetDisk(GenerateVDDiskName(bd.Name), SetDiskOptions{ + PersistentVolumeClaim: pointer.GetPointer(vd.Status.Target.PersistentVolumeClaim), + Serial: GenerateSerialFromObject(vd), + BootOrder: bootOrder, + IsHotplugged: true, + }) - // Set annotation for request network configuration. - err := setNetworksAnnotation(kvvm, networkSpec) - if err != nil { - return err + default: + return fmt.Errorf("unknown block device kind %q. %w", bd.Kind, common.ErrUnknownType) } - return nil } func ApplyMigrationVolumes(kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdsByName map[string]*v1alpha2.VirtualDisk) error { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_handler.go index ec57ea56b2..de3166476d 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_handler.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "log/slog" + "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -195,8 +196,35 @@ func (h *BlockDeviceHandler) Name() string { return nameBlockDeviceHandler } -func (h *BlockDeviceHandler) getBlockDeviceWarnings(_ context.Context, _ state.VirtualMachineState) (string, error) { - return "", nil +func (h *BlockDeviceHandler) getBlockDeviceWarnings(ctx context.Context, s state.VirtualMachineState) (string, error) { + vmbdasByBlockDevice, err := s.VirtualMachineBlockDeviceAttachments(ctx) + if err != nil { + return "", err + } + + var conflictedRefs []string + + for _, bdSpecRef := range s.VirtualMachine().Current().Spec.BlockDeviceRefs { + // It is a precaution to not apply changes in spec.blockDeviceRefs if disk is already + // hotplugged using the VMBDA resource. + // spec check is done by VirtualDisk status + // the reverse check is done by the vmbda-controller. + _, conflict := vmbdasByBlockDevice[v1alpha2.VMBDAObjectRef{ + Kind: v1alpha2.VMBDAObjectRefKind(bdSpecRef.Kind), + Name: bdSpecRef.Name, + }] + if conflict { + conflictedRefs = append(conflictedRefs, fmt.Sprintf("%s/%s", strings.ToLower(string(bdSpecRef.Kind)), bdSpecRef.Name)) + continue + } + } + + var warning string + if len(conflictedRefs) > 0 { + warning = fmt.Sprintf("spec.blockDeviceRefs field contains block devices to hotplug (%s): unplug or remove them from spec to continue.", strings.Join(conflictedRefs, ", ")) + } + + return warning, nil } // setFinalizersOnBlockDevices sets protection finalizers on CVMI and VMD attached to the VM. diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go index 6a99305796..3be7850d1c 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go @@ -33,31 +33,6 @@ type nameKindKey struct { name string } -func computeBootOrders(bdRefs []v1alpha2.BlockDeviceSpecRef) map[nameKindKey]int { - hasExplicit := false - for _, bd := range bdRefs { - if bd.BootOrder != nil { - hasExplicit = true - break - } - } - - result := make(map[nameKindKey]int, len(bdRefs)) - for i, bd := range bdRefs { - key := nameKindKey{kind: bd.Kind, name: bd.Name} - if hasExplicit { - if bd.BootOrder != nil { - result[key] = *bd.BootOrder - } else { - result[key] = -1 - } - } else { - result[key] = i - } - } - return result -} - func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s state.VirtualMachineState) ([]v1alpha2.BlockDeviceStatusRef, error) { kvvm, err := s.KVVM(ctx) if err != nil { @@ -65,7 +40,6 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta } specRefs := s.VirtualMachine().Current().Spec.BlockDeviceRefs - bootOrders := computeBootOrders(specRefs) specDevices := make(map[nameKindKey]struct{}, len(specRefs)) for _, bd := range specRefs { @@ -76,8 +50,7 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta if kvvm == nil { for _, specBlockDeviceRef := range specRefs { - key := nameKindKey{kind: specBlockDeviceRef.Kind, name: specBlockDeviceRef.Name} - ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name, bootOrders[key]) + ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name) ref.Size, err = h.getBlockDeviceRefSize(ctx, ref, s) if err != nil { return nil, err @@ -114,12 +87,8 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta } key := nameKindKey{kind: kind, name: bdName} - bo, hasBo := bootOrders[key] - if !hasBo { - bo = -1 - } - ref := h.getBlockDeviceStatusRef(kind, bdName, bo) + ref := h.getBlockDeviceStatusRef(kind, bdName) ref.Target, ref.Attached = h.getBlockDeviceTarget(volume, kvvmiVolumeStatusByName) ref.Size, err = h.getBlockDeviceRefSize(ctx, ref, s) if err != nil { @@ -147,7 +116,7 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta continue } - ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name, bootOrders[key]) + ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name) ref.Size, err = h.getBlockDeviceRefSize(ctx, ref, s) if err != nil { return nil, err @@ -158,11 +127,10 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta return refs, nil } -func (h *BlockDeviceHandler) getBlockDeviceStatusRef(kind v1alpha2.BlockDeviceKind, name string, bootOrder int) v1alpha2.BlockDeviceStatusRef { +func (h *BlockDeviceHandler) getBlockDeviceStatusRef(kind v1alpha2.BlockDeviceKind, name string) v1alpha2.BlockDeviceStatusRef { return v1alpha2.BlockDeviceStatusRef{ - Kind: kind, - Name: name, - BootOrder: bootOrder, + Kind: kind, + Name: name, } } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go index aafd444809..e414fdd571 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go @@ -1,5 +1,5 @@ /* -Copyright 2024 Flant JSC +Copyright 2026 Flant JSC Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -19,7 +19,6 @@ package internal import ( "context" "fmt" - "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" virtv1 "kubevirt.io/api/core/v1" @@ -52,31 +51,23 @@ type HotplugHandler struct { func (h *HotplugHandler) Handle(ctx context.Context, s state.VirtualMachineState) (reconcile.Result, error) { log := logger.FromContext(ctx).With(logger.SlogHandler(nameHotplugHandler)) - if s.VirtualMachine().IsEmpty() { + if s.VirtualMachine().IsEmpty() || isDeletion(s.VirtualMachine().Current()) { return reconcile.Result{}, nil } current := s.VirtualMachine().Current() - if isDeletion(current) { - return reconcile.Result{}, nil - } - kvvmi, err := s.KVVMI(ctx) - if err != nil { + if err != nil || kvvmi == nil { return reconcile.Result{}, err } - if kvvmi == nil { - return reconcile.Result{}, nil - } if current.Status.Phase == v1alpha2.MachineMigrating { log.Info("VM is migrating, skip hotplug") - return reconcile.Result{RequeueAfter: 30 * time.Second}, nil + return reconcile.Result{}, nil } - bdReady, ok := conditions.GetCondition(vmcondition.TypeBlockDevicesReady, current.Status.Conditions) - if ok && bdReady.Status != metav1.ConditionTrue { + if bdReady, ok := conditions.GetCondition(vmcondition.TypeBlockDevicesReady, current.Status.Conditions); ok && bdReady.Status != metav1.ConditionTrue { return reconcile.Result{}, nil } @@ -85,65 +76,30 @@ func (h *HotplugHandler) Handle(ctx context.Context, s state.VirtualMachineState return reconcile.Result{}, err } - specDevices := make(map[nameKindKey]v1alpha2.BlockDeviceSpecRef) + specDevices := make(map[nameKindKey]struct{}) for _, bd := range current.Spec.BlockDeviceRefs { - specDevices[nameKindKey{kind: bd.Kind, name: bd.Name}] = bd + specDevices[nameKindKey{kind: bd.Kind, name: bd.Name}] = struct{}{} } - type kvvmVolume struct { - name string - hotpluggable bool - } - kvvmDevices := make(map[nameKindKey]kvvmVolume) - if kvvm.Spec.Template != nil { - for _, vol := range kvvm.Spec.Template.Spec.Volumes { - name, kind := kvbuilder.GetOriginalDiskName(vol.Name) - if kind == "" { - continue - } - hp := (vol.PersistentVolumeClaim != nil && vol.PersistentVolumeClaim.Hotpluggable) || - (vol.ContainerDisk != nil && vol.ContainerDisk.Hotpluggable) - kvvmDevices[nameKindKey{kind: kind, name: name}] = kvvmVolume{name: vol.Name, hotpluggable: hp} - } - } - - pendingRequests := make(map[string]struct{}) - for _, vr := range kvvm.Status.VolumeRequests { - if vr.AddVolumeOptions != nil { - pendingRequests[vr.AddVolumeOptions.Name] = struct{}{} - } - if vr.RemoveVolumeOptions != nil { - pendingRequests[vr.RemoveVolumeOptions.Name] = struct{}{} - } - } - - bdState := NewBlockDeviceState(s) - if err = bdState.Reload(ctx); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to reload block device state: %w", err) - } + kvvmDevices, pending := parseKVVMVolumes(kvvm) var errs []error + // 1. Hotplugging for key := range specDevices { - if _, attached := kvvmDevices[key]; attached { + volName := generateVolumeName(key) + if _, onKVVM := kvvmDevices[key]; onKVVM { continue } - - var volumeName string - switch key.kind { - case v1alpha2.DiskDevice: - volumeName = kvbuilder.GenerateVDDiskName(key.name) - case v1alpha2.ImageDevice: - volumeName = kvbuilder.GenerateVIDiskName(key.name) - case v1alpha2.ClusterImageDevice: - volumeName = kvbuilder.GenerateCVIDiskName(key.name) + if _, ok := pending[volName]; ok { + continue } - if _, pending := pendingRequests[volumeName]; pending { + ad, adErr := h.buildAttachmentDisk(ctx, key, s) + if adErr != nil { + errs = append(errs, fmt.Errorf("build attachment disk %s/%s: %w", key.kind, key.name, adErr)) continue } - - ad := h.buildAttachmentDisk(key, bdState) if ad == nil { log.Info("Block device not ready for hotplug", "kind", key.kind, "name", key.name) continue @@ -154,16 +110,12 @@ func (h *HotplugHandler) Handle(ctx context.Context, s state.VirtualMachineState } } + // 2. Unplugging for key, vol := range kvvmDevices { - if _, inSpec := specDevices[key]; inSpec { + if _, wanted := specDevices[key]; wanted || !vol.hotpluggable { continue } - - if !vol.hotpluggable { - continue - } - - if _, pending := pendingRequests[vol.name]; pending { + if _, ok := pending[vol.name]; ok { continue } @@ -183,26 +135,72 @@ func (h *HotplugHandler) Name() string { return nameHotplugHandler } -func (h *HotplugHandler) buildAttachmentDisk(key nameKindKey, bdState BlockDevicesState) *service.AttachmentDisk { +type kvvmVolume struct { + name string + hotpluggable bool +} + +func parseKVVMVolumes(kvvm *virtv1.VirtualMachine) (map[nameKindKey]kvvmVolume, map[string]struct{}) { + devices := make(map[nameKindKey]kvvmVolume) + pending := make(map[string]struct{}) + + if kvvm.Spec.Template != nil { + for _, vol := range kvvm.Spec.Template.Spec.Volumes { + name, kind := kvbuilder.GetOriginalDiskName(vol.Name) + if kind == "" { + continue + } + hp := (vol.PersistentVolumeClaim != nil && vol.PersistentVolumeClaim.Hotpluggable) || + (vol.ContainerDisk != nil && vol.ContainerDisk.Hotpluggable) + devices[nameKindKey{kind: kind, name: name}] = kvvmVolume{name: vol.Name, hotpluggable: hp} + } + } + + for _, vr := range kvvm.Status.VolumeRequests { + if vr.AddVolumeOptions != nil { + pending[vr.AddVolumeOptions.Name] = struct{}{} + } + if vr.RemoveVolumeOptions != nil { + pending[vr.RemoveVolumeOptions.Name] = struct{}{} + } + } + + return devices, pending +} + +func generateVolumeName(key nameKindKey) string { + return kvbuilder.GenerateDiskName(key.kind, key.name) +} + +func (h *HotplugHandler) buildAttachmentDisk(ctx context.Context, key nameKindKey, s state.VirtualMachineState) (*service.AttachmentDisk, error) { switch key.kind { case v1alpha2.DiskDevice: - vd, ok := bdState.VDByName[key.name] - if !ok || vd == nil || vd.Status.Target.PersistentVolumeClaim == "" { - return nil + vd, err := s.VirtualDisk(ctx, key.name) + if err != nil { + return nil, err } - return service.NewAttachmentDiskFromVirtualDisk(vd) + if vd == nil || vd.Status.Target.PersistentVolumeClaim == "" { + return nil, nil + } + return service.NewAttachmentDiskFromVirtualDisk(vd), nil case v1alpha2.ImageDevice: - vi, ok := bdState.VIByName[key.name] - if !ok || vi == nil { - return nil + vi, err := s.VirtualImage(ctx, key.name) + if err != nil { + return nil, err + } + if vi == nil { + return nil, nil } - return service.NewAttachmentDiskFromVirtualImage(vi) + return service.NewAttachmentDiskFromVirtualImage(vi), nil case v1alpha2.ClusterImageDevice: - cvi, ok := bdState.CVIByName[key.name] - if !ok || cvi == nil { - return nil + cvi, err := s.ClusterVirtualImage(ctx, key.name) + if err != nil { + return nil, err + } + if cvi == nil { + return nil, nil } - return service.NewAttachmentDiskFromClusterVirtualImage(cvi) + return service.NewAttachmentDiskFromClusterVirtualImage(cvi), nil } - return nil + return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 2c15a71537..547f27bfe8 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -434,13 +434,7 @@ func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virt networkSpec := network.CreateNetworkSpec(current, vmmacs) - vmbdas, err := s.VirtualMachineBlockDeviceAttachments(ctx) - if err != nil { - return nil, fmt.Errorf("get vmbdas: %w", err) - } - - // Create kubevirt VirtualMachine resource from d8 VirtualMachine spec. - err = kvbuilder.ApplyVirtualMachineSpec(kvvmBuilder, current, bdState.VDByName, bdState.VIByName, bdState.CVIByName, vmbdas, class, ipAddress, networkSpec) + err = kvbuilder.ApplyVirtualMachineSpec(kvvmBuilder, current, bdState.VDByName, bdState.VIByName, bdState.CVIByName, class, ipAddress, networkSpec) if err != nil { return nil, err } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/block_device_refs_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/block_device_refs_validator.go index 1dc843fb86..547e2ca183 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/block_device_refs_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/block_device_refs_validator.go @@ -96,8 +96,8 @@ func (v *BlockDeviceSpecRefsValidator) validateBootOrder(vm *v1alpha2.VirtualMac if bdRef.BootOrder == nil { continue } - if *bdRef.BootOrder < 0 { - return fmt.Errorf("bootOrder must be >= 0, got %d for %s %q", *bdRef.BootOrder, bdRef.Kind, bdRef.Name) + if *bdRef.BootOrder < 1 { + return fmt.Errorf("bootOrder must be >= 1, got %d for %s %q", *bdRef.BootOrder, bdRef.Kind, bdRef.Name) } if prev, exists := seen[*bdRef.BootOrder]; exists { return fmt.Errorf("duplicate bootOrder %d: already used by %s, conflicts with %s %q", *bdRef.BootOrder, prev, bdRef.Kind, bdRef.Name) From ccbe3a407b26d32a09464e4e2e5da94e39bcb7e9 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Fri, 27 Feb 2026 20:14:14 +0300 Subject: [PATCH 03/28] wip Signed-off-by: Daniil Loktev --- api/core/v1alpha2/block_device.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/core/v1alpha2/block_device.go b/api/core/v1alpha2/block_device.go index 10dae40deb..f83e03473c 100644 --- a/api/core/v1alpha2/block_device.go +++ b/api/core/v1alpha2/block_device.go @@ -20,7 +20,7 @@ type BlockDeviceSpecRef struct { Kind BlockDeviceKind `json:"kind"` // The name of attached resource. Name string `json:"name"` - // Boot priority for the block device. 1-based: 1 = first to boot. Smaller value = higher priority. + // Boot priority for the block device. Smaller value = higher priority. // +optional // +kubebuilder:validation:Minimum=1 BootOrder *int `json:"bootOrder,omitempty"` From b46c4ab797203baf4802d596f90779b48d445021 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 2 Mar 2026 11:10:49 +0300 Subject: [PATCH 04/28] wip Signed-off-by: Daniil Loktev --- crds/doc-ru-virtualmachines.yaml | 2 +- crds/virtualmachines.yaml | 2 +- .../pkg/controller/kvbuilder/kvvm_utils.go | 4 ++++ .../pkg/controller/vm/internal/block_device_status.go | 11 +++++++++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/crds/doc-ru-virtualmachines.yaml b/crds/doc-ru-virtualmachines.yaml index 78a7d423c7..d17e2ca59f 100644 --- a/crds/doc-ru-virtualmachines.yaml +++ b/crds/doc-ru-virtualmachines.yaml @@ -369,7 +369,7 @@ spec: Имя ресурса заданного типа. bootOrder: description: | - Приоритет загрузки блочного устройства. Нумерация с 1: 1 = загружается первым. Меньшее значение означает более высокий приоритет. + Приоритет загрузки блочного устройства. Меньшее значение означает более высокий приоритет. Если не задано ни для одного устройства, порядок загрузки определяется позицией в списке (начиная с 1). Если задано хотя бы для одного устройства, явные значения определяют приоритет загрузки. bootloader: diff --git a/crds/virtualmachines.yaml b/crds/virtualmachines.yaml index a34e8e1e4e..468bf879c4 100644 --- a/crds/virtualmachines.yaml +++ b/crds/virtualmachines.yaml @@ -960,7 +960,7 @@ spec: type: integer minimum: 1 description: | - Boot priority for the block device. 1-based: 1 = first to boot. Smaller value means higher boot priority. + Boot priority for the block device. Smaller value means higher boot priority. If not set for any device, boot order follows the position in the list (starting from 1). If set for at least one device, explicit values determine boot priority. diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index 3248b60578..b397d694ba 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -144,12 +144,16 @@ func ApplyVirtualMachineSpec( kvvm.AddFinalizer(v1alpha2.FinalizerKVVMProtection) if ipAddress != "" { + // Set ip address cni request annotation. kvvm.SetKVVMIAnnotation(netmanager.AnnoIPAddressCNIRequest, ipAddress) } + // Set live migration annotation. kvvm.SetKVVMIAnnotation(virtv1.AllowPodBridgeNetworkLiveMigrationAnnotation, "true") + // Set label to skip the check for PodSecurityStandards to avoid irrelevant alerts related to a privileged virtual machine pod. kvvm.SetKVVMILabel(annotations.SkipPodSecurityStandardsCheckLabel, "true") + // Set annotation for request network configuration. return setNetworksAnnotation(kvvm, networkSpec) } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go index 3be7850d1c..9e74a62b10 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go @@ -33,6 +33,9 @@ type nameKindKey struct { name string } +// getBlockDeviceStatusRefs returns block device refs to populate .status.blockDeviceRefs of the virtual machine. +// If kvvm is present, this method will reflect all volumes with prefixes (vi,vd, or cvi) into the slice of `BlockDeviceStatusRef`. +// Block devices from the virtual machine specification will be added to the resulting slice if they have not been included in the previous step. func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s state.VirtualMachineState) ([]v1alpha2.BlockDeviceStatusRef, error) { kvvm, err := s.KVVM(ctx) if err != nil { @@ -48,6 +51,7 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta var refs []v1alpha2.BlockDeviceStatusRef + // 1. There is no kvvm yet: populate block device refs with the spec. if kvvm == nil { for _, specBlockDeviceRef := range specRefs { ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name) @@ -80,9 +84,11 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta attachedBlockDeviceRefs := make(map[nameKindKey]struct{}) + // 2. The kvvm already exists: populate block device refs with the kvvm volumes. for _, volume := range kvvm.Spec.Template.Spec.Volumes { bdName, kind := kvbuilder.GetOriginalDiskName(volume.Name) if kind == "" { + // Reflect only vi, vd, or cvi block devices in status. continue } @@ -110,6 +116,7 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta attachedBlockDeviceRefs[key] = struct{}{} } + // 3. The kvvm may be missing some block devices from the spec; they need to be added as well. for _, specBlockDeviceRef := range specRefs { key := nameKindKey{kind: specBlockDeviceRef.Kind, name: specBlockDeviceRef.Name} if _, ok := attachedBlockDeviceRefs[key]; ok { @@ -187,14 +194,18 @@ func (h *BlockDeviceHandler) getBlockDeviceTarget(volume virtv1.Volume, kvvmiVol func (h *BlockDeviceHandler) isHotplugged(volume virtv1.Volume, kvvmiVolumeStatusByName map[string]virtv1.VolumeStatus) bool { switch { + // 1. If kvvmi has volume status with hotplugVolume reference then it's 100% hot-plugged volume. case kvvmiVolumeStatusByName[volume.Name].HotplugVolume != nil: return true + // 2. If kvvm has volume with hot-pluggable pvc reference then it's 100% hot-plugged volume. case volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.Hotpluggable: return true + // 3. If kvvm has volume with hot-pluggable disk reference then it's 100% hot-plugged volume. case volume.ContainerDisk != nil && volume.ContainerDisk.Hotpluggable: return true } + // 4. Is not hot-plugged. return false } From a04c63edaeca1102ceb4bd95be7dc5ee7901d684 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 2 Mar 2026 15:11:54 +0300 Subject: [PATCH 05/28] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/vm/internal/block_device_status.go | 3 +++ .../pkg/controller/vm/internal/sync_kvvm.go | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go index 9e74a62b10..eab336e020 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go @@ -89,6 +89,7 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta bdName, kind := kvbuilder.GetOriginalDiskName(volume.Name) if kind == "" { // Reflect only vi, vd, or cvi block devices in status. + // This is neither of them, so skip. continue } @@ -197,9 +198,11 @@ func (h *BlockDeviceHandler) isHotplugged(volume virtv1.Volume, kvvmiVolumeStatu // 1. If kvvmi has volume status with hotplugVolume reference then it's 100% hot-plugged volume. case kvvmiVolumeStatusByName[volume.Name].HotplugVolume != nil: return true + // 2. If kvvm has volume with hot-pluggable pvc reference then it's 100% hot-plugged volume. case volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.Hotpluggable: return true + // 3. If kvvm has volume with hot-pluggable disk reference then it's 100% hot-plugged volume. case volume.ContainerDisk != nil && volume.ContainerDisk.Hotpluggable: return true diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 547f27bfe8..2b5123a9ee 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -434,6 +434,7 @@ func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virt networkSpec := network.CreateNetworkSpec(current, vmmacs) + // Create kubevirt VirtualMachine resource from d8 VirtualMachine spec. err = kvbuilder.ApplyVirtualMachineSpec(kvvmBuilder, current, bdState.VDByName, bdState.VIByName, bdState.CVIByName, class, ipAddress, networkSpec) if err != nil { return nil, err @@ -640,6 +641,7 @@ func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.Virt switch action { case vmchange.ActionRestart: + // Update KVVM spec according the current VM spec. if err = h.updateKVVM(ctx, s); err != nil { return fmt.Errorf("update virtual machine instance with new spec: %w", err) } @@ -691,8 +693,6 @@ func (h *SyncKvvmHandler) updateKVVMLastAppliedSpec( return nil } - log := logger.FromContext(ctx) - err := kvbuilder.SetLastAppliedSpec(kvvm, vm) if err != nil { return fmt.Errorf("set vm last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err) @@ -706,6 +706,7 @@ func (h *SyncKvvmHandler) updateKVVMLastAppliedSpec( return fmt.Errorf("unable to update KubeVirt VM '%s': %w", kvvm.GetName(), err) } + log := logger.FromContext(ctx) log.Info("Update last applied spec on KubeVirt VM done", "name", kvvm.GetName()) return nil From 331ba6a2861855351673c7f743c95854f62b67a1 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 2 Mar 2026 15:37:39 +0300 Subject: [PATCH 06/28] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/vm/internal/sync_kvvm.go | 45 +------------------ .../pkg/controller/vmchange/spec_changes.go | 13 ------ 2 files changed, 2 insertions(+), 56 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 2b5123a9ee..3ac6216e04 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -654,18 +654,8 @@ func (h *SyncKvvmHandler) applyVMChangesToKVVM(ctx context.Context, s state.Virt h.recorder.Event(current, corev1.EventTypeNormal, v1alpha2.ReasonVMChangesApplied, message) log.Debug(message, "vm.name", current.GetName(), "changes", changes) - if changes.IsBlockDeviceRefsOnly() && kvvmi != nil { - class, err := s.Class(ctx) - if err != nil { - return fmt.Errorf("failed to get vmclass: %w", err) - } - if err = h.patchKVVMLastAppliedSpec(ctx, current, kvvm, class); err != nil { - return fmt.Errorf("unable to patch last-applied-spec on KVVM: %w", err) - } - } else { - if err := h.updateKVVM(ctx, s); err != nil { - return fmt.Errorf("unable to update KVVM using new VM spec: %w", err) - } + if err := h.updateKVVM(ctx, s); err != nil { + return fmt.Errorf("unable to update KVVM using new VM spec: %w", err) } case vmchange.ActionNone: @@ -712,37 +702,6 @@ func (h *SyncKvvmHandler) updateKVVMLastAppliedSpec( return nil } -// patchKVVMLastAppliedSpec updates only the last-applied-spec annotations on KubeVirt VirtualMachine -// using a merge patch. Unlike updateKVVMLastAppliedSpec, this does not send the full KVVM object, -// so it won't accidentally overwrite spec changes made by KubeVirt (e.g., from AddVolume processing). -func (h *SyncKvvmHandler) patchKVVMLastAppliedSpec( - ctx context.Context, - vm *v1alpha2.VirtualMachine, - kvvm *virtv1.VirtualMachine, - class *v1alpha2.VirtualMachineClass, -) error { - if vm == nil || kvvm == nil { - return nil - } - - patch := client.MergeFrom(kvvm.DeepCopy()) - - if err := kvbuilder.SetLastAppliedSpec(kvvm, vm); err != nil { - return fmt.Errorf("set vm last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err) - } - if err := kvbuilder.SetLastAppliedClassSpec(kvvm, class); err != nil { - return fmt.Errorf("set vmclass last applied spec on KubeVirt VM '%s': %w", kvvm.GetName(), err) - } - - if err := h.client.Patch(ctx, kvvm, patch); err != nil { - return fmt.Errorf("unable to patch KubeVirt VM '%s': %w", kvvm.GetName(), err) - } - - log := logger.FromContext(ctx) - log.Info("Patch last applied spec on KubeVirt VM done", "name", kvvm.GetName()) - - return nil -} func (h *SyncKvvmHandler) isVMUnschedulable( vm *v1alpha2.VirtualMachine, diff --git a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go index 88bf5ad61e..857b1c1312 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go @@ -20,7 +20,6 @@ import ( "encoding/json" "fmt" "sort" - "strings" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "sigs.k8s.io/yaml" @@ -267,18 +266,6 @@ func (s *SpecChanges) IsDisruptive() bool { return s.ActionType() == ActionRestart } -func (s *SpecChanges) IsBlockDeviceRefsOnly() bool { - if s.IsEmpty() { - return false - } - for _, change := range s.changes { - if !strings.HasPrefix(change.Path, BlockDevicesPath) { - return false - } - } - return true -} - func (s *SpecChanges) ToJSON() string { // Sort by path. sort.SliceStable(s.changes, func(i, j int) bool { From eb51b47dd4384ee6a62cf38cd069364096ef47c5 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 2 Mar 2026 15:55:13 +0300 Subject: [PATCH 07/28] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/vm/internal/sync_kvvm.go | 1 - .../pkg/controller/vmchange/comparator_block_devices.go | 5 ++--- .../pkg/controller/vmchange/compare_test.go | 8 ++++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 3ac6216e04..530551a938 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -702,7 +702,6 @@ func (h *SyncKvvmHandler) updateKVVMLastAppliedSpec( return nil } - func (h *SyncKvvmHandler) isVMUnschedulable( vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine, diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go b/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go index 0cae958499..5a0d516295 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparator_block_devices.go @@ -35,7 +35,7 @@ func compareBlockDevices(current, desired *v1alpha2.VirtualMachineSpec) []FieldC BlockDevicesPath, NewValue(current.BlockDeviceRefs, len(current.BlockDeviceRefs) == 0, false), NewValue(desired.BlockDeviceRefs, len(desired.BlockDeviceRefs) == 0, false), - ActionRestart, + ActionApplyImmediate, ) if len(fullChanges) > 0 { @@ -81,13 +81,12 @@ func compareBlockDevices(current, desired *v1alpha2.VirtualMachineSpec) []FieldC switch { case isAdded && isRemoved: // Compact add+remove for the same index into one replace. - // A different device at the same index requires restart. changes = append(changes, FieldChange{ Operation: ChangeReplace, Path: itemPath, CurrentValue: current.BlockDeviceRefs[idx], DesiredValue: desired.BlockDeviceRefs[idx], - ActionRequired: ActionRestart, + ActionRequired: ActionApplyImmediate, }) case isAdded: changes = append(changes, FieldChange{ diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go index 74212d554b..f089142280 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go @@ -130,7 +130,7 @@ memory: ), }, { - "restart on blockDeviceRefs section add", + "apply immediate on blockDeviceRefs section add", ``, ` blockDeviceRefs: @@ -138,12 +138,12 @@ blockDeviceRefs: name: linux `, assertChanges( - actionRequired(ActionRestart), + actionRequired(ActionApplyImmediate), requirePathOperation("blockDeviceRefs", ChangeAdd), ), }, { - "restart on blockDeviceRefs section remove", + "apply immediate on blockDeviceRefs section remove", ` blockDeviceRefs: - kind: VirtualImage @@ -151,7 +151,7 @@ blockDeviceRefs: `, ``, assertChanges( - actionRequired(ActionRestart), + actionRequired(ActionApplyImmediate), requirePathOperation("blockDeviceRefs", ChangeRemove), ), }, From 6312d9f98fd850ce1f917b8692b4f16046919497 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 2 Mar 2026 20:40:00 +0300 Subject: [PATCH 08/28] fix linter errors Signed-off-by: Daniil Loktev --- .../pkg/controller/kvbuilder/kvvm_utils.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index b397d694ba..fae433c379 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -23,6 +23,11 @@ import ( "slices" "strings" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/ptr" + virtv1 "kubevirt.io/api/core/v1" + "github.com/deckhouse/virtualization-controller/pkg/common" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/imageformat" @@ -32,10 +37,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/netmanager" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/utils/ptr" - virtv1 "kubevirt.io/api/core/v1" ) const ( From bebae37565e2143b8fbcdf34ac2be8e36585c5ef Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Tue, 3 Mar 2026 11:31:03 +0300 Subject: [PATCH 09/28] move attachment service back Signed-off-by: Daniil Loktev --- .../service/attachment_service.go | 2 +- .../service/attachment_service_test.go | 0 .../vmbda/internal/block_device_ready.go | 7 +- .../vmbda/internal/block_device_ready_test.go | 15 +- .../controller/vmbda/internal/interfaces.go | 5 +- .../controller/vmbda/internal/life_cycle.go | 20 +- .../pkg/controller/vmbda/internal/mock.go | 17 +- .../vmbda/internal/service/interfaces.go | 23 - .../controller/vmbda/internal/service/mock.go | 682 ------------------ .../attachment_conflict_validator.go | 7 +- .../pkg/controller/vmbda/vmbda_controller.go | 4 +- .../pkg/controller/vmbda/vmbda_webhook.go | 4 +- 12 files changed, 43 insertions(+), 743 deletions(-) rename images/virtualization-artifact/pkg/controller/{vmbda/internal => }/service/attachment_service.go (99%) rename images/virtualization-artifact/pkg/controller/{vmbda/internal => }/service/attachment_service_test.go (100%) delete mode 100644 images/virtualization-artifact/pkg/controller/vmbda/internal/service/interfaces.go delete mode 100644 images/virtualization-artifact/pkg/controller/vmbda/internal/service/mock.go diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/service/attachment_service.go b/images/virtualization-artifact/pkg/controller/service/attachment_service.go similarity index 99% rename from images/virtualization-artifact/pkg/controller/vmbda/internal/service/attachment_service.go rename to images/virtualization-artifact/pkg/controller/service/attachment_service.go index 31440c3a3a..66925b0fb8 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/service/attachment_service.go +++ b/images/virtualization-artifact/pkg/controller/service/attachment_service.go @@ -124,7 +124,7 @@ func (s AttachmentService) CanHotPlug(ad *AttachmentDisk, vm *v1alpha2.VirtualMa } for _, vr := range kvvm.Status.VolumeRequests { - if vr.AddVolumeOptions.Name == name { + if vr.AddVolumeOptions != nil && vr.AddVolumeOptions.Name == name { return false, ErrHotPlugRequestAlreadySent } } diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/service/attachment_service_test.go b/images/virtualization-artifact/pkg/controller/service/attachment_service_test.go similarity index 100% rename from images/virtualization-artifact/pkg/controller/vmbda/internal/service/attachment_service_test.go rename to images/virtualization-artifact/pkg/controller/service/attachment_service_test.go diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready.go index d23b3b490d..287613d03a 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready.go @@ -26,10 +26,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/service" + "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" ) type BlockDeviceReadyHandler struct { @@ -109,7 +110,7 @@ func (h BlockDeviceReadyHandler) Handle(ctx context.Context, vmbda *v1alpha2.Vir Message("Waiting until VirtualImage has associated PersistentVolumeClaim name.") return reconcile.Result{}, nil } - ad := intsvc.NewAttachmentDiskFromVirtualImage(vi) + ad := service.NewAttachmentDiskFromVirtualImage(vi) pvc, err := h.attachment.GetPersistentVolumeClaim(ctx, ad) if err != nil { return reconcile.Result{}, err @@ -265,7 +266,7 @@ func (h BlockDeviceReadyHandler) ValidateVirtualDiskReady(ctx context.Context, v return nil } - ad := intsvc.NewAttachmentDiskFromVirtualDisk(vd) + ad := service.NewAttachmentDiskFromVirtualDisk(vd) pvc, err := h.attachment.GetPersistentVolumeClaim(ctx, ad) if err != nil { return err diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready_test.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready_test.go index 334bc017d9..6a9fba1fff 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready_test.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready_test.go @@ -27,7 +27,8 @@ import ( vmbdaBuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vmbda" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/service" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition" @@ -48,7 +49,7 @@ var _ = Describe("BlockDeviceReadyHandler ValidateVirtualDiskReady", func() { GetVirtualDiskFunc: func(_ context.Context, _, _ string) (*v1alpha2.VirtualDisk, error) { return nil, nil }, - GetPersistentVolumeClaimFunc: func(_ context.Context, _ *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { + GetPersistentVolumeClaimFunc: func(_ context.Context, _ *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { return nil, nil }, } @@ -88,7 +89,7 @@ var _ = Describe("BlockDeviceReadyHandler ValidateVirtualDiskReady", func() { attachmentServiceMock.GetVirtualDiskFunc = func(_ context.Context, _, _ string) (*v1alpha2.VirtualDisk, error) { return generateVD(v1alpha2.DiskReady, metav1.ConditionTrue), nil } - attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { + attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { return nil, errors.New("error") } err := NewBlockDeviceReadyHandler(&attachmentServiceMock).ValidateVirtualDiskReady(ctx, vmbda, cb) @@ -99,7 +100,7 @@ var _ = Describe("BlockDeviceReadyHandler ValidateVirtualDiskReady", func() { attachmentServiceMock.GetVirtualDiskFunc = func(_ context.Context, _, _ string) (*v1alpha2.VirtualDisk, error) { return generateVD(v1alpha2.DiskReady, metav1.ConditionTrue), nil } - attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { + attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { return nil, nil } err := NewBlockDeviceReadyHandler(&attachmentServiceMock).ValidateVirtualDiskReady(ctx, vmbda, cb) @@ -112,7 +113,7 @@ var _ = Describe("BlockDeviceReadyHandler ValidateVirtualDiskReady", func() { attachmentServiceMock.GetVirtualDiskFunc = func(_ context.Context, _, _ string) (*v1alpha2.VirtualDisk, error) { return generateVD(v1alpha2.DiskReady, metav1.ConditionFalse), nil } - attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { + attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { return nil, nil } err := NewBlockDeviceReadyHandler(&attachmentServiceMock).ValidateVirtualDiskReady(ctx, vmbda, cb) @@ -125,7 +126,7 @@ var _ = Describe("BlockDeviceReadyHandler ValidateVirtualDiskReady", func() { attachmentServiceMock.GetVirtualDiskFunc = func(_ context.Context, _, _ string) (*v1alpha2.VirtualDisk, error) { return generateVD(phase, metav1.ConditionTrue), nil } - attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { + attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { return &corev1.PersistentVolumeClaim{ Status: corev1.PersistentVolumeClaimStatus{ Phase: corev1.ClaimBound, @@ -156,7 +157,7 @@ var _ = Describe("BlockDeviceReadyHandler ValidateVirtualDiskReady", func() { attachmentServiceMock.GetVirtualDiskFunc = func(_ context.Context, _, _ string) (*v1alpha2.VirtualDisk, error) { return generateVD(vdPhase, metav1.ConditionTrue), nil } - attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { + attachmentServiceMock.GetPersistentVolumeClaimFunc = func(_ context.Context, _ *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { return &corev1.PersistentVolumeClaim{ Status: corev1.PersistentVolumeClaimStatus{ Phase: pvcPhase, diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go index 38222b31e8..155201ee13 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go @@ -22,8 +22,9 @@ import ( corev1 "k8s.io/api/core/v1" virtv1 "kubevirt.io/api/core/v1" - intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/service" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" ) //go:generate go tool moq -rm -out mock.go . AttachmentService @@ -35,5 +36,5 @@ type AttachmentService interface { GetVirtualDisk(ctx context.Context, name, namespace string) (*v1alpha2.VirtualDisk, error) GetVirtualImage(ctx context.Context, name, namespace string) (*v1alpha2.VirtualImage, error) GetClusterVirtualImage(ctx context.Context, name string) (*v1alpha2.ClusterVirtualImage, error) - GetPersistentVolumeClaim(ctx context.Context, ad *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) + GetPersistentVolumeClaim(ctx context.Context, ad *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) } diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go index d12c94f99d..f04815ab74 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go @@ -28,17 +28,17 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" - intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/service" + "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition" ) type LifeCycleHandler struct { - attacher *intsvc.AttachmentService + attacher *service.AttachmentService } -func NewLifeCycleHandler(attacher *intsvc.AttachmentService) *LifeCycleHandler { +func NewLifeCycleHandler(attacher *service.AttachmentService) *LifeCycleHandler { return &LifeCycleHandler{ attacher: attacher, } @@ -56,7 +56,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac cb.Status(metav1.ConditionUnknown).Reason(conditions.ReasonUnknown) } - var ad *intsvc.AttachmentDisk + var ad *service.AttachmentDisk switch vmbda.Spec.BlockDeviceRef.Kind { case v1alpha2.VMBDAObjectRefKindVirtualDisk: vd, err := h.attacher.GetVirtualDisk(ctx, vmbda.Spec.BlockDeviceRef.Name, vmbda.Namespace) @@ -64,7 +64,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac return reconcile.Result{}, err } if vd != nil { - ad = intsvc.NewAttachmentDiskFromVirtualDisk(vd) + ad = service.NewAttachmentDiskFromVirtualDisk(vd) } case v1alpha2.VMBDAObjectRefKindVirtualImage: vi, err := h.attacher.GetVirtualImage(ctx, vmbda.Spec.BlockDeviceRef.Name, vmbda.Namespace) @@ -72,7 +72,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac return reconcile.Result{}, err } if vi != nil { - ad = intsvc.NewAttachmentDiskFromVirtualImage(vi) + ad = service.NewAttachmentDiskFromVirtualImage(vi) } case v1alpha2.VMBDAObjectRefKindClusterVirtualImage: cvi, err := h.attacher.GetClusterVirtualImage(ctx, vmbda.Spec.BlockDeviceRef.Name) @@ -80,7 +80,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac return reconcile.Result{}, err } if cvi != nil { - ad = intsvc.NewAttachmentDiskFromClusterVirtualImage(cvi) + ad = service.NewAttachmentDiskFromClusterVirtualImage(cvi) } } @@ -197,7 +197,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac isHotPlugged, err := h.attacher.IsHotPlugged(ad, vm, kvvmi) if err != nil { - if errors.Is(err, intsvc.ErrVolumeStatusNotReady) { + if errors.Is(err, service.ErrVolumeStatusNotReady) { vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseInProgress cb. Status(metav1.ConditionFalse). @@ -278,7 +278,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac Reason(vmbdacondition.AttachmentRequestSent). Message("Attachment request has sent: attachment is in progress.") return reconcile.Result{}, nil - case errors.Is(err, intsvc.ErrBlockDeviceIsSpecAttached): + case errors.Is(err, service.ErrBlockDeviceIsSpecAttached): log.Info("VirtualDisk is already attached to the virtual machine spec") vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseFailed @@ -287,7 +287,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac Reason(vmbdacondition.Conflict). Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, nil - case errors.Is(err, intsvc.ErrHotPlugRequestAlreadySent): + case errors.Is(err, service.ErrHotPlugRequestAlreadySent): log.Info("Attachment request sent: attachment is in progress.") vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseInProgress diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go index 522cf3ee5c..2c3a6925e4 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go @@ -5,11 +5,12 @@ package internal import ( "context" - intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/service" + "github.com/deckhouse/virtualization/api/core/v1alpha2" corev1 "k8s.io/api/core/v1" virtv1 "kubevirt.io/api/core/v1" "sync" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" ) // Ensure, that AttachmentServiceMock does implement AttachmentService. @@ -31,7 +32,7 @@ var _ AttachmentService = &AttachmentServiceMock{} // GetKVVMIFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine) (*virtv1.VirtualMachineInstance, error) { // panic("mock out the GetKVVMI method") // }, -// GetPersistentVolumeClaimFunc: func(ctx context.Context, ad *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { +// GetPersistentVolumeClaimFunc: func(ctx context.Context, ad *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { // panic("mock out the GetPersistentVolumeClaim method") // }, // GetVirtualDiskFunc: func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualDisk, error) { @@ -60,7 +61,7 @@ type AttachmentServiceMock struct { GetKVVMIFunc func(ctx context.Context, vm *v1alpha2.VirtualMachine) (*virtv1.VirtualMachineInstance, error) // GetPersistentVolumeClaimFunc mocks the GetPersistentVolumeClaim method. - GetPersistentVolumeClaimFunc func(ctx context.Context, ad *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) + GetPersistentVolumeClaimFunc func(ctx context.Context, ad *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) // GetVirtualDiskFunc mocks the GetVirtualDisk method. GetVirtualDiskFunc func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualDisk, error) @@ -99,7 +100,7 @@ type AttachmentServiceMock struct { // Ctx is the ctx argument value. Ctx context.Context // Ad is the ad argument value. - Ad *intsvc.AttachmentDisk + Ad *service.AttachmentDisk } // GetVirtualDisk holds details about calls to the GetVirtualDisk method. GetVirtualDisk []struct { @@ -247,13 +248,13 @@ func (mock *AttachmentServiceMock) GetKVVMICalls() []struct { } // GetPersistentVolumeClaim calls GetPersistentVolumeClaimFunc. -func (mock *AttachmentServiceMock) GetPersistentVolumeClaim(ctx context.Context, ad *intsvc.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { +func (mock *AttachmentServiceMock) GetPersistentVolumeClaim(ctx context.Context, ad *service.AttachmentDisk) (*corev1.PersistentVolumeClaim, error) { if mock.GetPersistentVolumeClaimFunc == nil { panic("AttachmentServiceMock.GetPersistentVolumeClaimFunc: method is nil but AttachmentService.GetPersistentVolumeClaim was just called") } callInfo := struct { Ctx context.Context - Ad *intsvc.AttachmentDisk + Ad *service.AttachmentDisk }{ Ctx: ctx, Ad: ad, @@ -270,11 +271,11 @@ func (mock *AttachmentServiceMock) GetPersistentVolumeClaim(ctx context.Context, // len(mockedAttachmentService.GetPersistentVolumeClaimCalls()) func (mock *AttachmentServiceMock) GetPersistentVolumeClaimCalls() []struct { Ctx context.Context - Ad *intsvc.AttachmentDisk + Ad *service.AttachmentDisk } { var calls []struct { Ctx context.Context - Ad *intsvc.AttachmentDisk + Ad *service.AttachmentDisk } mock.lockGetPersistentVolumeClaim.RLock() calls = mock.calls.GetPersistentVolumeClaim diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/service/interfaces.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/service/interfaces.go deleted file mode 100644 index 1550f019c8..0000000000 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/service/interfaces.go +++ /dev/null @@ -1,23 +0,0 @@ -/* -Copyright 2026 Flant JSC - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package service - -import "sigs.k8s.io/controller-runtime/pkg/client" - -//go:generate go tool moq -rm -out mock.go . Client - -type Client = client.Client diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/service/mock.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/service/mock.go deleted file mode 100644 index 32f3b129e8..0000000000 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/service/mock.go +++ /dev/null @@ -1,682 +0,0 @@ -// Code generated by moq; DO NOT EDIT. -// github.com/matryer/moq - -package service - -import ( - "context" - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/client" - "sync" -) - -// Ensure, that ClientMock does implement Client. -// If this is not the case, regenerate this file with moq. -var _ Client = &ClientMock{} - -// ClientMock is a mock implementation of Client. -// -// func TestSomethingThatUsesClient(t *testing.T) { -// -// // make and configure a mocked Client -// mockedClient := &ClientMock{ -// CreateFunc: func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { -// panic("mock out the Create method") -// }, -// DeleteFunc: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { -// panic("mock out the Delete method") -// }, -// DeleteAllOfFunc: func(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { -// panic("mock out the DeleteAllOf method") -// }, -// GetFunc: func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { -// panic("mock out the Get method") -// }, -// GroupVersionKindForFunc: func(obj runtime.Object) (schema.GroupVersionKind, error) { -// panic("mock out the GroupVersionKindFor method") -// }, -// IsObjectNamespacedFunc: func(obj runtime.Object) (bool, error) { -// panic("mock out the IsObjectNamespaced method") -// }, -// ListFunc: func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { -// panic("mock out the List method") -// }, -// PatchFunc: func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { -// panic("mock out the Patch method") -// }, -// RESTMapperFunc: func() meta.RESTMapper { -// panic("mock out the RESTMapper method") -// }, -// SchemeFunc: func() *runtime.Scheme { -// panic("mock out the Scheme method") -// }, -// StatusFunc: func() client.SubResourceWriter { -// panic("mock out the Status method") -// }, -// SubResourceFunc: func(subResource string) client.SubResourceClient { -// panic("mock out the SubResource method") -// }, -// UpdateFunc: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { -// panic("mock out the Update method") -// }, -// } -// -// // use mockedClient in code that requires Client -// // and then make assertions. -// -// } -type ClientMock struct { - // CreateFunc mocks the Create method. - CreateFunc func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error - - // DeleteFunc mocks the Delete method. - DeleteFunc func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error - - // DeleteAllOfFunc mocks the DeleteAllOf method. - DeleteAllOfFunc func(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error - - // GetFunc mocks the Get method. - GetFunc func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error - - // GroupVersionKindForFunc mocks the GroupVersionKindFor method. - GroupVersionKindForFunc func(obj runtime.Object) (schema.GroupVersionKind, error) - - // IsObjectNamespacedFunc mocks the IsObjectNamespaced method. - IsObjectNamespacedFunc func(obj runtime.Object) (bool, error) - - // ListFunc mocks the List method. - ListFunc func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error - - // PatchFunc mocks the Patch method. - PatchFunc func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error - - // RESTMapperFunc mocks the RESTMapper method. - RESTMapperFunc func() meta.RESTMapper - - // SchemeFunc mocks the Scheme method. - SchemeFunc func() *runtime.Scheme - - // StatusFunc mocks the Status method. - StatusFunc func() client.SubResourceWriter - - // SubResourceFunc mocks the SubResource method. - SubResourceFunc func(subResource string) client.SubResourceClient - - // UpdateFunc mocks the Update method. - UpdateFunc func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error - - // calls tracks calls to the methods. - calls struct { - // Create holds details about calls to the Create method. - Create []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Obj is the obj argument value. - Obj client.Object - // Opts is the opts argument value. - Opts []client.CreateOption - } - // Delete holds details about calls to the Delete method. - Delete []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Obj is the obj argument value. - Obj client.Object - // Opts is the opts argument value. - Opts []client.DeleteOption - } - // DeleteAllOf holds details about calls to the DeleteAllOf method. - DeleteAllOf []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Obj is the obj argument value. - Obj client.Object - // Opts is the opts argument value. - Opts []client.DeleteAllOfOption - } - // Get holds details about calls to the Get method. - Get []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Key is the key argument value. - Key client.ObjectKey - // Obj is the obj argument value. - Obj client.Object - // Opts is the opts argument value. - Opts []client.GetOption - } - // GroupVersionKindFor holds details about calls to the GroupVersionKindFor method. - GroupVersionKindFor []struct { - // Obj is the obj argument value. - Obj runtime.Object - } - // IsObjectNamespaced holds details about calls to the IsObjectNamespaced method. - IsObjectNamespaced []struct { - // Obj is the obj argument value. - Obj runtime.Object - } - // List holds details about calls to the List method. - List []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // List is the list argument value. - List client.ObjectList - // Opts is the opts argument value. - Opts []client.ListOption - } - // Patch holds details about calls to the Patch method. - Patch []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Obj is the obj argument value. - Obj client.Object - // Patch is the patch argument value. - Patch client.Patch - // Opts is the opts argument value. - Opts []client.PatchOption - } - // RESTMapper holds details about calls to the RESTMapper method. - RESTMapper []struct { - } - // Scheme holds details about calls to the Scheme method. - Scheme []struct { - } - // Status holds details about calls to the Status method. - Status []struct { - } - // SubResource holds details about calls to the SubResource method. - SubResource []struct { - // SubResource is the subResource argument value. - SubResource string - } - // Update holds details about calls to the Update method. - Update []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Obj is the obj argument value. - Obj client.Object - // Opts is the opts argument value. - Opts []client.UpdateOption - } - } - lockCreate sync.RWMutex - lockDelete sync.RWMutex - lockDeleteAllOf sync.RWMutex - lockGet sync.RWMutex - lockGroupVersionKindFor sync.RWMutex - lockIsObjectNamespaced sync.RWMutex - lockList sync.RWMutex - lockPatch sync.RWMutex - lockRESTMapper sync.RWMutex - lockScheme sync.RWMutex - lockStatus sync.RWMutex - lockSubResource sync.RWMutex - lockUpdate sync.RWMutex -} - -// Create calls CreateFunc. -func (mock *ClientMock) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { - if mock.CreateFunc == nil { - panic("ClientMock.CreateFunc: method is nil but Client.Create was just called") - } - callInfo := struct { - Ctx context.Context - Obj client.Object - Opts []client.CreateOption - }{ - Ctx: ctx, - Obj: obj, - Opts: opts, - } - mock.lockCreate.Lock() - mock.calls.Create = append(mock.calls.Create, callInfo) - mock.lockCreate.Unlock() - return mock.CreateFunc(ctx, obj, opts...) -} - -// CreateCalls gets all the calls that were made to Create. -// Check the length with: -// -// len(mockedClient.CreateCalls()) -func (mock *ClientMock) CreateCalls() []struct { - Ctx context.Context - Obj client.Object - Opts []client.CreateOption -} { - var calls []struct { - Ctx context.Context - Obj client.Object - Opts []client.CreateOption - } - mock.lockCreate.RLock() - calls = mock.calls.Create - mock.lockCreate.RUnlock() - return calls -} - -// Delete calls DeleteFunc. -func (mock *ClientMock) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { - if mock.DeleteFunc == nil { - panic("ClientMock.DeleteFunc: method is nil but Client.Delete was just called") - } - callInfo := struct { - Ctx context.Context - Obj client.Object - Opts []client.DeleteOption - }{ - Ctx: ctx, - Obj: obj, - Opts: opts, - } - mock.lockDelete.Lock() - mock.calls.Delete = append(mock.calls.Delete, callInfo) - mock.lockDelete.Unlock() - return mock.DeleteFunc(ctx, obj, opts...) -} - -// DeleteCalls gets all the calls that were made to Delete. -// Check the length with: -// -// len(mockedClient.DeleteCalls()) -func (mock *ClientMock) DeleteCalls() []struct { - Ctx context.Context - Obj client.Object - Opts []client.DeleteOption -} { - var calls []struct { - Ctx context.Context - Obj client.Object - Opts []client.DeleteOption - } - mock.lockDelete.RLock() - calls = mock.calls.Delete - mock.lockDelete.RUnlock() - return calls -} - -// DeleteAllOf calls DeleteAllOfFunc. -func (mock *ClientMock) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { - if mock.DeleteAllOfFunc == nil { - panic("ClientMock.DeleteAllOfFunc: method is nil but Client.DeleteAllOf was just called") - } - callInfo := struct { - Ctx context.Context - Obj client.Object - Opts []client.DeleteAllOfOption - }{ - Ctx: ctx, - Obj: obj, - Opts: opts, - } - mock.lockDeleteAllOf.Lock() - mock.calls.DeleteAllOf = append(mock.calls.DeleteAllOf, callInfo) - mock.lockDeleteAllOf.Unlock() - return mock.DeleteAllOfFunc(ctx, obj, opts...) -} - -// DeleteAllOfCalls gets all the calls that were made to DeleteAllOf. -// Check the length with: -// -// len(mockedClient.DeleteAllOfCalls()) -func (mock *ClientMock) DeleteAllOfCalls() []struct { - Ctx context.Context - Obj client.Object - Opts []client.DeleteAllOfOption -} { - var calls []struct { - Ctx context.Context - Obj client.Object - Opts []client.DeleteAllOfOption - } - mock.lockDeleteAllOf.RLock() - calls = mock.calls.DeleteAllOf - mock.lockDeleteAllOf.RUnlock() - return calls -} - -// Get calls GetFunc. -func (mock *ClientMock) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { - if mock.GetFunc == nil { - panic("ClientMock.GetFunc: method is nil but Client.Get was just called") - } - callInfo := struct { - Ctx context.Context - Key client.ObjectKey - Obj client.Object - Opts []client.GetOption - }{ - Ctx: ctx, - Key: key, - Obj: obj, - Opts: opts, - } - mock.lockGet.Lock() - mock.calls.Get = append(mock.calls.Get, callInfo) - mock.lockGet.Unlock() - return mock.GetFunc(ctx, key, obj, opts...) -} - -// GetCalls gets all the calls that were made to Get. -// Check the length with: -// -// len(mockedClient.GetCalls()) -func (mock *ClientMock) GetCalls() []struct { - Ctx context.Context - Key client.ObjectKey - Obj client.Object - Opts []client.GetOption -} { - var calls []struct { - Ctx context.Context - Key client.ObjectKey - Obj client.Object - Opts []client.GetOption - } - mock.lockGet.RLock() - calls = mock.calls.Get - mock.lockGet.RUnlock() - return calls -} - -// GroupVersionKindFor calls GroupVersionKindForFunc. -func (mock *ClientMock) GroupVersionKindFor(obj runtime.Object) (schema.GroupVersionKind, error) { - if mock.GroupVersionKindForFunc == nil { - panic("ClientMock.GroupVersionKindForFunc: method is nil but Client.GroupVersionKindFor was just called") - } - callInfo := struct { - Obj runtime.Object - }{ - Obj: obj, - } - mock.lockGroupVersionKindFor.Lock() - mock.calls.GroupVersionKindFor = append(mock.calls.GroupVersionKindFor, callInfo) - mock.lockGroupVersionKindFor.Unlock() - return mock.GroupVersionKindForFunc(obj) -} - -// GroupVersionKindForCalls gets all the calls that were made to GroupVersionKindFor. -// Check the length with: -// -// len(mockedClient.GroupVersionKindForCalls()) -func (mock *ClientMock) GroupVersionKindForCalls() []struct { - Obj runtime.Object -} { - var calls []struct { - Obj runtime.Object - } - mock.lockGroupVersionKindFor.RLock() - calls = mock.calls.GroupVersionKindFor - mock.lockGroupVersionKindFor.RUnlock() - return calls -} - -// IsObjectNamespaced calls IsObjectNamespacedFunc. -func (mock *ClientMock) IsObjectNamespaced(obj runtime.Object) (bool, error) { - if mock.IsObjectNamespacedFunc == nil { - panic("ClientMock.IsObjectNamespacedFunc: method is nil but Client.IsObjectNamespaced was just called") - } - callInfo := struct { - Obj runtime.Object - }{ - Obj: obj, - } - mock.lockIsObjectNamespaced.Lock() - mock.calls.IsObjectNamespaced = append(mock.calls.IsObjectNamespaced, callInfo) - mock.lockIsObjectNamespaced.Unlock() - return mock.IsObjectNamespacedFunc(obj) -} - -// IsObjectNamespacedCalls gets all the calls that were made to IsObjectNamespaced. -// Check the length with: -// -// len(mockedClient.IsObjectNamespacedCalls()) -func (mock *ClientMock) IsObjectNamespacedCalls() []struct { - Obj runtime.Object -} { - var calls []struct { - Obj runtime.Object - } - mock.lockIsObjectNamespaced.RLock() - calls = mock.calls.IsObjectNamespaced - mock.lockIsObjectNamespaced.RUnlock() - return calls -} - -// List calls ListFunc. -func (mock *ClientMock) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - if mock.ListFunc == nil { - panic("ClientMock.ListFunc: method is nil but Client.List was just called") - } - callInfo := struct { - Ctx context.Context - List client.ObjectList - Opts []client.ListOption - }{ - Ctx: ctx, - List: list, - Opts: opts, - } - mock.lockList.Lock() - mock.calls.List = append(mock.calls.List, callInfo) - mock.lockList.Unlock() - return mock.ListFunc(ctx, list, opts...) -} - -// ListCalls gets all the calls that were made to List. -// Check the length with: -// -// len(mockedClient.ListCalls()) -func (mock *ClientMock) ListCalls() []struct { - Ctx context.Context - List client.ObjectList - Opts []client.ListOption -} { - var calls []struct { - Ctx context.Context - List client.ObjectList - Opts []client.ListOption - } - mock.lockList.RLock() - calls = mock.calls.List - mock.lockList.RUnlock() - return calls -} - -// Patch calls PatchFunc. -func (mock *ClientMock) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { - if mock.PatchFunc == nil { - panic("ClientMock.PatchFunc: method is nil but Client.Patch was just called") - } - callInfo := struct { - Ctx context.Context - Obj client.Object - Patch client.Patch - Opts []client.PatchOption - }{ - Ctx: ctx, - Obj: obj, - Patch: patch, - Opts: opts, - } - mock.lockPatch.Lock() - mock.calls.Patch = append(mock.calls.Patch, callInfo) - mock.lockPatch.Unlock() - return mock.PatchFunc(ctx, obj, patch, opts...) -} - -// PatchCalls gets all the calls that were made to Patch. -// Check the length with: -// -// len(mockedClient.PatchCalls()) -func (mock *ClientMock) PatchCalls() []struct { - Ctx context.Context - Obj client.Object - Patch client.Patch - Opts []client.PatchOption -} { - var calls []struct { - Ctx context.Context - Obj client.Object - Patch client.Patch - Opts []client.PatchOption - } - mock.lockPatch.RLock() - calls = mock.calls.Patch - mock.lockPatch.RUnlock() - return calls -} - -// RESTMapper calls RESTMapperFunc. -func (mock *ClientMock) RESTMapper() meta.RESTMapper { - if mock.RESTMapperFunc == nil { - panic("ClientMock.RESTMapperFunc: method is nil but Client.RESTMapper was just called") - } - callInfo := struct { - }{} - mock.lockRESTMapper.Lock() - mock.calls.RESTMapper = append(mock.calls.RESTMapper, callInfo) - mock.lockRESTMapper.Unlock() - return mock.RESTMapperFunc() -} - -// RESTMapperCalls gets all the calls that were made to RESTMapper. -// Check the length with: -// -// len(mockedClient.RESTMapperCalls()) -func (mock *ClientMock) RESTMapperCalls() []struct { -} { - var calls []struct { - } - mock.lockRESTMapper.RLock() - calls = mock.calls.RESTMapper - mock.lockRESTMapper.RUnlock() - return calls -} - -// Scheme calls SchemeFunc. -func (mock *ClientMock) Scheme() *runtime.Scheme { - if mock.SchemeFunc == nil { - panic("ClientMock.SchemeFunc: method is nil but Client.Scheme was just called") - } - callInfo := struct { - }{} - mock.lockScheme.Lock() - mock.calls.Scheme = append(mock.calls.Scheme, callInfo) - mock.lockScheme.Unlock() - return mock.SchemeFunc() -} - -// SchemeCalls gets all the calls that were made to Scheme. -// Check the length with: -// -// len(mockedClient.SchemeCalls()) -func (mock *ClientMock) SchemeCalls() []struct { -} { - var calls []struct { - } - mock.lockScheme.RLock() - calls = mock.calls.Scheme - mock.lockScheme.RUnlock() - return calls -} - -// Status calls StatusFunc. -func (mock *ClientMock) Status() client.SubResourceWriter { - if mock.StatusFunc == nil { - panic("ClientMock.StatusFunc: method is nil but Client.Status was just called") - } - callInfo := struct { - }{} - mock.lockStatus.Lock() - mock.calls.Status = append(mock.calls.Status, callInfo) - mock.lockStatus.Unlock() - return mock.StatusFunc() -} - -// StatusCalls gets all the calls that were made to Status. -// Check the length with: -// -// len(mockedClient.StatusCalls()) -func (mock *ClientMock) StatusCalls() []struct { -} { - var calls []struct { - } - mock.lockStatus.RLock() - calls = mock.calls.Status - mock.lockStatus.RUnlock() - return calls -} - -// SubResource calls SubResourceFunc. -func (mock *ClientMock) SubResource(subResource string) client.SubResourceClient { - if mock.SubResourceFunc == nil { - panic("ClientMock.SubResourceFunc: method is nil but Client.SubResource was just called") - } - callInfo := struct { - SubResource string - }{ - SubResource: subResource, - } - mock.lockSubResource.Lock() - mock.calls.SubResource = append(mock.calls.SubResource, callInfo) - mock.lockSubResource.Unlock() - return mock.SubResourceFunc(subResource) -} - -// SubResourceCalls gets all the calls that were made to SubResource. -// Check the length with: -// -// len(mockedClient.SubResourceCalls()) -func (mock *ClientMock) SubResourceCalls() []struct { - SubResource string -} { - var calls []struct { - SubResource string - } - mock.lockSubResource.RLock() - calls = mock.calls.SubResource - mock.lockSubResource.RUnlock() - return calls -} - -// Update calls UpdateFunc. -func (mock *ClientMock) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - if mock.UpdateFunc == nil { - panic("ClientMock.UpdateFunc: method is nil but Client.Update was just called") - } - callInfo := struct { - Ctx context.Context - Obj client.Object - Opts []client.UpdateOption - }{ - Ctx: ctx, - Obj: obj, - Opts: opts, - } - mock.lockUpdate.Lock() - mock.calls.Update = append(mock.calls.Update, callInfo) - mock.lockUpdate.Unlock() - return mock.UpdateFunc(ctx, obj, opts...) -} - -// UpdateCalls gets all the calls that were made to Update. -// Check the length with: -// -// len(mockedClient.UpdateCalls()) -func (mock *ClientMock) UpdateCalls() []struct { - Ctx context.Context - Obj client.Object - Opts []client.UpdateOption -} { - var calls []struct { - Ctx context.Context - Obj client.Object - Opts []client.UpdateOption - } - mock.lockUpdate.RLock() - calls = mock.calls.Update - mock.lockUpdate.RUnlock() - return calls -} diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/validators/attachment_conflict_validator.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/validators/attachment_conflict_validator.go index eeef8ca152..d64a7afaff 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/validators/attachment_conflict_validator.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/validators/attachment_conflict_validator.go @@ -23,16 +23,17 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/deckhouse/deckhouse/pkg/log" - intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/service" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" ) type AttachmentConflictValidator struct { log *log.Logger - service *intsvc.AttachmentService + service *service.AttachmentService } -func NewAttachmentConflictValidator(service *intsvc.AttachmentService, log *log.Logger) *AttachmentConflictValidator { +func NewAttachmentConflictValidator(service *service.AttachmentService, log *log.Logger) *AttachmentConflictValidator { return &AttachmentConflictValidator{ log: log, service: service, diff --git a/images/virtualization-artifact/pkg/controller/vmbda/vmbda_controller.go b/images/virtualization-artifact/pkg/controller/vmbda/vmbda_controller.go index 7dc7d00ed7..aaf6d58d9a 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/vmbda_controller.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/vmbda_controller.go @@ -29,7 +29,7 @@ import ( "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal" - intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/service" + "github.com/deckhouse/virtualization-controller/pkg/logger" vmbdametrics "github.com/deckhouse/virtualization-controller/pkg/monitoring/metrics/vmbda" "github.com/deckhouse/virtualization/api/client/kubeclient" @@ -45,7 +45,7 @@ func NewController( lg *log.Logger, ns string, ) (controller.Controller, error) { - attacher := intsvc.NewAttachmentService(mgr.GetClient(), virtClient, ns) + attacher := service.NewAttachmentService(mgr.GetClient(), virtClient, ns) blockDeviceService := service.NewBlockDeviceService(mgr.GetClient()) reconciler := NewReconciler( diff --git a/images/virtualization-artifact/pkg/controller/vmbda/vmbda_webhook.go b/images/virtualization-artifact/pkg/controller/vmbda/vmbda_webhook.go index 3dab83b997..9c96360bd9 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/vmbda_webhook.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/vmbda_webhook.go @@ -25,7 +25,7 @@ import ( "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/virtualization-controller/pkg/controller/service" - intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/service" + "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/validators" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -40,7 +40,7 @@ type Validator struct { log *log.Logger } -func NewValidator(attachmentService *intsvc.AttachmentService, service *service.BlockDeviceService, log *log.Logger) *Validator { +func NewValidator(attachmentService *service.AttachmentService, service *service.BlockDeviceService, log *log.Logger) *Validator { return &Validator{ log: log.With("webhook", "validation"), validators: []VirtualMachineBlockDeviceAttachmentValidator{ From 2b40420648fd4fb46307d17f8e19743f4abff2ef Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Tue, 3 Mar 2026 11:42:20 +0300 Subject: [PATCH 10/28] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/vmbda/internal/mock.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go index 2c3a6925e4..34b4f669ef 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go @@ -5,12 +5,11 @@ package internal import ( "context" - + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization/api/core/v1alpha2" corev1 "k8s.io/api/core/v1" virtv1 "kubevirt.io/api/core/v1" "sync" - "github.com/deckhouse/virtualization-controller/pkg/controller/service" ) // Ensure, that AttachmentServiceMock does implement AttachmentService. From 1a9ea6fe0999a2fe4f84045195cfe6e3587cc227 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Tue, 3 Mar 2026 11:55:59 +0300 Subject: [PATCH 11/28] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/vmbda/internal/block_device_ready.go | 3 +-- .../pkg/controller/vmbda/internal/block_device_ready_test.go | 1 - .../pkg/controller/vmbda/internal/interfaces.go | 3 +-- .../pkg/controller/vmbda/internal/life_cycle.go | 1 - .../vmbda/internal/validators/attachment_conflict_validator.go | 3 +-- .../pkg/controller/vmbda/vmbda_controller.go | 1 - .../pkg/controller/vmbda/vmbda_webhook.go | 1 - 7 files changed, 3 insertions(+), 10 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready.go index 287613d03a..97e4c247f4 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready.go @@ -26,11 +26,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition" - "github.com/deckhouse/virtualization-controller/pkg/controller/service" ) type BlockDeviceReadyHandler struct { diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready_test.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready_test.go index 6a9fba1fff..79d34f6ede 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready_test.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/block_device_ready_test.go @@ -28,7 +28,6 @@ import ( vmbdaBuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vmbda" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" - "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition" diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go index 155201ee13..5fcbf16099 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go @@ -22,9 +22,8 @@ import ( corev1 "k8s.io/api/core/v1" virtv1 "kubevirt.io/api/core/v1" - - "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization/api/core/v1alpha2" ) //go:generate go tool moq -rm -out mock.go . AttachmentService diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go index f04815ab74..52b3c92f7b 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go @@ -28,7 +28,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" - "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition" diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/validators/attachment_conflict_validator.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/validators/attachment_conflict_validator.go index d64a7afaff..82d5147299 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/validators/attachment_conflict_validator.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/validators/attachment_conflict_validator.go @@ -23,9 +23,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/deckhouse/deckhouse/pkg/log" - - "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization/api/core/v1alpha2" ) type AttachmentConflictValidator struct { diff --git a/images/virtualization-artifact/pkg/controller/vmbda/vmbda_controller.go b/images/virtualization-artifact/pkg/controller/vmbda/vmbda_controller.go index aaf6d58d9a..99cff3e675 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/vmbda_controller.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/vmbda_controller.go @@ -29,7 +29,6 @@ import ( "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal" - "github.com/deckhouse/virtualization-controller/pkg/logger" vmbdametrics "github.com/deckhouse/virtualization-controller/pkg/monitoring/metrics/vmbda" "github.com/deckhouse/virtualization/api/client/kubeclient" diff --git a/images/virtualization-artifact/pkg/controller/vmbda/vmbda_webhook.go b/images/virtualization-artifact/pkg/controller/vmbda/vmbda_webhook.go index 9c96360bd9..b355b03710 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/vmbda_webhook.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/vmbda_webhook.go @@ -25,7 +25,6 @@ import ( "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/virtualization-controller/pkg/controller/service" - "github.com/deckhouse/virtualization-controller/pkg/controller/vmbda/internal/validators" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) From 9f0b01573e7ec9ee972f49393c91705888cdad69 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Tue, 3 Mar 2026 20:00:13 +0300 Subject: [PATCH 12/28] wip Signed-off-by: Daniil Loktev --- .../controller/vm/internal/hotplug_handler.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go index e414fdd571..c97f2ebf78 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go @@ -81,6 +81,15 @@ func (h *HotplugHandler) Handle(ctx context.Context, s state.VirtualMachineState specDevices[nameKindKey{kind: bd.Kind, name: bd.Name}] = struct{}{} } + vmbdaMap, err := s.VirtualMachineBlockDeviceAttachments(ctx) + if err != nil { + return reconcile.Result{}, err + } + vmbdaDevices := make(map[nameKindKey]struct{}) + for ref := range vmbdaMap { + vmbdaDevices[nameKindKey{kind: v1alpha2.BlockDeviceKind(ref.Kind), name: ref.Name}] = struct{}{} + } + kvvmDevices, pending := parseKVVMVolumes(kvvm) var errs []error @@ -110,9 +119,12 @@ func (h *HotplugHandler) Handle(ctx context.Context, s state.VirtualMachineState } } - // 2. Unplugging + // 2. Unplugging: only unplug volumes that are not in spec and not managed by VMBDA. for key, vol := range kvvmDevices { - if _, wanted := specDevices[key]; wanted || !vol.hotpluggable { + if _, wanted := specDevices[key]; wanted { + continue + } + if _, isVMBDA := vmbdaDevices[key]; isVMBDA { continue } if _, ok := pending[vol.name]; ok { From ababbb20e298374fc6a630c5cb13bb986c951b87 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Tue, 3 Mar 2026 20:24:07 +0300 Subject: [PATCH 13/28] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/service/restorer/secret_restorer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go index 98f0c3944f..5f99c311fc 100644 --- a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go +++ b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go @@ -147,7 +147,7 @@ func (r SecretRestorer) setVirtualMachineBlockDeviceAttachments(ctx context.Cont var vmbdas []*v1alpha2.VirtualMachineBlockDeviceAttachment for _, bdr := range vm.Status.BlockDeviceRefs { - if !bdr.Hotplugged { + if !bdr.Hotplugged || bdr.VirtualMachineBlockDeviceAttachmentName == "" { continue } From fab1cb00d326a53c885a49e95553416d7baea93d Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Wed, 4 Mar 2026 10:48:29 +0300 Subject: [PATCH 14/28] add e2e Signed-off-by: Daniil Loktev --- .../vm/internal/block_device_status.go | 1 - test/e2e/vm/block_device_hotplug.go | 160 ++++++++++++++++++ 2 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 test/e2e/vm/block_device_hotplug.go diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go index 6e98a35d9c..e61e428b1e 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_status.go @@ -227,7 +227,6 @@ func (h *BlockDeviceHandler) getBlockDeviceAttachmentName(ctx context.Context, k switch len(vmbdas) { case 0: - log.Error("No one vmbda was found for hot-plugged block device") return "", nil case 1: // OK. diff --git a/test/e2e/vm/block_device_hotplug.go b/test/e2e/vm/block_device_hotplug.go new file mode 100644 index 0000000000..ce2513ff4f --- /dev/null +++ b/test/e2e/vm/block_device_hotplug.go @@ -0,0 +1,160 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vm + +import ( + "context" + "fmt" + "strconv" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/utils/ptr" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + + vdbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vd" + vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/test/e2e/internal/framework" + "github.com/deckhouse/virtualization/test/e2e/internal/object" + "github.com/deckhouse/virtualization/test/e2e/internal/util" +) + +var _ = Describe("VirtualMachineBlockDeviceHotplug", Ordered, func() { + f := framework.NewFramework("vm-bd-hotplug") + + var ( + vm *v1alpha2.VirtualMachine + vdRoot *v1alpha2.VirtualDisk + vdBlank *v1alpha2.VirtualDisk + initialDiskCount string + ) + + BeforeAll(func() { + DeferCleanup(f.After) + f.Before() + + vdRoot = vdbuilder.New( + vdbuilder.WithName("vd-root"), + vdbuilder.WithNamespace(f.Namespace().Name), + vdbuilder.WithSize(ptr.To(resource.MustParse("350Mi"))), + vdbuilder.WithDataSourceHTTP(&v1alpha2.DataSourceHTTP{ + URL: object.ImageURLAlpineBIOS, + }), + ) + + vdBlank = vdbuilder.New( + vdbuilder.WithName("vd-blank"), + vdbuilder.WithNamespace(f.Namespace().Name), + vdbuilder.WithSize(ptr.To(resource.MustParse("100Mi"))), + ) + + vm = vmbuilder.New( + vmbuilder.WithName("vm"), + vmbuilder.WithNamespace(f.Namespace().Name), + vmbuilder.WithCPU(1, ptr.To("5%")), + vmbuilder.WithMemory(resource.MustParse("256Mi")), + vmbuilder.WithLiveMigrationPolicy(v1alpha2.AlwaysSafeMigrationPolicy), + vmbuilder.WithVirtualMachineClass(object.DefaultVMClass), + vmbuilder.WithProvisioningUserData(object.DefaultCloudInit), + vmbuilder.WithBlockDeviceRefs( + v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, + Name: vdRoot.Name, + }, + ), + vmbuilder.WithRestartApprovalMode(v1alpha2.Automatic), + ) + + err := f.CreateWithDeferredDeletion(context.Background(), vm, vdRoot, vdBlank) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for VM agent to be ready") + util.UntilVMAgentReady(crclient.ObjectKeyFromObject(vm), framework.LongTimeout) + + By("Recording initial disk count") + out, err := f.SSHCommand(vm.Name, vm.Namespace, "lsblk --nodeps --noheadings | wc -l") + Expect(err).NotTo(HaveOccurred()) + initialDiskCount = strings.TrimSpace(out) + }) + + It("should hotplug a disk without restart", func() { + By("Adding blank disk to spec.blockDeviceRefs") + err := f.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(vm), vm) + Expect(err).NotTo(HaveOccurred()) + + vm.Spec.BlockDeviceRefs = append(vm.Spec.BlockDeviceRefs, v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, + Name: vdBlank.Name, + }) + err = f.Clients.GenericClient().Update(context.Background(), vm) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying no restart is required") + Expect(util.IsRestartRequired(vm, 5*time.Second)).To(BeFalse()) + + By("Waiting for disk to be attached") + Eventually(func(g Gomega) { + g.Expect(f.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(vm), vm)).To(Succeed()) + g.Expect(util.IsVDAttached(vm, vdBlank)).To(BeTrue()) + }, framework.LongTimeout, 5*time.Second).Should(Succeed()) + + By("Verifying disk count increased inside the guest") + initCount, err := strconv.Atoi(initialDiskCount) + Expect(err).NotTo(HaveOccurred()) + expected := fmt.Sprintf("%d", initCount+1) + Eventually(func(g Gomega) { + out, sshErr := f.SSHCommand(vm.Name, vm.Namespace, "lsblk --nodeps --noheadings | wc -l") + g.Expect(sshErr).NotTo(HaveOccurred()) + g.Expect(strings.TrimSpace(out)).To(Equal(expected)) + }, framework.MiddleTimeout, 5*time.Second).Should(Succeed()) + }) + + It("should unplug a disk without restart", func() { + By("Removing blank disk from spec.blockDeviceRefs") + err := f.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(vm), vm) + Expect(err).NotTo(HaveOccurred()) + + vm.Spec.BlockDeviceRefs = []v1alpha2.BlockDeviceSpecRef{ + { + Kind: v1alpha2.DiskDevice, + Name: vdRoot.Name, + }, + } + err = f.Clients.GenericClient().Update(context.Background(), vm) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying no restart is required") + Expect(util.IsRestartRequired(vm, 5*time.Second)).To(BeFalse()) + + By("Waiting for disk to be detached") + Eventually(func(g Gomega) { + g.Expect(f.Clients.GenericClient().Get(context.Background(), crclient.ObjectKeyFromObject(vm), vm)).To(Succeed()) + g.Expect(util.IsVDAttached(vm, vdBlank)).To(BeFalse()) + }, framework.LongTimeout, 5*time.Second).Should(Succeed()) + + By("Verifying disk count decreased inside the guest") + Eventually(func(g Gomega) { + out, sshErr := f.SSHCommand(vm.Name, vm.Namespace, "lsblk --nodeps --noheadings | wc -l") + g.Expect(sshErr).NotTo(HaveOccurred()) + g.Expect(strings.TrimSpace(out)).To(Equal(initialDiskCount)) + }, framework.MiddleTimeout, 5*time.Second).Should(Succeed()) + }) +}) From 3f9bbe140566962bdbe3fc881f4ffa0f556e435e Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Wed, 4 Mar 2026 11:41:07 +0300 Subject: [PATCH 15/28] add unit tests Signed-off-by: Daniil Loktev --- .../controller/vm/internal/hotplug_handler.go | 9 +- .../vm/internal/hotplug_handler_test.go | 287 ++++++++++++++++++ .../pkg/controller/vm/internal/interfaces.go | 9 +- .../pkg/controller/vm/internal/mock.go | 142 +++++++++ 4 files changed, 439 insertions(+), 8 deletions(-) create mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler_test.go diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go index c97f2ebf78..068b1393b8 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go @@ -35,17 +35,12 @@ import ( const nameHotplugHandler = "HotplugHandler" -type hotplugService interface { - HotPlugDisk(ctx context.Context, ad *service.AttachmentDisk, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) error - UnplugDisk(ctx context.Context, kvvm *virtv1.VirtualMachine, diskName string) error -} - -func NewHotplugHandler(svc hotplugService) *HotplugHandler { +func NewHotplugHandler(svc HotplugService) *HotplugHandler { return &HotplugHandler{svc: svc} } type HotplugHandler struct { - svc hotplugService + svc HotplugService } func (h *HotplugHandler) Handle(ctx context.Context, s state.VirtualMachineState) (reconcile.Result, error) { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler_test.go new file mode 100644 index 0000000000..aa13cbb25f --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler_test.go @@ -0,0 +1,287 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package internal + +import ( + "context" + "log/slog" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + virtv1 "kubevirt.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization-controller/pkg/logger" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" +) + +var _ = Describe("HotplugHandler", func() { + const ( + vmName = "test-vm" + vmNamespace = "default" + vdName = "test-vd" + vdPVCName = "pvc-test-vd" + ) + + var ( + ctx context.Context + mockSvc *HotplugServiceMock + handler *HotplugHandler + ) + + BeforeEach(func() { + ctx = logger.ToContext(context.Background(), slog.Default()) + mockSvc = &HotplugServiceMock{ + HotPlugDiskFunc: func(_ context.Context, _ *service.AttachmentDisk, _ *v1alpha2.VirtualMachine, _ *virtv1.VirtualMachine) error { + return nil + }, + UnplugDiskFunc: func(_ context.Context, _ *virtv1.VirtualMachine, _ string) error { + return nil + }, + } + handler = NewHotplugHandler(mockSvc) + }) + + newVM := func(phase v1alpha2.MachinePhase, bdRefs ...v1alpha2.BlockDeviceSpecRef) *v1alpha2.VirtualMachine { + return &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{Name: vmName, Namespace: vmNamespace}, + Spec: v1alpha2.VirtualMachineSpec{ + BlockDeviceRefs: bdRefs, + }, + Status: v1alpha2.VirtualMachineStatus{ + Phase: phase, + Conditions: []metav1.Condition{ + { + Type: vmcondition.TypeBlockDevicesReady.String(), + Status: metav1.ConditionTrue, + }, + }, + }, + } + } + + newKVVM := func(volumes []virtv1.Volume, volumeRequests ...virtv1.VirtualMachineVolumeRequest) *virtv1.VirtualMachine { + kvvm := newEmptyKVVM(vmName, vmNamespace) + kvvm.Spec.Template = &virtv1.VirtualMachineInstanceTemplateSpec{} + kvvm.Spec.Template.Spec.Volumes = volumes + kvvm.Status.VolumeRequests = volumeRequests + return kvvm + } + + newVD := func(name, pvcName string) *v1alpha2.VirtualDisk { + return &v1alpha2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: vmNamespace}, + Status: v1alpha2.VirtualDiskStatus{ + Target: v1alpha2.DiskTarget{PersistentVolumeClaim: pvcName}, + }, + } + } + + runHandle := func(vm *v1alpha2.VirtualMachine, objs ...client.Object) (reconcile.Result, error) { + _, _, vmState := setupEnvironment(vm, objs...) + return handler.Handle(ctx, vmState) + } + + It("should skip when VM is empty", func() { + vm := newVM(v1alpha2.MachineRunning) + vm.Name = "" + vm.Namespace = "" + _, _, vmState := setupEnvironment(&v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{Name: "empty", Namespace: vmNamespace}, + }) + result, err := handler.Handle(ctx, vmState) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.HotPlugDiskCalls()).To(BeEmpty()) + Expect(mockSvc.UnplugDiskCalls()).To(BeEmpty()) + }) + + It("should skip when KVVMI does not exist", func() { + vm := newVM(v1alpha2.MachineRunning, v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, Name: vdName, + }) + kvvm := newKVVM(nil) + result, err := runHandle(vm, kvvm) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.HotPlugDiskCalls()).To(BeEmpty()) + }) + + It("should skip when VM is migrating", func() { + vm := newVM(v1alpha2.MachineMigrating, v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, Name: vdName, + }) + kvvm := newKVVM(nil) + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + result, err := runHandle(vm, kvvm, kvvmi) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.HotPlugDiskCalls()).To(BeEmpty()) + }) + + It("should hotplug a disk that is in spec but not on KVVM", func() { + vm := newVM(v1alpha2.MachineRunning, v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, Name: vdName, + }) + kvvm := newKVVM(nil) + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + vd := newVD(vdName, vdPVCName) + + result, err := runHandle(vm, kvvm, kvvmi, vd) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.HotPlugDiskCalls()).To(HaveLen(1)) + Expect(mockSvc.HotPlugDiskCalls()[0].Ad.PVCName).To(Equal(vdPVCName)) + Expect(mockSvc.UnplugDiskCalls()).To(BeEmpty()) + }) + + It("should not hotplug a disk already on KVVM", func() { + vm := newVM(v1alpha2.MachineRunning, v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, Name: vdName, + }) + kvvm := newKVVM([]virtv1.Volume{ + { + Name: "vd-" + vdName, + VolumeSource: virtv1.VolumeSource{ + PersistentVolumeClaim: &virtv1.PersistentVolumeClaimVolumeSource{ + PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ClaimName: vdPVCName}, + Hotpluggable: true, + }, + }, + }, + }) + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + + result, err := runHandle(vm, kvvm, kvvmi) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.HotPlugDiskCalls()).To(BeEmpty()) + Expect(mockSvc.UnplugDiskCalls()).To(BeEmpty()) + }) + + It("should not hotplug a disk with a pending AddVolume request", func() { + vm := newVM(v1alpha2.MachineRunning, v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, Name: vdName, + }) + kvvm := newKVVM(nil, virtv1.VirtualMachineVolumeRequest{ + AddVolumeOptions: &virtv1.AddVolumeOptions{Name: "vd-" + vdName}, + }) + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + vd := newVD(vdName, vdPVCName) + + result, err := runHandle(vm, kvvm, kvvmi, vd) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.HotPlugDiskCalls()).To(BeEmpty()) + }) + + It("should unplug a hotpluggable disk removed from spec", func() { + vm := newVM(v1alpha2.MachineRunning) + kvvm := newKVVM([]virtv1.Volume{ + { + Name: "vd-" + vdName, + VolumeSource: virtv1.VolumeSource{ + PersistentVolumeClaim: &virtv1.PersistentVolumeClaimVolumeSource{ + PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ClaimName: vdPVCName}, + Hotpluggable: true, + }, + }, + }, + }) + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + + result, err := runHandle(vm, kvvm, kvvmi) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.UnplugDiskCalls()).To(HaveLen(1)) + Expect(mockSvc.UnplugDiskCalls()[0].DiskName).To(Equal("vd-" + vdName)) + Expect(mockSvc.HotPlugDiskCalls()).To(BeEmpty()) + }) + + It("should not unplug a VMBDA-managed disk", func() { + vm := newVM(v1alpha2.MachineRunning) + kvvm := newKVVM([]virtv1.Volume{ + { + Name: "vd-" + vdName, + VolumeSource: virtv1.VolumeSource{ + PersistentVolumeClaim: &virtv1.PersistentVolumeClaimVolumeSource{ + PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ClaimName: vdPVCName}, + Hotpluggable: true, + }, + }, + }, + }) + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + vmbda := &v1alpha2.VirtualMachineBlockDeviceAttachment{ + ObjectMeta: metav1.ObjectMeta{Name: "vmbda-test", Namespace: vmNamespace}, + Spec: v1alpha2.VirtualMachineBlockDeviceAttachmentSpec{ + VirtualMachineName: vmName, + BlockDeviceRef: v1alpha2.VMBDAObjectRef{ + Kind: v1alpha2.VMBDAObjectRefKindVirtualDisk, + Name: vdName, + }, + }, + } + + result, err := runHandle(vm, kvvm, kvvmi, vmbda) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.UnplugDiskCalls()).To(BeEmpty()) + }) + + It("should not unplug a disk with a pending RemoveVolume request", func() { + vm := newVM(v1alpha2.MachineRunning) + kvvm := newKVVM([]virtv1.Volume{ + { + Name: "vd-" + vdName, + VolumeSource: virtv1.VolumeSource{ + PersistentVolumeClaim: &virtv1.PersistentVolumeClaimVolumeSource{ + PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ClaimName: vdPVCName}, + Hotpluggable: true, + }, + }, + }, + }, virtv1.VirtualMachineVolumeRequest{ + RemoveVolumeOptions: &virtv1.RemoveVolumeOptions{Name: "vd-" + vdName}, + }) + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + + result, err := runHandle(vm, kvvm, kvvmi) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.UnplugDiskCalls()).To(BeEmpty()) + }) + + It("should skip hotplug when VD has no PVC yet", func() { + vm := newVM(v1alpha2.MachineRunning, v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, Name: vdName, + }) + kvvm := newKVVM(nil) + kvvmi := newEmptyKVVMI(vmName, vmNamespace) + vd := newVD(vdName, "") + + result, err := runHandle(vm, kvvm, kvvmi, vd) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(mockSvc.HotPlugDiskCalls()).To(BeEmpty()) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go index b5cfbe8596..1a2976ba8a 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go @@ -19,15 +19,22 @@ package internal import ( "context" + virtv1 "kubevirt.io/api/core/v1" "k8s.io/client-go/tools/record" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) -//go:generate go tool moq -rm -out mock.go . EventRecorder BlockDeviceService +//go:generate go tool moq -rm -out mock.go . EventRecorder BlockDeviceService HotplugService type EventRecorder = record.EventRecorder type BlockDeviceService interface { CountBlockDevicesAttachedToVM(ctx context.Context, vm *v1alpha2.VirtualMachine) (int, error) } + +type HotplugService interface { + HotPlugDisk(ctx context.Context, ad *service.AttachmentDisk, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) error + UnplugDisk(ctx context.Context, kvvm *virtv1.VirtualMachine, diskName string) error +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/mock.go b/images/virtualization-artifact/pkg/controller/vm/internal/mock.go index 47bc1a209e..31bc2b1569 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/mock.go @@ -5,8 +5,10 @@ package internal import ( "context" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization/api/core/v1alpha2" "k8s.io/apimachinery/pkg/runtime" + virtv1 "kubevirt.io/api/core/v1" "sync" ) @@ -307,3 +309,143 @@ func (mock *BlockDeviceServiceMock) CountBlockDevicesAttachedToVMCalls() []struc mock.lockCountBlockDevicesAttachedToVM.RUnlock() return calls } + +// Ensure, that HotplugServiceMock does implement HotplugService. +// If this is not the case, regenerate this file with moq. +var _ HotplugService = &HotplugServiceMock{} + +// HotplugServiceMock is a mock implementation of HotplugService. +// +// func TestSomethingThatUsesHotplugService(t *testing.T) { +// +// // make and configure a mocked HotplugService +// mockedHotplugService := &HotplugServiceMock{ +// HotPlugDiskFunc: func(ctx context.Context, ad *service.AttachmentDisk, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) error { +// panic("mock out the HotPlugDisk method") +// }, +// UnplugDiskFunc: func(ctx context.Context, kvvm *virtv1.VirtualMachine, diskName string) error { +// panic("mock out the UnplugDisk method") +// }, +// } +// +// // use mockedHotplugService in code that requires HotplugService +// // and then make assertions. +// +// } +type HotplugServiceMock struct { + // HotPlugDiskFunc mocks the HotPlugDisk method. + HotPlugDiskFunc func(ctx context.Context, ad *service.AttachmentDisk, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) error + + // UnplugDiskFunc mocks the UnplugDisk method. + UnplugDiskFunc func(ctx context.Context, kvvm *virtv1.VirtualMachine, diskName string) error + + // calls tracks calls to the methods. + calls struct { + // HotPlugDisk holds details about calls to the HotPlugDisk method. + HotPlugDisk []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Ad is the ad argument value. + Ad *service.AttachmentDisk + // VM is the vm argument value. + VM *v1alpha2.VirtualMachine + // Kvvm is the kvvm argument value. + Kvvm *virtv1.VirtualMachine + } + // UnplugDisk holds details about calls to the UnplugDisk method. + UnplugDisk []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Kvvm is the kvvm argument value. + Kvvm *virtv1.VirtualMachine + // DiskName is the diskName argument value. + DiskName string + } + } + lockHotPlugDisk sync.RWMutex + lockUnplugDisk sync.RWMutex +} + +// HotPlugDisk calls HotPlugDiskFunc. +func (mock *HotplugServiceMock) HotPlugDisk(ctx context.Context, ad *service.AttachmentDisk, vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine) error { + if mock.HotPlugDiskFunc == nil { + panic("HotplugServiceMock.HotPlugDiskFunc: method is nil but HotplugService.HotPlugDisk was just called") + } + callInfo := struct { + Ctx context.Context + Ad *service.AttachmentDisk + VM *v1alpha2.VirtualMachine + Kvvm *virtv1.VirtualMachine + }{ + Ctx: ctx, + Ad: ad, + VM: vm, + Kvvm: kvvm, + } + mock.lockHotPlugDisk.Lock() + mock.calls.HotPlugDisk = append(mock.calls.HotPlugDisk, callInfo) + mock.lockHotPlugDisk.Unlock() + return mock.HotPlugDiskFunc(ctx, ad, vm, kvvm) +} + +// HotPlugDiskCalls gets all the calls that were made to HotPlugDisk. +// Check the length with: +// +// len(mockedHotplugService.HotPlugDiskCalls()) +func (mock *HotplugServiceMock) HotPlugDiskCalls() []struct { + Ctx context.Context + Ad *service.AttachmentDisk + VM *v1alpha2.VirtualMachine + Kvvm *virtv1.VirtualMachine +} { + var calls []struct { + Ctx context.Context + Ad *service.AttachmentDisk + VM *v1alpha2.VirtualMachine + Kvvm *virtv1.VirtualMachine + } + mock.lockHotPlugDisk.RLock() + calls = mock.calls.HotPlugDisk + mock.lockHotPlugDisk.RUnlock() + return calls +} + +// UnplugDisk calls UnplugDiskFunc. +func (mock *HotplugServiceMock) UnplugDisk(ctx context.Context, kvvm *virtv1.VirtualMachine, diskName string) error { + if mock.UnplugDiskFunc == nil { + panic("HotplugServiceMock.UnplugDiskFunc: method is nil but HotplugService.UnplugDisk was just called") + } + callInfo := struct { + Ctx context.Context + Kvvm *virtv1.VirtualMachine + DiskName string + }{ + Ctx: ctx, + Kvvm: kvvm, + DiskName: diskName, + } + mock.lockUnplugDisk.Lock() + mock.calls.UnplugDisk = append(mock.calls.UnplugDisk, callInfo) + mock.lockUnplugDisk.Unlock() + return mock.UnplugDiskFunc(ctx, kvvm, diskName) +} + +// UnplugDiskCalls gets all the calls that were made to UnplugDisk. +// Check the length with: +// +// len(mockedHotplugService.UnplugDiskCalls()) +func (mock *HotplugServiceMock) UnplugDiskCalls() []struct { + Ctx context.Context + Kvvm *virtv1.VirtualMachine + DiskName string +} { + var calls []struct { + Ctx context.Context + Kvvm *virtv1.VirtualMachine + DiskName string + } + mock.lockUnplugDisk.RLock() + calls = mock.calls.UnplugDisk + mock.lockUnplugDisk.RUnlock() + return calls +} From 9fa1597f4c0164cb3ee24c95cded3f30d5240c2e Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Wed, 4 Mar 2026 12:05:23 +0300 Subject: [PATCH 16/28] add doc Signed-off-by: Daniil Loktev --- docs/USER_GUIDE.md | 31 ++++++++++++++++++++++++------- docs/USER_GUIDE.ru.md | 29 +++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 8d7781e1d9..201bd99258 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -2107,11 +2107,26 @@ Block devices and their features are shown in the table below: #### Boot Block Devices -Boot block devices are defined in the virtual machine specification in the `.spec.blockDeviceRefs` block as a list. The order of the devices in this list determines the sequence in which they are loaded. Thus, if a disk or image is specified first, the loader will first try to boot from it. If it fails, the system will go to the next device in the list and try to boot from it. And so on until the first boot loader is detected. +Boot block devices are defined in the virtual machine specification in the `.spec.blockDeviceRefs` block as a list. -Changing the composition and order of devices in the `.spec.blockDeviceRefs` block is possible only with a reboot of the virtual machine. +By default, the boot order follows the position in the list. You can override this with the optional `bootOrder` field — a smaller value means a higher boot priority. If `bootOrder` is set for at least one device, only devices with an explicit `bootOrder` participate in the boot sequence. Values must be unique and >= 1. -VirtualMachine configuration fragment with statically connected disk and project image: +Adding or removing block devices in the `.spec.blockDeviceRefs` block is applied without a reboot. Changing the order of devices or `bootOrder` values requires a reboot of the virtual machine. + +VirtualMachine configuration fragment with block devices and explicit boot order: + +```yaml +spec: + blockDeviceRefs: + - kind: VirtualDisk + name: + bootOrder: 1 + - kind: VirtualImage + name: + bootOrder: 2 +``` + +To attach a disk to a running virtual machine, add it to the `.spec.blockDeviceRefs` list: ```yaml spec: @@ -2120,8 +2135,12 @@ spec: name: - kind: VirtualImage name: + - kind: VirtualDisk + name: ``` +To detach a disk, remove it from the list. The disk will be detached from the running virtual machine without a reboot. + How to work with bootable block devices in the web interface: - Go to the "Projects" tab and select the desired project. @@ -2132,11 +2151,9 @@ How to work with bootable block devices in the web interface: #### Additional Block Devices -Additional block devices can be connected and disconnected from a virtual machine that is in a running state without having to reboot it. - -The `VirtualMachineBlockDeviceAttachment` (`vmbda`) resource is used to connect additional block devices. +Alternatively, additional block devices can be connected and disconnected from a running virtual machine using the `VirtualMachineBlockDeviceAttachment` (`vmbda`) resource. -As an example, create the following share that connects an empty blank-disk disk to a linux-vm virtual machine: +As an example, create the following resource that connects an empty blank-disk disk to a linux-vm virtual machine: ```yaml d8 k apply -f - <= 1. -Фрагмент конфигурации VirtualMachine со статически подключенными диском и проектным образом: +Добавление или удаление блочных устройств в блоке `.spec.blockDeviceRefs` выполняется без перезагрузки. Изменение порядка устройств или значений `bootOrder` требует перезагрузки виртуальной машины. + +Фрагмент конфигурации VirtualMachine с блочными устройствами и явным порядком загрузки: + +```yaml +spec: + blockDeviceRefs: + - kind: VirtualDisk + name: + bootOrder: 1 + - kind: VirtualImage + name: + bootOrder: 2 +``` + +Для подключения диска к работающей виртуальной машине добавьте его в список `.spec.blockDeviceRefs`: ```yaml spec: @@ -2142,8 +2157,12 @@ spec: name: - kind: VirtualImage name: + - kind: VirtualDisk + name: ``` +Для отключения диска удалите его из списка. Диск будет отсоединён от работающей виртуальной машины без перезагрузки. + Как работать со загрузочными блочными устройствами в веб-интерфейсе: - Перейдите на вкладку «Проекты» и выберите нужный проект. @@ -2154,9 +2173,7 @@ spec: #### Дополнительные блочные устройства -Дополнительные блочные устройства можно подключать и отключать от виртуальной машины, находящейся в запущенном состоянии, без необходимости её перезагрузки. - -Для подключения дополнительных блочных устройств используется ресурс `VirtualMachineBlockDeviceAttachment` (`vmbda`). +Альтернативно, дополнительные блочные устройства можно подключать и отключать от работающей виртуальной машины с помощью ресурса `VirtualMachineBlockDeviceAttachment` (`vmbda`). Создайте ресурс, который подключит пустой диск blank-disk к виртуальной машине linux-vm: From e4d602ed4b4b73f2a27676450d7fbc9ce4ec7848 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Wed, 4 Mar 2026 20:19:45 +0300 Subject: [PATCH 17/28] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/vm/internal/interfaces.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go index 1a2976ba8a..9a4517ff0a 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go @@ -19,8 +19,8 @@ package internal import ( "context" - virtv1 "kubevirt.io/api/core/v1" "k8s.io/client-go/tools/record" + virtv1 "kubevirt.io/api/core/v1" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization/api/core/v1alpha2" From d015889a91c18ef6d704af2d00e3f3a14c307719 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Wed, 4 Mar 2026 22:06:32 +0300 Subject: [PATCH 18/28] fix vmbda error Signed-off-by: Daniil Loktev --- .../controller/vmbda/internal/life_cycle.go | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go index 52b3c92f7b..9d5dea04be 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go @@ -192,6 +192,19 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac } log = log.With("vmName", vm.Name, "attachmentDiskName", ad.Name) + + _, canErr := h.attacher.CanHotPlug(ad, vm, kvvm) + if errors.Is(canErr, service.ErrBlockDeviceIsSpecAttached) { + log.Info("VirtualDisk is already attached to the virtual machine spec") + + vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseFailed + cb. + Status(metav1.ConditionFalse). + Reason(vmbdacondition.Conflict). + Message(service.CapitalizeFirstLetter(canErr.Error())) + return reconcile.Result{}, nil + } + log.Info("Check if hot plug is completed and disk is attached") isHotPlugged, err := h.attacher.IsHotPlugged(ad, vm, kvvmi) @@ -219,10 +232,8 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac return reconcile.Result{}, nil } - _, err = h.attacher.CanHotPlug(ad, vm, kvvm) - switch { - case err == nil: + case canErr == nil: blockDeviceLimitCondition, _ := conditions.GetCondition(vmbdacondition.DiskAttachmentCapacityAvailableType, vmbda.Status.Conditions) if blockDeviceLimitCondition.Status != metav1.ConditionTrue { log.Info("Virtual machine block device capacity reached") @@ -277,16 +288,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac Reason(vmbdacondition.AttachmentRequestSent). Message("Attachment request has sent: attachment is in progress.") return reconcile.Result{}, nil - case errors.Is(err, service.ErrBlockDeviceIsSpecAttached): - log.Info("VirtualDisk is already attached to the virtual machine spec") - - vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseFailed - cb. - Status(metav1.ConditionFalse). - Reason(vmbdacondition.Conflict). - Message(service.CapitalizeFirstLetter(err.Error())) - return reconcile.Result{}, nil - case errors.Is(err, service.ErrHotPlugRequestAlreadySent): + case errors.Is(canErr, service.ErrHotPlugRequestAlreadySent): log.Info("Attachment request sent: attachment is in progress.") vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseInProgress @@ -296,6 +298,6 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac Message("Attachment request sent: attachment is in progress.") return reconcile.Result{}, nil default: - return reconcile.Result{}, err + return reconcile.Result{}, canErr } } From a8fc9cf2d7ea13dffff78e8f0a978c51069dbcc0 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Thu, 5 Mar 2026 10:44:00 +0300 Subject: [PATCH 19/28] add validation for conflicts Signed-off-by: Daniil Loktev --- .../validators/vmbda_conflict_validator.go | 89 +++++++++++++++++++ .../pkg/controller/vm/vm_webhook.go | 1 + 2 files changed, 90 insertions(+) create mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/validators/vmbda_conflict_validator.go diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/vmbda_conflict_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/vmbda_conflict_validator.go new file mode 100644 index 0000000000..2d32724a06 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/vmbda_conflict_validator.go @@ -0,0 +1,89 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validators + +import ( + "context" + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +type VMBDAConflictValidator struct { + client client.Client +} + +func NewVMBDAConflictValidator(client client.Client) *VMBDAConflictValidator { + return &VMBDAConflictValidator{client: client} +} + +func (v *VMBDAConflictValidator) ValidateCreate(ctx context.Context, vm *v1alpha2.VirtualMachine) (admission.Warnings, error) { + return nil, v.checkConflicts(ctx, vm, vm.Spec.BlockDeviceRefs) +} + +func (v *VMBDAConflictValidator) ValidateUpdate(ctx context.Context, oldVM, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) { + added := findAdded(oldVM.Spec.BlockDeviceRefs, newVM.Spec.BlockDeviceRefs) + return nil, v.checkConflicts(ctx, newVM, added) +} + +func (v *VMBDAConflictValidator) checkConflicts(ctx context.Context, vm *v1alpha2.VirtualMachine, refs []v1alpha2.BlockDeviceSpecRef) error { + if len(refs) == 0 { + return nil + } + + var vmbdaList v1alpha2.VirtualMachineBlockDeviceAttachmentList + if err := v.client.List(ctx, &vmbdaList, + client.InNamespace(vm.Namespace), + client.MatchingFields{indexer.IndexFieldVMBDAByVM: vm.Name}, + ); err != nil { + return err + } + + for _, vmbda := range vmbdaList.Items { + if vmbda.Status.Phase == v1alpha2.BlockDeviceAttachmentPhaseTerminating { + continue + } + ref := vmbda.Spec.BlockDeviceRef + for _, bd := range refs { + if string(bd.Kind) == string(ref.Kind) && bd.Name == ref.Name { + return fmt.Errorf( + "block device %s %q is already attached to the virtual machine via VirtualMachineBlockDeviceAttachment %q", + bd.Kind, bd.Name, vmbda.Name, + ) + } + } + } + return nil +} + +func findAdded(old, new []v1alpha2.BlockDeviceSpecRef) []v1alpha2.BlockDeviceSpecRef { + existing := make(map[blockDeviceKey]struct{}, len(old)) + for _, bd := range old { + existing[blockDeviceKey{Kind: bd.Kind, Name: bd.Name}] = struct{}{} + } + var added []v1alpha2.BlockDeviceSpecRef + for _, bd := range new { + if _, ok := existing[blockDeviceKey{Kind: bd.Kind, Name: bd.Name}]; !ok { + added = append(added, bd) + } + } + return added +} diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go b/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go index cb56986437..06e058313f 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go @@ -56,6 +56,7 @@ func NewValidator(client client.Client, service *service.BlockDeviceService, fea validators.NewNetworksValidator(featureGate), validators.NewFirstDiskValidator(client), validators.NewUSBDevicesValidator(client), + validators.NewVMBDAConflictValidator(client), }, log: log.With("webhook", "validation"), } From 1843d0e3e05ebe9deb508b03b66acb8086fe9713 Mon Sep 17 00:00:00 2001 From: Vladislav Panfilov Date: Tue, 10 Mar 2026 16:37:21 +0400 Subject: [PATCH 20/28] docs: review Signed-off-by: Vladislav Panfilov --- crds/doc-ru-virtualmachines.yaml | 7 +++---- crds/virtualmachines.yaml | 6 +++--- docs/USER_GUIDE.md | 10 +++++----- docs/USER_GUIDE.ru.md | 10 +++++----- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/crds/doc-ru-virtualmachines.yaml b/crds/doc-ru-virtualmachines.yaml index d17e2ca59f..bccbbc2d2d 100644 --- a/crds/doc-ru-virtualmachines.yaml +++ b/crds/doc-ru-virtualmachines.yaml @@ -369,10 +369,9 @@ spec: Имя ресурса заданного типа. bootOrder: description: | - Приоритет загрузки блочного устройства. Меньшее значение означает более высокий приоритет. - Если не задано ни для одного устройства, порядок загрузки определяется позицией в списке (начиная с 1). - Если задано хотя бы для одного устройства, явные значения определяют приоритет загрузки. - bootloader: + Порядок загрузки блочного устройства. Меньшее значение означает более высокий приоритет. + Если параметр не задан ни для одного устройства, порядок загрузки определяется позицией устройства в списке (начиная с 1). + Если параметр задан хотя бы для одного устройства, порядок загрузки определяется указанными значениями. description: | Загрузчик для ВМ: diff --git a/crds/virtualmachines.yaml b/crds/virtualmachines.yaml index 468bf879c4..fa97fbaa24 100644 --- a/crds/virtualmachines.yaml +++ b/crds/virtualmachines.yaml @@ -960,9 +960,9 @@ spec: type: integer minimum: 1 description: | - Boot priority for the block device. Smaller value means higher boot priority. - If not set for any device, boot order follows the position in the list (starting from 1). - If set for at least one device, explicit values determine boot priority. + Boot order of the block device. A smaller value means a higher priority. + If the parameter is not set for any device, the boot order follows the device position in the list (starting from 1). + If the parameter is set for at least one device, the boot order is determined by the specified values. liveMigrationPolicy: type: string diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 201bd99258..fc92a8384f 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -2109,11 +2109,11 @@ Block devices and their features are shown in the table below: Boot block devices are defined in the virtual machine specification in the `.spec.blockDeviceRefs` block as a list. -By default, the boot order follows the position in the list. You can override this with the optional `bootOrder` field — a smaller value means a higher boot priority. If `bootOrder` is set for at least one device, only devices with an explicit `bootOrder` participate in the boot sequence. Values must be unique and >= 1. +By default, the boot order follows the position in the list. You can override it using the optional `bootOrder` field — a smaller value means a higher boot priority. If `bootOrder` is set for at least one device, only devices with an explicit `bootOrder` are included in the boot order. Values must be unique and ≥ 1. -Adding or removing block devices in the `.spec.blockDeviceRefs` block is applied without a reboot. Changing the order of devices or `bootOrder` values requires a reboot of the virtual machine. +Adding or removing block devices in the `.spec.blockDeviceRefs` block is applied without a reboot. Changing the device order or `bootOrder` values requires a reboot of the virtual machine. -VirtualMachine configuration fragment with block devices and explicit boot order: +Virtual machine configuration fragment with block devices and explicit boot order: ```yaml spec: @@ -2151,9 +2151,9 @@ How to work with bootable block devices in the web interface: #### Additional Block Devices -Alternatively, additional block devices can be connected and disconnected from a running virtual machine using the `VirtualMachineBlockDeviceAttachment` (`vmbda`) resource. +Alternatively, additional block devices can be connected and disconnected from a running virtual machine using the [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) resource. -As an example, create the following resource that connects an empty blank-disk disk to a linux-vm virtual machine: +As an example, create the following resource that connects an empty `blank-disk` disk to a `linux-vm` virtual machine: ```yaml d8 k apply -f - <= 1. +По умолчанию порядок загрузки определяется позицией устройства в списке. Его можно переопределить с помощью необязательного поля `bootOrder` — меньшее значение означает более высокий приоритет загрузки. Если `bootOrder` задан хотя бы для одного устройства, в порядок загрузки включаются только устройства с явно указанным `bootOrder`. Значения должны быть уникальными и ≥ 1. -Добавление или удаление блочных устройств в блоке `.spec.blockDeviceRefs` выполняется без перезагрузки. Изменение порядка устройств или значений `bootOrder` требует перезагрузки виртуальной машины. +Добавление или удаление блочных устройств в блоке `.spec.blockDeviceRefs` выполняются без перезагрузки. Изменение порядка устройств или значений `bootOrder` требует перезагрузки виртуальной машины. -Фрагмент конфигурации VirtualMachine с блочными устройствами и явным порядком загрузки: +Фрагмент конфигурации виртуальной машины с блочными устройствами и явным порядком загрузки: ```yaml spec: @@ -2173,9 +2173,9 @@ spec: #### Дополнительные блочные устройства -Альтернативно, дополнительные блочные устройства можно подключать и отключать от работающей виртуальной машины с помощью ресурса `VirtualMachineBlockDeviceAttachment` (`vmbda`). +Альтернативно, дополнительные блочные устройства можно подключать и отключать от работающей виртуальной машины с помощью ресурса [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment). -Создайте ресурс, который подключит пустой диск blank-disk к виртуальной машине linux-vm: +Создайте ресурс, который подключит пустой диск `blank-disk` к виртуальной машине `linux-vm`: ```yaml d8 k apply -f - < Date: Wed, 11 Mar 2026 10:49:17 +0300 Subject: [PATCH 21/28] fix merge Signed-off-by: Daniil Loktev --- crds/doc-ru-virtualmachines.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/crds/doc-ru-virtualmachines.yaml b/crds/doc-ru-virtualmachines.yaml index d7a5763753..91c83d3729 100644 --- a/crds/doc-ru-virtualmachines.yaml +++ b/crds/doc-ru-virtualmachines.yaml @@ -374,6 +374,7 @@ spec: Порядок загрузки блочного устройства. Меньшее значение означает более высокий приоритет. Если параметр не задан ни для одного устройства, порядок загрузки определяется позицией устройства в списке (начиная с 1). Если параметр задан хотя бы для одного устройства, порядок загрузки определяется указанными значениями. + bootloader: description: | Загрузчик для ВМ: From 8e9a8a1238d7c2288798e9a2a0aab84e53f43c00 Mon Sep 17 00:00:00 2001 From: Vladislav Panfilov Date: Fri, 13 Mar 2026 14:58:16 +0400 Subject: [PATCH 22/28] docs: update user guide for block device attachment methods Signed-off-by: Vladislav Panfilov --- docs/USER_GUIDE.md | 55 +++++++++++++++++++++++-------------------- docs/USER_GUIDE.ru.md | 41 ++++++++++++++++++-------------- 2 files changed, 53 insertions(+), 43 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index fdc6c2750c..008189be99 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -2095,23 +2095,28 @@ d8 k get nodes -o custom-columns=NAME:.metadata.name,ZONE:.metadata.labels.topol ### Attaching block devices (disks and images) -Block devices can be divided into two types based on how they are connected: static and dynamic (hotplug). +You can attach disks and images to a virtual machine. They are described as block devices (BlockDevices). -Block devices and their features are shown in the table below: +Two attachment methods are available: -| Block device type | Comment | -| --------------------- | --------------------------------------------------------- | -| `VirtualImage` | connected in read-only mode, or as a cdrom for iso images | -| `ClusterVirtualImage` | connected in read-only mode, or as a cdrom for iso images | -| `VirtualDisk` | connects in read/write mode | +- Static attachment: Devices are listed in the VM specification at creation or start and form the initial configuration. +- Dynamic attachment (hotplug): Attaching and detaching devices to or from a running VM without a reboot. This can be done either by changing the `.spec.blockDeviceRefs` list or by using the [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) resource. -#### Boot Block Devices +Block device types and access modes: -Boot block devices are defined in the virtual machine specification in the `.spec.blockDeviceRefs` block as a list. +| Block device type | Comment | +| --------------------------------------------------------------------------------- | ------------------------------------------------------------- | +| [VirtualImage](/modules/virtualization/cr.html#virtualimage) | Connected in read-only mode, or as a CD-ROM for ISO images. | +| [ClusterVirtualImage](/modules/virtualization/cr.html#clustervirtualimage) | Connected in read-only mode, or as a CD-ROM for ISO images. | +| [VirtualDisk](/modules/virtualization/cr.html#virtualdisk) | Connected in read/write mode. | -By default, the boot order follows the position in the list. You can override it using the optional `bootOrder` field — a smaller value means a higher boot priority. If `bootOrder` is set for at least one device, only devices with an explicit `bootOrder` are included in the boot order. Values must be unique and ≥ 1. +#### Attaching via the VM specification -Adding or removing block devices in the `.spec.blockDeviceRefs` block is applied without a reboot. Changing the device order or `bootOrder` values requires a reboot of the virtual machine. +The list of block devices is defined in the `.spec.blockDeviceRefs` field of the [VirtualMachine](/modules/virtualization/cr.html#virtualmachine) resource. + +By default, boot order follows the order of devices in the list. You can set it explicitly with the optional `bootOrder` field (smaller value means higher priority). If `bootOrder` is set for at least one device, only devices with `bootOrder` set are included in the boot sequence. Allowed values: integers ≥ 1, unique within the list. + +Adding or removing entries in `.spec.blockDeviceRefs` is applied to a running VM without a reboot. Changing the order of devices in the list or their `bootOrder` values takes effect after a VM reboot. Virtual machine configuration fragment with block devices and explicit boot order: @@ -2149,13 +2154,13 @@ How to work with bootable block devices in the web interface: - On the "Configuration" tab, scroll down to the "Disks and Images" section. - You can add, extract, delete, resize, and reorder bootable block devices in the "Boot Disks" section. -#### Additional Block Devices +#### Attaching via VirtualMachineBlockDeviceAttachment (vmbda) -Alternatively, additional block devices can be connected and disconnected from a running virtual machine using the [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) resource. +The [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) resource provides hot-plug: attach and detach a block device to or from a running VM without changing its spec and without a reboot. Suited for automation and scenarios when the user does not have permission to edit the VM. -As an example, create the following resource that connects an empty `blank-disk` disk to a `linux-vm` virtual machine: +Create a resource that attaches the empty disk `blank-disk` to the virtual machine `linux-vm`: -```yaml +```shell d8 k apply -f - < Date: Fri, 13 Mar 2026 15:23:43 +0400 Subject: [PATCH 23/28] docs: fix Signed-off-by: Vladislav Panfilov --- docs/USER_GUIDE.md | 10 +++++----- docs/USER_GUIDE.ru.md | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 008189be99..f80f10cbb2 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -2104,11 +2104,11 @@ Two attachment methods are available: Block device types and access modes: -| Block device type | Comment | -| --------------------------------------------------------------------------------- | ------------------------------------------------------------- | -| [VirtualImage](/modules/virtualization/cr.html#virtualimage) | Connected in read-only mode, or as a CD-ROM for ISO images. | -| [ClusterVirtualImage](/modules/virtualization/cr.html#clustervirtualimage) | Connected in read-only mode, or as a CD-ROM for ISO images. | -| [VirtualDisk](/modules/virtualization/cr.html#virtualdisk) | Connected in read/write mode. | +| Block device type | Comment | +|----------------------------------------------------------------------------|-------------------------------------------------------------| +| [VirtualImage](/modules/virtualization/cr.html#virtualimage) | Connected in read-only mode, or as a CD-ROM for ISO images. | +| [ClusterVirtualImage](/modules/virtualization/cr.html#clustervirtualimage) | Connected in read-only mode, or as a CD-ROM for ISO images. | +| [VirtualDisk](/modules/virtualization/cr.html#virtualdisk) | Connected in read/write mode. | #### Attaching via the VM specification diff --git a/docs/USER_GUIDE.ru.md b/docs/USER_GUIDE.ru.md index 156d60d641..e781cb8dad 100644 --- a/docs/USER_GUIDE.ru.md +++ b/docs/USER_GUIDE.ru.md @@ -2128,7 +2128,7 @@ d8 k get nodes -o custom-columns=NAME:.metadata.name,ZONE:.metadata.labels.topol | Тип блочного устройства | Комментарий | |----------------------------------------------------------------------------|-------------------------------------------------------------------| -| [VirtualImage](/modules/virtualization/cr.html#virtualimage) | Подключается в режиме для чтения, или как cdrom для ISO-образов. | +| [VirtualImage](/modules/virtualization/cr.html#virtualimage) | Подключается в режиме для чтения, или как CD-ROM для ISO-образов. | | [ClusterVirtualImage](/modules/virtualization/cr.html#clustervirtualimage) | Подключается в режиме для чтения, или как CD-ROM для ISO-образов. | | [VirtualDisk](/modules/virtualization/cr.html#virtualdisk) | Подключается в режиме для чтения и записи. | From a29e584b01208f1aefa146bb713122cf300f72b0 Mon Sep 17 00:00:00 2001 From: Vladislav Panfilov Date: Fri, 13 Mar 2026 15:52:22 +0400 Subject: [PATCH 24/28] docs: add additional info from ADR Signed-off-by: Vladislav Panfilov --- docs/USER_GUIDE.md | 4 ++-- docs/USER_GUIDE.ru.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index f80f10cbb2..92c5d2466c 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -2114,9 +2114,9 @@ Block device types and access modes: The list of block devices is defined in the `.spec.blockDeviceRefs` field of the [VirtualMachine](/modules/virtualization/cr.html#virtualmachine) resource. -By default, boot order follows the order of devices in the list. You can set it explicitly with the optional `bootOrder` field (smaller value means higher priority). If `bootOrder` is set for at least one device, only devices with `bootOrder` set are included in the boot sequence. Allowed values: integers ≥ 1, unique within the list. +By default, boot order follows the order of devices in the list. You can set it explicitly with the optional `bootOrder` field (smaller value means higher priority). If `bootOrder` is set for at least one device, only devices with `bootOrder` set are included in the boot sequence. Allowed values: integers ≥ 1, unique within the list. When you remove a device from the list, boot order is recalculated for the remaining devices. -Adding or removing entries in `.spec.blockDeviceRefs` is applied to a running VM without a reboot. Changing the order of devices in the list or their `bootOrder` values takes effect after a VM reboot. +Adding or removing entries in `.spec.blockDeviceRefs` is applied to a running VM without a reboot (hotplug). Changing the order of devices in the list or their `bootOrder` values takes effect after a VM reboot. This allows you to attach an ISO image for OS installation with the desired boot priority, then remove it from the list and detach it without rebooting the VM. Virtual machine configuration fragment with block devices and explicit boot order: diff --git a/docs/USER_GUIDE.ru.md b/docs/USER_GUIDE.ru.md index e781cb8dad..d38d0e5447 100644 --- a/docs/USER_GUIDE.ru.md +++ b/docs/USER_GUIDE.ru.md @@ -2136,9 +2136,9 @@ d8 k get nodes -o custom-columns=NAME:.metadata.name,ZONE:.metadata.labels.topol Список блочных устройств задаётся в поле `.spec.blockDeviceRefs` ресурса [VirtualMachine](/modules/virtualization/cr.html#virtualmachine). -Порядок загрузки по умолчанию совпадает с порядком устройств в списке. Его можно задать явно с помощью необязательного поля `bootOrder` (меньшее значение — выше приоритет). Если `bootOrder` указан хотя бы у одного устройства, в цепочку загрузки попадают только устройства с заданным `bootOrder`. Допустимые значения: целые ≥ 1, уникальные в пределах списка. +Порядок загрузки по умолчанию совпадает с порядком устройств в списке. Его можно задать явно с помощью необязательного поля `bootOrder` (меньшее значение — выше приоритет). Если `bootOrder` указан хотя бы у одного устройства, в цепочку загрузки попадают только устройства с заданным `bootOrder`. Допустимые значения: целые ≥ 1, уникальные в пределах списка. При удалении устройства из списка порядок загрузки пересчитывается для оставшихся устройств. -Добавление и удаление элементов в `.spec.blockDeviceRefs` применяется к работающей ВМ без перезагрузки. Изменение порядка устройств в списке или значений `bootOrder` вступает в силу после перезагрузки ВМ. +Добавление и удаление элементов в `.spec.blockDeviceRefs` применяется к работающей ВМ без перезагрузки (hotplug). Изменение порядка устройств в списке или значений `bootOrder` вступает в силу после перезагрузки ВМ. Это позволяет, например, подключить ISO-образ для установки ОС с нужным приоритетом загрузки, а после установки удалить его из списка и отключить без перезагрузки ВМ. Фрагмент конфигурации виртуальной машины с блочными устройствами и явным порядком загрузки: From bfc9a1bfc1ed33976cd7707b171b91723d14d9e1 Mon Sep 17 00:00:00 2001 From: Vladislav Panfilov Date: Fri, 13 Mar 2026 16:44:31 +0400 Subject: [PATCH 25/28] docs: post-review updates Signed-off-by: Vladislav Panfilov --- docs/USER_GUIDE.md | 20 +++++++++++--------- docs/USER_GUIDE.ru.md | 20 +++++++++++--------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 92c5d2466c..7d09f2d6ce 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -2097,11 +2097,6 @@ d8 k get nodes -o custom-columns=NAME:.metadata.name,ZONE:.metadata.labels.topol You can attach disks and images to a virtual machine. They are described as block devices (BlockDevices). -Two attachment methods are available: - -- Static attachment: Devices are listed in the VM specification at creation or start and form the initial configuration. -- Dynamic attachment (hotplug): Attaching and detaching devices to or from a running VM without a reboot. This can be done either by changing the `.spec.blockDeviceRefs` list or by using the [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) resource. - Block device types and access modes: | Block device type | Comment | @@ -2110,13 +2105,20 @@ Block device types and access modes: | [ClusterVirtualImage](/modules/virtualization/cr.html#clustervirtualimage) | Connected in read-only mode, or as a CD-ROM for ISO images. | | [VirtualDisk](/modules/virtualization/cr.html#virtualdisk) | Connected in read/write mode. | +Two attachment methods are available: + +- Via the VM specification (`.spec.blockDeviceRefs`): Disks are listed in the [VirtualMachine](/modules/virtualization/cr.html#virtualmachine) configuration and boot order is set for them (by position in the list or via the `bootOrder` field). Recommended when configuring the VM manually or via GitOps, or when you need to control boot order (e.g., an ISO for OS installation). +- Via [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) (`vmbda`): Disk is attached via a separate resource and does not participate in boot order. Recommended for automation and when you do not have permission to edit the VM. + +Both methods support hotplug (add or remove without rebooting the VM). + #### Attaching via the VM specification The list of block devices is defined in the `.spec.blockDeviceRefs` field of the [VirtualMachine](/modules/virtualization/cr.html#virtualmachine) resource. By default, boot order follows the order of devices in the list. You can set it explicitly with the optional `bootOrder` field (smaller value means higher priority). If `bootOrder` is set for at least one device, only devices with `bootOrder` set are included in the boot sequence. Allowed values: integers ≥ 1, unique within the list. When you remove a device from the list, boot order is recalculated for the remaining devices. -Adding or removing entries in `.spec.blockDeviceRefs` is applied to a running VM without a reboot (hotplug). Changing the order of devices in the list or their `bootOrder` values takes effect after a VM reboot. This allows you to attach an ISO image for OS installation with the desired boot priority, then remove it from the list and detach it without rebooting the VM. +Changing the order of devices in the list or their `bootOrder` values takes effect after a VM reboot. For example, you can attach an ISO image for OS installation with the desired boot priority, then remove it from the list after installation. Virtual machine configuration fragment with block devices and explicit boot order: @@ -2144,7 +2146,7 @@ spec: name: ``` -To detach a disk, remove it from the list. The disk will be detached from the running virtual machine without a reboot. +To detach a disk, remove it from the list. How to work with bootable block devices in the web interface: @@ -2156,7 +2158,7 @@ How to work with bootable block devices in the web interface: #### Attaching via VirtualMachineBlockDeviceAttachment (vmbda) -The [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) resource provides hot-plug: attach and detach a block device to or from a running VM without changing its spec and without a reboot. Suited for automation and scenarios when the user does not have permission to edit the VM. +The [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) resource attaches and detaches a block device to or from a VM without changing its spec. Suited for automation and scenarios when the user does not have permission to edit the VM. Create a resource that attaches the empty disk `blank-disk` to the virtual machine `linux-vm`: @@ -2219,7 +2221,7 @@ To detach the disk from the virtual machine, delete the previously created resou d8 k delete vmbda attach-blank-disk ``` -Attaching images is done by analogy. Set `kind` to `VirtualImage` or `ClusterVirtualImage` and specify the image name: +Attaching images is done by analogy: set the `kind` field to VirtualImage or ClusterVirtualImage and the image name. ```bash d8 k apply -f - < ``` -Для отключения диска удалите его из списка. Диск будет отсоединён от работающей виртуальной машины без перезагрузки. +Для отключения диска удалите его из списка. Как работать со загрузочными блочными устройствами в веб-интерфейсе: @@ -2178,7 +2180,7 @@ spec: #### Подключение через VirtualMachineBlockDeviceAttachment (vmbda) -Ресурс [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) обеспечивает hot-plug: подключение и отключение блочного устройства к работающей ВМ без изменения её спецификации и без перезагрузки. Подходит для автоматизации и сценариев, когда у пользователя нет прав на редактирование ВМ. +Ресурс [VirtualMachineBlockDeviceAttachment](/modules/virtualization/cr.html#virtualmachineblockdeviceattachment) подключает и отключает блочное устройство у ВМ без изменения её спецификации. Подходит для автоматизации и сценариев, когда у пользователя нет прав на редактирование ВМ. Создайте ресурс, который подключит пустой диск `blank-disk` к виртуальной машине `linux-vm`: @@ -2241,7 +2243,7 @@ sdc 8:32 0 95.9M 0 disk <--- динамически подключен d8 k delete vmbda attach-blank-disk ``` -Подключение образов, осуществляется по аналогии. Для этого в качестве `kind` указать VirtualImage или ClusterVirtualImage и имя образа: +Подключение образов осуществляется по аналогии: укажите в поле `kind` значение VirtualImage или ClusterVirtualImage и имя образа. ```bash d8 k apply -f - < Date: Fri, 13 Mar 2026 17:54:53 +0300 Subject: [PATCH 26/28] fix backward compatility Signed-off-by: Daniil Loktev --- .../pkg/controller/vm/internal/sync_kvvm.go | 17 +++++++++++++++++ .../pkg/controller/vmchange/spec_changes.go | 9 +++++++++ 2 files changed, 26 insertions(+) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 530551a938..456c90808d 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -138,6 +138,10 @@ func (h *SyncKvvmHandler) Handle(ctx context.Context, s state.VirtualMachineStat lastClassAppliedSpec := h.loadClassLastAppliedSpec(class, kvvm) changes = h.detectSpecChanges(ctx, kvvm, ¤t.Spec, lastAppliedSpec) if !changes.IsEmpty() { + kvvmi, kvvmiErr := s.KVVMI(ctx) + if kvvmiErr == nil && hasNonHotpluggableVolumes(kvvmi) { + changes.UpgradeBlockDeviceChangesToRestart() + } allChanges.Add(changes.GetAll()...) } if class != nil { @@ -714,6 +718,19 @@ func (h *SyncKvvmHandler) isVMUnschedulable( } // isPlacementPolicyChanged returns true if any of the Affinity, NodePlacement, or Toleration rules have changed. +func hasNonHotpluggableVolumes(kvvmi *virtv1.VirtualMachineInstance) bool { + if kvvmi == nil { + return false + } + for _, v := range kvvmi.Spec.Volumes { + if v.PersistentVolumeClaim != nil && !v.PersistentVolumeClaim.Hotpluggable || + v.ContainerDisk != nil && !v.ContainerDisk.Hotpluggable { + return true + } + } + return false +} + func (h *SyncKvvmHandler) isPlacementPolicyChanged(allChanges vmchange.SpecChanges) bool { for _, c := range allChanges.GetAll() { switch c.Path { diff --git a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go index 857b1c1312..6cd1c414b3 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/spec_changes.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "sort" + "strings" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "sigs.k8s.io/yaml" @@ -231,6 +232,14 @@ func (s *SpecChanges) ConvertPendingChanges() ([]apiextensionsv1.JSON, error) { return res, nil } +func (s *SpecChanges) UpgradeBlockDeviceChangesToRestart() { + for i := range s.changes { + if strings.HasPrefix(s.changes[i].Path, BlockDevicesPath) && s.changes[i].ActionRequired == ActionApplyImmediate { + s.changes[i].ActionRequired = ActionRestart + } + } +} + func (s *SpecChanges) Add(changes ...FieldChange) { if s.changes == nil { s.changes = make([]FieldChange, 0) From 6ce1e9629d8cc6c8a8097d6a0594f571e44f1ab6 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Fri, 13 Mar 2026 19:38:16 +0300 Subject: [PATCH 27/28] fix comments Signed-off-by: Daniil Loktev --- .../pkg/controller/kvbuilder/kvvm_utils.go | 29 +++++++++++++++---- test/e2e/vm/block_device_hotplug.go | 6 ++-- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index c8c526c505..4e52766af9 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -173,7 +173,8 @@ func applyBlockDeviceRefs( kvvmVolumes := kvvm.Resource.Spec.Template.Spec.Volumes for i, bd := range vm.Spec.BlockDeviceRefs { - if len(kvvmVolumes) > 0 && !slices.ContainsFunc(kvvmVolumes, func(v virtv1.Volume) bool { return v.Name == GenerateDiskName(bd.Kind, bd.Name) }) { + diskName := GenerateDiskName(bd.Kind, bd.Name) + if len(kvvmVolumes) > 0 && !slices.ContainsFunc(kvvmVolumes, func(v virtv1.Volume) bool { return v.Name == diskName }) { continue } @@ -186,7 +187,8 @@ func applyBlockDeviceRefs( kvBootOrder = uint(i) + 1 } - if err := setBlockDeviceDisk(kvvm, bd, kvBootOrder, vdByName, viByName, cviByName); err != nil { + hotpluggable := isVolumeHotpluggable(kvvmVolumes, diskName) + if err := setBlockDeviceDisk(kvvm, bd, kvBootOrder, hotpluggable, vdByName, viByName, cviByName); err != nil { return err } } @@ -194,8 +196,23 @@ func applyBlockDeviceRefs( return nil } +func isVolumeHotpluggable(volumes []virtv1.Volume, name string) bool { + for _, v := range volumes { + if v.Name != name { + continue + } + if v.PersistentVolumeClaim != nil { + return v.PersistentVolumeClaim.Hotpluggable + } + if v.ContainerDisk != nil { + return v.ContainerDisk.Hotpluggable + } + } + return true +} + func setBlockDeviceDisk( - kvvm *KVVM, bd v1alpha2.BlockDeviceSpecRef, bootOrder uint, + kvvm *KVVM, bd v1alpha2.BlockDeviceSpecRef, bootOrder uint, hotpluggable bool, vdByName map[string]*v1alpha2.VirtualDisk, viByName map[string]*v1alpha2.VirtualImage, cviByName map[string]*v1alpha2.ClusterVirtualImage, @@ -209,7 +226,7 @@ func setBlockDeviceDisk( opts := SetDiskOptions{ Serial: GenerateSerialFromObject(vi), BootOrder: bootOrder, - IsHotplugged: true, + IsHotplugged: hotpluggable, } switch vi.Spec.Storage { case v1alpha2.StorageKubernetes, v1alpha2.StoragePersistentVolumeClaim: @@ -233,7 +250,7 @@ func setBlockDeviceDisk( IsCdrom: imageformat.IsISO(cvi.Status.Format), Serial: GenerateSerialFromObject(cvi), BootOrder: bootOrder, - IsHotplugged: true, + IsHotplugged: hotpluggable, }) case v1alpha2.DiskDevice: @@ -248,7 +265,7 @@ func setBlockDeviceDisk( PersistentVolumeClaim: ptr.To(vd.Status.Target.PersistentVolumeClaim), Serial: GenerateSerialFromObject(vd), BootOrder: bootOrder, - IsHotplugged: true, + IsHotplugged: hotpluggable, }) default: diff --git a/test/e2e/vm/block_device_hotplug.go b/test/e2e/vm/block_device_hotplug.go index ce2513ff4f..f540a41dc7 100644 --- a/test/e2e/vm/block_device_hotplug.go +++ b/test/e2e/vm/block_device_hotplug.go @@ -80,14 +80,14 @@ var _ = Describe("VirtualMachineBlockDeviceHotplug", Ordered, func() { Name: vdRoot.Name, }, ), - vmbuilder.WithRestartApprovalMode(v1alpha2.Automatic), + vmbuilder.WithRestartApprovalMode(v1alpha2.Manual), ) err := f.CreateWithDeferredDeletion(context.Background(), vm, vdRoot, vdBlank) Expect(err).NotTo(HaveOccurred()) - By("Waiting for VM agent to be ready") - util.UntilVMAgentReady(crclient.ObjectKeyFromObject(vm), framework.LongTimeout) + By("Waiting for SSH to be ready") + util.UntilSSHReady(f, vm, framework.LongTimeout) By("Recording initial disk count") out, err := f.SSHCommand(vm.Name, vm.Namespace, "lsblk --nodeps --noheadings | wc -l") From f9ec5d873c3db11ffb8d77f4d002fe67704007b0 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Fri, 13 Mar 2026 22:29:01 +0300 Subject: [PATCH 28/28] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/kvbuilder/kvvm_utils.go | 49 +++++++++++++------ .../pkg/controller/vm/internal/sync_kvvm.go | 7 ++- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go index 4e52766af9..1b132ddb3f 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -100,6 +100,7 @@ func ApplyVirtualMachineSpec( class *v1alpha2.VirtualMachineClass, ipAddress string, networkSpec network.InterfaceSpecList, + isVmRunning bool, ) error { if err := kvvm.SetRunPolicy(vm.Spec.RunPolicy); err != nil { return err @@ -128,7 +129,7 @@ func ApplyVirtualMachineSpec( return err } - if err := applyBlockDeviceRefs(kvvm, vm, vdByName, viByName, cviByName); err != nil { + if err := applyBlockDeviceRefs(kvvm, vm, isVmRunning, vdByName, viByName, cviByName); err != nil { return err } @@ -158,7 +159,7 @@ func ApplyVirtualMachineSpec( } func applyBlockDeviceRefs( - kvvm *KVVM, vm *v1alpha2.VirtualMachine, + kvvm *KVVM, vm *v1alpha2.VirtualMachine, isVmRunning bool, vdByName map[string]*v1alpha2.VirtualDisk, viByName map[string]*v1alpha2.VirtualImage, cviByName map[string]*v1alpha2.ClusterVirtualImage, @@ -171,6 +172,18 @@ func applyBlockDeviceRefs( } } + specDiskNames := make(map[string]struct{}, len(vm.Spec.BlockDeviceRefs)) + for _, bd := range vm.Spec.BlockDeviceRefs { + specDiskNames[GenerateDiskName(bd.Kind, bd.Name)] = struct{}{} + } + hotpluggableVolumes := make(map[string]struct{}, len(kvvm.Resource.Spec.Template.Spec.Volumes)) + for _, v := range kvvm.Resource.Spec.Template.Spec.Volumes { + if v.ContainerDisk != nil && v.ContainerDisk.Hotpluggable || v.PersistentVolumeClaim != nil && v.PersistentVolumeClaim.Hotpluggable { + hotpluggableVolumes[v.Name] = struct{}{} + } + } + cleanupRemovedStaticDisks(kvvm, specDiskNames, hotpluggableVolumes) + kvvmVolumes := kvvm.Resource.Spec.Template.Spec.Volumes for i, bd := range vm.Spec.BlockDeviceRefs { diskName := GenerateDiskName(bd.Kind, bd.Name) @@ -187,8 +200,9 @@ func applyBlockDeviceRefs( kvBootOrder = uint(i) + 1 } - hotpluggable := isVolumeHotpluggable(kvvmVolumes, diskName) - if err := setBlockDeviceDisk(kvvm, bd, kvBootOrder, hotpluggable, vdByName, viByName, cviByName); err != nil { + _, hotpluggable := hotpluggableVolumes[diskName] + // isVmRunning is used to set static disks as hotpluggable on VM restart + if err := setBlockDeviceDisk(kvvm, bd, kvBootOrder, hotpluggable || !isVmRunning, vdByName, viByName, cviByName); err != nil { return err } } @@ -196,19 +210,22 @@ func applyBlockDeviceRefs( return nil } -func isVolumeHotpluggable(volumes []virtv1.Volume, name string) bool { - for _, v := range volumes { - if v.Name != name { - continue - } - if v.PersistentVolumeClaim != nil { - return v.PersistentVolumeClaim.Hotpluggable - } - if v.ContainerDisk != nil { - return v.ContainerDisk.Hotpluggable - } +func cleanupRemovedStaticDisks(kvvm *KVVM, specDiskNames map[string]struct{}, hotpluggableVolumes map[string]struct{}) { + isRemovedStatic := func(name string) bool { + _, kind := GetOriginalDiskName(name) + _, inSpec := specDiskNames[name] + _, hotpluggable := hotpluggableVolumes[name] + return kind != "" && !inSpec && !hotpluggable } - return true + + kvvm.Resource.Spec.Template.Spec.Domain.Devices.Disks = slices.DeleteFunc( + kvvm.Resource.Spec.Template.Spec.Domain.Devices.Disks, + func(d virtv1.Disk) bool { return isRemovedStatic(d.Name) }, + ) + kvvm.Resource.Spec.Template.Spec.Volumes = slices.DeleteFunc( + kvvm.Resource.Spec.Template.Spec.Volumes, + func(v virtv1.Volume) bool { return isRemovedStatic(v.Name) }, + ) } func setBlockDeviceDisk( diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 456c90808d..6ae6e666bb 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -438,8 +438,13 @@ func MakeKVVMFromVMSpec(ctx context.Context, s state.VirtualMachineState) (*virt networkSpec := network.CreateNetworkSpec(current, vmmacs) + kvvmi, err := s.KVVMI(ctx) + if err != nil { + return nil, err + } + // Create kubevirt VirtualMachine resource from d8 VirtualMachine spec. - err = kvbuilder.ApplyVirtualMachineSpec(kvvmBuilder, current, bdState.VDByName, bdState.VIByName, bdState.CVIByName, class, ipAddress, networkSpec) + err = kvbuilder.ApplyVirtualMachineSpec(kvvmBuilder, current, bdState.VDByName, bdState.VIByName, bdState.CVIByName, class, ipAddress, networkSpec, kvvmi != nil && kvvmi.Status.Phase == virtv1.Running) if err != nil { return nil, err }