fix(vd,vi): fix issues with storage class condition display (#947)

1. The Start and StartImmediate methods of DiskService now take a *storagev1.StorageClass instead of a string as the storage class parameter.

2. The global services VirtualImageStorageClassService and VirtualDiskStorageClassService have been moved to the internal/service package of the corresponding resource.

3. All checks related to StorageClass are now performed exclusively in the StorageClassReady handlers. The LifeCycle handler (and all source handlers) now rely on the StorageClassReady condition and no longer perform redundant checks.

4. A new service, BaseStorageClassService, which integrates internal storage class services in order to use the base logic for working with StorageClass.

5. Refactored LifeCycle for vi similarly to how it’s done for vd.

Signed-off-by: Isteb4k <dmitry.rakitin@flant.com>

Author: Isteb4k

api/core/v1alpha2/events.go

@@ -64,12 +64,11 @@ const (
 	// ReasonVMOPInProgress is event reason that the operation is in progress
 	ReasonVMOPInProgress = "VirtualMachineOperationInProgress"
 
-	// ReasonVDStorageClassWasDeleted is event reason that VDStorageClass was deleted.
-	ReasonVDStorageClassWasDeleted = "VirtualDiskStorageClassWasDeleted"
-	// ReasonVDStorageClassNotFound is event reason that VDStorageClass not found.
-	ReasonVDStorageClassNotFound = "VirtualDiskStorageClassNotFound"
-	// ReasonVDSpecChanged is event reason that VDStorageClass is chanded.
-	ReasonVDSpecChanged = "VirtualDiskSpecChanged"
+	// ReasonVDSpecHasBeenChanged is event reason that spec of virtual disk has been changed.
+	ReasonVDSpecHasBeenChanged = "VirtualDiskSpecHasBeenChanged"
+	// ReasonVISpecHasBeenChanged is event reason that spec of virtual image has been changed.
+	ReasonVISpecHasBeenChanged = "VirtualImageSpecHasBeenChanged"
+
 	// ReasonVDContainerRegistrySecretNotFound is event reason that VDContainerRegistrySecret not found.
 	ReasonVDContainerRegistrySecretNotFound = "VirtualDiskContainerRegistrySecretNotFound"
 

api/core/v1alpha2/vicondition/condition.go

@@ -87,6 +87,8 @@ const (
 	QuotaExceeded ReadyReason = "QuotaExceeded"
 	// ImagePullFailed indicates that there was an issue with importing from DVCR.
 	ImagePullFailed ReadyReason = "ImagePullFailed"
+	// DatasourceNotReady indicates that the datasource is not ready, which prevents the import process from starting.
+	DatasourceNotReady ReadyReason = "DatasourceNotReady"
 
 	// Lost indicates that the underlying PersistentVolumeClaim has been lost and the `VirtualImage` can no longer be used.
 	Lost ReadyReason = "PVCLost"

images/virtualization-artifact/pkg/controller/service/base_storage_class_service.go

@@ -0,0 +1,77 @@
+/*
+Copyright 2025 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 (
+	"context"
+	"sort"
+
+	corev1 "k8s.io/api/core/v1"
+	storagev1 "k8s.io/api/storage/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/deckhouse/virtualization-controller/pkg/common/annotations"
+	"github.com/deckhouse/virtualization-controller/pkg/common/object"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/supplements"
+)
+
+type BaseStorageClassService struct {
+	client client.Client
+}
+
+func NewBaseStorageClassService(client client.Client) *BaseStorageClassService {
+	return &BaseStorageClassService{client: client}
+}
+
+func (s BaseStorageClassService) GetDefaultStorageClass(ctx context.Context) (*storagev1.StorageClass, error) {
+	var scs storagev1.StorageClassList
+	err := s.client.List(ctx, &scs, &client.ListOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	var defaultClasses []*storagev1.StorageClass
+	for idx := range scs.Items {
+		if scs.Items[idx].Annotations[annotations.AnnDefaultStorageClass] == "true" {
+			defaultClasses = append(defaultClasses, &scs.Items[idx])
+		}
+	}
+
+	if len(defaultClasses) == 0 {
+		return nil, ErrDefaultStorageClassNotFound
+	}
+
+	// Primary sort by creation timestamp, newest first.
+	// Secondary sort by class name, ascending order.
+	sort.Slice(defaultClasses, func(i, j int) bool {
+		if defaultClasses[i].CreationTimestamp.UnixNano() == defaultClasses[j].CreationTimestamp.UnixNano() {
+			return defaultClasses[i].Name < defaultClasses[j].Name
+		}
+		return defaultClasses[i].CreationTimestamp.UnixNano() > defaultClasses[j].CreationTimestamp.UnixNano()
+	})
+
+	return defaultClasses[0], nil
+}
+
+func (s BaseStorageClassService) GetStorageClass(ctx context.Context, scName string) (*storagev1.StorageClass, error) {
+	return object.FetchObject(ctx, types.NamespacedName{Name: scName}, s.client, &storagev1.StorageClass{})
+}
+
+func (s BaseStorageClassService) GetPersistentVolumeClaim(ctx context.Context, sup *supplements.Generator) (*corev1.PersistentVolumeClaim, error) {
+	return object.FetchObject(ctx, sup.PersistentVolumeClaim(), s.client, &corev1.PersistentVolumeClaim{})
+}

images/virtualization-artifact/pkg/controller/service/disk_service.go

@@ -22,13 +22,12 @@ import (
 	"errors"
 	"fmt"
 	"slices"
-	"sort"
 	"strconv"
 	"strings"
 
 	vsv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
 	corev1 "k8s.io/api/core/v1"
-	storev1 "k8s.io/api/storage/v1"
+	storagev1 "k8s.io/api/storage/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -73,12 +72,16 @@ func NewDiskService(
 func (s DiskService) Start(
 	ctx context.Context,
 	pvcSize resource.Quantity,
-	storageClass *string,
+	sc *storagev1.StorageClass,
 	source *cdiv1.DataVolumeSource,
 	obj ObjectKind,
 	sup *supplements.Generator,
 	opts ...Option,
 ) error {
+	if sc == nil {
+		return errors.New("cannot create DataVolume: StorageClass must not be nil")
+	}
+
 	dvBuilder := kvbuilder.NewDV(sup.DataVolume())
 	dvBuilder.SetDataSource(source)
 	dvBuilder.SetOwnerRef(obj, obj.GroupVersionKind())
@@ -95,17 +98,12 @@ func (s DiskService) Start(
 		}
 	}
 
-	sc, err := s.GetStorageClass(ctx, storageClass)
-	if err != nil {
-		return fmt.Errorf("get storage class: %w", err)
-	}
-
 	volumeMode, accessMode, err := s.GetVolumeAndAccessModes(ctx, sc)
 	if err != nil {
 		return fmt.Errorf("get volume and access modes: %w", err)
 	}
 
-	dvBuilder.SetPVC(storageClass, pvcSize, accessMode, volumeMode)
+	dvBuilder.SetPVC(&sc.Name, pvcSize, accessMode, volumeMode)
 
 	if s.isImmediateBindingMode(sc) {
 		dvBuilder.SetImmediate()
@@ -129,7 +127,7 @@ func (s DiskService) Start(
 	return supplements.EnsureForDataVolume(ctx, s.client, sup, dvBuilder.GetResource(), s.dvcrSettings)
 }
 
-func (s DiskService) GetVolumeAndAccessModes(ctx context.Context, sc *storev1.StorageClass) (corev1.PersistentVolumeMode, corev1.PersistentVolumeAccessMode, error) {
+func (s DiskService) GetVolumeAndAccessModes(ctx context.Context, sc *storagev1.StorageClass) (corev1.PersistentVolumeMode, corev1.PersistentVolumeAccessMode, error) {
 	if sc == nil {
 		return "", "", errors.New("storage class is nil")
 	}
@@ -163,14 +161,13 @@ func (s DiskService) GetVolumeAndAccessModes(ctx context.Context, sc *storev1.St
 func (s DiskService) StartImmediate(
 	ctx context.Context,
 	pvcSize resource.Quantity,
-	storageClass *string,
+	sc *storagev1.StorageClass,
 	source *cdiv1.DataVolumeSource,
 	obj ObjectKind,
 	sup *supplements.Generator,
 ) error {
-	sc, err := s.GetStorageClass(ctx, storageClass)
-	if err != nil {
-		return err
+	if sc == nil {
+		return errors.New("cannot create DataVolume: StorageClass must not be nil")
 	}
 
 	dvBuilder := kvbuilder.NewDV(sup.DataVolume())
@@ -180,7 +177,7 @@ func (s DiskService) StartImmediate(
 	dvBuilder.SetImmediate()
 	dv := dvBuilder.GetResource()
 
-	err = s.client.Create(ctx, dv)
+	err := s.client.Create(ctx, dv)
 	if err != nil && !k8serrors.IsAlreadyExists(err) {
 		return err
 	}
@@ -455,7 +452,7 @@ func getAccessModeMax(modes []corev1.PersistentVolumeAccessMode) corev1.Persiste
 	return m
 }
 
-func (s DiskService) parseVolumeMode(sc *storev1.StorageClass) (corev1.PersistentVolumeMode, bool) {
+func (s DiskService) parseVolumeMode(sc *storagev1.StorageClass) (corev1.PersistentVolumeMode, bool) {
 	if sc == nil {
 		return "", false
 	}
@@ -469,7 +466,7 @@ func (s DiskService) parseVolumeMode(sc *storev1.StorageClass) (corev1.Persisten
 	}
 }
 
-func (s DiskService) parseAccessMode(sc *storev1.StorageClass) (corev1.PersistentVolumeAccessMode, bool) {
+func (s DiskService) parseAccessMode(sc *storagev1.StorageClass) (corev1.PersistentVolumeAccessMode, bool) {
 	if sc == nil {
 		return "", false
 	}
@@ -483,11 +480,11 @@ func (s DiskService) parseAccessMode(sc *storev1.StorageClass) (corev1.Persisten
 	}
 }
 
-func (s DiskService) isImmediateBindingMode(sc *storev1.StorageClass) bool {
+func (s DiskService) isImmediateBindingMode(sc *storagev1.StorageClass) bool {
 	if sc == nil {
 		return false
 	}
-	return sc.GetAnnotations()[annotations.AnnVirtualDiskBindingMode] == string(storev1.VolumeBindingImmediate)
+	return sc.GetAnnotations()[annotations.AnnVirtualDiskBindingMode] == string(storagev1.VolumeBindingImmediate)
 }
 
 func (s DiskService) parseStorageCapabilities(status cdiv1.StorageProfileStatus) StorageCapabilities {
@@ -521,6 +518,10 @@ func (s DiskService) parseStorageCapabilities(status cdiv1.StorageProfileStatus)
 	return storageCapabilities[len(storageCapabilities)-1]
 }
 
+func (s DiskService) GetStorageClass(ctx context.Context, scName string) (*storagev1.StorageClass, error) {
+	return object.FetchObject(ctx, types.NamespacedName{Name: scName}, s.client, &storagev1.StorageClass{})
+}
+
 func (s DiskService) GetDataVolume(ctx context.Context, sup *supplements.Generator) (*cdiv1.DataVolume, error) {
 	return object.FetchObject(ctx, sup.DataVolume(), s.client, &cdiv1.DataVolume{})
 }
@@ -600,59 +601,6 @@ func (s DiskService) CheckImportProcess(ctx context.Context, dv *cdiv1.DataVolum
 	return nil
 }
 
-func (s DiskService) GetStorageClass(ctx context.Context, storageClassName *string) (*storev1.StorageClass, error) {
-	if storageClassName == nil || *storageClassName == "" {
-		return s.GetDefaultStorageClass(ctx)
-	}
-	return s.getStorageClass(ctx, *storageClassName)
-}
-
-func (s DiskService) GetDefaultStorageClass(ctx context.Context) (*storev1.StorageClass, error) {
-	var scs storev1.StorageClassList
-	err := s.client.List(ctx, &scs, &client.ListOptions{})
-	if err != nil {
-		return nil, err
-	}
-
-	var defaultClasses []*storev1.StorageClass
-	for idx := range scs.Items {
-		if scs.Items[idx].Annotations[annotations.AnnDefaultStorageClass] == "true" {
-			defaultClasses = append(defaultClasses, &scs.Items[idx])
-		}
-	}
-
-	if len(defaultClasses) == 0 {
-		return nil, ErrDefaultStorageClassNotFound
-	}
-
-	// Primary sort by creation timestamp, newest first
-	// Secondary sort by class name, ascending order
-	sort.Slice(defaultClasses, func(i, j int) bool {
-		if defaultClasses[i].CreationTimestamp.UnixNano() == defaultClasses[j].CreationTimestamp.UnixNano() {
-			return defaultClasses[i].Name < defaultClasses[j].Name
-		}
-		return defaultClasses[i].CreationTimestamp.UnixNano() > defaultClasses[j].CreationTimestamp.UnixNano()
-	})
-
-	return defaultClasses[0], nil
-}
-
-func (s DiskService) getStorageClass(ctx context.Context, storageClassName string) (*storev1.StorageClass, error) {
-	var sc storev1.StorageClass
-	err := s.client.Get(ctx, types.NamespacedName{
-		Name: storageClassName,
-	}, &sc, &client.GetOptions{})
-	if err != nil {
-		if k8serrors.IsNotFound(err) {
-			return nil, ErrStorageClassNotFound
-		}
-
-		return nil, err
-	}
-
-	return &sc, nil
-}
-
 var ErrInsufficientPVCSize = errors.New("the specified pvc size is insufficient")
 
 func GetValidatedPVCSize(pvcSize *resource.Quantity, requiredSize resource.Quantity) (resource.Quantity, error) {

images/virtualization-artifact/pkg/controller/service/errors.go

@@ -18,14 +18,11 @@ package service
 
 import (
 	"errors"
-	"fmt"
 )
 
 var (
-	ErrStorageClassNotFound               = errors.New("storage class not found")
 	ErrStorageProfileNotFound             = errors.New("storage profile not found")
 	ErrDefaultStorageClassNotFound        = errors.New("default storage class not found")
-	ErrStorageClassNotAllowed             = errors.New("storage class not allowed")
 	ErrDataVolumeNotRunning               = errors.New("pvc importer is not running")
 	ErrDataVolumeProvisionerUnschedulable = errors.New("provisioner unschedulable")
 )
@@ -34,31 +31,3 @@ var (
 	ErrIPAddressAlreadyExist = errors.New("the IP address is already allocated")
 	ErrIPAddressOutOfRange   = errors.New("the IP address is out of range")
 )
-
-type VirtualDiskUsedByImageError struct {
-	vdName string
-}
-
-func (e VirtualDiskUsedByImageError) Error() string {
-	return fmt.Sprintf("the virtual disk %q already used by creating image", e.vdName)
-}
-
-func NewVirtualDiskUsedByImageError(name string) error {
-	return VirtualDiskUsedByImageError{
-		vdName: name,
-	}
-}
-
-type VirtualDiskUsedByVirtualMachineError struct {
-	vdName string
-}
-
-func (e VirtualDiskUsedByVirtualMachineError) Error() string {
-	return fmt.Sprintf("the virtual disk %q already used by running virtual machine", e.vdName)
-}
-
-func NewVirtualDiskUsedByVirtualMachineError(name string) error {
-	return VirtualDiskUsedByVirtualMachineError{
-		vdName: name,
-	}
-}