diff --git a/api/core/v1alpha2/block_device.go b/api/core/v1alpha2/block_device.go index 4e7eb743e4..f83e03473c 100644 --- a/api/core/v1alpha2/block_device.go +++ b/api/core/v1alpha2/block_device.go @@ -20,6 +20,10 @@ 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 + // +kubebuilder:validation:Minimum=1 + BootOrder *int `json:"bootOrder,omitempty"` } type BlockDeviceStatusRef struct { diff --git a/api/core/v1alpha2/zz_generated.deepcopy.go b/api/core/v1alpha2/zz_generated.deepcopy.go index 09418f9713..313b401fe3 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 } @@ -3423,7 +3428,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 281876d766..d17e2ca59f 100644 --- a/crds/doc-ru-virtualmachines.yaml +++ b/crds/doc-ru-virtualmachines.yaml @@ -367,6 +367,11 @@ spec: name: description: | Имя ресурса заданного типа. + bootOrder: + description: | + Приоритет загрузки блочного устройства. Меньшее значение означает более высокий приоритет. + Если не задано ни для одного устройства, порядок загрузки определяется позицией в списке (начиная с 1). + Если задано хотя бы для одного устройства, явные значения определяют приоритет загрузки. bootloader: description: | Загрузчик для ВМ: diff --git a/crds/virtualmachines.yaml b/crds/virtualmachines.yaml index a55a3c7f20..468bf879c4 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: 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. liveMigrationPolicy: type: string 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: diff --git a/images/virtualization-artifact/cmd/virtualization-controller/main.go b/images/virtualization-artifact/cmd/virtualization-controller/main.go index f1a685a130..70c3d9c62f 100644 --- a/images/virtualization-artifact/cmd/virtualization-controller/main.go +++ b/images/virtualization-artifact/cmd/virtualization-controller/main.go @@ -347,7 +347,7 @@ func main() { } vmLogger := logger.NewControllerLogger(vm.ControllerName, logLevel, logOutput, logDebugVerbosity, logDebugControllerList) - if err = vm.SetupController(ctx, mgr, virtClient, vmLogger, dvcrSettings, firmwareImage); err != nil { + if err = vm.SetupController(ctx, mgr, virtClient, vmLogger, dvcrSettings, firmwareImage, 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..fae433c379 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_utils.go @@ -20,13 +20,13 @@ 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" @@ -57,6 +57,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 +93,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,162 +129,14 @@ func ApplyVirtualMachineSpec( return err } - hotpluggedDevices := make([]HotPlugDeviceSettings, 0) - for _, volume := range kvvm.Resource.Spec.Template.Spec.Volumes { - if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.Hotpluggable { - hotpluggedDevices = append(hotpluggedDevices, HotPlugDeviceSettings{ - VolumeName: volume.Name, - PVCName: volume.PersistentVolumeClaim.ClaimName, - }) - } - - if volume.ContainerDisk != nil && volume.ContainerDisk.Hotpluggable { - hotpluggedDevices = append(hotpluggedDevices, HotPlugDeviceSettings{ - VolumeName: volume.Name, - Image: volume.ContainerDisk.Image, - }) - } - } - - kvvm.ClearDisks() - bootOrder := uint(1) - for _, bd := range vm.Spec.BlockDeviceRefs { - // bootOrder starts from 1. - switch bd.Kind { - case v1alpha2.ImageDevice: - // Attach ephemeral disk for storage: Kubernetes. - // Attach containerDisk for storage: ContainerRegistry (i.e. image from DVCR). - - 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: - // 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, - }); 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: bootOrder, - }); 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) - } - - 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: bootOrder, - }); 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 - } - - name := GenerateVDDiskName(bd.Name) - if err := kvvm.SetDisk(name, SetDiskOptions{ - PersistentVolumeClaim: pointer.GetPointer(pvcName), - Serial: GenerateSerialFromObject(vd), - BootOrder: bootOrder, - }); err != nil { - return err - } - bootOrder++ - default: - return fmt.Errorf("unknown block device kind %q. %w", bd.Kind, common.ErrUnknownType) - } + if err := applyBlockDeviceRefs(kvvm, vm, 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 - - switch kind { - case v1alpha2.ImageDevice: - obj, exists = viByName[name] - case v1alpha2.ClusterImageDevice: - obj, exists = cviByName[name] - case v1alpha2.DiskDevice: - obj, exists = vdByName[name] - default: - return fmt.Errorf("unknown block device kind %q. %w", kind, common.ErrUnknownType) - } - - if !exists || obj == nil || obj.GetUID() == "" { - continue - } - - 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 - } - } - } - kvvm.SetOwnerRef(vm, schema.GroupVersionKind{ Group: v1alpha2.SchemeGroupVersion.Group, Version: v1alpha2.SchemeGroupVersion.Version, @@ -299,13 +155,108 @@ func ApplyVirtualMachineSpec( kvvm.SetKVVMILabel(annotations.SkipPodSecurityStandardsCheckLabel, "true") // Set annotation for request network configuration. - err := setNetworksAnnotation(kvvm, networkSpec) - if err != nil { - return err + 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 { + hasExplicitBootOrder = true + break + } } + + 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) + } + } else { + kvBootOrder = uint(i) + 1 + } + + if err := setBlockDeviceDisk(kvvm, bd, kvBootOrder, vdByName, viByName, cviByName); err != nil { + return err + } + } + return nil } +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("unexpected storage type %q for vi %s. %w", vi.Spec.Storage, vi.Name, common.ErrUnknownType) + } + return kvvm.SetDisk(GenerateVIDiskName(bd.Name), opts) + + 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) + } + 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) + } + 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, + }) + + default: + return fmt.Errorf("unknown block device kind %q. %w", bd.Kind, common.ErrUnknownType) + } +} + func ApplyMigrationVolumes(kvvm *KVVM, vm *v1alpha2.VirtualMachine, vdsByName map[string]*v1alpha2.VirtualDisk) error { bootOrder := uint(1) var updateVolumesStrategy *virtv1.UpdateVolumesStrategy = nil 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/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 } 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 af0260b697..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 @@ -42,11 +42,18 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta return nil, err } + specRefs := s.VirtualMachine().Current().Spec.BlockDeviceRefs + + 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 { + for _, specBlockDeviceRef := range specRefs { ref := h.getBlockDeviceStatusRef(specBlockDeviceRef.Kind, specBlockDeviceRef.Name) ref.Size, err = h.getBlockDeviceRefSize(ctx, ref, s) if err != nil { @@ -86,6 +93,8 @@ func (h *BlockDeviceHandler) getBlockDeviceStatusRefs(ctx context.Context, s sta continue } + key := nameKindKey{kind: kind, name: bdName} + ref := h.getBlockDeviceStatusRef(kind, bdName) _, ref.Attached = h.getBlockDeviceTarget(volume, kvvmiVolumeStatusByName) ref.Size, err = h.getBlockDeviceRefSize(ctx, ref, s) @@ -95,26 +104,23 @@ 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 } @@ -221,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/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..068b1393b8 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vm/internal/hotplug_handler.go @@ -0,0 +1,213 @@ +/* +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" + "fmt" + + 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" + +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() || isDeletion(s.VirtualMachine().Current()) { + return reconcile.Result{}, nil + } + + current := s.VirtualMachine().Current() + + kvvmi, err := s.KVVMI(ctx) + if err != nil || kvvmi == nil { + return reconcile.Result{}, err + } + + if current.Status.Phase == v1alpha2.MachineMigrating { + log.Info("VM is migrating, skip hotplug") + return reconcile.Result{}, nil + } + + if bdReady, ok := conditions.GetCondition(vmcondition.TypeBlockDevicesReady, current.Status.Conditions); 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]struct{}) + for _, bd := range current.Spec.BlockDeviceRefs { + 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 + + // 1. Hotplugging + for key := range specDevices { + volName := generateVolumeName(key) + if _, onKVVM := kvvmDevices[key]; onKVVM { + continue + } + if _, ok := pending[volName]; ok { + continue + } + + 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 + } + 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)) + } + } + + // 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 { + continue + } + if _, isVMBDA := vmbdaDevices[key]; isVMBDA { + continue + } + if _, ok := pending[vol.name]; ok { + 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 +} + +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, err := s.VirtualDisk(ctx, key.name) + if err != nil { + return nil, err + } + if vd == nil || vd.Status.Target.PersistentVolumeClaim == "" { + return nil, nil + } + return service.NewAttachmentDiskFromVirtualDisk(vd), nil + case v1alpha2.ImageDevice: + vi, err := s.VirtualImage(ctx, key.name) + if err != nil { + return nil, err + } + if vi == nil { + return nil, nil + } + return service.NewAttachmentDiskFromVirtualImage(vi), nil + case v1alpha2.ClusterImageDevice: + cvi, err := s.ClusterVirtualImage(ctx, key.name) + if err != nil { + return nil, err + } + if cvi == nil { + return nil, nil + } + return service.NewAttachmentDiskFromClusterVirtualImage(cvi), nil + } + return nil, nil +} 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..9a4517ff0a 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go @@ -20,14 +20,21 @@ import ( "context" "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" ) -//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 +} 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..530551a938 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,8 @@ 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 f578aef5cc..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 @@ -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 < 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) + } + seen[*bdRef.BootOrder] = fmt.Sprintf("%s/%s", bdRef.Kind, bdRef.Name) + } + return nil +} 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_controller.go b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go index dd22ee3236..348983b209 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go @@ -51,12 +51,14 @@ func SetupController( log *log.Logger, dvcrSettings *dvcr.Settings, firmwareImage string, + 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) @@ -77,6 +79,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/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"), } 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..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,7 +26,7 @@ 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-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" @@ -109,7 +109,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 +265,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..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 @@ -27,7 +27,7 @@ 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 +48,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 +88,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 +99,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 +112,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 +125,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 +156,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..5fcbf16099 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/interfaces.go @@ -22,7 +22,7 @@ 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-controller/pkg/controller/service" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -35,5 +35,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..9d5dea04be 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,16 @@ 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 +55,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 +63,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 +71,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 +79,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) } } @@ -193,11 +192,24 @@ 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) if err != nil { - if errors.Is(err, intsvc.ErrVolumeStatusNotReady) { + if errors.Is(err, service.ErrVolumeStatusNotReady) { vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseInProgress cb. Status(metav1.ConditionFalse). @@ -220,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") @@ -278,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, intsvc.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, intsvc.ErrHotPlugRequestAlreadySent): + case errors.Is(canErr, service.ErrHotPlugRequestAlreadySent): log.Info("Attachment request sent: attachment is in progress.") vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseInProgress @@ -297,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 } } diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go index 522cf3ee5c..34b4f669ef 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/mock.go @@ -5,7 +5,7 @@ package internal import ( "context" - 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" corev1 "k8s.io/api/core/v1" virtv1 "kubevirt.io/api/core/v1" @@ -31,7 +31,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 +60,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 +99,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 +247,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 +270,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..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,16 +23,16 @@ 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-controller/pkg/controller/service" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) 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..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" - 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 +44,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..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" - 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 +39,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{ 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..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 { @@ -86,21 +86,21 @@ func compareBlockDevices(current, desired *v1alpha2.VirtualMachineSpec) []FieldC Path: itemPath, CurrentValue: current.BlockDeviceRefs[idx], DesiredValue: desired.BlockDeviceRefs[idx], - ActionRequired: ActionRestart, + ActionRequired: ActionApplyImmediate, }) case isAdded: changes = append(changes, FieldChange{ 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 +108,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..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,12 +151,12 @@ blockDeviceRefs: `, ``, assertChanges( - actionRequired(ActionRestart), + actionRequired(ActionApplyImmediate), requirePathOperation("blockDeviceRefs", ChangeRemove), ), }, { - "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/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()) + }) +})