mgr/nfs: adjust export creation CLI args, and allow rgw user exports #43611

liewegas · 2021-10-20T21:36:43Z

for nfs export create rgw, take either bucket or user or both. If we get only the user, do an export for the user that shows buckets at the top level. If bucket+user, then use that user instead of the bucket owner. If bucket only, behavior is as before (use bucket owner as the user).
adjust hte CLI arg position for nfs export create cephfs to put the fsname as the last positional argument. This aligns the 2 export create commands so that cluster_id and pseudo_path both come first (which FWIW is more intutive anyway)
adjust tests so that all callers use named arguments for all of these commands, so that they work both before and after the CLI positional argument order change.

Note that the change for the nfs export create cephfs could be dropped, but I think it si worth the potential user breakage to end up with a better CLI.

TODO

add release note, and encourage users to use named arguments whenever using these interfaces
add test for rgw user-level export (including bucket creation with mkdir)

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

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

ajarr · 2021-10-25T22:51:21Z

do we need a tracker ticket for this PR to make it easier for pacific backport?

alfonsomthd · 2021-10-27T14:02:53Z

@liewegas In the GUI we dropped the rgw user id selection because the rgw user was not customizable: the bucket was used as the path and the bucket owner as the user. Can you give more context here on what are the implications for the Dashboard?

liewegas · 2021-10-27T14:27:21Z

@liewegas In the GUI we dropped the rgw user id selection because the rgw user was not customizable: the bucket was used as the path and the bucket owner as the user. Can you give more context here on what are the implications for the Dashboard?

It turns out there are two kinds of NFS exports for RGW: bucket and user. The bucket ones are currently implemented (and you're right--we just use the bucket owner as the user in that case). What I missed was that you can also export an RGW user, in which case the top-level directory is a list of the buckets owned by that user, mkdir creates a bucket, etc.

So this PR makes 2 changes:

you can create a user export (by passing --user <user> only)
you can create a bucket export and also specify the rgw user (by passing both --user ... and --bucket ...).

I forgot about the dashboard implications, as per usual (sigh!). I suspect the UI probably should have a radio box to select the type of export? And make the user optional the bucket case?

alfonsomthd · 2021-10-27T15:03:39Z

you can create a bucket export and also specify the rgw user (by passing both --user ... and --bucket ...).

I understand that if both (user & bucket) are passed, it creates an export that:

the export user is the bucket owner.
The bucket is the export path.
If the bucket does not exist for that user, the NFS module will trigger the bucket creation.

Is this correct?

liewegas · 2021-10-27T15:13:34Z

the export user is the bucket owner.

Almost. If you pass both user and bucket, then

it is a bucket export
the user is the specified user (not the bucket owner)

I don't think it will create the bucket for you (at least I have not heard of such a thing).

If you pass bucket only, then we default to the bucket owner.

github-actions · 2021-10-31T13:02:37Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

Signed-off-by: Sage Weil <sage@newdream.net>

Put fsname after cluster_id + pseudo_path so that it aligns with the change to the rgw command. Signed-off-by: Sage Weil <sage@newdream.net>

Put bucket name last so that it paves the way for an optional 'user' arg to go along with it. Signed-off-by: Sage Weil <sage@newdream.net>

liewegas · 2021-11-02T21:56:37Z

jenkins test make check

liewegas · 2021-11-03T02:07:05Z

jenkins test make check

liewegas · 2021-11-03T02:07:41Z

@sebastian-philipp @alfonsomthd This is passing tests now!

liewegas · 2021-11-03T14:37:31Z

jenkins test api

mgfritch

one minor issue in the error path, but otherwise lgtm!

src/pybind/mgr/nfs/export.py

ajarr

Looks good!

src/pybind/mgr/nfs/module.py

src/pybind/mgr/nfs/tests/test_nfs.py

- move the bucket / user position after the cluster_id and pseudo_path (since they are optional) - require bucket or user or both - if bucket, use the bucket owner - if bucket+user, use that user - if user only, then export at top-level (all users buckets) Fixes: https://tracker.ceph.com/issues/53134 Signed-off-by: Sage Weil <sage@newdream.net>

Signed-off-by: Sage Weil <sage@newdream.net>

liewegas requested a review from a team as a code owner October 20, 2021 21:36

github-actions bot added cephadm documentation mgr nfs pybind labels Oct 20, 2021

liewegas force-pushed the nfs-rgw-by-user branch from 0b4864c to 6262367 Compare October 25, 2021 14:54

epuertat requested a review from alfonsomthd October 27, 2021 11:05

github-actions bot added the needs-rebase label Oct 31, 2021

liewegas force-pushed the nfs-rgw-by-user branch from 4ed1e43 to 7cce068 Compare November 1, 2021 15:48

liewegas added wip-sage-testing wip-sage2-testing and removed wip-sage-testing labels Nov 1, 2021

github-actions bot removed the needs-rebase label Nov 1, 2021

liewegas force-pushed the nfs-rgw-by-user branch 4 times, most recently from df104aa to 70b092a Compare November 2, 2021 13:43

pybind/ceph_argparse: handle misordered keyword arguments

1b2ed89

Signed-off-by: Sage Weil <sage@newdream.net>

liewegas force-pushed the nfs-rgw-by-user branch from 70b092a to 6916abf Compare November 2, 2021 16:38

github-actions bot added cephfs Ceph File System tests labels Nov 2, 2021

liewegas force-pushed the nfs-rgw-by-user branch 2 times, most recently from a1e6a78 to 900c6de Compare November 2, 2021 20:29

liewegas added 5 commits November 2, 2021 17:06

qa/tasks/cephfs/test_nfs: use keyword args

fa4ee0f

Signed-off-by: Sage Weil <sage@newdream.net>

mgr/nfs: document and use keyword args for 'nfs export create cephfs'

9467f1e

Signed-off-by: Sage Weil <sage@newdream.net>

mgr/nfs: use keyword args for 'nfs export create rgw'

aef952b

Signed-off-by: Sage Weil <sage@newdream.net>

mgr/nfs: reorder 'nfs export create cephfs' arguments

a07ca24

Put fsname after cluster_id + pseudo_path so that it aligns with the change to the rgw command. Signed-off-by: Sage Weil <sage@newdream.net>

mgr/nfs: reorder 'nfs export creage rgw' arguments

89e73a2

Put bucket name last so that it paves the way for an optional 'user' arg to go along with it. Signed-off-by: Sage Weil <sage@newdream.net>

liewegas force-pushed the nfs-rgw-by-user branch from 900c6de to 4be9b2f Compare November 2, 2021 21:07

liewegas force-pushed the nfs-rgw-by-user branch from 4be9b2f to 428a872 Compare November 3, 2021 02:11

mgfritch approved these changes Nov 4, 2021

View reviewed changes

src/pybind/mgr/nfs/export.py Show resolved Hide resolved

ajarr approved these changes Nov 4, 2021

View reviewed changes

src/pybind/mgr/nfs/module.py Show resolved Hide resolved

alfonsomthd approved these changes Nov 4, 2021

View reviewed changes

sebastian-philipp approved these changes Nov 4, 2021

View reviewed changes

src/pybind/mgr/nfs/tests/test_nfs.py Outdated Show resolved Hide resolved

liewegas added 4 commits November 4, 2021 10:42

qa/suites/orch/cephadm/smoke-roleless: add rgw user nfs export case

d354baa

Signed-off-by: Sage Weil <sage@newdream.net>

PendingReleaseNotes: add note about nfs CLI change(s)

48ce4e9

Signed-off-by: Sage Weil <sage@newdream.net>

doc/mgr/nfs: document rgw user and bucket exports

b6d85e3

Signed-off-by: Sage Weil <sage@newdream.net>

liewegas force-pushed the nfs-rgw-by-user branch from 428a872 to b6d85e3 Compare November 4, 2021 14:42

liewegas merged commit d87d2bd into ceph:master Nov 4, 2021
8 checks passed

liewegas mentioned this pull request Nov 4, 2021

pacific: backport mgr/nfs bits #43811

Merged

1 task

liewegas mentioned this pull request Nov 11, 2021

pacific: backport mgr/nfs bits #43900

Closed

3 tasks

mgr/nfs: adjust export creation CLI args, and allow rgw user exports #43611

mgr/nfs: adjust export creation CLI args, and allow rgw user exports #43611

liewegas commented Oct 20, 2021 •

edited

ajarr commented Oct 25, 2021

alfonsomthd commented Oct 27, 2021

liewegas commented Oct 27, 2021

alfonsomthd commented Oct 27, 2021 •

edited

liewegas commented Oct 27, 2021

github-actions bot commented Oct 31, 2021

liewegas commented Nov 2, 2021

liewegas commented Nov 3, 2021

liewegas commented Nov 3, 2021

liewegas commented Nov 3, 2021

mgfritch left a comment

ajarr left a comment

mgr/nfs: adjust export creation CLI args, and allow rgw user exports #43611

mgr/nfs: adjust export creation CLI args, and allow rgw user exports #43611

Conversation

liewegas commented Oct 20, 2021 • edited

Checklist

ajarr commented Oct 25, 2021

alfonsomthd commented Oct 27, 2021

liewegas commented Oct 27, 2021

alfonsomthd commented Oct 27, 2021 • edited

liewegas commented Oct 27, 2021

github-actions bot commented Oct 31, 2021

liewegas commented Nov 2, 2021

liewegas commented Nov 3, 2021

liewegas commented Nov 3, 2021

liewegas commented Nov 3, 2021

mgfritch left a comment

Choose a reason for hiding this comment

ajarr left a comment

Choose a reason for hiding this comment

liewegas commented Oct 20, 2021 •

edited

alfonsomthd commented Oct 27, 2021 •

edited