Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mgr/nfs: add NFSv3 protocol to default protocols #55693

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

adk3798
Copy link
Contributor

@adk3798 adk3798 commented Feb 21, 2024

To allow users to use NFSv3 with cephadm deployed
nfs daemons and nfs module created exports

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

To allow users to use NFSv3 by default with
exports created through the nfs module

Signed-off-by: Adam King <adking@redhat.com>
To allow users to use NFSv3 with cephadm deployed
nfs daemons by default

Signed-off-by: Adam King <adking@redhat.com>
This seems to be necessary in order to mount exports
wit NFSv3, evne with protocol 3 present in the ganesha
conf and the export

Signed-off-by: Adam King <adking@redhat.com>
@adk3798
Copy link
Contributor Author

adk3798 commented Feb 21, 2024

with all 3 commits here, mounting with NFSv3 seems to work

[root@vm-00 ~]# mount -t nfs -o vers=3 -o port=2049 192.168.122.75:/export_foo /mnt/foo
[root@vm-00 ~]# 
[root@vm-00 ~]# echo "hello" > /mnt/foo/test.txt
[root@vm-00 ~]# 
[root@vm-00 ~]# cat /mnt/foo/test.txt 
hello
[root@vm-00 ~]# 
[root@vm-00 ~]# ssh vm-01
The authenticity of host 'vm-01 (fefe:fefe:fefe:ff00::139)' can't be established.
ED25519 key fingerprint is SHA256:7eKAX0q1Woqvy4vX2tDB5yyyRhy0irgiIoCJs3pGVYc.
This host key is known by the following other names/addresses:
    ~/.ssh/known_hosts:1: 192.168.122.79
Are you sure you want to continue connecting (yes/no/[fingerprint])? yes
Warning: Permanently added 'vm-01' (ED25519) to the list of known hosts.
Activate the web console with: systemctl enable --now cockpit.socket

[root@vm-01 ~]# 
[root@vm-01 ~]# mkdir -p /mnt/foo
[root@vm-01 ~]# 
[root@vm-01 ~]# mount -t nfs -o vers=3 -o port=2049 192.168.122.75:/export_foo /mnt/foo
[root@vm-01 ~]# 
[root@vm-01 ~]# cat /mnt/foo/test.txt 
hello
[root@vm-01 ~]# 

Note that without the mount_path_pseudo = true; patch but still with the other two, this still fails with ganesha.nfsd-2[svc_12] mnt_Mnt :NFS3 :INFO :MOUNT: Export entry / does not support NFS v3 for client ::ffff:192.168.122.75

Copy link
Contributor

@mgfritch mgfritch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure NFSv3 is needed?

Support for this protocol was removed (for both rook and cephadm deployments) when we containerized the nfs-ganesha service (ceph >= octopus)

There was quite a bit of discussion about re-adding this protocol:
#37484 (comment)

But it was eventually marked as deprecated/disabled in various places:
#40008
#41081

@adk3798
Copy link
Contributor Author

adk3798 commented Feb 21, 2024

Are we sure NFSv3 is needed?

Support for this protocol was removed (for both rook and cephadm deployments) when we containerized the nfs-ganesha service (ceph >= octopus)

There was quite a bit of discussion about re-adding this protocol: #37484 (comment)

But it was eventually marked as deprecated/disabled in various places: #40008 #41081

Are we sure NFSv3 is needed?

Support for this protocol was removed (for both rook and cephadm deployments) when we containerized the nfs-ganesha service (ceph >= octopus)

There was quite a bit of discussion about re-adding this protocol: #37484 (comment)

But it was eventually marked as deprecated/disabled in various places: #40008 #41081

for context, this was coming from downstream, i believe so that some Windows client could mount the export using NFSv3. I wouldn't actually expect people to use NFSv3 over NFSv4 for the most part. But, I'm willing to mention those problems discussed in the comment you linked or maybe lock this behind some flag rather than having it by default (and not documenting it). The idea was just to make it possible for those certain clients without them needing to manually update the ganesha conf and export to have NFSv3 listed as a protocol.

@adk3798
Copy link
Contributor Author

adk3798 commented Feb 28, 2024

If we were to go for this, test failure

2024-02-27T06:26:42.790 INFO:tasks.cephfs_test_runner:FAIL: test_create_and_delete_export (tasks.cephfs.test_nfs.TestNFS)
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:Test successful creation and deletion of the cephfs export.
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:----------------------------------------------------------------------
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:Traceback (most recent call last):
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:  File "/home/teuthworker/src/git.ceph.com_ceph-c_635ee6850c57c98cc6ba932cade45f9cc015aa32/qa/tasks/cephfs/test_nfs.py", line 449, in test_create_and_delete_export
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:    self._test_get_export()
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:  File "/home/teuthworker/src/git.ceph.com_ceph-c_635ee6850c57c98cc6ba932cade45f9cc015aa32/qa/tasks/cephfs/test_nfs.py", line 285, in _test_get_export
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:    self.assertDictEqual(self.sample_export, nfs_output)
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:AssertionError: {'export_id': 1, 'path': '/', 'cluster_id':[219 chars]: []} != {'access_type': 'RW', 'clients': [], 'clust[251 chars]CP']}
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:  {'access_type': 'RW',
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:   'clients': [],
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:   'cluster_id': 'test',
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:   'export_id': 1,
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:-  'fsal': {'fs_name': 'nfs-cephfs', 'name': 'CEPH', 'user_id': 'nfs.test.1'},
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:+  'fsal': {'cmount_path': '/',
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:+           'fs_name': 'nfs-cephfs',
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:+           'name': 'CEPH',
2024-02-27T06:26:42.791 INFO:tasks.cephfs_test_runner:+           'user_id': 'nfs.test.nfs-cephfs'},
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:   'path': '/',
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:-  'protocols': [4],
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:+  'protocols': [3, 4],
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:?                +++
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:   'pseudo': '/cephfs',
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:   'security_label': True,
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:   'squash': 'none',
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:   'transports': ['TCP']}
2024-02-27T06:26:42.792 INFO:tasks.cephfs_test_runner:

only the protocols difference is actually from this PR, the rest is form another one in the run, but that would have to be fixed if this was to be merged.

Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants