Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(vm): refactor blockDeviceReady handler #800

Merged
merged 3 commits into from
Apr 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/core/v1alpha2/virtual_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ type VirtualDiskStatsCreationDuration struct {

// List of VirtualMachines that use the disk.
type AttachedVirtualMachine struct {
// Name of attached VirtualMachine.
Name string `json:"name,omitempty"`
// Flag indicating that VirtualDisk is currently being used by this attached VirtualMachine.
Mounted bool `json:"mounted,omitempty"`
}

// +kubebuilder:validation:XValidation:rule="self.type == 'HTTP' ? has(self.http) && !has(self.containerImage) && !has(self.objectRef) : true",message="HTTP requires http and cannot have ContainerImage or ObjectRef."
Expand Down
7 changes: 7 additions & 0 deletions crds/doc-ru-virtualdisks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ spec:
attachedToVirtualMachines:
description: |
Список виртуальных машин, использующих данный диск.
properties:
mounted:
description:
Флаг, указывающий, что данный VirtualDisk в настоящее время используется
присоединённой VirtualMachine.
name:
description: Имя присоединённой VirtualMachine.
stats:
description: |
Статистика по виртуальному диску.
Expand Down
6 changes: 6 additions & 0 deletions crds/virtualdisks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,13 @@ spec:
items:
description: List of VirtualMachines that use the disk.
properties:
mounted:
description:
Flag indicating that this VirtualDisk is currently
used by the attached VirtualMachine.
type: boolean
name:
description: Name of attached VirtualMachine.
type: string
type: object
type: array
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, cvi *virtv2.Cluster
}

inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions)
if inUseCondition.Status != metav1.ConditionTrue || inUseCondition.ObservedGeneration != vd.Generation {
if inUseCondition.Status != metav1.ConditionTrue || !conditions.IsLastUpdated(inUseCondition, vd) {
return NewVirtualDiskNotReadyForUseError(vd.Name)
}

Expand Down

This file was deleted.

150 changes: 113 additions & 37 deletions images/virtualization-artifact/pkg/controller/vd/internal/inuse.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,15 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon
inUseCondition = cb.Condition()
}

err := h.updateAttachedVirtualMachines(ctx, vd)
if err != nil {
return reconcile.Result{}, err
}

usedByVM, usedByImage := false, false
var err error

if inUseCondition.Reason != vdcondition.UsedForImageCreation.String() {
usedByVM, err = h.checkUsageByVM(ctx, vd)
if err != nil {
return reconcile.Result{}, err
}
usedByVM = h.checkUsageByVM(vd)

if !usedByVM {
usedByImage, err = h.checkImageUsage(ctx, vd)
Expand All @@ -82,10 +83,7 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon
}

if !usedByImage {
usedByVM, err = h.checkUsageByVM(ctx, vd)
if err != nil {
return reconcile.Result{}, err
}
usedByVM = h.checkUsageByVM(vd)
}
}

Expand Down Expand Up @@ -141,6 +139,11 @@ func canStartVM(conditions []metav1.Condition) bool {
}

func (h InUseHandler) checkImageUsage(ctx context.Context, vd *virtv2.VirtualDisk) (bool, error) {
// If disk is not ready, it cannot be used for create image
if vd.Status.Phase != virtv2.DiskReady {
return false, nil
}

usedByImage, err := h.checkUsageByVI(ctx, vd)
if err != nil {
return false, err
Expand All @@ -155,62 +158,135 @@ func (h InUseHandler) checkImageUsage(ctx context.Context, vd *virtv2.VirtualDis
return usedByImage, nil
}

func (h InUseHandler) checkUsageByVM(ctx context.Context, vd *virtv2.VirtualDisk) (bool, error) {
func (h InUseHandler) updateAttachedVirtualMachines(ctx context.Context, vd *virtv2.VirtualDisk) error {
var vms virtv2.VirtualMachineList
err := h.client.List(ctx, &vms, &client.ListOptions{
Namespace: vd.GetNamespace(),
})
if err != nil {
return false, fmt.Errorf("error getting virtual machines: %w", err)
return fmt.Errorf("error getting virtual machines: %w", err)
}

usageMap, err := h.getVirtualMachineUsageMap(ctx, vd, vms)
if err != nil {
return err
}

h.updateAttachedVirtualMachinesStatus(vd, usageMap)
return nil
}

func (h InUseHandler) getVirtualMachineUsageMap(ctx context.Context, vd *virtv2.VirtualDisk, vms virtv2.VirtualMachineList) (map[string]bool, error) {
usageMap := make(map[string]bool)

for _, vm := range vms.Items {
if !h.isVDAttachedToVM(vd.GetName(), vm) {
continue
}

switch vm.Status.Phase {
case "":
return false, nil

usageMap[vm.GetName()] = false
case virtv2.MachinePending:
usedByVM := canStartVM(vm.Status.Conditions)

if usedByVM {
return true, nil
}
usageMap[vm.GetName()] = canStartVM(vm.Status.Conditions)
case virtv2.MachineStopped:
kvvm, err := object.FetchObject(ctx, types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace}, h.client, &virtv1.VirtualMachine{})
vmIsActive, err := h.isVMActive(ctx, vm)
if err != nil {
return false, fmt.Errorf("error getting kvvms: %w", err)
return nil, err
}

if kvvm != nil && kvvm.Status.StateChangeRequests != nil {
return true, nil
}
usageMap[vm.GetName()] = vmIsActive
default:
usageMap[vm.GetName()] = true
}
}

podList := corev1.PodList{}
err = h.client.List(ctx, &podList, &client.ListOptions{
Namespace: vm.GetNamespace(),
LabelSelector: labels.SelectorFromSet(map[string]string{virtv1.VirtualMachineNameLabel: vm.GetName()}),
})
if err != nil {
return false, fmt.Errorf("unable to list virt-launcher Pod for VM %q: %w", vm.GetName(), err)
}
return usageMap, nil
}

for _, pod := range podList.Items {
if pod.Status.Phase == corev1.PodRunning {
return true, nil
}
}
default:
func (h InUseHandler) isVMActive(ctx context.Context, vm virtv2.VirtualMachine) (bool, error) {
kvvm, err := object.FetchObject(ctx, types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace}, h.client, &virtv1.VirtualMachine{})
if err != nil {
return false, fmt.Errorf("error getting kvvms: %w", err)
}
if kvvm != nil && kvvm.Status.StateChangeRequests != nil {
return true, nil
}

podList := corev1.PodList{}
err = h.client.List(ctx, &podList, &client.ListOptions{
Namespace: vm.GetNamespace(),
LabelSelector: labels.SelectorFromSet(map[string]string{virtv1.VirtualMachineNameLabel: vm.GetName()}),
})
if err != nil {
return false, fmt.Errorf("unable to list virt-launcher Pod for VM %q: %w", vm.GetName(), err)
}

for _, pod := range podList.Items {
if pod.Status.Phase == corev1.PodRunning {
return true, nil
}
}

return false, nil
}

func (h InUseHandler) updateAttachedVirtualMachinesStatus(vd *virtv2.VirtualDisk, usageMap map[string]bool) {
var currentlyMountedVM string
for _, attachedVM := range vd.Status.AttachedToVirtualMachines {
if attachedVM.Mounted {
currentlyMountedVM = attachedVM.Name
break
}
}

attachedVMs := make([]virtv2.AttachedVirtualMachine, 0, len(usageMap))
setAnyToTrue := false

if used, exists := usageMap[currentlyMountedVM]; exists && used {
for key := range usageMap {
if key == currentlyMountedVM {
attachedVMs = append(attachedVMs, virtv2.AttachedVirtualMachine{
Name: key,
Mounted: true,
})
} else {
attachedVMs = append(attachedVMs, virtv2.AttachedVirtualMachine{
Name: key,
Mounted: false,
})
}
}
} else {
for key, value := range usageMap {
if !setAnyToTrue && value {
attachedVMs = append(attachedVMs, virtv2.AttachedVirtualMachine{
Name: key,
Mounted: true,
})
setAnyToTrue = true
} else {
attachedVMs = append(attachedVMs, virtv2.AttachedVirtualMachine{
Name: key,
Mounted: false,
})
}
}
}

vd.Status.AttachedToVirtualMachines = attachedVMs
}

func (h InUseHandler) checkUsageByVM(vd *virtv2.VirtualDisk) bool {
for _, attachedVM := range vd.Status.AttachedToVirtualMachines {
if attachedVM.Mounted {
return true
}
}

return false
}

func (h InUseHandler) checkUsageByVI(ctx context.Context, vd *virtv2.VirtualDisk) (bool, error) {
var vis virtv2.VirtualImageList
err := h.client.List(ctx, &vis, &client.ListOptions{
Expand Down
Loading
Loading