Skip to content

Commit

Permalink
Clean up unit test
Browse files Browse the repository at this point in the history 
Signed-off-by: Benamar Mekhissi <[email protected]>
(cherry picked from commit aae07f6)
  • Loading branch information
Benamar Mekhissi committed Jan 9, 2025
1 parent a256e65 commit d3b4c33
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 138 deletions.
2 changes: 1 addition & 1 deletion internal/controller/cephfscg/replicationgroupsource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
22 changes: 12 additions & 10 deletions internal/controller/cephfscg/volumegroupsourcehandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/drplacementcontrol.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions internal/controller/volsync/vshandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand Down
135 changes: 13 additions & 122 deletions internal/controller/volsync/vshandler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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))
})
})
})
Expand Down
8 changes: 8 additions & 0 deletions internal/controller/volumereplicationgroup_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,14 @@ 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
if storageClass.Provisioner == DefaultCephFSCSIDriverName {
storageID = "cephfs-" + storageID
} else {
storageID = "rbd-" + storageID
}

// Add label for PVC, showing that this PVC is part of consistency group
return util.NewResourceUpdater(pvc).
AddLabel(ConsistencyGroupLabel, storageID).
Expand Down

0 comments on commit d3b4c33

Please sign in to comment.