fix(vi,vd): fix default storage class usage

- handle default storage class from the module config
- fix `StorageClassReady` condition

Signed-off-by: Roman Sysoev <roman.sysoev@flant.com>

Author: Roman Sysoev

api/core/v1alpha2/events.go

@@ -82,6 +82,9 @@ const (
 	// ReasonVDResizingNotAvailable is event reason that VD Resizing is not available.
 	ReasonVDResizingNotAvailable = "VirtualDiskResizingNotAvailable"
 
+	// ReasonVIStorageClassNotFound is event reason that VIStorageClass not found.
+	ReasonVIStorageClassNotFound = "VirtualImageStorageClassNotFound"
+
 	// ReasonDataSourceSyncStarted is event reason that DataSource sync is started.
 	ReasonDataSourceSyncStarted = "DataSourceImportStarted"
 	// ReasonDataSourceSyncInProgress is event reason that DataSource sync is in progress.

api/core/v1alpha2/vicondition/condition.go

@@ -79,8 +79,6 @@ const (
 	ProvisioningNotStarted ReadyReason = "ProvisioningNotStarted"
 	// ProvisioningFailed indicates that the provisioning process has failed.
 	ProvisioningFailed ReadyReason = "ProvisioningFailed"
-	// StorageClassNotReady indicates that the provisioning process pending because `StorageClass` not ready.
-	StorageClassNotReady ReadyReason = "StorageClassNotReady"
 	// Ready indicates that the import process is complete and the `VirtualImage` is ready for use.
 	Ready ReadyReason = "Ready"
 	// QuotaExceeded indicates that the VirtualImage is reached project quotas and can not be provisioned.
@@ -93,6 +91,8 @@ const (
 
 	// StorageClassReady indicates that the chosen StorageClass exists.
 	StorageClassReady StorageClassReadyReason = "StorageClassReady"
+	// StorageClassNotReady indicates that the provisioning process pending because `StorageClass` not ready.
+	StorageClassNotReady ReadyReason = "StorageClassNotReady"
 	// StorageClassNotFound indicates that the chosen StorageClass not found.
 	StorageClassNotFound StorageClassReadyReason = "StorageClassNotFound"
 	// DVCRTypeUsed indicates that the DVCR provisioning chosen.

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

@@ -17,20 +17,25 @@ limitations under the License.
 package service
 
 import (
+	"context"
 	"slices"
 
+	corev1 "k8s.io/api/core/v1"
 	storev1 "k8s.io/api/storage/v1"
 
 	"github.com/deckhouse/virtualization-controller/pkg/config"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/supplements"
 )
 
 type VirtualImageStorageClassService struct {
 	storageClassSettings config.VirtualImageStorageClassSettings
+	scGetter             StorageClassGetter
 }
 
-func NewVirtualImageStorageClassService(settings config.VirtualImageStorageClassSettings) *VirtualImageStorageClassService {
+func NewVirtualImageStorageClassService(settings config.VirtualImageStorageClassSettings, scGetter StorageClassGetter) *VirtualImageStorageClassService {
 	return &VirtualImageStorageClassService{
 		storageClassSettings: settings,
+		scGetter:             scGetter,
 	}
 }
 
@@ -47,7 +52,7 @@ func NewVirtualImageStorageClassService(settings config.VirtualImageStorageClass
 // Errors:
 // 1. Return error if no storage class is specified.
 // 2. Return error if specified non-empty class is not allowed.
-func (svc *VirtualImageStorageClassService) GetStorageClass(storageClassFromSpec *string, clusterDefaultStorageClass *storev1.StorageClass) (*string, error) {
+func (svc *VirtualImageStorageClassService) GetValidatedStorageClass(storageClassFromSpec *string, clusterDefaultStorageClass *storev1.StorageClass) (*string, error) {
 	if svc.storageClassSettings.DefaultStorageClassName == "" && len(svc.storageClassSettings.AllowedStorageClassNames) == 0 {
 		if svc.storageClassSettings.StorageClassName == "" {
 			return storageClassFromSpec, nil
@@ -86,3 +91,35 @@ func (svc *VirtualImageStorageClassService) GetStorageClass(storageClassFromSpec
 
 	return nil, ErrStorageClassNotFound
 }
+
+func (svc *VirtualImageStorageClassService) IsStorageClassAllowed(scName string) bool {
+	if svc.storageClassSettings.DefaultStorageClassName == "" && len(svc.storageClassSettings.AllowedStorageClassNames) == 0 {
+		return true
+	}
+
+	if slices.Contains(svc.storageClassSettings.AllowedStorageClassNames, scName) {
+		return true
+	}
+
+	if svc.storageClassSettings.DefaultStorageClassName != "" && svc.storageClassSettings.DefaultStorageClassName == scName {
+		return true
+	}
+
+	return false
+}
+
+func (svc *VirtualImageStorageClassService) GetModuleStorageClass(ctx context.Context) (*storev1.StorageClass, error) {
+	return svc.GetStorageClass(ctx, svc.storageClassSettings.DefaultStorageClassName)
+}
+
+func (svc *VirtualImageStorageClassService) GetPersistentVolumeClaim(ctx context.Context, sup *supplements.Generator) (*corev1.PersistentVolumeClaim, error) {
+	return svc.scGetter.GetPersistentVolumeClaim(ctx, sup)
+}
+
+func (svc *VirtualImageStorageClassService) GetStorageClass(ctx context.Context, scName string) (*storev1.StorageClass, error) {
+	return svc.scGetter.GetStorageClass(ctx, &scName)
+}
+
+func (svc *VirtualImageStorageClassService) GetDefaultStorageClass(ctx context.Context) (*storev1.StorageClass, error) {
+	return svc.scGetter.GetDefaultStorageClass(ctx)
+}

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

@@ -40,9 +40,9 @@ var _ = Describe("VirtualImageStorageClassService", func() {
 	Context("when settings are empty", func() {
 		It("returns the storageClassFromSpec", func() {
 			storageClassSettings = config.VirtualImageStorageClassSettings{}
-			service = NewVirtualImageStorageClassService(storageClassSettings)
+			service = NewVirtualImageStorageClassService(storageClassSettings, nil)
 			sc := ptr.To("requested-storage-class")
-			storageClass, err := service.GetStorageClass(sc, clusterDefaultStorageClass)
+			storageClass, err := service.GetValidatedStorageClass(sc, clusterDefaultStorageClass)
 
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(sc))
@@ -52,9 +52,9 @@ var _ = Describe("VirtualImageStorageClassService", func() {
 	Context("when settings are empty and storageClassFromSpec is empty", func() {
 		It("returns the storageClassFromSpec", func() {
 			storageClassSettings = config.VirtualImageStorageClassSettings{}
-			service = NewVirtualImageStorageClassService(storageClassSettings)
+			service = NewVirtualImageStorageClassService(storageClassSettings, nil)
 
-			storageClass, err := service.GetStorageClass(nil, clusterDefaultStorageClass)
+			storageClass, err := service.GetValidatedStorageClass(nil, clusterDefaultStorageClass)
 
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(BeNil())
@@ -64,9 +64,9 @@ var _ = Describe("VirtualImageStorageClassService", func() {
 	Context("when settings and clusterDefaultStorageClass are empty", func() {
 		It("returns the storageClassFromSpec", func() {
 			storageClassSettings = config.VirtualImageStorageClassSettings{}
-			service = NewVirtualImageStorageClassService(storageClassSettings)
+			service = NewVirtualImageStorageClassService(storageClassSettings, nil)
 			sc := ptr.To("requested-storage-class")
-			storageClass, err := service.GetStorageClass(sc, nil)
+			storageClass, err := service.GetValidatedStorageClass(sc, nil)
 
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(sc))
@@ -78,24 +78,24 @@ var _ = Describe("VirtualImageStorageClassService", func() {
 			storageClassSettings = config.VirtualImageStorageClassSettings{
 				StorageClassName: "storage-class-name",
 			}
-			service = NewVirtualImageStorageClassService(storageClassSettings)
+			service = NewVirtualImageStorageClassService(storageClassSettings, nil)
 		})
 
 		It("return the StorageClassName if storageClassFromSpec is empty", func() {
-			storageClass, err := service.GetStorageClass(nil, nil)
+			storageClass, err := service.GetValidatedStorageClass(nil, nil)
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(&storageClassSettings.StorageClassName))
 		})
 
 		It("return the StorageClassName if storageClassFromSpec equal StorageClassName", func() {
-			storageClass, err := service.GetStorageClass(&storageClassSettings.StorageClassName, nil)
+			storageClass, err := service.GetValidatedStorageClass(&storageClassSettings.StorageClassName, nil)
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(&storageClassSettings.StorageClassName))
 		})
 
 		It("return the err if storageClassFromSpec not equal StorageClassName", func() {
 			sc := ptr.To("requested-storage-class")
-			_, err := service.GetStorageClass(sc, nil)
+			_, err := service.GetValidatedStorageClass(sc, nil)
 			Expect(err).To(Equal(ErrStorageClassNotAllowed))
 		})
 	})
@@ -106,25 +106,25 @@ var _ = Describe("VirtualImageStorageClassService", func() {
 				AllowedStorageClassNames: []string{"allowed-storage-class"},
 				DefaultStorageClassName:  "",
 			}
-			service = NewVirtualImageStorageClassService(storageClassSettings)
+			service = NewVirtualImageStorageClassService(storageClassSettings, nil)
 		})
 
 		It("returns the requested storage class if it's in the allowed list", func() {
 			sc := ptr.To("allowed-storage-class")
-			storageClass, err := service.GetStorageClass(sc, clusterDefaultStorageClass)
+			storageClass, err := service.GetValidatedStorageClass(sc, clusterDefaultStorageClass)
 
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(sc))
 		})
 
 		It("returns an error if the requested storage class is not in the allowed list", func() {
-			_, err := service.GetStorageClass(ptr.To("not-allowed-storage-class"), clusterDefaultStorageClass)
+			_, err := service.GetValidatedStorageClass(ptr.To("not-allowed-storage-class"), clusterDefaultStorageClass)
 
 			Expect(err).To(Equal(ErrStorageClassNotAllowed))
 		})
 
 		It("returns an error if storageClassFromSpec is empty", func() {
-			_, err := service.GetStorageClass(nil, clusterDefaultStorageClass)
+			_, err := service.GetValidatedStorageClass(nil, clusterDefaultStorageClass)
 
 			Expect(err).To(Equal(ErrStorageClassNotAllowed))
 		})
@@ -136,26 +136,26 @@ var _ = Describe("VirtualImageStorageClassService", func() {
 				AllowedStorageClassNames: []string{},
 				DefaultStorageClassName:  "default-storage-class",
 			}
-			service = NewVirtualImageStorageClassService(storageClassSettings)
+			service = NewVirtualImageStorageClassService(storageClassSettings, nil)
 		})
 
 		It("returns the default storage class if storageClassFromSpec is empty", func() {
-			storageClass, err := service.GetStorageClass(nil, clusterDefaultStorageClass)
+			storageClass, err := service.GetValidatedStorageClass(nil, clusterDefaultStorageClass)
 
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(ptr.To("default-storage-class")))
 		})
 
 		It("returns the requested storage class if it matches the default storage class", func() {
 			sc := ptr.To("default-storage-class")
-			storageClass, err := service.GetStorageClass(sc, clusterDefaultStorageClass)
+			storageClass, err := service.GetValidatedStorageClass(sc, clusterDefaultStorageClass)
 
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(sc))
 		})
 
 		It("returns an error if the requested storage class does not match the default", func() {
-			_, err := service.GetStorageClass(ptr.To("different-storage-class"), clusterDefaultStorageClass)
+			_, err := service.GetValidatedStorageClass(ptr.To("different-storage-class"), clusterDefaultStorageClass)
 
 			Expect(err).To(Equal(ErrStorageClassNotAllowed))
 		})
@@ -167,26 +167,26 @@ var _ = Describe("VirtualImageStorageClassService", func() {
 				AllowedStorageClassNames: []string{"allowed-storage-class"},
 				DefaultStorageClassName:  "default-storage-class",
 			}
-			service = NewVirtualImageStorageClassService(storageClassSettings)
+			service = NewVirtualImageStorageClassService(storageClassSettings, nil)
 		})
 
 		It("returns the default storage class if storageClassFromSpec is empty", func() {
-			storageClass, err := service.GetStorageClass(nil, clusterDefaultStorageClass)
+			storageClass, err := service.GetValidatedStorageClass(nil, clusterDefaultStorageClass)
 
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(ptr.To("default-storage-class")))
 		})
 
 		It("returns the requested storage class if it's in the allowed list", func() {
 			sc := ptr.To("allowed-storage-class")
-			storageClass, err := service.GetStorageClass(sc, clusterDefaultStorageClass)
+			storageClass, err := service.GetValidatedStorageClass(sc, clusterDefaultStorageClass)
 
 			Expect(err).To(BeNil())
 			Expect(storageClass).To(Equal(sc))
 		})
 
 		It("returns an error if the requested storage class is not in the allowed list", func() {
-			_, err := service.GetStorageClass(ptr.To("not-allowed-storage-class"), clusterDefaultStorageClass)
+			_, err := service.GetValidatedStorageClass(ptr.To("not-allowed-storage-class"), clusterDefaultStorageClass)
 
 			Expect(err).To(Equal(ErrStorageClassNotAllowed))
 		})

images/virtualization-artifact/pkg/controller/vd/internal/storageclass_ready.go

@@ -18,6 +18,7 @@ package internal
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
@@ -27,6 +28,7 @@ import (
 
 	"github.com/deckhouse/virtualization-controller/pkg/common/annotations"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/service"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/supplements"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition"
@@ -76,7 +78,7 @@ func (h StorageClassReadyHandler) Handle(ctx context.Context, vd *virtv2.Virtual
 
 	// 3. Try to use default storage class from the module settings.
 	moduleStorageClass, err := h.svc.GetModuleStorageClass(ctx)
-	if err != nil {
+	if err != nil && !errors.Is(err, service.ErrDefaultStorageClassNotFound) {
 		return reconcile.Result{}, fmt.Errorf("get module storage class: %w", err)
 	}
 
@@ -87,7 +89,7 @@ func (h StorageClassReadyHandler) Handle(ctx context.Context, vd *virtv2.Virtual
 
 	// 4. Try to use default storage class from the cluster.
 	defaultStorageClass, err := h.svc.GetDefaultStorageClass(ctx)
-	if err != nil {
+	if err != nil && !errors.Is(err, service.ErrDefaultStorageClassNotFound) {
 		return reconcile.Result{}, fmt.Errorf("get default storage class: %w", err)
 	}
 

images/virtualization-artifact/pkg/controller/vi/internal/interfaces.go

@@ -20,14 +20,14 @@ import (
 	"context"
 
 	corev1 "k8s.io/api/core/v1"
-	storev1 "k8s.io/api/storage/v1"
+	storagev1 "k8s.io/api/storage/v1"
 
 	"github.com/deckhouse/virtualization-controller/pkg/controller/supplements"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vi/internal/source"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
 )
 
-//go:generate moq -rm -out mock.go . DiskService Sources
+//go:generate moq -rm -out mock.go . DiskService Sources StorageClassService
 
 type Sources interface {
 	Changed(ctx context.Context, vi *virtv2.VirtualImage) bool
@@ -36,6 +36,14 @@ type Sources interface {
 }
 
 type DiskService interface {
-	GetStorageClass(ctx context.Context, storageClassName *string) (*storev1.StorageClass, error)
+	GetStorageClass(ctx context.Context, storageClassName *string) (*storagev1.StorageClass, error)
+	GetPersistentVolumeClaim(ctx context.Context, sup *supplements.Generator) (*corev1.PersistentVolumeClaim, error)
+}
+
+type StorageClassService interface {
+	IsStorageClassAllowed(sc string) bool
+	GetModuleStorageClass(ctx context.Context) (*storagev1.StorageClass, error)
+	GetDefaultStorageClass(ctx context.Context) (*storagev1.StorageClass, error)
+	GetStorageClass(ctx context.Context, sc string) (*storagev1.StorageClass, error)
 	GetPersistentVolumeClaim(ctx context.Context, sup *supplements.Generator) (*corev1.PersistentVolumeClaim, error)
 }