From 0dae45d0a2af7037034b056b72600677cd1649b9 Mon Sep 17 00:00:00 2001 From: Praveen M Date: Mon, 16 Oct 2023 13:44:44 +0530 Subject: [PATCH 1/3] cephfs: remove subvolumegroup creation Signed-off-by: Praveen M --- e2e/cephfs.go | 38 +++++++++++++++++++++++++++----- e2e/nfs.go | 12 +++++++++- e2e/utils.go | 28 +++++++++++++++++++++++ internal/cephfs/core/metadata.go | 26 ---------------------- internal/cephfs/core/volume.go | 32 --------------------------- 5 files changed, 72 insertions(+), 64 deletions(-) diff --git a/e2e/cephfs.go b/e2e/cephfs.go index f4366c1d231..43171537f49 100644 --- a/e2e/cephfs.go +++ b/e2e/cephfs.go @@ -215,6 +215,11 @@ var _ = Describe(cephfsType, func() { if err != nil { framework.Failf("timeout waiting for deployment update %s/%s: %v", cephCSINamespace, cephFSDeploymentName, err) } + + err = createSubvolumegroup(f, fileSystemName, subvolumegroup) + if err != nil { + framework.Failf("%v", err) + } }) AfterEach(func() { @@ -254,10 +259,15 @@ var _ = Describe(cephfsType, func() { } deleteVault() + err = deleteSubvolumegroup(f, fileSystemName, subvolumegroup) + if err != nil { + framework.Failf("%v", err) + } + if deployCephFS { deleteCephfsPlugin() if cephCSINamespace != defaultNs { - err := deleteNamespace(c, cephCSINamespace) + err = deleteNamespace(c, cephCSINamespace) if err != nil { framework.Failf("failed to delete namespace %s: %v", cephCSINamespace, err) } @@ -939,14 +949,24 @@ var _ = Describe(cephfsType, func() { } // re-define configmap with information of multiple clusters. + subvolgrp1 := "subvolgrp1" + subvolgrp2 := "subvolgrp2" clusterInfo := map[string]map[string]string{} clusterID1 := "clusterID-1" clusterID2 := "clusterID-2" clusterInfo[clusterID1] = map[string]string{} - clusterInfo[clusterID1]["subvolumeGroup"] = "subvolgrp1" + clusterInfo[clusterID1]["subvolumeGroup"] = subvolgrp1 clusterInfo[clusterID2] = map[string]string{} - clusterInfo[clusterID2]["subvolumeGroup"] = "subvolgrp2" + clusterInfo[clusterID2]["subvolumeGroup"] = subvolgrp2 + err = createSubvolumegroup(f, fileSystemName, subvolgrp1) + if err != nil { + framework.Failf("%v", err) + } + err = createSubvolumegroup(f, fileSystemName, subvolgrp2) + if err != nil { + framework.Failf("%v", err) + } err = createCustomConfigMap(f.ClientSet, cephFSDirPath, clusterInfo) if err != nil { framework.Failf("failed to create configmap: %v", err) @@ -967,7 +987,7 @@ var _ = Describe(cephfsType, func() { framework.Failf("failed to delete storageclass: %v", err) } // verify subvolume group creation. - err = validateSubvolumegroup(f, "subvolgrp1") + err = validateSubvolumegroup(f, subvolgrp1) if err != nil { framework.Failf("failed to validate subvolume group: %v", err) } @@ -987,10 +1007,18 @@ var _ = Describe(cephfsType, func() { if err != nil { framework.Failf("failed to delete storageclass: %v", err) } - err = validateSubvolumegroup(f, "subvolgrp2") + err = validateSubvolumegroup(f, subvolgrp2) if err != nil { framework.Failf("failed to validate subvolume group: %v", err) } + err = deleteSubvolumegroup(f, fileSystemName, subvolgrp1) + if err != nil { + framework.Failf("%v", err) + } + err = deleteSubvolumegroup(f, fileSystemName, subvolgrp2) + if err != nil { + framework.Failf("%v", err) + } err = deleteConfigMap(cephFSDirPath) if err != nil { framework.Failf("failed to delete configmap: %v", err) diff --git a/e2e/nfs.go b/e2e/nfs.go index 56f9cc7f434..59628c94a86 100644 --- a/e2e/nfs.go +++ b/e2e/nfs.go @@ -276,6 +276,11 @@ var _ = Describe("nfs", func() { if err != nil { framework.Failf("failed to create node secret: %v", err) } + + err = createSubvolumegroup(f, fileSystemName, subvolumegroup) + if err != nil { + framework.Failf("%v", err) + } }) AfterEach(func() { @@ -313,10 +318,15 @@ var _ = Describe("nfs", func() { if err != nil { framework.Failf("failed to delete storageclass: %v", err) } + err = deleteSubvolumegroup(f, fileSystemName, subvolumegroup) + if err != nil { + framework.Failf("%v", err) + } + if deployNFS { deleteNFSPlugin() if cephCSINamespace != defaultNs { - err := deleteNamespace(c, cephCSINamespace) + err = deleteNamespace(c, cephCSINamespace) if err != nil { framework.Failf("failed to delete namespace %s: %v", cephCSINamespace, err) } diff --git a/e2e/utils.go b/e2e/utils.go index ff1618bf063..08a8d4b510b 100644 --- a/e2e/utils.go +++ b/e2e/utils.go @@ -1853,3 +1853,31 @@ func checkExports(f *framework.Framework, clusterID, clientString string) bool { return true } + +// createSubvolumegroup creates a subvolumegroup. +func createSubvolumegroup(f *framework.Framework, fileSystemName, subvolumegroup string) error { + cmd := fmt.Sprintf("ceph fs subvolumegroup create %s %s", fileSystemName, subvolumegroup) + _, stdErr, err := execCommandInToolBoxPod(f, cmd, rookNamespace) + if err != nil { + return fmt.Errorf("failed to exec command in toolbox: %w", err) + } + if stdErr != "" { + return fmt.Errorf("failed to create subvolumegroup %s : %v", subvolumegroup, stdErr) + } + + return nil +} + +// deleteSubvolumegroup creates a subvolumegroup. +func deleteSubvolumegroup(f *framework.Framework, fileSystemName, subvolumegroup string) error { + cmd := fmt.Sprintf("ceph fs subvolumegroup rm %s %s", fileSystemName, subvolumegroup) + _, stdErr, err := execCommandInToolBoxPod(f, cmd, rookNamespace) + if err != nil { + return fmt.Errorf("failed to exec command in toolbox: %w", err) + } + if stdErr != "" { + return fmt.Errorf("failed to remove subvolumegroup %s : %v", subvolumegroup, stdErr) + } + + return nil +} diff --git a/internal/cephfs/core/metadata.go b/internal/cephfs/core/metadata.go index 1dfd1b5c4e8..9e2b90d5f46 100644 --- a/internal/cephfs/core/metadata.go +++ b/internal/cephfs/core/metadata.go @@ -51,32 +51,6 @@ func (s *subVolumeClient) isUnsupportedSubVolMetadata(err error) bool { return true } -// isSubVolumeGroupCreated returns true if subvolume group is created. -func (s *subVolumeClient) isSubVolumeGroupCreated() bool { - newLocalClusterState(s.clusterID) - clusterAdditionalInfo[s.clusterID].subVolumeGroupsRWMutex.RLock() - defer clusterAdditionalInfo[s.clusterID].subVolumeGroupsRWMutex.RUnlock() - - if clusterAdditionalInfo[s.clusterID].subVolumeGroupsCreated == nil { - return false - } - - return clusterAdditionalInfo[s.clusterID].subVolumeGroupsCreated[s.SubvolumeGroup] -} - -// updateSubVolumeGroupCreated updates subvolume group created map. -// If the map is nil, it creates a new map and updates it. -func (s *subVolumeClient) updateSubVolumeGroupCreated(state bool) { - clusterAdditionalInfo[s.clusterID].subVolumeGroupsRWMutex.Lock() - defer clusterAdditionalInfo[s.clusterID].subVolumeGroupsRWMutex.Unlock() - - if clusterAdditionalInfo[s.clusterID].subVolumeGroupsCreated == nil { - clusterAdditionalInfo[s.clusterID].subVolumeGroupsCreated = make(map[string]bool) - } - - clusterAdditionalInfo[s.clusterID].subVolumeGroupsCreated[s.SubvolumeGroup] = state -} - // setMetadata sets custom metadata on the subvolume in a volume as a // key-value pair. func (s *subVolumeClient) setMetadata(key, value string) error { diff --git a/internal/cephfs/core/volume.go b/internal/cephfs/core/volume.go index 0754b8a23f3..a92025ec93d 100644 --- a/internal/cephfs/core/volume.go +++ b/internal/cephfs/core/volume.go @@ -210,14 +210,6 @@ type localClusterState struct { resizeState operationState subVolMetadataState operationState subVolSnapshotMetadataState operationState - // A cluster can have multiple filesystem for that we need to have a map of - // subvolumegroups to check filesystem is created nor not. - // set true once a subvolumegroup is created - // for corresponding filesystem in a cluster. - subVolumeGroupsCreated map[string]bool - // subVolumeGroupsRWMutex is used to protect subVolumeGroupsCreated map - // against concurrent writes while allowing multiple readers. - subVolumeGroupsRWMutex sync.RWMutex } func newLocalClusterState(clusterID string) { @@ -241,24 +233,6 @@ func (s *subVolumeClient) CreateVolume(ctx context.Context) error { return err } - // create subvolumegroup if not already created for the cluster. - if !s.isSubVolumeGroupCreated() { - opts := fsAdmin.SubVolumeGroupOptions{} - err = ca.CreateSubVolumeGroup(s.FsName, s.SubvolumeGroup, &opts) - if err != nil { - log.ErrorLog( - ctx, - "failed to create subvolume group %s, for the vol %s: %s", - s.SubvolumeGroup, - s.VolID, - err) - - return err - } - log.DebugLog(ctx, "cephfs: created subvolume group %s", s.SubvolumeGroup) - s.updateSubVolumeGroupCreated(true) - } - opts := fsAdmin.SubVolumeOptions{ Size: fsAdmin.ByteCount(s.Size), } @@ -271,12 +245,6 @@ func (s *subVolumeClient) CreateVolume(ctx context.Context) error { if err != nil { log.ErrorLog(ctx, "failed to create subvolume %s in fs %s: %s", s.VolID, s.FsName, err) - if errors.Is(err, rados.ErrNotFound) { - // Reset the subVolumeGroupsCreated so that we can try again to create the - // subvolumegroup in next request if the error is Not Found. - s.updateSubVolumeGroupCreated(false) - } - return err } From 4727fc2bdec072be4b71cb91781ffca5fe3e5139 Mon Sep 17 00:00:00 2001 From: Praveen M Date: Thu, 2 Nov 2023 13:40:37 +0530 Subject: [PATCH 2/3] doc: updated doc for subvolumegroup creation Signed-off-by: Praveen M --- PendingReleaseNotes.md | 3 +++ deploy/csi-config-map-sample.yaml | 1 + docs/deploy-cephfs.md | 6 ++++++ 3 files changed, 10 insertions(+) diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index 3ee8d31d7b0..5b1be3f9f3b 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -3,6 +3,9 @@ ## Breaking Changes - Removed the deprecated grpc metrics flag's in [PR](https://github.com/ceph/ceph-csi/pull/4225) + +- Support for pre-creation of cephFS subvolumegroup before creating subvolume + is removed in [PR](https://github.com/ceph/ceph-csi/pull/4195) ## Features diff --git a/deploy/csi-config-map-sample.yaml b/deploy/csi-config-map-sample.yaml index 7f8653a583a..1766d32270a 100644 --- a/deploy/csi-config-map-sample.yaml +++ b/deploy/csi-config-map-sample.yaml @@ -20,6 +20,7 @@ kind: ConfigMap # NOTE: Make sure you don't add radosNamespace option to a currently in use # configuration as it will cause issues. # The field "cephFS.subvolumeGroup" is optional and defaults to "csi". +# NOTE: The given subvolumeGroup must already exist in the filesystem. # The "cephFS.netNamespaceFilePath" fields are the various network namespace # path for the Ceph cluster identified by the , This will be used # by the CephFS CSI plugin to execute the mount -t in the diff --git a/docs/deploy-cephfs.md b/docs/deploy-cephfs.md index ccdf7062140..8428993ad8d 100644 --- a/docs/deploy-cephfs.md +++ b/docs/deploy-cephfs.md @@ -239,3 +239,9 @@ However, not all KMS are supported in order to be compatible with [fscrypt](https://github.com/google/fscrypt). In general KMS that either store secrets to use directly (Vault), or allow access to the plain password (Kubernetes Secrets) work. + +## CephFS PVC Provisioning + +Requires subvolumegroup to be created before provisioning the PVC. +If the subvolumegroup provided in `ceph-csi-config` ConfigMap is missing +in the ceph cluster, the PVC creation will fail and will stay in `Pending` state. From c5c7709fb445713f79e925925b5e1601da97fc1a Mon Sep 17 00:00:00 2001 From: Praveen M Date: Tue, 7 Nov 2023 16:55:13 +0530 Subject: [PATCH 3/3] ci: create default subvolumegroup Signed-off-by: Praveen M --- PendingReleaseNotes.md | 2 +- scripts/rook.sh | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index 5b1be3f9f3b..1ea10ba8464 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -3,7 +3,7 @@ ## Breaking Changes - Removed the deprecated grpc metrics flag's in [PR](https://github.com/ceph/ceph-csi/pull/4225) - + - Support for pre-creation of cephFS subvolumegroup before creating subvolume is removed in [PR](https://github.com/ceph/ceph-csi/pull/4195) diff --git a/scripts/rook.sh b/scripts/rook.sh index bb02fd85438..c71eec08db6 100755 --- a/scripts/rook.sh +++ b/scripts/rook.sh @@ -5,6 +5,7 @@ ROOK_DEPLOY_TIMEOUT=${ROOK_DEPLOY_TIMEOUT:-300} ROOK_URL="https://raw.githubusercontent.com/rook/rook/${ROOK_VERSION}/deploy/examples" ROOK_BLOCK_POOL_NAME=${ROOK_BLOCK_POOL_NAME:-"newrbdpool"} ROOK_BLOCK_EC_POOL_NAME=${ROOK_BLOCK_EC_POOL_NAME:-"ec-pool"} +ROOK_SUBVOLUMEGROUP_NAME=${ROOK_SUBVOLUMEGROUP_NAME:-"csi"} SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)" # shellcheck disable=SC1091 @@ -53,11 +54,14 @@ function deploy_rook() { cat "${TEMP_DIR}"/cluster-test.yaml kubectl_retry create -f "${TEMP_DIR}/cluster-test.yaml" fi + rm -rf "${TEMP_DIR}" kubectl_retry create -f "${ROOK_URL}/toolbox.yaml" kubectl_retry create -f "${ROOK_URL}/filesystem-test.yaml" kubectl_retry create -f "${ROOK_URL}/pool-test.yaml" + + create_or_delete_subvolumegroup "create" # Check if CephCluster is empty if ! kubectl_retry -n rook-ceph get cephclusters -oyaml | grep 'items: \[\]' &>/dev/null; then @@ -79,6 +83,7 @@ function deploy_rook() { } function teardown_rook() { + create_or_delete_subvolumegroup "delete" kubectl delete -f "${ROOK_URL}/pool-test.yaml" kubectl delete -f "${ROOK_URL}/filesystem-test.yaml" kubectl delete -f "${ROOK_URL}/toolbox.yaml" @@ -88,6 +93,21 @@ function teardown_rook() { kubectl delete -f "${ROOK_URL}/crds.yaml" } +# TODO: to be removed once issue is closed - https://github.com/rook/rook/issues/13040 +function create_or_delete_subvolumegroup() { + local action="$1" + curl -o "subvolumegroup.yaml" "${ROOK_URL}/subvolumegroup.yaml" + sed -i "s|name:.*|name: $ROOK_SUBVOLUMEGROUP_NAME|g" subvolumegroup.yaml + + if [ "$action" == "create" ]; then + kubectl_retry create -f subvolumegroup.yaml + else + kubectl delete -f subvolumegroup.yaml + fi + + rm -f "subvolumegroup.yaml" +} + function create_block_pool() { curl -o newpool.yaml "${ROOK_URL}/pool-test.yaml" sed -i "s/replicapool/$ROOK_BLOCK_POOL_NAME/g" newpool.yaml