Clean up unit test

Signed-off-by: Benamar Mekhissi <[email protected]>

internal/controller/cephfscg/replicationgroupsource.go

@@ -137,7 +137,7 @@ func (m *replicationGroupSourceMachine) Synchronize(ctx context.Context) (mover.
 
 		return mover.InProgress(), err
 	}
-	
+
 	replicationSources, err := m.VolumeGroupHandler.CreateOrUpdateReplicationSourceForRestoredPVCs(
 		ctx, m.ReplicationGroupSource.Status.LastSyncStartTime.String(), restoredPVCs, m.ReplicationGroupSource)
 	if err != nil {

internal/controller/cephfscg/volumegroupsourcehandler.go

@@ -143,7 +143,7 @@ func (h *volumeGroupSourceHandler) CreateOrUpdateVolumeGroupSnapshot(
 	return nil
 }
 
-// CleanVolumeGroupSnapshot delete restored pvc, replicationsource and VolumeGroupSnapshot
+// CleanVolumeGroupSnapshot delete restored pvc and VolumeGroupSnapshot
 //
 //nolint:funlen
 func (h *volumeGroupSourceHandler) CleanVolumeGroupSnapshot(
@@ -211,36 +211,38 @@ func (h *volumeGroupSourceHandler) CleanVolumeGroupSnapshot(
 	return nil
 }
 
-// RestoreVolumesFromVolumeGroupSnapshot restore VolumeGroupSnapshot to PVCs
+// RestoreVolumesFromVolumeGroupSnapshot restores VolumeGroupSnapshot to PVCs
+//
+//nolint:funlen,cyclop
 func (h *volumeGroupSourceHandler) RestoreVolumesFromVolumeGroupSnapshot(
 	ctx context.Context, owner metav1.Object,
 ) ([]RestoredPVC, error) {
 	logger := h.Logger.WithName("RestoreVolumesFromVolumeGroupSnapshot")
 	logger.Info("Get volume group snapshot")
 
-	volumeGroupSnapshot := &vgsv1alphfa1.VolumeGroupSnapshot{}
+	vgs := &vgsv1alphfa1.VolumeGroupSnapshot{}
 	if err := h.Client.Get(ctx,
 		types.NamespacedName{Name: h.VolumeGroupSnapshotName, Namespace: h.VolumeGroupSnapshotNamespace},
-		volumeGroupSnapshot); err != nil {
+		vgs); err != nil {
 		return nil, fmt.Errorf("failed to get volume group snapshot: %w", err)
 	}
 
-	if volumeGroupSnapshot.Status == nil || volumeGroupSnapshot.Status.ReadyToUse == nil ||
-		(volumeGroupSnapshot.Status.ReadyToUse != nil && !*volumeGroupSnapshot.Status.ReadyToUse) {
+	if vgs.Status == nil || vgs.Status.ReadyToUse == nil ||
+		(vgs.Status.ReadyToUse != nil && !*vgs.Status.ReadyToUse) {
 		return nil, fmt.Errorf("can't restore volume group snapshot: volume group snapshot is not ready to be used")
 	}
 
 	restoredPVCs := []RestoredPVC{}
 
-	for _, pvcVSRef := range volumeGroupSnapshot.Status.PVCVolumeSnapshotRefList {
+	for _, pvcVSRef := range vgs.Status.PVCVolumeSnapshotRefList {
 		logger.Info("Get PVCName from volume snapshot",
 			"PVCName", pvcVSRef.PersistentVolumeClaimRef.Name, "VolumeSnapshotName", pvcVSRef.VolumeSnapshotRef.Name)
 
 		pvc, err := util.GetPVC(ctx, h.Client,
-			types.NamespacedName{Name: pvcVSRef.PersistentVolumeClaimRef.Name, Namespace: volumeGroupSnapshot.Namespace})
+			types.NamespacedName{Name: pvcVSRef.PersistentVolumeClaimRef.Name, Namespace: vgs.Namespace})
 		if err != nil {
 			return nil, fmt.Errorf("failed to get PVC from VGS %s: %w",
-				volumeGroupSnapshot.Namespace+"/"+pvcVSRef.PersistentVolumeClaimRef.Name, err)
+				vgs.Namespace+"/"+pvcVSRef.PersistentVolumeClaimRef.Name, err)
 		}
 
 		storageClass, err := GetStorageClass(ctx, h.Client, pvc.Spec.StorageClassName)
@@ -425,7 +427,7 @@ func (h *volumeGroupSourceHandler) CreateOrUpdateReplicationSourceForRestoredPVC
 			}
 			replicationSource.Spec.RsyncTLS = &volsyncv1alpha1.ReplicationSourceRsyncTLSSpec{
 				ReplicationSourceVolumeOptions: volsyncv1alpha1.ReplicationSourceVolumeOptions{
-					CopyMethod:  volsyncv1alpha1.CopyMethodDirect,
+					CopyMethod: volsyncv1alpha1.CopyMethodDirect,
 				},
 
 				KeySecret: &h.VolsyncKeySecretName,

internal/controller/drplacementcontrol.go

@@ -1707,7 +1707,7 @@ func (d *DRPCInstance) updateVRGOptionalFields(vrg, vrgFromView *rmn.VolumeRepli
 		DoNotDeletePVCAnnotation:        d.instance.GetAnnotations()[DoNotDeletePVCAnnotation],
 		DRPCUIDAnnotation:               string(d.instance.UID),
 		rmnutil.IsCGEnabledAnnotation:   d.instance.GetAnnotations()[rmnutil.IsCGEnabledAnnotation],
-		rmnutil.UseVolSyncAnnotation:   d.instance.GetAnnotations()[rmnutil.UseVolSyncAnnotation],
+		rmnutil.UseVolSyncAnnotation:    d.instance.GetAnnotations()[rmnutil.UseVolSyncAnnotation],
 	}
 
 	vrg.Spec.ProtectedNamespaces = d.instance.Spec.ProtectedNamespaces

internal/controller/volsync/vshandler.go

@@ -1961,8 +1961,7 @@ func (v *VSHandler) createPVCFromSnapshot(rd *volsyncv1alpha1.ReplicationDestina
 	snapshotRef *corev1.TypedLocalObjectReference,
 	snapRestoreSize *resource.Quantity,
 ) (*corev1.PersistentVolumeClaim, error) {
-	l := v.log.WithValues("pvcName", rd.GetName(), "snapshotRef", snapshotRef,
-		"snapRestoreSize", snapRestoreSize)
+	l := v.log.WithValues("pvcName", rd.GetName(), "snapshotRef", snapshotRef, "snapRestoreSize", snapRestoreSize)
 
 	storageClass, err := v.getStorageClass(rdSpec.ProtectedPVC.StorageClassName)
 	if err != nil {
@@ -2012,8 +2011,6 @@ func (v *VSHandler) createPVCFromSnapshot(rd *volsyncv1alpha1.ReplicationDestina
 		return nil
 	})
 	if err != nil {
-		l.Error(err, "Unable to createOrUpdate PVC from snapshot for localRS")
-
 		return nil, fmt.Errorf("error creating or updating PVC from snapshot for localRS (%w)", err)
 	}
 

internal/controller/volsync/vshandler_test.go

@@ -253,11 +253,17 @@ var _ = Describe("VolSync Handler - Volume Replication Class tests", func() {
 				storageClassForTest)).To(Succeed())
 
 			vsHandler.ModifyRSSpecForCephFS(&testRsSpec, storageClassForTest)
-
-			Expect(testRsSpec.ProtectedPVC.AccessModes).To(Equal(
-				[]corev1.PersistentVolumeAccessMode{
-					corev1.ReadOnlyMany,
-				}))
+			if storageClassForTest.Provisioner == testCephFSStorageDriverName {
+				Expect(testRsSpec.ProtectedPVC.AccessModes).To(Equal(
+					[]corev1.PersistentVolumeAccessMode{
+						corev1.ReadOnlyMany,
+					}))
+			} else {
+				Expect(testRsSpec.ProtectedPVC.AccessModes).To(Equal(
+					[]corev1.PersistentVolumeAccessMode{
+						corev1.ReadWriteMany,
+					}))
+			}
 		})
 
 		Context("When the source PVC is not using a cephfs storageclass", func() {
@@ -272,128 +278,13 @@ var _ = Describe("VolSync Handler - Volume Replication Class tests", func() {
 		})
 
 		Context("When the sourcePVC is using a cephfs storageclass", func() {
-			customBackingSnapshotStorageClassName := testCephFSStorageClassName + "-vrg"
-
 			BeforeEach(func() {
 				// Make sure the source PVC uses the cephfs storageclass
 				testSourcePVC.Spec.StorageClassName = &testCephFSStorageClassName
 			})
 
-			JustBeforeEach(func() {
-				// Common tests - rsSpec should be modified with settings to allow pvc from snapshot
-				// to use our custom cephfs storageclass and ReadOnlyMany accessModes
-				Expect(testRsSpecOrig).NotTo(Equal(testRsSpec))
-
-				// Should use the custom storageclass with backingsnapshot: true parameter
-				Expect(*testRsSpec.ProtectedPVC.StorageClassName).To(Equal(customBackingSnapshotStorageClassName))
-
-				// AccessModes should be updated to ReadOnlyMany
-				Expect(testRsSpec.ProtectedPVC.AccessModes).To(Equal(
-					[]corev1.PersistentVolumeAccessMode{
-						corev1.ReadOnlyMany,
-					}))
-			})
-
-			AfterEach(func() {
-				// Delete the custom storage class that may have been created by test
-				custStorageClass := &storagev1.StorageClass{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: customBackingSnapshotStorageClassName,
-					},
-				}
-				err := k8sClient.Delete(ctx, custStorageClass)
-				if err != nil {
-					Expect(kerrors.IsNotFound(err)).To(BeTrue())
-				}
-
-				Eventually(func() bool {
-					err := k8sClient.Get(ctx, client.ObjectKeyFromObject(custStorageClass), custStorageClass)
-
-					return kerrors.IsNotFound(err)
-				}, maxWait, interval).Should(BeTrue())
-			})
-
-			Context("When the custom cephfs backing storage class for readonly pvc from snap does not exist", func() {
-				// Delete the custom vrg storageclass if it exists
-				BeforeEach(func() {
-					custStorageClass := &storagev1.StorageClass{
-						ObjectMeta: metav1.ObjectMeta{
-							Name: customBackingSnapshotStorageClassName,
-						},
-					}
-					err := k8sClient.Delete(ctx, custStorageClass)
-					if err != nil {
-						Expect(kerrors.IsNotFound(err)).To(BeTrue())
-					}
-
-					Eventually(func() bool {
-						err := k8sClient.Get(ctx, client.ObjectKeyFromObject(custStorageClass), custStorageClass)
-
-						return kerrors.IsNotFound(err)
-					}, maxWait, interval).Should(BeTrue())
-				})
-
-				It("ModifyRSSpecForCephFS should modify the rsSpec and create the new storageclass", func() {
-					// RSspec modification checks in the outer context JustBeforeEach()
-
-					newStorageClass := &storagev1.StorageClass{
-						ObjectMeta: metav1.ObjectMeta{
-							Name: customBackingSnapshotStorageClassName,
-						},
-					}
-
-					Eventually(func() error {
-						return k8sClient.Get(ctx, client.ObjectKeyFromObject(newStorageClass), newStorageClass)
-					}, maxWait, interval).Should(Succeed())
-
-					Expect(newStorageClass.Parameters["backingSnapshot"]).To(Equal("true"))
-
-					// Other parameters from the test cephfs storageclass should be copied over
-					for k, v := range testCephFSStorageClass.Parameters {
-						Expect(newStorageClass.Parameters[k]).To(Equal(v))
-					}
-				})
-			})
-
-			Context("When the custom cephfs backing storage class for readonly pvc from snap exists", func() {
-				var preExistingCustStorageClass *storagev1.StorageClass
-
-				BeforeEach(func() {
-					preExistingCustStorageClass = &storagev1.StorageClass{
-						ObjectMeta: metav1.ObjectMeta{
-							Name: customBackingSnapshotStorageClassName,
-						},
-						Provisioner: testCephFSStorageDriverName,
-						Parameters: map[string]string{ // Not the same params as our CephFS storageclass for test
-							"different-param-1": "abc",
-							"different-param-2": "def",
-							"backingSnapshot":   "true",
-						},
-					}
-					Expect(k8sClient.Create(ctx, preExistingCustStorageClass)).To(Succeed())
-
-					// Confirm it's created
-					Eventually(func() error {
-						return k8sClient.Get(ctx,
-							client.ObjectKeyFromObject(preExistingCustStorageClass), preExistingCustStorageClass)
-					}, maxWait, interval).Should(Succeed())
-				})
-
-				It("ModifyRSSpecForCephFS should modify the rsSpec but not modify the new custom storageclass", func() {
-					// Load the custom storageclass
-					newStorageClass := &storagev1.StorageClass{
-						ObjectMeta: metav1.ObjectMeta{
-							Name: customBackingSnapshotStorageClassName,
-						},
-					}
-
-					Eventually(func() error {
-						return k8sClient.Get(ctx, client.ObjectKeyFromObject(newStorageClass), newStorageClass)
-					}, maxWait, interval).Should(Succeed())
-
-					// Parameters should match the original, unmodified
-					Expect(newStorageClass.Parameters).To(Equal(preExistingCustStorageClass.Parameters))
-				})
+			It("ModifyRSSpecForCephFS should modify the rsSpec protectedPVC accessModes", func() {
+				Expect(testRsSpecOrig).ToNot(Equal(testRsSpec))
 			})
 		})
 	})

internal/controller/volumereplicationgroup_controller.go

@@ -757,6 +757,13 @@ func (v *VRGInstance) addConsistencyGroupLabel(pvc *corev1.PersistentVolumeClaim
 		return fmt.Errorf("missing storageID for PVC %s/%s", pvc.GetNamespace(), pvc.GetName())
 	}
 
+	// FIXME: a temporary workaround for issue DFBUGS-1209
+	// Remove this block once DFBUGS-1209 is fixed
+	storageID = "cephfs-" + storageID
+	if storageClass.Provisioner != DefaultCephFSCSIDriverName {
+		storageID = "rbd-" + storageID
+	}
+
 	// Add label for PVC, showing that this PVC is part of consistency group
 	return util.NewResourceUpdater(pvc).
 		AddLabel(ConsistencyGroupLabel, storageID).