ceph: add cephfs mirroring support by leseb · Pull Request #7062 · rook/rook

leseb · 2021-01-25T09:23:16Z

Description of your changes:

With Ceph Pacific, Rook can now deploy the cephfs-mirror daemon.
This initial commit covers the deployment of a single daemon only.
Multiple mirror daemons is currently untested.
Only a single mirror daemon is recommended.

The configuration of peers will come in a later PR since the mgr module
is still pending upstream: ceph/ceph#39050

The same goes for integration tests, they will get added later once we
start testing on Pacific.

Closes: #7002
Signed-off-by: Sébastien Han seb@redhat.com

Which issue is resolved by this Pull Request:
Resolves #

Checklist:

travisn · 2021-01-26T23:16:39Z

Documentation/ceph-filesystem.md

 kubectl create -f cluster/examples/kubernetes/ceph/csi/cephfs/storageclass.yaml
 ```

+## Mirroring


Why do we have both the mirror.enabled flag and the CRD? I don't see why we need both.

The CRD deploys the cephfs-mirror daemon, the CephFilesystem setting, turns on mirroring on that filesystem.

will this enable mirroring of complete filesystem by default? do we have an option to enable per subvolume not per filesystem like we do for RBD?

Yes, but just like rbd-mirroring must be first enabled on the pool and then per-image, we have to enable the mirroring on the fs and then we can do snapshot per-directory using subcommand like: ceph fs snapshot mirror add <fs> <path>.

travisn · 2021-01-26T23:18:55Z

Documentation/ceph-fs-mirror-crd.md

@@ -0,0 +1,46 @@
+---
+title: FilesystemMirror CRD
+weight: 3500


This weight should put it after the rbd mirroring doc in the TOC

Suggested change

weight: 3500

weight: 3600

travisn · 2021-01-26T23:26:26Z

Documentation/ceph-fs-mirror-crd.md

+
+# Ceph FilesystemMirror CRD
+
+Rook allows creation and updating fs-mirror daemon through the custom resource definitions (CRDs).


Suggested change

Rook allows creation and updating fs-mirror daemon through the custom resource definitions (CRDs).

Rook allows creation and updating the fs-mirror daemon through the custom resource definitions (CRDs).

travisn · 2021-01-26T23:27:52Z

Documentation/ceph-fs-mirror-crd.md

+metadata:
+  name: my-fs-mirror
+  namespace: rook-ceph
+```


Are there no settings like the number of mirroring daemons to start? Or is only a single daemon supported?

I didn't add a count since the doc says: Multiple mirror daemons is currently untested. Only a single mirror daemon is recommended..
We can always add it later.

do we need to pass some tokens which contain remote information that the mirroring daemon will connect to?

Yes, but this is coming in another PR, I'm waiting for ceph/ceph#39050 to merge so I can start the implementation.

travisn · 2021-01-26T23:30:10Z

cluster/examples/kubernetes/ceph/filesystem-mirror.yaml

+  #        - key: role
+  #          operator: In
+  #          values:
+  #          - rgw-node


Suggested change

# - rgw-node

# - cephfs-mirror

travisn · 2021-01-26T23:38:18Z

pkg/operator/ceph/file/controller.go

 	}

+	// Enable mirroring if needed
+	if r.clusterInfo.CephVersion.IsAtLeastPacific() {


Shouldn't this be done by the new cephfs mirroring controller? I must be missing why we need the mirroring.enabled setting in the filesystem since we have the separate CRD.

The CRD deploys the cephfs-mirror daemon, the CephFilesystem setting, turns on mirroring on that filesystem. So let's imagine we have multiple CephFilesystem deployed, we don't want all of them to be mirrored, that's why I'm basing the decision out of the CephFilesystem CRD and the CephFilesystemMirror CRD, which goal is to only deploy the cephfs-mirror at the moment. Later it will add/remove peers once available in Ceph.

I don't see anything specific to this filesystem being enabled, I only see a merge module being enabled. Is there another setting coming that will be specific to the filesystem?

Since enabling the module only needs to happen once for the entire cluster, this still seems like it belongs with the singleton mirroring CR instead of being enabled as a property on the filesystem. Why not enable the module with the mirroring CR, and get rid of the filesystem CR setting? With rbd mirroring there is a pool-specific setting so it makes sense to have the boolean on the pool CR. But if there is no filesystem-specific setting for mirroring, I don't see why a bool is needed on the filesystem CR.

You don't see anything specific to the fs because I'm waiting for ceph/ceph#39050 to merge so I can turn on mirroring for the fs. Now, we just enable the mirroring module. Anyway, we will always need the fs setting to turn on mirroring.

Honestly, I have no preference, I can move this cephclient.MgrEnableModule to the CephFilesysteMirror CRD. Either way, we will have to enable it.

What do you think?

Hope that clarifies.

Got it, this is basically a placeholder for the per-filesystem setting that will come later.
Whether to enable the merge module sounds fine to keep here, as discussed it could go either here or with the mirroring CR.

pkg/operator/ceph/file/mirror/config.go

pkg/operator/ceph/file/mirror/controller.go

pkg/operator/ceph/file/mirror/mirror.go

travisn · 2021-01-26T23:53:59Z

Documentation/ceph-filesystem.md

+Since Ceph Pacific, CephFS supports asynchronous replication of snapshots to a remote CephFS file system via cephfs-mirror tool. Snapshots are synchronized by mirroring snapshot data followed by creating a snapshot with the same name (for a given directory on the remote file system) as the snapshot being synchronized.
+It is generally useful when planning for Disaster Recovery.
+For clusters that are geographically distributed and stretching is not possible due to high latencies.
+


How do you setup the peer? Still working on that?

Yes, just waiting for ceph/ceph#39050

Madhu-1 · 2021-01-27T08:43:12Z

Documentation/ceph-filesystem.md

 kubectl create -f cluster/examples/kubernetes/ceph/csi/cephfs/storageclass.yaml
 ```

+## Mirroring


will this enable mirroring of complete filesystem by default? do we have an option to enable per subvolume not per filesystem like we do for RBD?

Madhu-1 · 2021-01-27T08:44:28Z

Documentation/ceph-fs-mirror-crd.md

+# Ceph FilesystemMirror CRD
+
+Rook allows creation and updating fs-mirror daemon through the custom resource definitions (CRDs).
+CephFS images can be asynchronously mirrored between two Ceph clusters.


CephFS images or filesystem or even subvolumes?

I should remove this line, only the following sentences are relevant.

Madhu-1 · 2021-01-27T08:48:28Z

Documentation/ceph-fs-mirror-crd.md

+metadata:
+  name: my-fs-mirror
+  namespace: rook-ceph
+```


do we need to pass some tokens which contain remote information that the mirroring daemon will connect to?

mergify · 2021-01-28T18:11:16Z

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/master/development-flow.html#updating-your-fork

With Ceph Pacific, Rook can now deploy the cephfs-mirror daemon. This initial commit covers the deployment of a single daemon only. Multiple mirror daemons is currently untested. Only a single mirror daemon is recommended. The configuration of peers will come in a later PR since the mgr module is still pending upstream: ceph/ceph#39050 The same goes for integration tests, they will get added later once we start testing on Pacific. Closes: rook#7002 Signed-off-by: Sébastien Han <seb@redhat.com>

travisn · 2021-01-28T20:53:03Z

pkg/operator/ceph/file/mirror/mirror.go

+		return errors.Wrapf(err, "failed to get controller %q owner reference", filesystemMirror.Name)
+	}
+
+	daemonID := k8sutil.IndexToName(0)


Only a single mirroring configuration is allowed per rook cluster, right? Do we need to prevent multiple mirroring CRs from being created? (similar to how we only allow a single CephCluster CR in a namespace)

Only a single mirroring configuration is allowed per rook cluster, right?

No, it's per filesystem.

Do we need to prevent multiple mirroring CRs from being created? (similar to how we only allow a single CephCluster CR in a namespace)

Not really since we support multiple fs.

Per discussion, agreed that only a single mirroring CR is supported, but no need for extra restrictions since we expect users to follow the docs and examples.

travisn · 2021-01-28T20:55:43Z

pkg/operator/ceph/file/controller.go

 	}

+	// Enable mirroring if needed
+	if r.clusterInfo.CephVersion.IsAtLeastPacific() {


Since enabling the module only needs to happen once for the entire cluster, this still seems like it belongs with the singleton mirroring CR instead of being enabled as a property on the filesystem. Why not enable the module with the mirroring CR, and get rid of the filesystem CR setting? With rbd mirroring there is a pool-specific setting so it makes sense to have the boolean on the pool CR. But if there is no filesystem-specific setting for mirroring, I don't see why a bool is needed on the filesystem CR.

travisn · 2021-01-29T15:45:40Z

pkg/operator/ceph/file/controller.go

 	}

+	// Enable mirroring if needed
+	if r.clusterInfo.CephVersion.IsAtLeastPacific() {


Got it, this is basically a placeholder for the per-filesystem setting that will come later.
Whether to enable the merge module sounds fine to keep here, as discussed it could go either here or with the mirroring CR.

travisn · 2021-01-29T15:48:00Z

pkg/operator/ceph/file/mirror/mirror.go

+		return errors.Wrapf(err, "failed to get controller %q owner reference", filesystemMirror.Name)
+	}
+
+	daemonID := k8sutil.IndexToName(0)


Per discussion, agreed that only a single mirroring CR is supported, but no need for extra restrictions since we expect users to follow the docs and examples.

mergify bot added the ceph main ceph tag label Jan 25, 2021

leseb requested review from BlaineEXE and travisn January 25, 2021 09:25

leseb force-pushed the cephfs-mirror branch 10 times, most recently from a5ba3a8 to d82e932 Compare January 26, 2021 13:25

travisn requested changes Jan 26, 2021

View reviewed changes

leseb force-pushed the cephfs-mirror branch from d82e932 to 6393012 Compare January 27, 2021 08:51

Madhu-1 reviewed Jan 27, 2021

View reviewed changes

leseb force-pushed the cephfs-mirror branch from 6393012 to a516e99 Compare January 27, 2021 09:16

leseb requested a review from travisn January 27, 2021 09:46

leseb force-pushed the cephfs-mirror branch from a516e99 to c0123cf Compare January 28, 2021 18:21

travisn requested changes Jan 28, 2021

View reviewed changes

travisn approved these changes Jan 29, 2021

View reviewed changes

travisn merged commit 0a1bfe5 into rook:master Jan 29, 2021

leseb deleted the cephfs-mirror branch January 29, 2021 16:35


		# Ceph FilesystemMirror CRD

		Rook allows creation and updating fs-mirror daemon through the custom resource definitions (CRDs).

	Rook allows creation and updating fs-mirror daemon through the custom resource definitions (CRDs).
	Rook allows creation and updating the fs-mirror daemon through the custom resource definitions (CRDs).

Conversation

leseb commented Jan 25, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Jan 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants