CR fixes

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

pkg/controller/clone/common.go

@@ -146,12 +146,13 @@ func GetStorageClassForClaim(ctx context.Context, c client.Client, pvc *corev1.P
 }
 
 func getSnapshotClassForClaim(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) (*string, error) {
-	if pvc.Spec.StorageClassName == nil || *pvc.Spec.StorageClassName == "" {
-		return nil, nil
+	sc, err := cc.GetStorageClassByNameWithK8sFallback(ctx, c, pvc.Spec.StorageClassName)
+	if err != nil || sc == nil {
+		return nil, err
 	}
 
 	sp := &cdiv1.StorageProfile{}
-	exists, err := getResource(ctx, c, "", *pvc.Spec.StorageClassName, sp)
+	exists, err := getResource(ctx, c, "", sc.Name, sp)
 	if err != nil {
 		return nil, err
 	}
@@ -247,7 +248,9 @@ func getCommonSnapshotClass(ctx context.Context, c client.Client, pvcs ...*corev
 }
 
 // GetCompatibleVolumeSnapshotClass returns a VolumeSnapshotClass name that works for all PVCs
-func GetCompatibleVolumeSnapshotClass(ctx context.Context, c client.Client, log logr.Logger, pvcs ...*corev1.PersistentVolumeClaim) (*string, error) {
+// Note the last PVC passed is considered the target PVC for logs and events
+func GetCompatibleVolumeSnapshotClass(ctx context.Context, c client.Client, log logr.Logger, recorder record.EventRecorder, pvcs ...*corev1.PersistentVolumeClaim) (*string, error) {
+	targetClaim := pvcs[len(pvcs)-1]
 	driver, err := GetCommonDriver(ctx, c, pvcs...)
 	if err != nil {
 		return nil, err
@@ -261,7 +264,7 @@ func GetCompatibleVolumeSnapshotClass(ctx context.Context, c client.Client, log
 		return nil, err
 	}
 
-	return cc.GetVolumeSnapshotClass(context.TODO(), c, *driver, snapshotClassName, log)
+	return cc.GetVolumeSnapshotClass(context.TODO(), c, targetClaim, *driver, snapshotClassName, log, recorder)
 }
 
 // SameVolumeMode returns true if all pvcs have the same volume mode

pkg/controller/clone/planner.go

@@ -340,7 +340,7 @@ func (p *Planner) computeStrategyForSourcePVC(ctx context.Context, args *ChooseS
 	}
 
 	if strategy == cdiv1.CloneStrategySnapshot {
-		n, err := GetCompatibleVolumeSnapshotClass(ctx, p.Client, args.Log, sourceClaim, args.TargetClaim)
+		n, err := GetCompatibleVolumeSnapshotClass(ctx, p.Client, args.Log, p.Recorder, sourceClaim, args.TargetClaim)
 		if err != nil {
 			return nil, err
 		}
@@ -660,7 +660,7 @@ func (p *Planner) planSnapshotFromPVC(ctx context.Context, args *PlanArgs) ([]Ph
 		return nil, fmt.Errorf("source claim does not exist")
 	}
 
-	vsc, err := GetCompatibleVolumeSnapshotClass(ctx, p.Client, args.Log, args.TargetClaim)
+	vsc, err := GetCompatibleVolumeSnapshotClass(ctx, p.Client, args.Log, p.Recorder, args.TargetClaim)
 	if err != nil {
 		return nil, err
 	}

pkg/controller/common/util.go

@@ -283,6 +283,15 @@ const (
 	// SourceVDDK is the source type of VDDK
 	SourceVDDK = "vddk"
 
+	// VolumeSnapshotClassSelected reports that a VolumeSnapshotClass was selected
+	VolumeSnapshotClassSelected = "VolumeSnapshotClassSelected"
+	// MessageStorageProfileVolumeSnapshotClassSelected reports that a VolumeSnapshotClass was selected according to StorageProfile
+	MessageStorageProfileVolumeSnapshotClassSelected = "VolumeSnapshotClass selected according to StorageProfile"
+	// MessageDefaultVolumeSnapshotClassSelected reports that the default VolumeSnapshotClass was selected
+	MessageDefaultVolumeSnapshotClassSelected = "Default VolumeSnapshotClass selected"
+	// MessageFirstVolumeSnapshotClassSelected reports that the first VolumeSnapshotClass was selected
+	MessageFirstVolumeSnapshotClassSelected = "First VolumeSnapshotClass selected"
+
 	// ClaimLost reason const
 	ClaimLost = "ClaimLost"
 	// NotFound reason const
@@ -1872,7 +1881,7 @@ func ValidateSnapshotCloneProvisioners(ctx context.Context, c client.Client, sna
 }
 
 // GetSnapshotClassForSmartClone looks up the snapshot class based on the storage class
-func GetSnapshotClassForSmartClone(dvName string, targetPvcStorageClassName, snapshotClassName *string, log logr.Logger, client client.Client) (string, error) {
+func GetSnapshotClassForSmartClone(pvc *corev1.PersistentVolumeClaim, targetPvcStorageClassName, snapshotClassName *string, log logr.Logger, client client.Client, recorder record.EventRecorder) (string, error) {
 	logger := log.WithName("GetSnapshotClassForSmartClone").V(3)
 	// Check if relevant CRDs are available
 	if !isCsiCrdsDeployed(client, log) {
@@ -1889,13 +1898,13 @@ func GetSnapshotClassForSmartClone(dvName string, targetPvcStorageClassName, sna
 		return "", nil
 	}
 
-	vscName, err := GetVolumeSnapshotClass(context.TODO(), client, targetStorageClass.Provisioner, snapshotClassName, logger)
+	vscName, err := GetVolumeSnapshotClass(context.TODO(), client, pvc, targetStorageClass.Provisioner, snapshotClassName, logger, recorder)
 	if err != nil {
 		return "", err
 	}
-	if vscName != nil {
+	if vscName != nil && pvc != nil {
 		logger.Info("smart-clone is applicable for datavolume", "datavolume",
-			dvName, "snapshot class", *vscName)
+			pvc.Name, "snapshot class", *vscName)
 		return *vscName, nil
 	}
 
@@ -1904,15 +1913,24 @@ func GetSnapshotClassForSmartClone(dvName string, targetPvcStorageClassName, sna
 }
 
 // GetVolumeSnapshotClass looks up the snapshot class based on the driver and an optional specific name
-func GetVolumeSnapshotClass(ctx context.Context, c client.Client, driver string, snapshotClassName *string, log logr.Logger) (*string, error) {
+func GetVolumeSnapshotClass(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim, driver string, snapshotClassName *string, log logr.Logger, recorder record.EventRecorder) (*string, error) {
 	logger := log.WithName("GetVolumeSnapshotClass").V(3)
+
+	logEvent := func(message, vscName string) {
+		logger.Info(message, "name", vscName)
+		if pvc != nil {
+			msg := fmt.Sprintf("%s %s", message, vscName)
+			recorder.Event(pvc, corev1.EventTypeNormal, VolumeSnapshotClassSelected, msg)
+		}
+	}
+
 	if snapshotClassName != nil {
 		vsc := &snapshotv1.VolumeSnapshotClass{}
 		if err := c.Get(context.TODO(), types.NamespacedName{Name: *snapshotClassName}, vsc); err != nil {
 			return nil, err
 		}
 		if vsc.Driver == driver {
-			logger.Info("VolumeSnapshotClass selected according to StorageProfile", "name", vsc.Name)
+			logEvent(MessageStorageProfileVolumeSnapshotClassSelected, vsc.Name)
 			return &vsc.Name, nil
 		}
 		return nil, nil
@@ -1930,7 +1948,7 @@ func GetVolumeSnapshotClass(ctx context.Context, c client.Client, driver string,
 	for _, vsc := range vscList.Items {
 		if vsc.Driver == driver {
 			if vsc.Annotations[AnnDefaultSnapshotClass] == "true" {
-				logger.Info("Default VolumeSnapshotClass selected", "name", vsc.Name)
+				logEvent(MessageDefaultVolumeSnapshotClassSelected, vsc.Name)
 				return &vsc.Name, nil
 			}
 			candidates = append(candidates, vsc.Name)
@@ -1939,7 +1957,7 @@ func GetVolumeSnapshotClass(ctx context.Context, c client.Client, driver string,
 
 	if len(candidates) > 0 {
 		sort.Strings(candidates)
-		logger.Info("First VolumeSnapshotClass selected", "name", candidates[0], "candidates", len(candidates))
+		logEvent(MessageFirstVolumeSnapshotClassSelected, candidates[0])
 		return &candidates[0], nil
 	}
 

pkg/controller/dataimportcron-controller.go

@@ -692,7 +692,7 @@ func (r *DataImportCronReconciler) handleSnapshot(ctx context.Context, dataImpor
 	if err := r.client.Get(ctx, types.NamespacedName{Name: desiredStorageClass.Name}, storageProfile); err != nil {
 		return err
 	}
-	className, err := cc.GetSnapshotClassForSmartClone(pvc.Name, &desiredStorageClass.Name, storageProfile.Status.SnapshotClass, r.log, r.client)
+	className, err := cc.GetSnapshotClassForSmartClone(pvc, &desiredStorageClass.Name, storageProfile.Status.SnapshotClass, r.log, r.client, r.recorder)
 	if err != nil {
 		return err
 	}

pkg/controller/storageprofile-controller.go

@@ -15,6 +15,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -34,6 +35,8 @@ import (
 	"kubevirt.io/containerized-data-importer/pkg/util"
 )
 
+const storageProfileControllerName = "storageprofile-controller"
+
 var (
 	// IncompleteProfileGauge is the metric we use to alert about incomplete storage profiles
 	IncompleteProfileGauge = prometheus.NewGauge(
@@ -54,6 +57,7 @@ type StorageProfileReconciler struct {
 	client client.Client
 	// use this for getting any resources not in the install namespace or cluster scope
 	uncachedClient  client.Client
+	recorder        record.EventRecorder
 	scheme          *runtime.Scheme
 	log             logr.Logger
 	installerLabels map[string]string
@@ -92,7 +96,7 @@ func (r *StorageProfileReconciler) reconcileStorageProfile(sc *storagev1.Storage
 
 	storageProfile.Status.StorageClass = &sc.Name
 	storageProfile.Status.Provisioner = &sc.Provisioner
-	snapClass, err := cc.GetSnapshotClassForSmartClone("", &sc.Name, storageProfile.Spec.SnapshotClass, r.log, r.client)
+	snapClass, err := cc.GetSnapshotClassForSmartClone(nil, &sc.Name, storageProfile.Spec.SnapshotClass, r.log, r.client, r.recorder)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
@@ -349,16 +353,18 @@ func NewStorageProfileController(mgr manager.Manager, log logr.Logger, installer
 	if err != nil {
 		return nil, err
 	}
+
 	reconciler := &StorageProfileReconciler{
 		client:          mgr.GetClient(),
 		uncachedClient:  uncachedClient,
+		recorder:        mgr.GetEventRecorderFor(storageProfileControllerName),
 		scheme:          mgr.GetScheme(),
-		log:             log.WithName("storageprofile-controller"),
+		log:             log.WithName(storageProfileControllerName),
 		installerLabels: installerLabels,
 	}
 
 	storageProfileController, err := controller.New(
-		"storageprofile-controller",
+		dataImportControllerName,
 		mgr,
 		controller.Options{Reconciler: reconciler, MaxConcurrentReconciles: 3})
 	if err != nil {