From 5efa78fc3dfb66e024d232ed25db5dfc06fc8aaf Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 19 May 2026 08:30:39 +0300 Subject: [PATCH 1/2] fix provisioning Signed-off-by: Valeriy Khorunzhin --- .../pkg/common/vd/vd.go | 49 +++++ ...ass_provisioner_compatibility_validator.go | 35 +--- .../pkg/controller/vm/internal/state/state.go | 171 +++++++++++++++- .../vm/internal/state/state_test.go | 182 +++++++++++++++++- .../rbac-for-us.yaml | 8 + 5 files changed, 404 insertions(+), 41 deletions(-) diff --git a/images/virtualization-artifact/pkg/common/vd/vd.go b/images/virtualization-artifact/pkg/common/vd/vd.go index eea442746c..db5b7503f4 100644 --- a/images/virtualization-artifact/pkg/common/vd/vd.go +++ b/images/virtualization-artifact/pkg/common/vd/vd.go @@ -18,6 +18,7 @@ package vd import ( "context" + "errors" "fmt" "log/slog" @@ -27,6 +28,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/deckhouse/virtualization-controller/pkg/common/object" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/featuregates" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -75,6 +77,53 @@ func StorageClassChanged(vd *v1alpha2.VirtualDisk) bool { return *specSc != "" && statusSc != "" } +type VirtualDiskStorageClassResolver interface { + GetModuleStorageClass(ctx context.Context) (*storagev1.StorageClass, error) + GetDefaultStorageClass(ctx context.Context) (*storagev1.StorageClass, error) +} + +// ResolveStorageClassName resolves storage class name for a VirtualDisk +// with the same precedence as VD handlers: +// 1. vd.Status.StorageClassName +// 2. vd.Spec.PersistentVolumeClaim.StorageClass +// 3. module default storage class (if resolver is provided) +// 4. cluster default storage class (if resolver is provided) +func ResolveStorageClassName(ctx context.Context, vd *v1alpha2.VirtualDisk, resolver VirtualDiskStorageClassResolver) (string, error) { + if vd == nil { + return "", nil + } + + if vd.Status.StorageClassName != "" { + return vd.Status.StorageClassName, nil + } + + if vd.Spec.PersistentVolumeClaim.StorageClass != nil && *vd.Spec.PersistentVolumeClaim.StorageClass != "" { + return *vd.Spec.PersistentVolumeClaim.StorageClass, nil + } + + if resolver == nil { + return "", nil + } + + moduleStorageClass, err := resolver.GetModuleStorageClass(ctx) + if err != nil { + return "", err + } + if moduleStorageClass != nil { + return moduleStorageClass.Name, nil + } + + defaultStorageClass, err := resolver.GetDefaultStorageClass(ctx) + if err != nil && !errors.Is(err, service.ErrDefaultStorageClassNotFound) { + return "", err + } + if defaultStorageClass != nil { + return defaultStorageClass.Name, nil + } + + return "", fmt.Errorf("storage class for VirtualDisk %q cannot be determined", vd.Name) +} + func ValidateVirtualImageStorageClassProvisionerCompatibility(ctx context.Context, vd *v1alpha2.VirtualDisk, client client.Client) error { if vd.Spec.DataSource == nil || vd.Spec.DataSource.Type != v1alpha2.DataSourceTypeObjectRef { return nil diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/validator/vi_pvc_storage_class_provisioner_compatibility_validator.go b/images/virtualization-artifact/pkg/controller/vd/internal/validator/vi_pvc_storage_class_provisioner_compatibility_validator.go index 2d1707e1ba..4929e78d7e 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/validator/vi_pvc_storage_class_provisioner_compatibility_validator.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/validator/vi_pvc_storage_class_provisioner_compatibility_validator.go @@ -18,8 +18,6 @@ package validator import ( "context" - "errors" - "fmt" "reflect" "sigs.k8s.io/controller-runtime/pkg/client" @@ -27,7 +25,6 @@ import ( commonvd "github.com/deckhouse/virtualization-controller/pkg/common/vd" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/service" intsvc "github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/source" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -47,7 +44,7 @@ func NewVirtualImagePVCStorageClassValidator(client client.Client, scService *in } func (v *VirtualImagePVCStorageClassValidator) ValidateCreate(ctx context.Context, vd *v1alpha2.VirtualDisk) (admission.Warnings, error) { - scName, err := v.extractVDStorageClassName(ctx, vd) + scName, err := commonvd.ResolveStorageClassName(ctx, vd, v.scService) if err != nil { return nil, err } @@ -70,33 +67,3 @@ func (v *VirtualImagePVCStorageClassValidator) ValidateUpdate(ctx context.Contex return nil, commonvd.ValidateVirtualImageStorageClassProvisionerCompatibility(ctx, newVD, v.client) } - -func (v *VirtualImagePVCStorageClassValidator) extractVDStorageClassName(ctx context.Context, vd *v1alpha2.VirtualDisk) (string, error) { - if vd.Status.StorageClassName != "" { - return vd.Status.StorageClassName, nil - } - - if vd.Spec.PersistentVolumeClaim.StorageClass != nil { - return *vd.Spec.PersistentVolumeClaim.StorageClass, nil - } - - moduleStorageClass, err := v.scService.GetModuleStorageClass(ctx) - if err != nil { - return "", err - } - - if moduleStorageClass != nil { - return moduleStorageClass.Name, nil - } - - defaultStorageClass, err := v.scService.GetDefaultStorageClass(ctx) - if err != nil && !errors.Is(err, service.ErrDefaultStorageClassNotFound) { - return "", err - } - - if defaultStorageClass != nil { - return defaultStorageClass.Name, nil - } - - return "", fmt.Errorf("storage class for VirtualDisk %q cannot be determined", vd.Name) -} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go index fddc328bae..bafea9cf7f 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go @@ -21,17 +21,22 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" kvvmutil "github.com/deckhouse/virtualization-controller/pkg/common/kvvm" "github.com/deckhouse/virtualization-controller/pkg/common/nodeaffinity" "github.com/deckhouse/virtualization-controller/pkg/common/object" + commonvd "github.com/deckhouse/virtualization-controller/pkg/common/vd" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" "github.com/deckhouse/virtualization-controller/pkg/controller/powerstate" @@ -410,7 +415,7 @@ func (s *state) PVNodeAffinityTerms(ctx context.Context) ([]corev1.NodeSelectorT continue } - terms, err := s.pvNodeAffinityTermsForPVC(ctx, pvcName, namespace) + terms, err := s.pvNodeAffinityTermsForPVC(ctx, ref.Kind, ref.Name, pvcName, namespace) if err != nil { return nil, fmt.Errorf("get PV node affinity for PVC %s: %w", pvcName, err) } @@ -495,19 +500,63 @@ func (s *state) resolvePVCName(ctx context.Context, kind v1alpha2.BlockDeviceKin } } -func (s *state) pvNodeAffinityTermsForPVC(ctx context.Context, pvcName, namespace string) ([]corev1.NodeSelectorTerm, error) { +func (s *state) resolveStorageClassName(ctx context.Context, kind v1alpha2.BlockDeviceKind, name, pvcName string) (string, error) { + switch kind { + case v1alpha2.DiskDevice: + vd, err := s.VirtualDisk(ctx, name) + if err != nil { + return "", err + } + if vd == nil { + return "", nil + } + // During migration, status.StorageClassName can still point to the old PVC's class. + // Avoid using it for the target PVC; rely on target PVC fields instead. + if vd.Status.MigrationState.TargetPVC == pvcName { + return "", nil + } + return commonvd.ResolveStorageClassName(ctx, vd, nil) + case v1alpha2.ImageDevice: + vi, err := s.VirtualImage(ctx, name) + if err != nil { + return "", err + } + if vi == nil { + return "", nil + } + if vi.Status.StorageClassName != "" { + return vi.Status.StorageClassName, nil + } + if vi.Spec.PersistentVolumeClaim.StorageClass != nil { + return *vi.Spec.PersistentVolumeClaim.StorageClass, nil + } + return "", nil + default: + return "", nil + } +} + +func (s *state) pvNodeAffinityTermsForPVC(ctx context.Context, kind v1alpha2.BlockDeviceKind, name, pvcName, namespace string) ([]corev1.NodeSelectorTerm, error) { pvc, err := object.FetchObject(ctx, types.NamespacedName{ Name: pvcName, Namespace: namespace, }, s.client, &corev1.PersistentVolumeClaim{}) if err != nil { return nil, err } - if pvc == nil || pvc.Spec.VolumeName == "" { + if pvc == nil { return nil, nil } + if pvc.Spec.VolumeName != "" { + return s.nodeAffinityTermsFromBoundPV(ctx, pvc.Spec.VolumeName) + } + + return s.nodeAffinityTermsFromUnboundPVC(ctx, kind, name, pvcName, pvc) +} + +func (s *state) nodeAffinityTermsFromBoundPV(ctx context.Context, pvName string) ([]corev1.NodeSelectorTerm, error) { pv, err := object.FetchObject(ctx, types.NamespacedName{ - Name: pvc.Spec.VolumeName, + Name: pvName, }, s.client, &corev1.PersistentVolume{}) if err != nil { return nil, err @@ -522,6 +571,120 @@ func (s *state) pvNodeAffinityTermsForPVC(ctx context.Context, pvcName, namespac return nil, nil } +// nodeAffinityTermsFromUnboundPVC determines node affinity for an unbound PVC by: +// 1. Looking for pre-provisioned Available PVs (static provisioning case). +// 2. If none found, deriving topology from StorageClass + LVMVolumeGroup (dynamic provisioning case). +func (s *state) nodeAffinityTermsFromUnboundPVC(ctx context.Context, kind v1alpha2.BlockDeviceKind, name, pvcName string, pvc *corev1.PersistentVolumeClaim) ([]corev1.NodeSelectorTerm, error) { + storageClassName, err := s.resolveStorageClassName(ctx, kind, name, pvcName) + if err != nil { + return nil, fmt.Errorf("resolve StorageClass for %s/%s: %w", kind, name, err) + } + if storageClassName == "" { + return nil, nil + } + + var pvList corev1.PersistentVolumeList + if err := s.client.List(ctx, &pvList); err != nil { + return nil, fmt.Errorf("list PVs: %w", err) + } + + var allTerms []corev1.NodeSelectorTerm + for i := range pvList.Items { + pv := &pvList.Items[i] + if pv.Status.Phase != corev1.VolumeAvailable { + continue + } + if pv.Spec.StorageClassName != storageClassName { + continue + } + if pv.Spec.NodeAffinity == nil || pv.Spec.NodeAffinity.Required == nil { + continue + } + allTerms = append(allTerms, pv.Spec.NodeAffinity.Required.NodeSelectorTerms...) + } + + if len(allTerms) > 0 { + return allTerms, nil + } + + return s.nodeAffinityTermsFromStorageClassTopology(ctx, storageClassName) +} + +const ( + localCSIProvisioner = "local.csi.storage.deckhouse.io" + lvmVolumeGroupsParam = "local.csi.storage.deckhouse.io/lvm-volume-groups" +) + +// nodeAffinityTermsFromStorageClassTopology resolves node topology for dynamic provisioning +// by reading the StorageClass parameters and looking up LVMVolumeGroup resources to determine +// which nodes can provision volumes for this StorageClass. +func (s *state) nodeAffinityTermsFromStorageClassTopology(ctx context.Context, storageClassName string) ([]corev1.NodeSelectorTerm, error) { + sc, err := object.FetchObject(ctx, types.NamespacedName{Name: storageClassName}, s.client, &storagev1.StorageClass{}) + if err != nil { + return nil, fmt.Errorf("get StorageClass %s: %w", storageClassName, err) + } + if sc == nil || sc.Provisioner != localCSIProvisioner { + return nil, nil + } + + lvgParam, ok := sc.Parameters[lvmVolumeGroupsParam] + if !ok || lvgParam == "" { + return nil, nil + } + + var lvgs []struct { + Name string `json:"name"` + } + if err := yaml.Unmarshal([]byte(lvgParam), &lvgs); err != nil { + return nil, nil + } + + var nodeNames []string + for _, lvg := range lvgs { + nodeName, err := s.getNodeNameFromLVMVolumeGroup(ctx, lvg.Name) + if err != nil { + return nil, err + } + if nodeName != "" { + nodeNames = append(nodeNames, nodeName) + } + } + + if len(nodeNames) == 0 { + return nil, nil + } + + return []corev1.NodeSelectorTerm{{ + MatchExpressions: []corev1.NodeSelectorRequirement{{ + Key: corev1.LabelHostname, + Operator: corev1.NodeSelectorOpIn, + Values: nodeNames, + }}, + }}, nil +} + +var lvmVolumeGroupGVK = schema.GroupVersionKind{ + Group: "storage.deckhouse.io", + Version: "v1alpha1", + Kind: "LVMVolumeGroup", +} + +func (s *state) getNodeNameFromLVMVolumeGroup(ctx context.Context, name string) (string, error) { + lvg := &unstructured.Unstructured{} + lvg.SetGroupVersionKind(lvmVolumeGroupGVK) + + err := s.client.Get(ctx, types.NamespacedName{Name: name}, lvg) + if err != nil { + if k8serrors.IsNotFound(err) { + return "", nil + } + return "", fmt.Errorf("get LVMVolumeGroup %s: %w", name, err) + } + + nodeName, _, _ := unstructured.NestedString(lvg.Object, "spec", "local", "nodeName") + return nodeName, nil +} + func (s *state) USBDevice(ctx context.Context, name string) (*v1alpha2.USBDevice, error) { return object.FetchObject(ctx, types.NamespacedName{ Name: name, diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go index 4977699de8..fa2822ed7d 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go @@ -24,8 +24,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" apiruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -148,6 +151,7 @@ var _ = Describe("PVNodeAffinityTerms", func() { v1alpha2.AddToScheme, virtv1.AddToScheme, corev1.AddToScheme, + storagev1.AddToScheme, } { err := f(scheme) Expect(err).NotTo(HaveOccurred()) @@ -305,7 +309,7 @@ var _ = Describe("PVNodeAffinityTerms", func() { Expect(terms[0].MatchExpressions[0].Values).To(ConsistOf(node2)) }) - It("should skip new WFFC disks where PVC is pending (no PV yet)", func() { + It("should skip unbound PVC when no available PVs match and SC is not local CSI", func() { vm := makeVM( v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "bound-disk"}, v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "pending-disk"}, @@ -315,19 +319,191 @@ var _ = Describe("PVNodeAffinityTerms", func() { pvBound := makePV("pv-bound", nodeAffinityTerm(node1)) vdPending := makeVD("pending-disk", "pvc-pending") + vdPending.Status.StorageClassName = "some-other-storage" pvcPending := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: "pvc-pending", Namespace: ns}, - Spec: corev1.PersistentVolumeClaimSpec{}, // no VolumeName yet } + otherSC := &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{Name: "some-other-storage"}, + Provisioner: "some-other-provisioner", + } + + s := buildState(vm, vdBound, pvcBound, pvBound, vdPending, pvcPending, otherSC) + ctx := logger.ToContext(context.TODO(), slog.Default()) + terms, err := s.PVNodeAffinityTerms(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(terms).To(HaveLen(1)) + Expect(terms[0].MatchExpressions[0].Values).To(ConsistOf(node1)) + }) - s := buildState(vm, vdBound, pvcBound, pvBound, vdPending, pvcPending) + It("should derive node affinity from LVMVolumeGroup for local CSI dynamic provisioning", func() { + vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "local-disk"}) + vd := makeVD("local-disk", "pvc-local") + vd.Status.StorageClassName = "local-storage-class-thin" + pvcLocal := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-local", Namespace: ns}, + } + localSC := &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{Name: "local-storage-class-thin"}, + Provisioner: localCSIProvisioner, + Parameters: map[string]string{ + lvmVolumeGroupsParam: "- name: vg-data-node-1\n thin:\n poolName: thin-data\n", + }, + } + lvg := &unstructured.Unstructured{} + lvg.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "storage.deckhouse.io", Version: "v1alpha1", Kind: "LVMVolumeGroup", + }) + lvg.SetName("vg-data-node-1") + _ = unstructured.SetNestedField(lvg.Object, node1, "spec", "local", "nodeName") + + s := buildState(vm, vd, pvcLocal, localSC, lvg) ctx := logger.ToContext(context.TODO(), slog.Default()) terms, err := s.PVNodeAffinityTerms(ctx) Expect(err).NotTo(HaveOccurred()) Expect(terms).To(HaveLen(1)) + Expect(terms[0].MatchExpressions).To(HaveLen(1)) + Expect(terms[0].MatchExpressions[0].Key).To(Equal(corev1.LabelHostname)) Expect(terms[0].MatchExpressions[0].Values).To(ConsistOf(node1)) }) + It("should intersect bound PV terms with LVMVolumeGroup topology", func() { + vm := makeVM( + v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "bound-disk"}, + v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "local-disk"}, + ) + vdBound := makeVD("bound-disk", "pvc-bound") + pvcBound := makePVC("pvc-bound", "pv-bound") + pvBound := makePV("pv-bound", nodeAffinityTerm(node1, node2)) + + vdLocal := makeVD("local-disk", "pvc-local") + vdLocal.Status.StorageClassName = "local-storage-class-thin" + pvcLocal := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-local", Namespace: ns}, + } + localSC := &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{Name: "local-storage-class-thin"}, + Provisioner: localCSIProvisioner, + Parameters: map[string]string{ + lvmVolumeGroupsParam: "- name: vg-data-node-1\n thin:\n poolName: thin-data\n", + }, + } + lvg := &unstructured.Unstructured{} + lvg.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "storage.deckhouse.io", Version: "v1alpha1", Kind: "LVMVolumeGroup", + }) + lvg.SetName("vg-data-node-1") + _ = unstructured.SetNestedField(lvg.Object, node1, "spec", "local", "nodeName") + + s := buildState(vm, vdBound, pvcBound, pvBound, vdLocal, pvcLocal, localSC, lvg) + ctx := logger.ToContext(context.TODO(), slog.Default()) + terms, err := s.PVNodeAffinityTerms(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(terms).NotTo(BeEmpty()) + }) + + It("should collect node affinity from available PVs for unbound WFFC PVC", func() { + vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "wffc-disk"}) + vd := makeVD("wffc-disk", "pvc-wffc") + vd.Status.StorageClassName = "local-storage" + pvcWFFC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-wffc", Namespace: ns}, + } + + pvAvail1 := makePV("pv-avail-1", nodeAffinityTerm(node1)) + pvAvail1.Spec.StorageClassName = "local-storage" + pvAvail1.Status.Phase = corev1.VolumeAvailable + + pvAvail2 := makePV("pv-avail-2", nodeAffinityTerm(node2)) + pvAvail2.Spec.StorageClassName = "local-storage" + pvAvail2.Status.Phase = corev1.VolumeAvailable + + pvBound := makePV("pv-bound-other", nodeAffinityTerm(node3)) + pvBound.Spec.StorageClassName = "local-storage" + pvBound.Status.Phase = corev1.VolumeBound + + s := buildState(vm, vd, pvcWFFC, pvAvail1, pvAvail2, pvBound) + ctx := logger.ToContext(context.TODO(), slog.Default()) + terms, err := s.PVNodeAffinityTerms(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(terms).To(HaveLen(2), "should have terms from 2 available PVs (not the bound one)") + }) + + It("should use VirtualDisk status storage class when PVC storage class is empty", func() { + vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "wffc-disk"}) + vd := makeVD("wffc-disk", "pvc-wffc") + vd.Status.StorageClassName = "local-storage" + pvcWFFC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-wffc", Namespace: ns}, + } + + pvAvail := makePV("pv-avail-1", nodeAffinityTerm(node2)) + pvAvail.Spec.StorageClassName = "local-storage" + pvAvail.Status.Phase = corev1.VolumeAvailable + + s := buildState(vm, vd, pvcWFFC, pvAvail) + ctx := logger.ToContext(context.TODO(), slog.Default()) + terms, err := s.PVNodeAffinityTerms(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(terms).To(HaveLen(1)) + Expect(terms[0].MatchExpressions).To(HaveLen(1)) + Expect(terms[0].MatchExpressions[0].Values).To(ConsistOf(node2)) + }) + + It("should use VirtualDisk spec storage class when PVC and status storage classes are empty", func() { + vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "wffc-disk"}) + vd := makeVD("wffc-disk", "pvc-wffc") + sc := "local-storage" + vd.Spec.PersistentVolumeClaim.StorageClass = &sc + pvcWFFC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-wffc", Namespace: ns}, + } + + pvAvail := makePV("pv-avail-1", nodeAffinityTerm(node3)) + pvAvail.Spec.StorageClassName = "local-storage" + pvAvail.Status.Phase = corev1.VolumeAvailable + + s := buildState(vm, vd, pvcWFFC, pvAvail) + ctx := logger.ToContext(context.TODO(), slog.Default()) + terms, err := s.PVNodeAffinityTerms(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(terms).To(HaveLen(1)) + Expect(terms[0].MatchExpressions).To(HaveLen(1)) + Expect(terms[0].MatchExpressions[0].Values).To(ConsistOf(node3)) + }) + + It("should intersect available PV terms with bound disk terms", func() { + vm := makeVM( + v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "bound-disk"}, + v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "wffc-disk"}, + ) + + vdBound := makeVD("bound-disk", "pvc-bound") + pvcBound := makePVC("pvc-bound", "pv-bound") + pvBound := makePV("pv-bound", nodeAffinityTerm(node1, node2)) + + vdWFFC := makeVD("wffc-disk", "pvc-wffc") + vdWFFC.Status.StorageClassName = "local-storage" + pvcWFFC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-wffc", Namespace: ns}, + } + pvAvail1 := makePV("pv-avail-1", nodeAffinityTerm(node2)) + pvAvail1.Spec.StorageClassName = "local-storage" + pvAvail1.Status.Phase = corev1.VolumeAvailable + + pvAvail2 := makePV("pv-avail-2", nodeAffinityTerm(node3)) + pvAvail2.Spec.StorageClassName = "local-storage" + pvAvail2.Status.Phase = corev1.VolumeAvailable + + s := buildState(vm, vdBound, pvcBound, pvBound, vdWFFC, pvcWFFC, pvAvail1, pvAvail2) + ctx := logger.ToContext(context.TODO(), slog.Default()) + terms, err := s.PVNodeAffinityTerms(ctx) + Expect(err).NotTo(HaveOccurred()) + // bound-disk allows node1,node2; wffc-disk allows node2,node3 + // intersection (cross-product) should yield terms matching node2 + Expect(terms).NotTo(BeEmpty()) + }) + It("should collect PV nodeAffinity from VirtualImage with PVC storage", func() { vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.ImageDevice, Name: "pvc-image"}) vi := makeVI("pvc-image", "pvc-vi", v1alpha2.StoragePersistentVolumeClaim) diff --git a/templates/virtualization-controller/rbac-for-us.yaml b/templates/virtualization-controller/rbac-for-us.yaml index 8bc92dc654..75e1297afe 100644 --- a/templates/virtualization-controller/rbac-for-us.yaml +++ b/templates/virtualization-controller/rbac-for-us.yaml @@ -95,6 +95,14 @@ rules: - get - list - watch +- apiGroups: + - storage.deckhouse.io + resources: + - lvmvolumegroups + verbs: + - get + - list + - watch - apiGroups: - "" resources: From ed457fda2f4285604ced6bd611bf814843de8189 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 19 May 2026 08:35:49 +0300 Subject: [PATCH 2/2] fix migration Signed-off-by: Valeriy Khorunzhin --- .../pkg/controller/vm/internal/state/state.go | 6 +++ .../vm/internal/state/state_test.go | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go index bafea9cf7f..555ea533fd 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/state/state.go @@ -579,6 +579,12 @@ func (s *state) nodeAffinityTermsFromUnboundPVC(ctx context.Context, kind v1alph if err != nil { return nil, fmt.Errorf("resolve StorageClass for %s/%s: %w", kind, name, err) } + // During migration we intentionally do not trust VD status storage class for target PVC, + // because it can still point to the source PVC class while the source VM pod is running. + // In this case use target PVC storage class if it is already set. + if storageClassName == "" && pvc.Spec.StorageClassName != nil { + storageClassName = *pvc.Spec.StorageClassName + } if storageClassName == "" { return nil, nil } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go index fa2822ed7d..0563c1417c 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/state/state_test.go @@ -569,6 +569,48 @@ var _ = Describe("PVNodeAffinityTerms", func() { "affinity should follow the migration target PVC's PV (node-2), not the source PVC's PV (node-1)") }) + It("should use target PVC storage class for unbound target PVC during migration", func() { + vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "local-disk"}) + + vd := makeVD("local-disk", "pvc-source") + vd.Generation = 1 + vd.Status.StorageClassName = "source-sc" + vd.Status.Conditions = []metav1.Condition{{ + Type: vdcondition.MigratingType.String(), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + Reason: "Migrating", + }} + vd.Status.MigrationState = v1alpha2.VirtualDiskMigrationState{ + SourcePVC: "pvc-source", + TargetPVC: "pvc-target", + } + + pvcSource := makePVC("pvc-source", "pv-source") + pvSource := makePV("pv-source", nodeAffinityTerm(node1)) + pvSource.Spec.StorageClassName = "source-sc" + pvSource.Status.Phase = corev1.VolumeBound + + targetSC := "target-sc" + pvcTarget := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-target", Namespace: ns}, + Spec: corev1.PersistentVolumeClaimSpec{ + StorageClassName: &targetSC, + }, + } + pvAvailTarget := makePV("pv-target-avail", nodeAffinityTerm(node2)) + pvAvailTarget.Spec.StorageClassName = "target-sc" + pvAvailTarget.Status.Phase = corev1.VolumeAvailable + + s := buildState(vm, vd, pvcSource, pvSource, pvcTarget, pvAvailTarget) + ctx := logger.ToContext(context.TODO(), slog.Default()) + terms, err := s.PVNodeAffinityTerms(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(terms).To(HaveLen(1)) + Expect(terms[0].MatchExpressions[0].Values).To(ConsistOf(node2), + "affinity for unbound target PVC should use target storage class, not source VD status class") + }) + It("should fall back to source PVC when migration condition is False (e.g. reverted)", func() { vm := makeVM(v1alpha2.BlockDeviceSpecRef{Kind: v1alpha2.DiskDevice, Name: "local-disk"})