Allow ImmediateBinding annotation when using populators

pkg/controller/clone-controller.go

@@ -722,7 +722,7 @@ func MakeCloneSourcePodSpec(sourceVolumeMode corev1.PersistentVolumeMode, image,
 	}
 
 	pod.Spec.Containers[0].Env = append(pod.Spec.Containers[0].Env, addVars...)
-	setPodPvcAnnotations(pod, targetPvc)
+	cc.SetPvcAllowedAnnotations(pod, targetPvc)
 	cc.SetRestrictedSecurityContext(&pod.Spec)
 	return pod
 }

pkg/controller/common/util.go

@@ -648,6 +648,12 @@ func GetPreallocation(ctx context.Context, client client.Client, preallocation *
 	return cdiconfig.Status.Preallocation
 }
 
+// ImmediateBindingRequested returns if an object has the ImmediateBinding annotation
+func ImmediateBindingRequested(obj metav1.Object) bool {
+	_, isImmediateBindingRequested := obj.GetAnnotations()[AnnImmediateBinding]
+	return isImmediateBindingRequested
+}
+
 // GetPriorityClass gets PVC priority class
 func GetPriorityClass(pvc *corev1.PersistentVolumeClaim) string {
 	anno := pvc.GetAnnotations()
@@ -1327,7 +1333,7 @@ func GetCloneSourceInfo(dv *cdiv1.DataVolume) (sourceType, sourceName, sourceNam
 // IsWaitForFirstConsumerEnabled tells us if we should respect "real" WFFC behavior or just let our worker pods randomly spawn
 func IsWaitForFirstConsumerEnabled(obj metav1.Object, gates featuregates.FeatureGates) (bool, error) {
 	// when PVC requests immediateBinding it cannot honor wffc logic
-	_, isImmediateBindingRequested := obj.GetAnnotations()[AnnImmediateBinding]
+	isImmediateBindingRequested := ImmediateBindingRequested(obj)
 	pvcHonorWaitForFirstConsumer := !isImmediateBindingRequested
 	globalHonorWaitForFirstConsumer, err := gates.HonorWaitForFirstConsumerEnabled()
 	if err != nil {
@@ -1337,6 +1343,18 @@ func IsWaitForFirstConsumerEnabled(obj metav1.Object, gates featuregates.Feature
 	return pvcHonorWaitForFirstConsumer && globalHonorWaitForFirstConsumer, nil
 }
 
+// AddImmediateBindingAnnotationIfWFFCDisabled adds the immediateBinding annotation if wffc feature gate is disabled
+func AddImmediateBindingAnnotationIfWFFCDisabled(obj metav1.Object, gates featuregates.FeatureGates) error {
+	globalHonorWaitForFirstConsumer, err := gates.HonorWaitForFirstConsumerEnabled()
+	if err != nil {
+		return err
+	}
+	if !globalHonorWaitForFirstConsumer {
+		AddAnnotation(obj, AnnImmediateBinding, "")
+	}
+	return nil
+}
+
 // GetRequiredSpace calculates space required taking file system overhead into account
 func GetRequiredSpace(filesystemOverhead float64, requestedSpace int64) int64 {
 	// the `image` has to be aligned correctly, so the space requested has to be aligned to
@@ -1867,3 +1885,21 @@ func OwnedByDataVolume(obj metav1.Object) bool {
 	owner := metav1.GetControllerOf(obj)
 	return owner != nil && owner.Kind == "DataVolume"
 }
+
+// SetPvcAllowedAnnotations applies PVC annotations on the given obj
+func SetPvcAllowedAnnotations(obj metav1.Object, pvc *corev1.PersistentVolumeClaim) {
+	allowedAnnotations := map[string]string{
+		AnnPodNetwork:              "",
+		AnnPodSidecarInjection:     AnnPodSidecarInjectionDefault,
+		AnnPodMultusDefaultNetwork: ""}
+	for ann, def := range allowedAnnotations {
+		val, ok := pvc.Annotations[ann]
+		if !ok && def != "" {
+			val = def
+		}
+		if val != "" {
+			klog.V(1).Info("Applying PVC annotation", "Name", obj.GetName(), ann, val)
+			AddAnnotation(obj, ann, val)
+		}
+	}
+}

pkg/controller/datavolume/clone-controller-base.go

@@ -191,6 +191,9 @@ func (r *CloneReconcilerBase) updatePVCForPopulation(dataVolume *cdiv1.DataVolum
 	if err := addCloneToken(dataVolume, pvc); err != nil {
 		return err
 	}
+	if err := cc.AddImmediateBindingAnnotationIfWFFCDisabled(pvc, r.featureGates); err != nil {
+		return err
+	}
 	if isCrossNamespaceClone(dataVolume) {
 		_, _, sourcNamespace := cc.GetCloneSourceInfo(dataVolume)
 		cc.AddAnnotation(pvc, populators.AnnDataSourceNamespace, sourcNamespace)

pkg/controller/datavolume/controller-base.go

@@ -1150,8 +1150,9 @@ func (r *ReconcilerBase) shouldBeMarkedPendingPopulation(pvc *corev1.PersistentV
 		return false, err
 	}
 	nodeName := pvc.Annotations[cc.AnnSelectedNode]
+	immediateBindingRequested := cc.ImmediateBindingRequested(pvc)
 
-	return wffc && nodeName == "", nil
+	return wffc && nodeName == "" && !immediateBindingRequested, nil
 }
 
 // handlePvcCreation works as a wrapper for non-clone PVC creation and error handling
@@ -1200,11 +1201,6 @@ func (r *ReconcilerBase) shouldUseCDIPopulator(syncState *dvSyncState) (bool, er
 		log.Info("Not using CDI populators, currently we don't support populators with retainAfterCompletion annotation")
 		return false, nil
 	}
-	// currently populators don't support immediate bind annotation so don't use populators in that case
-	if forceBind := dv.Annotations[cc.AnnImmediateBinding]; forceBind == "true" {
-		log.Info("Not using CDI populators, currently we don't support populators with bind immediate annotation")
-		return false, nil
-	}
 	// currently we don't support populator with import source of VDDK or Imageio
 	// or clone either from PVC nor snapshot
 	if dv.Spec.Source.Imageio != nil ||
@@ -1213,23 +1209,6 @@ func (r *ReconcilerBase) shouldUseCDIPopulator(syncState *dvSyncState) (bool, er
 		return false, nil
 	}
 
-	wffc, err := r.storageClassWaitForFirstConsumer(syncState.pvcSpec.StorageClassName)
-	if err != nil {
-		return false, err
-	}
-
-	honorWaitForFirstConsumerEnabled, err := r.featureGates.HonorWaitForFirstConsumerEnabled()
-	if err != nil {
-		return false, err
-	}
-	// currently since we don't support force bind in populators in case the storage class
-	// is wffc but the honorWaitForFirstConsumer feature gate is disabled we can't
-	// do immediate bind anyways, instead do regular flow
-	if wffc && !honorWaitForFirstConsumerEnabled {
-		log.Info("Not using CDI populators, currently we don't WFFC storage with disabled honorWaitForFirstConsumer feature gate")
-		return false, nil
-	}
-
 	usePopulator, err := storageClassCSIDriverExists(r.client, r.log, syncState.pvcSpec.StorageClassName)
 	if err != nil {
 		return false, err

pkg/controller/datavolume/import-controller.go

@@ -131,6 +131,9 @@ func (r *ImportReconciler) updatePVCForPopulation(dataVolume *cdiv1.DataVolume,
 		dataVolume.Spec.Source.Blank == nil {
 		return errors.Errorf("no source set for import datavolume")
 	}
+	if err := cc.AddImmediateBindingAnnotationIfWFFCDisabled(pvc, r.featureGates); err != nil {
+		return err
+	}
 	apiGroup := cc.AnnAPIGroup
 	pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{
 		APIGroup: &apiGroup,

pkg/controller/datavolume/import-controller_test.go

@@ -194,7 +194,7 @@ var _ = Describe("All DataVolume Tests", func() {
 				},
 			}
 			dv := NewImportDataVolume("test-dv")
-			reconciler = createImportReconciler(dv, sc, csiDriver)
+			reconciler = createImportReconcilerWFFCDisabled(dv, sc, csiDriver)
 			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
 			Expect(err).ToNot(HaveOccurred())
 			pvc := &corev1.PersistentVolumeClaim{}
@@ -208,6 +208,8 @@ var _ = Describe("All DataVolume Tests", func() {
 			Expect(pvc.Spec.DataSourceRef.Name).To(Equal(importSourceName))
 			Expect(pvc.Spec.DataSourceRef.Kind).To(Equal(cdiv1.VolumeImportSourceRef))
 			Expect(pvc.GetAnnotations()[AnnUsePopulator]).To(Equal("true"))
+			_, annExists := pvc.Annotations[AnnImmediateBinding]
+			Expect(annExists).To(BeTrue())
 		})
 
 		It("Should report import population progress when use populators", func() {
@@ -1566,7 +1568,6 @@ var _ = Describe("All DataVolume Tests", func() {
 			Entry("AnnUsePopulator=true return true", AnnUsePopulator, "true", true),
 			Entry("AnnUsePopulator=false return false", AnnUsePopulator, "false", false),
 			Entry("AnnPodRetainAfterCompletion return false", AnnPodRetainAfterCompletion, "true", false),
-			Entry("AnnImmediateBinding return false", AnnImmediateBinding, "true", false),
 		)
 
 		DescribeTable("Should return false if source is", func(source *cdiv1.DataVolumeSource) {
@@ -1588,19 +1589,20 @@ var _ = Describe("All DataVolume Tests", func() {
 			}),
 		)
 
-		It("Should return false if storage class has wffc bindingMode and honorWaitForFirstConsumer feature gate is disabled", func() {
+		It("Should return true if storage class has wffc bindingMode and honorWaitForFirstConsumer feature gate is disabled", func() {
 			sc := createStorageClassWithBindingMode(scName,
 				map[string]string{
 					AnnDefaultStorageClass: "true",
 				},
 				storagev1.VolumeBindingWaitForFirstConsumer)
+			sc.Provisioner = "csi-plugin"
 			httpSource := &cdiv1.DataVolumeSource{
 				HTTP: &cdiv1.DataVolumeSourceHTTP{},
 			}
 			storageSpec := &cdiv1.StorageSpec{}
 			dv := createDataVolumeWithStorageAPI("test-dv", metav1.NamespaceDefault, httpSource, storageSpec)
 
-			reconciler = createImportReconcilerWFFCDisabled(sc)
+			reconciler = createImportReconcilerWFFCDisabled(sc, csiDriver)
 			syncState := dvSyncState{
 				dvMutated: dv,
 				pvcSpec: &corev1.PersistentVolumeClaimSpec{
@@ -1609,7 +1611,7 @@ var _ = Describe("All DataVolume Tests", func() {
 			}
 			usePopulator, err := reconciler.shouldUseCDIPopulator(&syncState)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(usePopulator).To(BeFalse())
+			Expect(usePopulator).To(BeTrue())
 		})
 
 		It("Should return false if storage class doesnt exist", func() {

pkg/controller/datavolume/pvc-clone-controller_test.go

@@ -123,7 +123,7 @@ var _ = Describe("All DataVolume Tests", func() {
 					dv.Finalizers = append(dv.Finalizers, crossNamespaceFinalizer)
 				}
 				srcPvc := CreatePvcInStorageClass("test", sourceNamespace, &scName, nil, nil, corev1.ClaimBound)
-				reconciler = createCloneReconciler(storageClass, csiDriver, dv, srcPvc)
+				reconciler = createCloneReconcilerWFFCDisabled(storageClass, csiDriver, dv, srcPvc)
 				result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(result.Requeue).To(BeFalse())
@@ -146,6 +146,8 @@ var _ = Describe("All DataVolume Tests", func() {
 				Expect(pvc.Spec.DataSourceRef.Name).To(Equal(cloneSourceName))
 				Expect(pvc.Spec.DataSourceRef.Kind).To(Equal(cdiv1.VolumeCloneSourceRef))
 				Expect(pvc.GetAnnotations()[AnnUsePopulator]).To(Equal("true"))
+				_, annExists := pvc.Annotations[AnnImmediateBinding]
+				Expect(annExists).To(BeTrue())
 				vcs := &cdiv1.VolumeCloneSource{}
 				err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: cloneSourceName, Namespace: sourceNamespace}, vcs)
 				Expect(err).ToNot(HaveOccurred())
@@ -711,6 +713,20 @@ var _ = Describe("All DataVolume Tests", func() {
 	})
 })
 
+func createCloneReconcilerWFFCDisabled(objects ...runtime.Object) *PvcCloneReconciler {
+	cdiConfig := MakeEmptyCDIConfigSpec(common.ConfigName)
+	cdiConfig.Status = cdiv1.CDIConfigStatus{
+		ScratchSpaceStorageClass: testStorageClass,
+	}
+	cdiConfig.Spec.FeatureGates = []string{}
+
+	objs := []runtime.Object{}
+	objs = append(objs, objects...)
+	objs = append(objs, cdiConfig)
+
+	return createCloneReconcilerWithoutConfig(objs...)
+}
+
 func createCloneReconciler(objects ...runtime.Object) *PvcCloneReconciler {
 	cdiConfig := MakeEmptyCDIConfigSpec(common.ConfigName)
 	cdiConfig.Status = cdiv1.CDIConfigStatus{

pkg/controller/datavolume/snapshot-clone-controller_test.go

@@ -268,7 +268,7 @@ var _ = Describe("All DataVolume Tests", func() {
 						dv.Finalizers = append(dv.Finalizers, crossNamespaceFinalizer)
 					}
 					snapshot := createSnapshotInVolumeSnapshotClass("test-snap", sourceNamespace, &expectedSnapshotClass, nil, nil, true)
-					reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, snapshot)
+					reconciler = createSnapshotCloneReconcilerWFFCDisabled(storageClass, csiDriver, dv, snapshot)
 					result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
 					Expect(err).ToNot(HaveOccurred())
 					Expect(result.Requeue).To(BeFalse())
@@ -291,6 +291,8 @@ var _ = Describe("All DataVolume Tests", func() {
 					Expect(pvc.Spec.DataSourceRef.Name).To(Equal(cloneSourceName))
 					Expect(pvc.Spec.DataSourceRef.Kind).To(Equal(cdiv1.VolumeCloneSourceRef))
 					Expect(pvc.GetAnnotations()[AnnUsePopulator]).To(Equal("true"))
+					_, annExists := pvc.Annotations[AnnImmediateBinding]
+					Expect(annExists).To(BeTrue())
 					vcs := &cdiv1.VolumeCloneSource{}
 					err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: cloneSourceName, Namespace: sourceNamespace}, vcs)
 					Expect(err).ToNot(HaveOccurred())
@@ -474,6 +476,20 @@ var _ = Describe("All DataVolume Tests", func() {
 	})
 })
 
+func createSnapshotCloneReconcilerWFFCDisabled(objects ...runtime.Object) *SnapshotCloneReconciler {
+	cdiConfig := MakeEmptyCDIConfigSpec(common.ConfigName)
+	cdiConfig.Status = cdiv1.CDIConfigStatus{
+		ScratchSpaceStorageClass: testStorageClass,
+	}
+	cdiConfig.Spec.FeatureGates = []string{}
+
+	objs := []runtime.Object{}
+	objs = append(objs, objects...)
+	objs = append(objs, cdiConfig)
+
+	return createSnapshotCloneReconcilerWithoutConfig(objs...)
+}
+
 func createSnapshotCloneReconciler(objects ...runtime.Object) *SnapshotCloneReconciler {
 	cdiConfig := MakeEmptyCDIConfigSpec(common.ConfigName)
 	cdiConfig.Status = cdiv1.CDIConfigStatus{

pkg/controller/datavolume/upload-controller.go

@@ -119,6 +119,9 @@ func (r *UploadReconciler) updatePVCForPopulation(dataVolume *cdiv1.DataVolume,
 	if dataVolume.Spec.Source.Upload == nil {
 		return errors.Errorf("no source set for upload datavolume")
 	}
+	if err := cc.AddImmediateBindingAnnotationIfWFFCDisabled(pvc, r.featureGates); err != nil {
+		return err
+	}
 	apiGroup := cc.AnnAPIGroup
 	pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{
 		APIGroup: &apiGroup,

pkg/controller/import-controller.go

@@ -1180,7 +1180,7 @@ func setImporterPodCommons(pod *corev1.Pod, podEnvVar *importPodEnvVar, pvc *cor
 
 	pod.Spec.Containers[0].Env = makeImportEnv(podEnvVar, ownerUID)
 
-	setPodPvcAnnotations(pod, pvc)
+	cc.SetPvcAllowedAnnotations(pod, pvc)
 }
 
 func makeImporterContainerSpec(image, verbose, pullPolicy string) *corev1.Container {

pkg/controller/populators/populator-base.go

@@ -199,6 +199,7 @@ func (r *ReconcilerBase) createPVCPrime(pvc *corev1.PersistentVolumeClaim, sourc
 			Kind:    "PersistentVolumeClaim",
 		}),
 	}
+	cc.SetPvcAllowedAnnotations(pvcPrime, pvc)
 	util.SetRecommendedLabels(pvcPrime, r.installerLabels, "cdi-controller")
 
 	// We use the populator-specific pvcModifierFunc to add required annotations

pkg/controller/populators/util.go

@@ -120,7 +120,8 @@ func claimReadyForPopulation(ctx context.Context, c client.Client, pvc *corev1.P
 
 	if storageClass.VolumeBindingMode != nil && *storageClass.VolumeBindingMode == storagev1.VolumeBindingWaitForFirstConsumer {
 		nodeName = pvc.Annotations[cc.AnnSelectedNode]
-		if nodeName == "" {
+		isImmediateBindingRequested := cc.ImmediateBindingRequested(pvc)
+		if nodeName == "" && !isImmediateBindingRequested {
 			// Wait for the PVC to get a node name before continuing
 			return false, nodeName, nil
 		}

pkg/controller/upload-controller.go

@@ -893,7 +893,7 @@ func (r *UploadReconciler) makeUploadPodSpec(args UploadPodArgs, resourceRequire
 			MountPath: common.ScratchDataDir,
 		})
 	}
-	setPodPvcAnnotations(pod, args.PVC)
+	cc.SetPvcAllowedAnnotations(pod, args.PVC)
 	cc.SetNodeNameIfPopulator(args.PVC, &pod.Spec)
 	cc.SetRestrictedSecurityContext(&pod.Spec)
 	return pod

pkg/controller/util.go

@@ -374,27 +374,6 @@ func getScratchNameFromPod(pod *v1.Pod) (string, bool) {
 	return "", false
 }
 
-// setPodPvcAnnotations applies PVC annotations on the pod
-func setPodPvcAnnotations(pod *v1.Pod, pvc *v1.PersistentVolumeClaim) {
-	allowedAnnotations := map[string]string{
-		cc.AnnPodNetwork:              "",
-		cc.AnnPodSidecarInjection:     cc.AnnPodSidecarInjectionDefault,
-		cc.AnnPodMultusDefaultNetwork: ""}
-	for ann, def := range allowedAnnotations {
-		val, ok := pvc.Annotations[ann]
-		if !ok && def != "" {
-			val = def
-		}
-		if val != "" {
-			klog.V(1).Info("Applying PVC annotation on the pod", ann, val)
-			if pod.Annotations == nil {
-				pod.Annotations = map[string]string{}
-			}
-			pod.Annotations[ann] = val
-		}
-	}
-}
-
 func podUsingPVC(pvc *corev1.PersistentVolumeClaim, readOnly bool) *corev1.Pod {
 	return &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{