docs: update NFS docs #9281

BlaineEXE · 2021-11-30T23:26:04Z

NFS docs need to be updated to reflect changes between Octopus and
Pacific, how users are affected, how NFS is configured differently
between the Ceph versions, and how to upgrade from one Ceph version to
the other.

Signed-off-by: Blaine Gardner blaine.gardner@redhat.com

Description of your changes:

Which issue is resolved by this Pull Request:
Resolves #

Checklist:

travisn · 2021-12-07T20:56:23Z

Documentation/ceph-nfs-crd.md


 ## Overview

-Rook allows exporting NFS shares of the filesystem or object store through the CephNFS custom resource definition. This will spin up a cluster of [NFS Ganesha](https://github.com/nfs-ganesha/nfs-ganesha) servers that coordinate with one another via shared RADOS objects. The servers will be configured for NFSv4.1+ access, as serving earlier protocols can inhibit responsiveness after a server restart.
+> **WARNING**: We do not recommend using NFS in Ceph v16.2.0 through v16.2.6. If you are using Ceph


How about moving the warning after the opening paragraph? Then we can start off with a description about the feature before we warn them not to use it. :)

travisn · 2021-12-07T20:57:17Z

Documentation/ceph-nfs-crd.md


 ## Overview

-Rook allows exporting NFS shares of the filesystem or object store through the CephNFS custom resource definition. This will spin up a cluster of [NFS Ganesha](https://github.com/nfs-ganesha/nfs-ganesha) servers that coordinate with one another via shared RADOS objects. The servers will be configured for NFSv4.1+ access, as serving earlier protocols can inhibit responsiveness after a server restart.
+> **WARNING**: We do not recommend using NFS in Ceph v16.2.0 through v16.2.6. If you are using Ceph
+> v15, please wait to upgrade until v16.2.7 is released.


This statement will hopefully be obsolete in the next few days when it's released. What if we just assume it's released so we don't need to revisit?

travisn · 2021-12-07T21:02:30Z

Documentation/ceph-nfs-crd.md


-The following sample will create a two-node active-active cluster of NFS Ganesha gateways. The recovery objects are stored in a RADOS pool named `myfs-data0` with a RADOS namespace of `nfs-ns`.
+## Samples
+The following sample assumes Ceph v15 and will create a two-node active-active cluster of NFS


Are we actually assuming v16.2.7 now, instead of v15?

I can change this and the example manifest I think

travisn · 2021-12-07T21:03:48Z

Documentation/ceph-nfs-crd.md

+When a CephNFS is first created, all NFS daemons within the CephNFS cluster will share a
+configuration with no exports defined.
+
+### For Ceph v15


Do we need to document v15 exports? Seems like we should concentrate docs on 16.2.7+.

We still support v15, and I still took the time to write the documentation, so I don't see the hurt.

This looks more like a Ceph documentation rather than a Rook at this point 😅

Documentation/ceph-nfs-crd.md

thotz · 2021-12-08T05:52:39Z

Documentation/ceph-nfs-crd.md

@@ -72,55 +82,395 @@ spec:
    priorityClassName:
 ```



@BlaineEXE : I have a generic question, is it planned to support CRs for Exports in near future?

Seb and I believe that exports are something that should be enabled by a new mode added to the Ceph-CSI driver rather than CRs.

leseb · 2021-12-08T11:05:09Z

Documentation/ceph-nfs-crd.md


 ## Overview
+Rook allows exporting NFS shares of the filesystem or object store through the CephNFS custom


For the "object store" are you referring to rgw-nfs? If so, it's not using CephFS but simply RGW.

I will clarify by changing this to CephFilesystem and CephObjectStore. I'm not sure what rgw-nfs is?

leseb · 2021-12-08T11:08:48Z

Documentation/ceph-nfs-crd.md

+NFS configuration is stored in a Ceph pool so that it is highly available and protected. How that is
+configured changes depending on the Ceph version. Configuring the pool is done via the `rados` config.
+
+> **WARNING**: Do not use error corrected (EC) pools for NFS. NFS-Ganesha uses OMAP which is not


Error corrected pools? Did you mean Erasure Coded pools? It's the first time I'm hearing this term in Ceph... If I missed something, how do people know they are using an error corrected pool?

YES LOL!

I'll add a link

leseb · 2021-12-08T11:11:57Z

Documentation/ceph-nfs-crd.md

+When a CephNFS is first created, all NFS daemons within the CephNFS cluster will share a
+configuration with no exports defined.
+
+### For Ceph v15


This looks more like a Ceph documentation rather than a Rook at this point 😅

leseb · 2021-12-08T11:13:36Z

Documentation/ceph-nfs-crd.md

+ensure the necessary Ceph mgr modules are enabled and that the Ceph orchestrator backend is set to
+Rook.
+```console
+ceph mgr module enable rook


Shouldn't we recommend setting the enabled module in the mgr spec instead of jumping in the toolbox? Also I think the porch rook backend might be enabled already...

They have to use the toolbox to create the exports anyway. Because of that, IMO, it's simpler just to say "enable this in the toolbox" so we don't have to cross-link to the CephCluster. But this is a nuance I think we could talk about more. Maybe that would be fine for a follow-up? @leseb

Yes and I don't feel really strong about it too, also later on you recommend disabling the module so it's probably not worth editing the CR for this :).

leseb · 2021-12-08T11:14:59Z

Documentation/ceph-nfs-crd.md

+```
+mount -t nfs4 -o proto=tcp <nfs-service-ip>:/ <mount-location>
+```
+


unecessary blank line?

leseb · 2021-12-08T11:15:36Z

Documentation/ceph-nfs-crd.md

+
+## Upgrading from Ceph v15 to v16
+We do not recommend using NFS in Ceph v16.2.0 through v16.2.6 due to bugs in Ceph's NFS
+implementation. If you are using Ceph v15, please wait to upgrade until Ceph v16.2.7 is released.


v16.2.7 is available now.

leseb · 2021-12-08T11:18:55Z

Documentation/ceph-nfs-crd.md

+started. Scaling down the cluster requires that clients be migrated from servers that will be
+eliminated to others. That process is currently a manual one and should be performed before reducing
+the size of the cluster.
+


unecessary blank line?

I've been keeping two newlines between the major lvl-2 headers to help with navigating the raw .md file a bit easier.

leseb · 2021-12-08T11:20:38Z

deploy/examples/nfs-test.yaml

-    # RADOS namespace where NFS client recovery data is stored in the pool.
-    namespace: nfs-ns
+
+  # For Ceph v15, use the block here.


Do we care for the -test.taml? The cluster-test.yaml points to v16.2 already so I don't think we need to repeat this.

Yeah, I can remove the changes in nfs-test.yaml

Documentation/ceph-nfs-crd.md

thotz · 2021-12-08T15:20:50Z

Documentation/ceph-nfs-crd.md

+Rook allows exporting NFS shares of the filesystem or object store through the CephNFS custom
+resource definition. This will spin up a cluster of
+[NFS Ganesha](https://github.com/nfs-ganesha/nfs-ganesha) servers that coordinate with one another
+via shared RADOS objects. The servers will be configured for NFSv4.1+ access only, as serving


Here it is mention nfsv4.1+ but all the below examples is defined with nfsv4

I think that's fine in the two other places where it appears. The .1+ isn't strictly necessary to know for those since v4.1 is also still v4.

BlaineEXE · 2021-12-08T16:09:03Z

deploy/examples/nfs-test.yaml

-    # RADOS namespace where NFS client recovery data is stored in the pool.
-    namespace: nfs-ns


I did remove this from nfs-test.yaml because it is only relevant for octopus.

NFS docs need to be updated to reflect changes between Octopus and Pacific, how users are affected, how NFS is configured differently between the Ceph versions, and how to upgrade from one Ceph version to the other. Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>

Documentation/ceph-nfs-crd.md

travisn · 2021-12-08T16:54:02Z

deploy/examples/nfs.yaml

  rados:
-    # The Ganesha pool spec. Must use replication.
    poolConfig:


Should the poolConfig be under rados? If so, the comment above on line 8 is confusing about how the rados settings aren't necessary now.

I suppose we do have time to change this before cutting the 1.8 release since it was added recently by Seb. IMO, I would put poolConfig on the top level, like you are suggesting. I think having a rados block seems a little too specific. Let's bring @leseb into this discussion.

For now, I'll merge this since I think changing the spec is orthogonal and good for a follow-up PR.

Oh right, we still have time before cutting 1.8. I guess this used to be valid while the RadosSpec made sense, but not anymore. +1 from moving to the top-level.

I also wonder whether we should have this here or not. If there are multiple CephNFS specs, they may contend with each other to set different replication settings on the same .nfs pool. Maybe we should just remove this config and allow .nfs to be managed by @travisn 's suggestion here: #9209 (comment)

I'm not sure, but I agree the multiple CR is a concern. However, is this a valid use case? Should we support it? Especially if they all end up using the same pools.
I'm not really keen on leaving the pool management to the user, the nfs implementation is already complex enough (at least the migration).

docs: update NFS docs (backport #9281)

BlaineEXE force-pushed the nfs-docs branch 2 times, most recently from 3bc6980 to f013032 Compare December 7, 2021 20:37

BlaineEXE marked this pull request as ready for review December 7, 2021 20:37

BlaineEXE requested review from travisn and leseb December 7, 2021 20:37

travisn added the backport-release-1.8 label Dec 7, 2021

travisn requested changes Dec 7, 2021

View reviewed changes

BlaineEXE force-pushed the nfs-docs branch from f013032 to d0fed43 Compare December 7, 2021 21:27

BlaineEXE requested a review from travisn December 7, 2021 21:27

travisn requested changes Dec 7, 2021

View reviewed changes

Documentation/ceph-nfs-crd.md Show resolved Hide resolved

Documentation/ceph-nfs-crd.md Show resolved Hide resolved

thotz reviewed Dec 8, 2021

View reviewed changes

leseb reviewed Dec 8, 2021

View reviewed changes

thotz reviewed Dec 8, 2021

View reviewed changes

BlaineEXE force-pushed the nfs-docs branch from d0fed43 to 5c3329c Compare December 8, 2021 16:06

BlaineEXE requested review from travisn, leseb and thotz December 8, 2021 16:06

BlaineEXE commented Dec 8, 2021

View reviewed changes

BlaineEXE force-pushed the nfs-docs branch 2 times, most recently from 24108e2 to 27a0a6e Compare December 8, 2021 16:12

BlaineEXE force-pushed the nfs-docs branch from 27a0a6e to e98959c Compare December 8, 2021 16:14

leseb approved these changes Dec 8, 2021

View reviewed changes

travisn approved these changes Dec 8, 2021

View reviewed changes

BlaineEXE merged commit d20692a into rook:master Dec 8, 2021

BlaineEXE deleted the nfs-docs branch December 8, 2021 17:00

mergify bot mentioned this pull request Dec 8, 2021

docs: update NFS docs (backport #9281) #9351

Merged

BlaineEXE added a commit that referenced this pull request Dec 8, 2021

Merge pull request #9351 from rook/mergify/bp/release-1.8/pr-9281

f0e19ea

docs: update NFS docs (backport #9281)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: update NFS docs #9281

docs: update NFS docs #9281

BlaineEXE commented Nov 30, 2021

travisn Dec 7, 2021

BlaineEXE Dec 7, 2021

travisn Dec 7, 2021

BlaineEXE Dec 7, 2021

travisn Dec 7, 2021

BlaineEXE Dec 7, 2021

travisn Dec 7, 2021

BlaineEXE Dec 7, 2021

leseb Dec 8, 2021

thotz Dec 8, 2021

BlaineEXE Dec 8, 2021

leseb Dec 8, 2021

BlaineEXE Dec 8, 2021

leseb Dec 8, 2021

BlaineEXE Dec 8, 2021

leseb Dec 8, 2021

leseb Dec 8, 2021

BlaineEXE Dec 8, 2021

leseb Dec 8, 2021

leseb Dec 8, 2021

leseb Dec 8, 2021

leseb Dec 8, 2021

BlaineEXE Dec 8, 2021

leseb Dec 8, 2021

BlaineEXE Dec 8, 2021

thotz Dec 8, 2021

BlaineEXE Dec 8, 2021

BlaineEXE Dec 8, 2021

travisn Dec 8, 2021

BlaineEXE Dec 8, 2021 •

edited

BlaineEXE Dec 8, 2021

leseb Dec 8, 2021

BlaineEXE Dec 8, 2021

leseb Dec 8, 2021


		## Overview
		Rook allows exporting NFS shares of the filesystem or object store through the CephNFS custom

		# RADOS namespace where NFS client recovery data is stored in the pool.
		namespace: nfs-ns

docs: update NFS docs #9281

docs: update NFS docs #9281

Conversation

BlaineEXE commented Nov 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlaineEXE Dec 8, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlaineEXE Dec 8, 2021 •

edited