resolve

Signed-off-by: Valeriy Khorunzhin <valeriy.khorunzhin@flant.com>

Author: Valeriy Khorunzhin

api/core/v1alpha2/vmbdacondition/condition.go

@@ -65,10 +65,6 @@ const (
 	Conflict AttachedReason = "Conflict"
 	// DeviceNotAvailableOnNode indicates that the block device's PersistentVolume is not available on the node where the virtual machine is running.
 	DeviceNotAvailableOnNode AttachedReason = "DeviceNotAvailableOnNode"
-	// HotPlugPodNotScheduled indicates that the hotplug pod cannot be scheduled on any node.
-	HotPlugPodNotScheduled AttachedReason = "HotPlugPodNotScheduled"
-	// FailedAttachVolume indicates that the hotplug pod failed to attach a volume.
-	FailedAttachVolume AttachedReason = "FailedAttachVolume"
 
 	// CapacityAvailable signifies that the capacity not reached and attaching available.
 	CapacityAvailable DiskAttachmentCapacityAvailableReason = "CapacityAvailable"

api/core/v1alpha2/vmcondition/condition.go

@@ -169,7 +169,6 @@ const (
 	ReasonVirtualMachineRunning       RunningReason = "Running"
 	ReasonInternalVirtualMachineError RunningReason = "InternalVirtualMachineError"
 	ReasonPodNotStarted               RunningReason = "PodNotStarted"
-	ReasonPodVolumeErrors             RunningReason = "PodVolumeErrors"
 	ReasonPodTerminating              RunningReason = "PodTerminating"
 	ReasonPodNotFound                 RunningReason = "PodNotFound"
 	ReasonPodConditionMissing         RunningReason = "PodConditionMissing"

images/virtualization-artifact/pkg/common/pod/pod.go

@@ -17,9 +17,14 @@ limitations under the License.
 package pod
 
 import (
+	"context"
+	"slices"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // MakeOwnerReference makes owner reference from a Pod
@@ -108,6 +113,46 @@ func IsPodComplete(pod *corev1.Pod) bool {
 	return pod != nil && pod.Status.Phase == corev1.PodSucceeded
 }
 
+func GetLastPodEvent(ctx context.Context, clientObject client.Client, pod *corev1.Pod) (*corev1.Event, error) {
+	if pod == nil {
+		return nil, nil
+	}
+
+	eventList := &corev1.EventList{}
+	err := clientObject.List(ctx, eventList, &client.ListOptions{
+		Namespace: pod.Namespace,
+		FieldSelector: fields.SelectorFromSet(fields.Set{
+			"involvedObject.name": pod.Name,
+			"involvedObject.kind": "Pod",
+		}),
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	if len(eventList.Items) == 0 {
+		return nil, nil
+	}
+
+	last := slices.MaxFunc(eventList.Items, func(a, b corev1.Event) int {
+		return a.LastTimestamp.Compare(b.LastTimestamp.Time)
+	})
+
+	return &last, nil
+}
+
+func IsContainerCreating(pod *corev1.Pod) bool {
+	if pod.Status.Phase != corev1.PodPending {
+		return false
+	}
+	for _, cs := range pod.Status.ContainerStatuses {
+		if cs.State.Waiting != nil && cs.State.Waiting.Reason == "ContainerCreating" {
+			return true
+		}
+	}
+	return false
+}
+
 // QemuSubGID is the gid used as the qemu group in fsGroup
 const QemuSubGID = int64(107)
 

images/virtualization-artifact/pkg/controller/vm/internal/lifecycle.go

@@ -20,17 +20,16 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
-	"slices"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	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/common/annotations"
+	ccpod "github.com/deckhouse/virtualization-controller/pkg/common/pod"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/service"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state"
@@ -55,11 +54,6 @@ type LifeCycleHandler struct {
 	recorder eventrecord.EventRecorderLogger
 }
 
-type podVolumeErrorEvent struct {
-	Reason  string
-	Message string
-}
-
 func (h *LifeCycleHandler) Handle(ctx context.Context, s state.VirtualMachineState) (reconcile.Result, error) {
 	if s.VirtualMachine().IsEmpty() {
 		return reconcile.Result{}, nil
@@ -109,19 +103,6 @@ func (h *LifeCycleHandler) Handle(ctx context.Context, s state.VirtualMachineSta
 	}
 
 	log := logger.FromContext(ctx).With(logger.SlogHandler(nameLifeCycleHandler))
-	// While the pod is not running, the VMI does not set the node and the method returns nil, so it is necessary to check if there are any issues with the pod
-	if pod == nil {
-		cb := conditions.NewConditionBuilder(vmcondition.TypeRunning).Generation(changed.GetGeneration())
-
-		if volumeErr := h.checkPodVolumeErrors(ctx, changed, log); volumeErr != nil {
-			cb.Status(metav1.ConditionFalse).
-				Reason(vmcondition.ReasonPodVolumeErrors).
-				Message(fmt.Sprintf("Error attaching block devices to virtual machine: %s: %s", volumeErr.Reason, volumeErr.Message))
-			conditions.SetCondition(cb, &changed.Status.Conditions)
-			return reconcile.Result{}, nil
-		}
-	}
-
 	h.syncRunning(ctx, changed, kvvm, kvvmi, pod, log)
 	return reconcile.Result{}, nil
 }
@@ -141,10 +122,10 @@ func (h *LifeCycleHandler) syncRunning(ctx context.Context, vm *v1alpha2.Virtual
 		return
 	}
 
-	if volumeError := h.checkPodVolumeErrors(ctx, vm, log); volumeError != nil {
+	if volumeError := h.checkVMPodVolumeErrors(ctx, vm, log); volumeError != nil {
 		cb.Status(metav1.ConditionFalse).
-			Reason(vmcondition.ReasonPodVolumeErrors).
-			Message(fmt.Sprintf("Error attaching block devices to virtual machine: %s: %s", volumeError.Reason, volumeError.Message))
+			Reason(vmcondition.ReasonPodNotStarted).
+			Message(fmt.Sprintf("Error attaching block devices to virtual machine: %s", volumeError.Error()))
 		conditions.SetCondition(cb, &vm.Status.Conditions)
 		return
 	}
@@ -233,7 +214,7 @@ func (h *LifeCycleHandler) syncRunning(ctx context.Context, vm *v1alpha2.Virtual
 	conditions.SetCondition(cb, &vm.Status.Conditions)
 }
 
-func (h *LifeCycleHandler) checkPodVolumeErrors(ctx context.Context, vm *v1alpha2.VirtualMachine, log *slog.Logger) *podVolumeErrorEvent {
+func (h *LifeCycleHandler) checkVMPodVolumeErrors(ctx context.Context, vm *v1alpha2.VirtualMachine, log *slog.Logger) error {
 	var podList corev1.PodList
 	err := h.client.List(ctx, &podList, &client.ListOptions{
 		Namespace: vm.Namespace,
@@ -243,62 +224,20 @@ func (h *LifeCycleHandler) checkPodVolumeErrors(ctx context.Context, vm *v1alpha
 	})
 	if err != nil {
 		log.Error("Failed to list pods", "error", err)
-		return nil
+		return err
 	}
 
-	for i := range podList.Items {
-		if volumeErr := h.getPodVolumeError(ctx, &podList.Items[i], log); volumeErr != nil {
-			return volumeErr
+	for _, pod := range podList.Items {
+		if !ccpod.IsContainerCreating(&pod) {
+			continue
 		}
-	}
-
-	return nil
-}
-
-func isContainerCreating(pod *corev1.Pod) bool {
-	if pod == nil {
-		return false
-	}
-	if pod.Status.Phase != corev1.PodPending {
-		return false
-	}
-	for _, cs := range pod.Status.ContainerStatuses {
-		if cs.State.Waiting != nil && cs.State.Waiting.Reason == "ContainerCreating" {
-			return true
+		lastEvent, err := ccpod.GetLastPodEvent(ctx, h.client, &pod)
+		if err != nil {
+			log.Error("Failed to get last pod event", "error", err)
+			return err
 		}
-	}
-	return false
-}
-
-func (h *LifeCycleHandler) getPodVolumeError(ctx context.Context, pod *corev1.Pod, log *slog.Logger) *podVolumeErrorEvent {
-	if !isContainerCreating(pod) {
-		return nil
-	}
-
-	eventList := &corev1.EventList{}
-	err := h.client.List(ctx, eventList, &client.ListOptions{
-		Namespace: pod.Namespace,
-		FieldSelector: fields.SelectorFromSet(fields.Set{
-			"involvedObject.name": pod.Name,
-			"involvedObject.kind": "Pod",
-		}),
-	})
-	if err != nil {
-		log.Error("Failed to list pod events", "error", err)
-		return nil
-	}
-
-	if len(eventList.Items) == 0 {
-		return nil
-	}
-
-	last := slices.MaxFunc(eventList.Items, func(a, b corev1.Event) int {
-		return a.LastTimestamp.Compare(b.LastTimestamp.Time)
-	})
-	if last.Reason == watcher.ReasonFailedAttachVolume || last.Reason == watcher.ReasonFailedMount {
-		return &podVolumeErrorEvent{
-			Reason:  last.Reason,
-			Message: last.Message,
+		if lastEvent != nil && (lastEvent.Reason == watcher.ReasonFailedAttachVolume || lastEvent.Reason == watcher.ReasonFailedMount) {
+			return fmt.Errorf("failed to attach volume: %s: %s", lastEvent.Reason, lastEvent.Message)
 		}
 	}
 

images/virtualization-artifact/pkg/controller/vmbda/internal/life_cycle.go

@@ -22,15 +22,13 @@ import (
 	"fmt"
 	"time"
 
-	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/reconcile"
 
 	"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/controller/vmbda/internal/watcher"
 	"github.com/deckhouse/virtualization-controller/pkg/logger"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition"
@@ -202,10 +200,6 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac
 		if errors.Is(err, intsvc.ErrVolumeStatusNotReady) {
 			vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhaseInProgress
 
-			if handled, podErr := h.handleHotPlugPodIssues(ctx, ad, kvvmi, vmbda, cb); podErr != nil || handled {
-				return reconcile.Result{}, podErr
-			}
-
 			cb.
 				Status(metav1.ConditionFalse).
 				Reason(vmbdacondition.AttachmentRequestSent).
@@ -307,58 +301,3 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac
 		return reconcile.Result{}, err
 	}
 }
-
-func (h LifeCycleHandler) handleHotPlugPodIssues(
-	ctx context.Context,
-	ad *intsvc.AttachmentDisk,
-	kvvmi *virtv1.VirtualMachineInstance,
-	vmbda *v1alpha2.VirtualMachineBlockDeviceAttachment,
-	cb *conditions.ConditionBuilder,
-) (bool, error) {
-	hotPlugPod, err := h.attacher.GetHotPlugPod(ctx, ad, kvvmi)
-	if err != nil {
-		return false, err
-	}
-	if hotPlugPod == nil {
-		return false, nil
-	}
-
-	for _, c := range hotPlugPod.Status.Conditions {
-		if c.Type == corev1.PodScheduled && c.Status == corev1.ConditionFalse && c.Message != "" {
-			vmbda.Status.Phase = v1alpha2.BlockDeviceAttachmentPhasePending
-			cb.
-				Status(metav1.ConditionFalse).
-				Reason(vmbdacondition.HotPlugPodNotScheduled).
-				Message(fmt.Sprintf("Error attaching block device to virtual machine: %s: %s", c.Reason, c.Message))
-			return true, nil
-		}
-	}
-
-	if isContainerCreating(hotPlugPod) {
-		lastEvent, err := h.attacher.GetLastPodEvent(ctx, hotPlugPod)
-		if err != nil {
-			return false, err
-		}
-		if lastEvent != nil && (lastEvent.Reason == watcher.ReasonFailedAttachVolume || lastEvent.Reason == watcher.ReasonFailedMount) {
-			cb.
-				Status(metav1.ConditionFalse).
-				Reason(vmbdacondition.FailedAttachVolume).
-				Message(fmt.Sprintf("Error attaching block device to virtual machine: %s: %s", lastEvent.Reason, lastEvent.Message))
-			return true, nil
-		}
-	}
-
-	return false, nil
-}
-
-func isContainerCreating(pod *corev1.Pod) bool {
-	if pod.Status.Phase != corev1.PodPending {
-		return false
-	}
-	for _, cs := range pod.Status.ContainerStatuses {
-		if cs.State.Waiting != nil && cs.State.Waiting.Reason == "ContainerCreating" {
-			return true
-		}
-	}
-	return false
-}

images/virtualization-artifact/pkg/controller/vmbda/internal/service/attachment_service.go

@@ -20,11 +20,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"slices"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/component-helpers/scheduling/corev1/nodeaffinity"
 	virtv1 "kubevirt.io/api/core/v1"
@@ -316,69 +314,6 @@ func (s AttachmentService) IsPVAvailableOnVMNode(ctx context.Context, pvc *corev
 	return true, nil
 }
 
-func (s AttachmentService) GetHotPlugPod(ctx context.Context, ad *AttachmentDisk, kvvmi *virtv1.VirtualMachineInstance) (*corev1.Pod, error) {
-	if ad == nil || kvvmi == nil {
-		return nil, nil
-	}
-
-	for _, vs := range kvvmi.Status.VolumeStatus {
-		if vs.HotplugVolume == nil || vs.Name != ad.GenerateName {
-			continue
-		}
-		if vs.HotplugVolume.AttachPodName == "" {
-			return nil, nil
-		}
-
-		return object.FetchObject(ctx, types.NamespacedName{
-			Namespace: kvvmi.Namespace,
-			Name:      vs.HotplugVolume.AttachPodName,
-		}, s.client, &corev1.Pod{})
-	}
-	return nil, nil
-}
-
-func (s AttachmentService) GetHotPlugPodCondition(ctx context.Context, ad *AttachmentDisk, kvvmi *virtv1.VirtualMachineInstance, condType corev1.PodConditionType) (*corev1.PodCondition, error) {
-	pod, err := s.GetHotPlugPod(ctx, ad, kvvmi)
-	if err != nil || pod == nil {
-		return nil, err
-	}
-
-	for i, c := range pod.Status.Conditions {
-		if c.Type == condType {
-			return &pod.Status.Conditions[i], nil
-		}
-	}
-	return nil, nil
-}
-
-func (s AttachmentService) GetLastPodEvent(ctx context.Context, pod *corev1.Pod) (*corev1.Event, error) {
-	if pod == nil {
-		return nil, nil
-	}
-
-	eventList := &corev1.EventList{}
-	err := s.client.List(ctx, eventList, &client.ListOptions{
-		Namespace: pod.Namespace,
-		FieldSelector: fields.SelectorFromSet(fields.Set{
-			"involvedObject.name": pod.Name,
-			"involvedObject.kind": "Pod",
-		}),
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	if len(eventList.Items) == 0 {
-		return nil, nil
-	}
-
-	last := slices.MaxFunc(eventList.Items, func(a, b corev1.Event) int {
-		return a.LastTimestamp.Compare(b.LastTimestamp.Time)
-	})
-
-	return &last, nil
-}
-
 func isSameBlockDeviceRefs(a, b v1alpha2.VMBDAObjectRef) bool {
 	return a.Kind == b.Kind && a.Name == b.Name
 }