diff --git a/internal/controller/cephfscg/replicationgroupsource.go b/internal/controller/cephfscg/replicationgroupsource.go index d217d0e1d..71a1258e9 100644 --- a/internal/controller/cephfscg/replicationgroupsource.go +++ b/internal/controller/cephfscg/replicationgroupsource.go @@ -140,7 +140,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 { diff --git a/internal/controller/cephfscg/volumegroupsourcehandler.go b/internal/controller/cephfscg/volumegroupsourcehandler.go index f23cb1575..e92e02a7c 100644 --- a/internal/controller/cephfscg/volumegroupsourcehandler.go +++ b/internal/controller/cephfscg/volumegroupsourcehandler.go @@ -146,7 +146,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( @@ -214,36 +214,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) @@ -428,7 +430,7 @@ func (h *volumeGroupSourceHandler) CreateOrUpdateReplicationSourceForRestoredPVC } replicationSource.Spec.RsyncTLS = &volsyncv1alpha1.ReplicationSourceRsyncTLSSpec{ ReplicationSourceVolumeOptions: volsyncv1alpha1.ReplicationSourceVolumeOptions{ - CopyMethod: volsyncv1alpha1.CopyMethodDirect, + CopyMethod: volsyncv1alpha1.CopyMethodDirect, }, KeySecret: &h.VolsyncKeySecretName, diff --git a/internal/controller/drplacementcontrol.go b/internal/controller/drplacementcontrol.go index b9cd51199..35800c903 100644 --- a/internal/controller/drplacementcontrol.go +++ b/internal/controller/drplacementcontrol.go @@ -1711,7 +1711,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 diff --git a/internal/controller/volsync/vshandler.go b/internal/controller/volsync/vshandler.go index e67e94912..7f0c9981e 100644 --- a/internal/controller/volsync/vshandler.go +++ b/internal/controller/volsync/vshandler.go @@ -1929,8 +1929,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 { @@ -1980,8 +1979,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) } diff --git a/internal/controller/volsync/vshandler_test.go b/internal/controller/volsync/vshandler_test.go index 92b6087af..a3ebfb264 100644 --- a/internal/controller/volsync/vshandler_test.go +++ b/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)) }) }) }) diff --git a/internal/controller/volumereplicationgroup_controller.go b/internal/controller/volumereplicationgroup_controller.go index d96432b04..1e8a71a1b 100644 --- a/internal/controller/volumereplicationgroup_controller.go +++ b/internal/controller/volumereplicationgroup_controller.go @@ -759,6 +759,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).