From 5d034b11242d5d3e7bd77976e450ef089af28a8e Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 30 Aug 2023 09:46:01 +0200 Subject: [PATCH 1/5] cephfs: block creation of ROX clone from ROX volume As there is no usecase currently, blocking the creation of ROX clone from the ROX volume. Signed-off-by: Madhu Rajanna --- internal/cephfs/controllerserver.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/cephfs/controllerserver.go b/internal/cephfs/controllerserver.go index 873b8175c..ce4c19dc5 100644 --- a/internal/cephfs/controllerserver.go +++ b/internal/cephfs/controllerserver.go @@ -214,6 +214,7 @@ func checkValidCreateVolumeRequest( sID *store.SnapshotIdentifier, req *csi.CreateVolumeRequest, ) error { + volCaps := req.GetVolumeCapabilities() switch { case pvID != nil: if vol.Size < parentVol.Size { @@ -224,12 +225,12 @@ func checkValidCreateVolumeRequest( vol.Size) } - if vol.BackingSnapshot { - return errors.New("cloning snapshot-backed volumes is currently not supported") + if parentVol.BackingSnapshot && store.IsVolumeCreateRO(volCaps) { + return errors.New("creating read-only clone from a snapshot-backed volume is not supported") } + case sID != nil: if vol.BackingSnapshot { - volCaps := req.GetVolumeCapabilities() isRO := store.IsVolumeCreateRO(volCaps) if !isRO { return errors.New("backingSnapshot may be used only with read-only access modes") From 771470d9755a32f78a98d6831d585151f2d7b6ad Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 30 Aug 2023 11:13:03 +0200 Subject: [PATCH 2/5] cephfs: add support for RWX clone from ROX Add support to create RWX clone from the ROX clone, in ceph no subvolume clone is created when ROX clone is created from a snapshot just a internal ref counter is added. This PR allows creating a RWX clone from a ROX clone which allows users to create RW copy of PVC where cephcsi will identify the snapshot created for the ROX volume and creates a subvolume from the CephFS snapshot. updates: #3603 Signed-off-by: Madhu Rajanna --- internal/cephfs/controllerserver.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/internal/cephfs/controllerserver.go b/internal/cephfs/controllerserver.go index ce4c19dc5..d1c126fc3 100644 --- a/internal/cephfs/controllerserver.go +++ b/internal/cephfs/controllerserver.go @@ -299,6 +299,23 @@ func (cs *ControllerServer) CreateVolume( return nil, status.Error(codes.InvalidArgument, err.Error()) } + // As we are trying to create RWX volume from backing snapshot, we need to + // retrieve the snapshot details from the backing snapshot and create a + // subvolume clone from the snapshot. + if parentVol != nil && parentVol.BackingSnapshot && !store.IsVolumeCreateRO(req.VolumeCapabilities) { + // unset pvID as we dont have real subvolume for the parent volumeID as its a backing snapshot + pvID = nil + parentVol, _, sID, err = store.NewSnapshotOptionsFromID(ctx, parentVol.BackingSnapshotID, cr, + req.GetSecrets(), cs.ClusterName, cs.SetMetadata) + if err != nil { + if errors.Is(err, cerrors.ErrSnapNotFound) { + return nil, status.Error(codes.NotFound, err.Error()) + } + + return nil, status.Error(codes.Internal, err.Error()) + } + } + vID, err := store.CheckVolExists(ctx, volOptions, parentVol, pvID, sID, cr, cs.ClusterName, cs.SetMetadata) if err != nil { if cerrors.IsCloneRetryError(err) { From 9809f365fcb6edc4b000533d547832bc8800f896 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 30 Aug 2023 12:18:29 +0200 Subject: [PATCH 3/5] e2e: add e2e test for RWX from ROX added an e2e test case to create RWX clone from ROX and also try to write extra data in the RWX cloned PVC. Signed-off-by: Madhu Rajanna --- e2e/cephfs.go | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/e2e/cephfs.go b/e2e/cephfs.go index 53fb03eb5..7e596c1c9 100644 --- a/e2e/cephfs.go +++ b/e2e/cephfs.go @@ -2030,6 +2030,167 @@ var _ = Describe(cephfsType, func() { } }) + By("create RWX clone from ROX PVC", func() { + pvc, err := loadPVC(pvcPath) + if err != nil { + framework.Failf("failed to load PVC: %v", err) + } + pvc.Namespace = f.UniqueName + err = createPVCAndvalidatePV(f.ClientSet, pvc, deployTimeout) + if err != nil { + framework.Failf("failed to create PVC: %v", err) + } + + _, pv, err := getPVCAndPV(f.ClientSet, pvc.Name, pvc.Namespace) + if err != nil { + framework.Failf("failed to get PV object for %s: %v", pvc.Name, err) + } + + app, err := loadApp(appPath) + if err != nil { + framework.Failf("failed to load application: %v", err) + } + app.Namespace = f.UniqueName + app.Spec.Volumes[0].PersistentVolumeClaim.ClaimName = pvc.Name + appLabels := map[string]string{ + appKey: appLabel, + } + app.Labels = appLabels + optApp := metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", appKey, appLabels[appKey]), + } + err = writeDataInPod(app, &optApp, f) + if err != nil { + framework.Failf("failed to write data: %v", err) + } + + appTestFilePath := app.Spec.Containers[0].VolumeMounts[0].MountPath + "/test" + + err = appendToFileInContainer(f, app, appTestFilePath, "hello", &optApp) + if err != nil { + framework.Failf("failed to append data: %v", err) + } + + parentFileSum, err := calculateSHA512sum(f, app, appTestFilePath, &optApp) + if err != nil { + framework.Failf("failed to get SHA512 sum for file: %v", err) + } + + snap := getSnapshot(snapshotPath) + snap.Namespace = f.UniqueName + snap.Spec.Source.PersistentVolumeClaimName = &pvc.Name + err = createSnapshot(&snap, deployTimeout) + if err != nil { + framework.Failf("failed to create snapshot: %v", err) + } + validateCephFSSnapshotCount(f, 1, subvolumegroup, pv) + + pvcClone, err := loadPVC(pvcClonePath) + if err != nil { + framework.Failf("failed to load PVC: %v", err) + } + // Snapshot-backed volumes support read-only access modes only. + pvcClone.Spec.DataSource.Name = snap.Name + pvcClone.Spec.AccessModes = []v1.PersistentVolumeAccessMode{v1.ReadOnlyMany} + + pvcClone.Namespace = f.UniqueName + err = createPVCAndvalidatePV(c, pvcClone, deployTimeout) + if err != nil { + framework.Failf("failed to create PVC: %v", err) + } + + validateSubvolumeCount(f, 1, fileSystemName, subvolumegroup) + + // create RWX clone from ROX PVC + pvcRWXClone, err := loadPVC(pvcSmartClonePath) + if err != nil { + framework.Failf("failed to load PVC: %v", err) + } + pvcRWXClone.Spec.DataSource.Name = pvcClone.Name + pvcRWXClone.Spec.AccessModes = []v1.PersistentVolumeAccessMode{v1.ReadWriteMany} + pvcRWXClone.Namespace = f.UniqueName + + appClone, err := loadApp(appPath) + if err != nil { + framework.Failf("failed to load application: %v", err) + } + appCloneLabels := map[string]string{ + appKey: appCloneLabel, + } + appClone.Name = f.UniqueName + "-app" + appClone.Namespace = f.UniqueName + appClone.Labels = appCloneLabels + appClone.Spec.Volumes[0].PersistentVolumeClaim.ClaimName = pvcRWXClone.Name + optAppClone := metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", appKey, appCloneLabels[appKey]), + } + + err = createPVCAndApp("", f, pvcRWXClone, appClone, deployTimeout) + if err != nil { + framework.Failf("failed to create PVC and app: %v", err) + } + // 2 subvolumes should be created 1 for parent PVC and 1 for + // RWX clone PVC. + validateSubvolumeCount(f, 2, fileSystemName, subvolumegroup) + + appCloneTestFilePath := appClone.Spec.Containers[0].VolumeMounts[0].MountPath + "/test" + + cloneFileSum, err := calculateSHA512sum(f, appClone, appCloneTestFilePath, &optAppClone) + if err != nil { + framework.Failf("failed to get SHA512 sum for file: %v", err) + } + + if parentFileSum != cloneFileSum { + framework.Failf( + "SHA512 sums of files in parent and ROX should not differ. parentFileSum: %s cloneFileSum: %s", + parentFileSum, + cloneFileSum) + } + + // Now try to write to the PVC as its a RWX PVC + err = appendToFileInContainer(f, app, appCloneTestFilePath, "testing", &optApp) + if err != nil { + framework.Failf("failed to append data: %v", err) + } + + // Deleting snapshot before deleting pvcClone should succeed. It will be + // deleted once all volumes that are backed by this snapshot are gone. + err = deleteSnapshot(&snap, deployTimeout) + if err != nil { + framework.Failf("failed to delete snapshot: %v", err) + } + + // delete parent pvc and app + err = deletePVCAndApp("", f, pvc, app) + if err != nil { + framework.Failf("failed to delete PVC or application: %v", err) + } + + // delete ROX clone PVC + err = deletePVCAndValidatePV(c, pvcClone, deployTimeout) + if err != nil { + framework.Failf("failed to delete PVC or application: %v", err) + } + // delete RWX clone PVC and app + err = deletePVCAndApp("", f, pvcRWXClone, appClone) + if err != nil { + framework.Failf("failed to delete PVC or application: %v", err) + } + + validateSubvolumeCount(f, 0, fileSystemName, subvolumegroup) + validateOmapCount(f, 0, cephfsType, metadataPool, volumesType) + + err = deleteResource(cephFSExamplePath + "storageclass.yaml") + if err != nil { + framework.Failf("failed to delete CephFS storageclass: %v", err) + } + + err = createCephfsStorageClass(f.ClientSet, f, false, nil) + if err != nil { + framework.Failf("failed to create CephFS storageclass: %v", err) + } + }) + if testCephFSFscrypt { kmsToTest := map[string]kmsConfig{ "secrets-metadata-test": secretsMetadataKMS, From 561e0bc04ab6e76f03c946503ab53ce39d2d9674 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Thu, 31 Aug 2023 10:13:15 +0200 Subject: [PATCH 4/5] ci: add ginkgo.v when running e2e tests Ginkgo is not output the logs when the tests are running, it logs only after completing the tests or if the test fails. Signed-off-by: Madhu Rajanna --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 64dfb581f..91865aa3e 100644 --- a/Makefile +++ b/Makefile @@ -186,7 +186,7 @@ run-e2e: NAMESPACE ?= cephcsi-e2e-$(shell uuidgen | cut -d- -f1) run-e2e: @test -e e2e.test || $(MAKE) e2e.test cd e2e && \ - ../e2e.test -test.v -ginkgo.timeout="${E2E_TIMEOUT}" --deploy-timeout="${DEPLOY_TIMEOUT}" --cephcsi-namespace=$(NAMESPACE) $(E2E_ARGS) + ../e2e.test -test.v -ginkgo.v -ginkgo.timeout="${E2E_TIMEOUT}" --deploy-timeout="${DEPLOY_TIMEOUT}" --cephcsi-namespace=$(NAMESPACE) $(E2E_ARGS) .container-cmd: @test -n "$(shell which $(CONTAINER_CMD) 2>/dev/null)" || { echo "Missing container support, install Podman or Docker"; exit 1; } From 683821c4073d731d85f1050d77d1f316c1bee5f8 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Sat, 2 Sep 2023 09:21:35 +0200 Subject: [PATCH 5/5] rbd: discard not found error from GetMetadata During ResyncVolume call, discard not found error from GetMetadata API. If the image gets resynced the local image creation time will be lost, if the key is not present in the image metadata then we can assume that the image is already resynced. Signed-off-by: Madhu Rajanna --- internal/csi-addons/rbd/replication.go | 27 +++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/internal/csi-addons/rbd/replication.go b/internal/csi-addons/rbd/replication.go index 3dd011576..197bb822b 100644 --- a/internal/csi-addons/rbd/replication.go +++ b/internal/csi-addons/rbd/replication.go @@ -513,6 +513,7 @@ func (rs *ReplicationServer) DemoteVolume(ctx context.Context, // store the image creation time for resync _, err = rbdVol.GetMetadata(imageCreationTimeKey) if err != nil && errors.Is(err, librbd.ErrNotFound) { + log.DebugLog(ctx, "setting image creation time %s for %s", creationTime, rbdVol) err = rbdVol.SetMetadata(imageCreationTimeKey, timestampToString(creationTime)) } if err != nil { @@ -670,8 +671,11 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context, // image creation time is stored in the image metadata. it looks like // `"seconds:1692879841 nanos:631526669"` + // If the image gets resynced the local image creation time will be + // lost, if the keys is not present in the image metadata then we can + // assume that the image is already resynced. savedImageTime, err := rbdVol.GetMetadata(imageCreationTimeKey) - if err != nil { + if err != nil && !errors.Is(err, librbd.ErrNotFound) { return nil, status.Errorf(codes.Internal, "failed to get %s key from image metadata for %s: %s", imageCreationTimeKey, @@ -679,16 +683,17 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context, err.Error()) } - st, err := timestampFromString(savedImageTime) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to parse image creation time: %s", err.Error()) - } - log.DebugLog(ctx, "image %s, savedImageTime=%v, currentImageTime=%v", rbdVol, st, creationTime.AsTime()) - - if req.Force && st.Equal(creationTime.AsTime()) { - err = rbdVol.ResyncVol(localStatus) - if err != nil { - return nil, getGRPCError(err) + if savedImageTime != "" { + st, sErr := timestampFromString(savedImageTime) + if sErr != nil { + return nil, status.Errorf(codes.Internal, "failed to parse image creation time: %s", sErr.Error()) + } + log.DebugLog(ctx, "image %s, savedImageTime=%v, currentImageTime=%v", rbdVol, st, creationTime.AsTime()) + if req.Force && st.Equal(creationTime.AsTime()) { + err = rbdVol.ResyncVol(localStatus) + if err != nil { + return nil, getGRPCError(err) + } } }