fix(vd): set validator warnings instead of errors

- VirtualDisk webhooks no longer return an error if an insufficient pvc size is specified or if the vd is created from an ISO image. Instead, a warning will be thrown. - Creating a disk from an HTTP source with an ISO suffix is allowed, but after uploading to the dvcr, we will identify that image format is iso and mark the disk as Failed. Signed-off-by: Isteb4k <dmitry.rakitin@flant.com>
deckhouse · Sep 4, 2024 · b173b08 · b173b08
1 parent ce38e10
commit b173b08
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 72 deletions.
diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/validators/iso_source_validator.go b/images/virtualization-artifact/pkg/controller/vd/internal/validators/iso_source_validator.go
@@ -18,13 +18,12 @@ package validators
 
 import (
 	"context"
-	"errors"
-	"strings"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/deckhouse/virtualization-controller/pkg/controller"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/service"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/source"
 	"github.com/deckhouse/virtualization-controller/pkg/imageformat"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
@@ -43,35 +42,26 @@ func (v *ISOSourceValidator) ValidateCreate(ctx context.Context, vd *virtv2.Virt
 		return nil, nil
 	}
 
-	switch vd.Spec.DataSource.Type {
-	case virtv2.DataSourceTypeObjectRef:
-		if vd.Spec.DataSource.ObjectRef == nil {
-			return nil, errors.New("data source object ref is omitted, but expected")
-		}
+	if vd.Spec.DataSource.Type != virtv2.DataSourceTypeObjectRef || vd.Spec.DataSource.ObjectRef == nil {
+		return nil, nil
+	}
 
-		switch vd.Spec.DataSource.ObjectRef.Kind {
-		case virtv2.VirtualDiskObjectRefKindVirtualImage,
-			virtv2.VirtualDiskObjectRefKindClusterVirtualImage:
-			dvcrDataSource, err := controller.NewDVCRDataSourcesForVMD(ctx, vd.Spec.DataSource, vd, v.client)
-			if err != nil {
-				return nil, err
-			}
-
-			if !dvcrDataSource.IsReady() {
-				return nil, nil
-			}
-
-			if imageformat.IsISO(dvcrDataSource.GetFormat()) {
-				return nil, source.ErrISOSourceNotSupported
-			}
+	switch vd.Spec.DataSource.ObjectRef.Kind {
+	case virtv2.VirtualDiskObjectRefKindVirtualImage,
+		virtv2.VirtualDiskObjectRefKindClusterVirtualImage:
+		dvcrDataSource, err := controller.NewDVCRDataSourcesForVMD(ctx, vd.Spec.DataSource, vd, v.client)
+		if err != nil {
+			return nil, err
 		}
-	case virtv2.DataSourceTypeHTTP:
-		if vd.Spec.DataSource.HTTP == nil {
-			return nil, errors.New("data source http is omitted, but expected")
+
+		if !dvcrDataSource.IsReady() {
+			return nil, nil
 		}
 
-		if strings.HasSuffix(strings.ToLower(vd.Spec.DataSource.HTTP.URL), imageformat.FormatISO) {
-			return nil, source.ErrISOSourceNotSupported
+		if imageformat.IsISO(dvcrDataSource.GetFormat()) {
+			return admission.Warnings{
+				service.CapitalizeFirstLetter(source.ErrISOSourceNotSupported.Error()),
+			}, nil
 		}
 	}
 
@@ -83,41 +73,28 @@ func (v *ISOSourceValidator) ValidateUpdate(ctx context.Context, _, newVD *virtv
 		return nil, nil
 	}
 
-	switch newVD.Spec.DataSource.Type {
-	case virtv2.DataSourceTypeObjectRef:
-		if newVD.Spec.DataSource.ObjectRef == nil {
-			return nil, errors.New("data source object ref is omitted, but expected")
-		}
+	if newVD.Spec.DataSource.Type != virtv2.DataSourceTypeObjectRef || newVD.Spec.DataSource.ObjectRef == nil {
+		return nil, nil
+	}
 
-		switch newVD.Spec.DataSource.ObjectRef.Kind {
-		case virtv2.VirtualDiskObjectRefKindVirtualImage,
-			virtv2.VirtualDiskObjectRefKindClusterVirtualImage:
-			dvcrDataSource, err := controller.NewDVCRDataSourcesForVMD(ctx, newVD.Spec.DataSource, newVD, v.client)
-			if err != nil {
-				return nil, err
-			}
-
-			if !dvcrDataSource.IsReady() {
-				return nil, nil
-			}
-
-			if imageformat.IsISO(dvcrDataSource.GetFormat()) {
-				return nil, source.ErrISOSourceNotSupported
-			}
+	switch newVD.Spec.DataSource.ObjectRef.Kind {
+	case virtv2.VirtualDiskObjectRefKindVirtualImage,
+		virtv2.VirtualDiskObjectRefKindClusterVirtualImage:
+		dvcrDataSource, err := controller.NewDVCRDataSourcesForVMD(ctx, newVD.Spec.DataSource, newVD, v.client)
+		if err != nil {
+			return nil, err
 		}
 
-		return nil, nil
-	case virtv2.DataSourceTypeHTTP:
-		if newVD.Spec.DataSource.HTTP == nil {
-			return nil, errors.New("data source http is omitted, but expected")
+		if !dvcrDataSource.IsReady() {
+			return nil, nil
 		}
 
-		if strings.HasSuffix(strings.ToLower(newVD.Spec.DataSource.HTTP.URL), imageformat.FormatISO) {
-			return nil, source.ErrISOSourceNotSupported
+		if imageformat.IsISO(dvcrDataSource.GetFormat()) {
+			return admission.Warnings{
+				service.CapitalizeFirstLetter(source.ErrISOSourceNotSupported.Error()),
+			}, nil
 		}
-
-		return nil, nil
-	default:
-		return nil, nil
 	}
+
+	return nil, nil
 }
diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/validators/pvc_size_validator.go b/images/virtualization-artifact/pkg/controller/vd/internal/validators/pvc_size_validator.go
@@ -104,11 +104,16 @@ func (v *PVCSizeValidator) ValidateCreate(ctx context.Context, vd *virtv2.Virtua
 	}
 
 	_, err := service.GetValidatedPVCSize(vd.Spec.PersistentVolumeClaim.Size, unpackedSize)
-	if err != nil {
+	switch {
+	case err == nil:
+		return nil, nil
+	case errors.Is(err, service.ErrInsufficientPVCSize):
+		return admission.Warnings{
+			service.CapitalizeFirstLetter(err.Error()),
+		}, nil
+	default:
 		return nil, err
 	}
-
-	return nil, nil
 }
 
 func (v *PVCSizeValidator) ValidateUpdate(ctx context.Context, oldVD, newVD *virtv2.VirtualDisk) (admission.Warnings, error) {
@@ -136,7 +141,7 @@ func (v *PVCSizeValidator) ValidateUpdate(ctx context.Context, oldVD, newVD *vir
 		return nil, nil
 	}
 
-	var unpackedBytes string
+	var unpackedSize resource.Quantity
 
 	switch newVD.Spec.DataSource.ObjectRef.Kind {
 	case virtv2.VirtualDiskObjectRefKindVirtualImage,
@@ -150,22 +155,51 @@ func (v *PVCSizeValidator) ValidateUpdate(ctx context.Context, oldVD, newVD *vir
 			return nil, nil
 		}
 
-		unpackedBytes = dvcrDataSource.GetSize().UnpackedBytes
+		unpackedSize, err = resource.ParseQuantity(dvcrDataSource.GetSize().UnpackedBytes)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse unpacked bytes %s: %w", unpackedSize.String(), err)
+		}
+
+	case virtv2.VirtualDiskObjectRefKindVirtualDiskSnapshot:
+		vdSnapshot, err := helper.FetchObject(ctx, types.NamespacedName{
+			Name:      newVD.Spec.DataSource.ObjectRef.Name,
+			Namespace: newVD.Namespace,
+		}, v.client, &virtv2.VirtualDiskSnapshot{})
+		if err != nil {
+			return nil, err
+		}
+
+		if vdSnapshot == nil || vdSnapshot.Status.Phase != virtv2.VirtualDiskSnapshotPhaseReady {
+			return nil, nil
+		}
+
+		vs, err := helper.FetchObject(ctx, types.NamespacedName{
+			Name:      vdSnapshot.Status.VolumeSnapshotName,
+			Namespace: vdSnapshot.Namespace,
+		}, v.client, &vsv1.VolumeSnapshot{})
+		if err != nil {
+			return nil, err
+		}
+
+		if vs == nil || vs.Status == nil || vs.Status.RestoreSize == nil {
+			return nil, nil
+		}
+
+		unpackedSize = *vs.Status.RestoreSize
 
-	// TODO validate for snapshot kind also.
 	default:
 		return nil, nil
 	}
 
-	unpackedSize, err := resource.ParseQuantity(unpackedBytes)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse unpacked bytes %s: %w", unpackedBytes, err)
-	}
-
-	_, err = service.GetValidatedPVCSize(newVD.Spec.PersistentVolumeClaim.Size, unpackedSize)
-	if err != nil {
+	_, err := service.GetValidatedPVCSize(newVD.Spec.PersistentVolumeClaim.Size, unpackedSize)
+	switch {
+	case err == nil:
+		return nil, nil
+	case errors.Is(err, service.ErrInsufficientPVCSize):
+		return admission.Warnings{
+			service.CapitalizeFirstLetter(err.Error()),
+		}, nil
+	default:
 		return nil, err
 	}
-
-	return nil, nil
 }