mgr/nfs: generate user_id & access_key for apply_export(CEPHFS) #54277

avanthakkar · 2023-10-31T16:01:31Z

Generate user_id & secret_access_key for CephFS based exports for apply export. Also ensure the export FSAL block has cmount_path.

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)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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

avanthakkar · 2023-10-31T16:10:04Z

After #53925

phlogistonjohn

Looks OK to me.

avanthakkar · 2023-11-07T12:56:15Z

jenkins test api

avanthakkar · 2024-01-25T15:40:54Z

jenkins test api

avanthakkar · 2024-02-04T14:28:58Z

@adk3798 I've rebased the PR & removed the DNM label. I think we can have this merged once CI passes & close #53925

adk3798 · 2024-02-12T23:19:37Z

src/pybind/mgr/nfs/export.py

+        fsal.user_id = f"nfs.{export.cluster_id}.{export.fsal.fs_name}"
+        fsal.cephx_key = self._create_user_key(
+            export.cluster_id, fsal.user_id, fsal.cmount_path, fsal.fs_name
+        )


I think this change in the naming structure is causing a test failure

2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner:====================================================================== 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner:FAIL: test_create_and_delete_export (tasks.cephfs.test_nfs.TestNFS) 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner:Test successful creation and deletion of the cephfs export. 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner:---------------------------------------------------------------------- 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner:Traceback (most recent call last): 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner: File "/home/teuthworker/src/git.ceph.com_ceph-c_2b25e10cc573cee6dfe2f5900f62ba4e83f9ac06/qa/tasks/cephfs/test_nfs.py", line 448, in test_create_and_delete_export 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner: self._create_default_export() 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner: File "/home/teuthworker/src/git.ceph.com_ceph-c_2b25e10cc573cee6dfe2f5900f62ba4e83f9ac06/qa/tasks/cephfs/test_nfs.py", line 231, in _create_default_export 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner: self._create_export(export_id='1', create_fs=True) 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner: File "/home/teuthworker/src/git.ceph.com_ceph-c_2b25e10cc573cee6dfe2f5900f62ba4e83f9ac06/qa/tasks/cephfs/test_nfs.py", line 219, in _create_export 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner: self._check_auth_ls(export_id, check_in=True) 2024-02-09T13:08:07.529 INFO:tasks.cephfs_test_runner: File "/home/teuthworker/src/git.ceph.com_ceph-c_2b25e10cc573cee6dfe2f5900f62ba4e83f9ac06/qa/tasks/cephfs/test_nfs.py", line 130, in _check_auth_ls 2024-02-09T13:08:07.530 INFO:tasks.cephfs_test_runner: self.assertIn(f'{client_id}.{export_id}', output) 2024-02-09T13:08:07.530 INFO:tasks.cephfs_test_runner:AssertionError: 'client.nfs.test.1' not found in 'mds.a.smithi179.omhiad\n\tkey: AQDdIsZlQFS3BhAAa9Zq6Mxs1XZHa10jwIjUew==\n\tcaps: [mds] allow\n\tcaps: [mon] profile mds\n\tcaps: [osd] allow rw tag cephfs *=*\nmds.a.smithi179.zzaanp\n\tkey: AQDJIsZl2tIqFhAAYWbJxC9qls0g8Qd53rH0kw==\n\tcaps: [mds] allow\n\tcaps: [mon] profile mds\n\tcaps: [osd] allow rw tag cephfs *=*\nmds.nfs-cephfs.smithi179.khleme\n\tkey: AQCrI8Zl8QNNJBAAZZwPFhqdWRSrikOpQ9gysg==\n\tcaps: [mds] allow\n\tcaps: [mon] profile mds\n\tcaps: [osd] allow rw tag cephfs *=*\nmds.nfs-cephfs.smithi179.uzhtks\n\tkey: AQCuI8ZllR3eHBAA0VJRJDxyy1sU6tiBWgLitw==\n\tcaps: [mds] allow\n\tcaps: [mon] profile mds\n\tcaps: [osd] allow rw tag cephfs *=*\nosd.0\n\tkey: AQBWIsZlf4w1MBAAJ1yyxRe0pGhVk8e3awIpGw==\n\tcaps: [mgr] allow profile osd\n\tcaps: [mon] allow profile osd\n\tcaps: [osd] allow *\nosd.1\n\tkey: AQBwIsZlr084CxAAIEkAoTVPxY+21BvFNkSHaw==\n\tcaps: [mgr] allow profile osd\n\tcaps: [mon] allow profile osd\n\tcaps: [osd] allow *\nosd.2\n\tkey: AQCKIsZl5jGuAhAAWOUQRCca+AqOJAvlYNL6jw==\n\tcaps: [mgr] allow profile osd\n\tcaps: [mon] allow profile osd\n\tcaps: [osd] allow *\nclient.0\n\tkey: AQCnIsZlY6ILDBAAXkuOToR/WbjXrvwtsfdyug==\n\tcaps: [mds] allow *\n\tcaps: [mgr] allow *\n\tcaps: [mon] allow *\n\tcaps: [osd] allow *\nclient.admin\n\tkey: AQDwIcZlAOt3EhAA6xc5jZh6DnaqxIKTKbaD8g==\n\tcaps: [mds] allow *\n\tcaps: [mgr] allow *\n\tcaps: [mon] allow *\n\tcaps: [osd] allow *\nclient.bootstrap-mds\n\tkey: AQD4IcZlnWWZJhAA1gZBv62MOILxtl0NujyqaQ==\n\tcaps: [mon] allow profile bootstrap-mds\nclient.bootstrap-mgr\n\tkey: AQD4IcZluG2ZJhAA6Bfwi8zXxZev9+26g1iSiw==\n\tcaps: [mon] allow profile bootstrap-mgr\nclient.bootstrap-osd\n\tkey: AQD4IcZltHSZJhAASlE8fs4zdpstBZvzS6g/lw==\n\tcaps: [mon] allow profile bootstrap-osd\nclient.bootstrap-rbd\n\tkey: AQD4IcZl4nuZJhAAvItM4qkKoYDonawnWxwqAA==\n\tcaps: [mon] allow profile bootstrap-rbd\nclient.bootstrap-rbd-mirror\n\tkey: AQD4IcZl/IKZJhAA7vtWvpndXPu96QpMgHafmA==\n\tcaps: [mon] allow profile bootstrap-rbd-mirror\nclient.bootstrap-rgw\n\tkey: AQD4IcZlXIuZJhAAeFumY4Gq/QKfnMSB9WZOow==\n\tcaps: [mon] allow profile bootstrap-rgw\nclient.nfs.test.0.0.smithi179.pokwoy\n\tkey: AQChI8ZlRA5YIBAA4jlB9BpkAz5LlpId8VwJUw==\n\tcaps: [mon] allow r\n\tcaps: [osd] allow rw pool=.nfs namespace=test\nclient.nfs.test.0.0.smithi179.pokwoy-rgw\n\tkey: AQChI8ZlmZBlJhAA/jVuNnhQdqlQzcQIETxO/w==\n\tcaps: [mon] allow r\n\tcaps: [osd] allow rwx tag rgw *=*\nclient.nfs.test.nfs-cephfs\n\tkey: AQC2I8ZlVtjCGRAAO9aBG88+wjm1YC8nGi3W8w==\n\tcaps: [mds] allow rw path=/\n\tcaps: [mon] allow r\n\tcaps: [osd] allow rw pool=.nfs namespace=test, allow rw tag cephfs data=nfs-cephfs\nclient.test\n\tkey: AQAmI8ZlsbrfBhAAUcmdKo3prqtJXbJ9BKFMyg==\n\tcaps: [mds] allow rw path=/\n\tcaps: [mon] allow r\n\tcaps: [osd] allow rw pool=.nfs namespace=test, allow rw tag cephfs data=user_test_fs\nmgr.x\n\tkey: AQDxIcZl7MpbChAA7p3Hju/uwUdRxjX6m41Z4w==\n\tcaps: [mds] allow *\n\tcaps: [mon] profile mgr\n\tcaps: [osd] allow *\n'

It's looking for client.nfs.test.1 but the keyring that's actually there is client.nfs.test.nfs-cephfs after this name change. The check comes from https://github.com/ceph/ceph/blob/main/qa/tasks/cephfs/test_nfs.py#L121 so I'd guess you'd have to adjust that to expect the new naming structure, although I'm not too familiar with the working s of the test myself.

https://pulpito.ceph.com/adking-2024-02-09_04:25:39-orch:cephadm-wip-adk-testing-2024-02-08-1828-distro-default-smithi/7553533/

@adk3798 I've fixed the API tests for this. Can you re-trigger teuthology?

@avanthakkar Changes to pybind/nfs would required reviews/acks from cephfs devs.

@avanthakkar I can at least rerun the test and see if this passes while we wait for review from the cephfs team, yeah.

@avanthakkar the test has gotten farther, but still seem to fail elsewhere

2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner:Traceback (most recent call last): 2024-02-27T06:26:42.789 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.789 INFO:tasks.cephfs_test_runner: self._test_get_export() 2024-02-27T06:26:42.789 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.789 INFO:tasks.cephfs_test_runner: self.assertDictEqual(self.sample_export, nfs_output) 2024-02-27T06:26:42.789 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.789 INFO:tasks.cephfs_test_runner: {'access_type': 'RW', 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner: 'clients': [], 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner: 'cluster_id': 'test', 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner: 'export_id': 1, 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner:- 'fsal': {'fs_name': 'nfs-cephfs', 'name': 'CEPH', 'user_id': 'nfs.test.1'}, 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner:+ 'fsal': {'cmount_path': '/', 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner:+ 'fs_name': 'nfs-cephfs', 2024-02-27T06:26:42.790 INFO:tasks.cephfs_test_runner:+ 'name': 'CEPH', 2024-02-27T06:26:42.790 INFO:tasks.cephfs_test_runner:+ 'user_id': 'nfs.test.nfs-cephfs'},

the user_id and cmount_path fields is not present in the dict its comparing to, and there is a difference in the user_id field.
https://pulpito.ceph.com/adking-2024-02-27_05:08:20-orch:cephadm-wip-adk-testing-2024-02-26-1600-distro-default-smithi/7574464/
Note in that test run it also complains about a difference in the protocols, but that was caused by another PR, not this one.

@avanthakkar the test has gotten farther, but still seem to fail elsewhere

2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner:Traceback (most recent call last): 2024-02-27T06:26:42.789 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.789 INFO:tasks.cephfs_test_runner: self._test_get_export() 2024-02-27T06:26:42.789 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.789 INFO:tasks.cephfs_test_runner: self.assertDictEqual(self.sample_export, nfs_output) 2024-02-27T06:26:42.789 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.789 INFO:tasks.cephfs_test_runner: {'access_type': 'RW', 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner: 'clients': [], 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner: 'cluster_id': 'test', 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner: 'export_id': 1, 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner:- 'fsal': {'fs_name': 'nfs-cephfs', 'name': 'CEPH', 'user_id': 'nfs.test.1'}, 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner:+ 'fsal': {'cmount_path': '/', 2024-02-27T06:26:42.789 INFO:tasks.cephfs_test_runner:+ 'fs_name': 'nfs-cephfs', 2024-02-27T06:26:42.790 INFO:tasks.cephfs_test_runner:+ 'name': 'CEPH', 2024-02-27T06:26:42.790 INFO:tasks.cephfs_test_runner:+ 'user_id': 'nfs.test.nfs-cephfs'},

the user_id and cmount_path fields is not present in the dict its comparing to, and there is a difference in the user_id field. https://pulpito.ceph.com/adking-2024-02-27_05:08:20-orch:cephadm-wip-adk-testing-2024-02-26-1600-distro-default-smithi/7574464/ Note in that test run it also complains about a difference in the protocols, but that was caused by another PR, not this one.

Need to add cmount_path and change user_id in self.sample_export present in def setUp()

fsal.user_id = f"nfs.{export.cluster_id}.{export.fsal.fs_name}" fsal.cephx_key = self._create_user_key( export.cluster_id, fsal.user_id, fsal.cmount_path, fsal.fs_name )

We didn't check whether the user exists till now since we're creating a new user for every export but since now this is not the case, I think it would be wise to check if the user exists and only then try create key, because if the user exists then it asserts that the key should be existent and valid, right?

FWIW, that _create_user_key perhaps has a misleading name. If the key already exists, (it use ceph auth get-or-create) it won't actually create a new one, it will just return the existing one, which we might need to do anyway.

FWIW, that _create_user_key perhaps has a misleading name. If the key already exists, (it use ceph auth get-or-create) it won't actually create a new one, it will just return the existing one, which we might need to do anyway.

Oh, didn't check that code since the name was sort of self-explanatory. I think that func should be renamed to something like _ensure_user_key. Thanks @adk3798!

vshankar · 2024-02-27T04:53:07Z

@avanthakkar Commit message for b81b0fe looks like a combination of various commits since the Signed-off-by tag overlaps with various commit messages. Please clean that up.

github-actions · 2024-02-27T06:28:36Z

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

dparmar18 · 2024-02-27T13:18:01Z

Generate user_id & secret_access_key for CephFS based exports for apply export. Also ensure the export FSAL block has cmount_path.

The cmount_path in the latest iteration of NFS-Ganesha isn't mandatory, right?

dparmar18 · 2024-02-27T14:03:51Z

Copied from commit description:

Furthermore, the newly created CephFS exports will now share the same user_id and secret_access_key, which are determined based
on the NFS cluster name and filesystem name. This results in each export on the same filesystem using a shared connection,
thereby optimizing resource usage.

Can this centralisation bring in any drawbacks? Like e.g. if lets say somehow the export rados object gets tampered, can this in any way impact other exports? Or if the user is gone or the secret key is tampered, it will bring down all the exports, isn't it? Do we have any crash handling mechanism to mitigate the risks that this unification brings?

src/pybind/mgr/nfs/export.py

dparmar18

There are some docs that would need changes since we're now including cmount_path in FSAL block, like:

https://docs.ceph.com/en/latest/mgr/ceph_api/
https://docs.ceph.com/en/latest/mgr/nfs/#set-customized-nfs-ganesha-configuration
https://docs.ceph.com/en/latest/mgr/nfs/#create-or-update-export-via-json-specification

src/pybind/mgr/nfs/export.py

dparmar18 · 2024-02-28T15:01:39Z

src/pybind/mgr/nfs/export.py

-        if old_export.fsal.name == NFS_GANESHA_SUPPORTED_FSALS[0]:
-            old_fsal = cast(CephFSFSAL, old_export.fsal)
-            new_fsal = cast(CephFSFSAL, new_export.fsal)
-            if old_fsal.user_id != new_fsal.user_id:
-                self._delete_export_user(old_export)
-                self._create_export_user(new_export)
-            elif (
-                old_export.path != new_export.path
-                or old_fsal.fs_name != new_fsal.fs_name
-            ):
-                self._update_user_id(
-                    cluster_id,
-                    new_export.path,
-                    cast(str, new_fsal.fs_name),
-                    cast(str, new_fsal.user_id)
-                )
-                new_fsal.cephx_key = old_fsal.cephx_key
-            else:
-                expected_mds_caps = 'allow rw path={}'.format(new_export.path)
-                entity = new_fsal.user_id
-                ret, out, err = self.mgr.mon_command({
-                    'prefix': 'auth get',
-                    'entity': 'client.{}'.format(entity),
-                    'format': 'json',
-                })
-                if ret:
-                    raise NFSException(f'Failed to fetch caps for {entity}: {err}')
-                actual_mds_caps = json.loads(out)[0]['caps'].get('mds')
-                if actual_mds_caps != expected_mds_caps:
-                    self._update_user_id(
-                        cluster_id,
-                        new_export.path,
-                        cast(str, new_fsal.fs_name),
-                        cast(str, new_fsal.user_id)
-                    )
-                elif old_export.pseudo == new_export.pseudo:
-                    need_nfs_service_restart = False
-                new_fsal.cephx_key = old_fsal.cephx_key


So when applying an export block if there is no change to the path or fs_name or user_id then else block here makes sure that mds caps are valid and exist as expected, I think this is an important check and must be preserved. I'm yet to reach the end of this PR but while scrambling through the patch I didn't find this being handled anywhere. So is this removal intentional? @vshankar any thoughts.

Not only this, else block also contain another important check:

elif old_export.pseudo == new_export.pseudo: need_nfs_service_restart = False

We should also ensure that this is preserved since this will make sure we don't unnecessarily restart the nfs service

So when applying an export block if there is no change to the path or fs_name or user_id then else block here makes sure that mds caps are valid and exist as expected, I think this is an important check and must be preserved. I'm yet to reach the end of this PR but while scrambling through the patch I didn't find this being handled anywhere. So is this removal intentional? @vshankar any thoughts.

Not only this, else block also contain another important check:

elif old_export.pseudo == new_export.pseudo: need_nfs_service_restart = False

We should also ensure that this is preserved since this will make sure we don't unnecessarily restart the nfs service

Yes you're correct @dparmar18 . The changes for user_id were removed because initially the assumption was to just have cmount_path & no requirement of user_id, but that's not the case now, so I'll revert these changes and preserve all this if block

qa/tasks/cephfs/test_nfs.py

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

dparmar18 · 2024-02-28T20:11:47Z

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

@@ -382,6 +403,29 @@ def test_export_parser_2(self) -> None:
        export = Export.from_export_block(blocks[0], self.cluster_id)
        self._validate_export_2(export)

+    def _validate_export_5(self, export: Export):
+        assert export.export_id == 3


this is confusing, its *_export_5 but here the id is 3 0_o?

oh wow, export_3 and export_4 use export_id = 1, that could be changed to the respective numbers so that this export uses export_id = 5 so that its simpler to understand.

oh wow, export_3 and export_4 use export_id = 1, that could be changed to the respective numbers so that this export uses export_id = 5 so that its simpler to understand.

Well , export_3 & export_4 seems for the same export id i tihnk its for testing the update of export so they are having same export id here

adk3798 · 2024-02-28T20:19:37Z

taking testing tag off for now due to merge conflict

dparmar18 · 2024-02-28T20:32:13Z

src/pybind/mgr/nfs/export.py

-            user_id = f"nfs.{cluster_id}.{ex_id}"
-            if "user_id" in fsal and fsal["user_id"] != user_id:
-                raise NFSInvalidOperation(f"export FSAL user_id must be '{user_id}'")


Oh so now there is no user_id validation, i.e. let's say even if someone provided a user_id = foo in an/the export block(s), the code would happily accept it, although this has no actual impact since def _ensure_cephfs_export_user creates a user_id with the cluster name and fs name but this looks wrong to me, I think a proper errno with a err msg must be sent to user that the user_id must be the syntax nfs.<cluster_name>.<fs_name>, makes sense?

Yeah, user_id also needs to be generated based on cmount_path which will avoid any caps issue. I'll add a commit in this PR itself to fix this

Please do check commit message of my last commit, that addresses the issue for generation of user_id now

vshankar · 2024-03-26T14:23:54Z

@avanthakkar status of this?

avanthakkar · 2024-04-12T08:13:34Z

@avanthakkar status of this?
I got caught up with some other stuff, so I couldn't update this PR. I plan to tackle it next week and make the necessary adjustments based on the feedback.

avanthakkar · 2024-05-07T06:15:07Z

Generate user_id & secret_access_key for CephFS based exports for apply export. Also ensure the export FSAL block has cmount_path.

The cmount_path in the latest iteration of NFS-Ganesha isn't mandatory, right?

Yes, but I think given memoery consumption issue with export we decided to have atleast cmount_path = / by default

Implementing necessary changes for the NFS module to align with the new export block format introduced in nfs-ganesha-V5.6. The purpose of these changes is to enhance memory efficiency for exports. To achieve this goal, we have introduced a new field called cmount_path under the FSAL block of export. Initially, this is applicable only to CephFS-based exports. Furthermore, the newly created CephFS exports will now share the same user_id and secret_access_key, which are determined based on the NFS cluster name and filesystem name. This results in each export on the same filesystem using a shared connection, thereby optimizing resource usage. Signed-off-by: avanthakkar <avanjohn@gmail.com> mgr/nfs: fix a unit test failure Signed-off-by: John Mulligan <jmulligan@redhat.com> mgr/nfs: fix a unit test failure Signed-off-by: John Mulligan <jmulligan@redhat.com> mgr/nfs: fix a unit test failure Signed-off-by: John Mulligan <jmulligan@redhat.com> mgr/nfs: enable user management on a per-fs basis Add back the ability to create a user for a cephfs export but do it only for a cluster+filesystem combination. According to the ganesha devs this ought to continue sharing a cephfs client connection across multiple exports. Signed-off-by: John Mulligan <jmulligan@redhat.com> mgr/nfs: add more unit tests with cmount_path Add more unit tests for CEPHFS based NFS exports with newly added cmount_path field under FSAL. Signed-off-by: avanthakkar <avanjohn@gmail.com> mgr/nfs: fix rgw nfs export when no existing exports Signed-off-by: avanthakkar <avanjohn@gmail.com>

Generate user_id & secret_access_key for CephFS based exports for apply export. Also ensure the export FSAL block has `cmount_path`. Fixes: https://tracker.ceph.com/issues/63377 Signed-off-by: avanthakkar <avanjohn@gmail.com>

Signed-off-by: Avan Thakkar <athakkar@redhat.com> The fix keeps the same user_id and cephx_key generation the same, no need to take into account fs name or cmount path because that will not handle the case well when we create export with same cmount_path & fs_name but different `path` for e.g. subvolume path & if we delete that export it'll also delete the client which the other export would be consuming. So taking into account the export_id & path should be enough for generation of user_id & cephx_key attached to it.

avanthakkar · 2024-05-08T08:05:51Z

@dparmar18 @vshankar @adk3798 I've updated the PR & addressed mostly all the comments, PTAL. Also I've added the commit message for the new commit I submitted to address the user_id & cephx_key generation. I think now it also avoids any CAPS issue and also different scenarios of exports (with same/diff cmount_path and/or path)

avanthakkar · 2024-05-08T17:03:07Z

jenkins test api

avanthakkar requested a review from phlogistonjohn October 31, 2023 16:01

github-actions bot added nfs pybind labels Oct 31, 2023

avanthakkar force-pushed the nfs-export-apply-fix branch from d36310c to 20f8404 Compare October 31, 2023 16:07

avanthakkar added the DNM label Oct 31, 2023

avanthakkar force-pushed the nfs-export-apply-fix branch from 20f8404 to 80f2e3c Compare November 2, 2023 13:39

avanthakkar marked this pull request as ready for review November 2, 2023 13:39

phlogistonjohn approved these changes Nov 2, 2023

View reviewed changes

avanthakkar force-pushed the nfs-export-apply-fix branch from 80f2e3c to ac03197 Compare November 7, 2023 07:53

adk3798 added wip-adk-testing and removed wip-adk-testing labels Nov 28, 2023

avanthakkar removed the DNM label Jan 25, 2024

avanthakkar force-pushed the nfs-export-apply-fix branch from ac03197 to 83d6c3f Compare February 4, 2024 14:27

adk3798 added the wip-adk-testing label Feb 8, 2024

adk3798 requested changes Feb 12, 2024

View reviewed changes

avanthakkar force-pushed the nfs-export-apply-fix branch from 83d6c3f to f78e425 Compare February 24, 2024 17:15

avanthakkar requested a review from adk3798 February 24, 2024 17:15

github-actions bot added cephfs Ceph File System tests labels Feb 24, 2024

vshankar requested review from dparmar18 and a team February 26, 2024 07:02

github-actions bot added the needs-rebase label Feb 27, 2024

dparmar18 requested changes Feb 28, 2024

View reviewed changes

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

dparmar18 requested changes Feb 28, 2024

View reviewed changes