Skip to content

mgr/volumes/nfs: Add interface for adding user defined configuration#36351

Merged
batrick merged 3 commits intoceph:masterfrom
varshar16:wip-nfs-update-user-config
Aug 11, 2020
Merged

mgr/volumes/nfs: Add interface for adding user defined configuration#36351
batrick merged 3 commits intoceph:masterfrom
varshar16:wip-nfs-update-user-config

Conversation

@varshar16
Copy link
Contributor

@varshar16 varshar16 commented Jul 29, 2020 •
edited
Loading

Set config command saves the user defined config in rados object, adds it's url to common config and restarts the ganesha service.

$ ceph nfs cluster config set <cluster_id> -i <config_file>

Reset config command deletes the user defined config object, removes url from common config and restarts the ganesha service.

$ ceph nfs cluster config reset <cluster_id>

Fixes: https://tracker.ceph.com/issues/45747

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 backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@varshar16 varshar16 marked this pull request as draft July 29, 2020 13:59
@varshar16 varshar16 added the DNM label Jul 29, 2020
@varshar16
Copy link
Contributor Author

varshar16 commented Jul 29, 2020

Depends on PR #36338

batrick
batrick requested changes Jul 29, 2020
View reviewed changes
@sebastian-philipp
Copy link
Contributor

sebastian-philipp commented Jul 30, 2020

cc @BlaineEXE

@varshar16 varshar16 force-pushed the wip-nfs-update-user-config branch 2 times, most recently from ec5d652 to dc0cd42 Compare July 31, 2020 10:33
@varshar16 varshar16 requested a review from batrick July 31, 2020 10:34
batrick
batrick requested changes Aug 4, 2020
View reviewed changes
@varshar16 varshar16 force-pushed the wip-nfs-update-user-config branch from dc0cd42 to ca18d0f Compare August 4, 2020 10:03
@BlaineEXE
Copy link
Collaborator

BlaineEXE commented Aug 4, 2020

With Rook (and I think cephadm) I am thinking about the workflow that will be used programmatically by the orchestrator. In my mind, thinking of Rook, it looks something like below.

  1. Rook needs to be able to set up NFS such that it connects to the Ceph cluster with some sort of minimal config
  2. Rook can programmatically apply additional user-defined configs to NFS in such a way that does not overwrite the minimal configs needed to connect NFS to Ceph as done in step 1.
    2.a. Rook will likely wish to use some pre-existing mechanism to accomplish 2 above, and it's very helpful if that mechanism can be used by users manually in the interim period before it is implemented in Rook. It seems like your idea of an override file here would meet this desire, but it needn't be the only way.

The biggest question I am left with after looking at this draft PR is whether the user must re-specify the minimal connection configs from my item 1 in order to apply config changes or whether applying config changes will "merge" with the Rook-applied minimal configs.

@varshar16
mgr/volumes/nfs: Add interface for adding user defined configuration
9535154 
Code related to creating and removing rados object has been rearranged abd
modified. This patch introduces interface to set and reset user config.

nfs cluster config set <clusterid> -i <config_file>
nfs cluster config reset <clusterid>

Fixes: https://tracker.ceph.com/issues/45747
Signed-off-by: Varsha Rao <varao@redhat.com>
@varshar16
doc/cephfs/nfs: Update the doc about 'reset' and 'set' config interfaces
287da11 
Signed-off-by: Varsha Rao <varao@redhat.com>
@varshar16
qa/tasks/cephfs/nfs: Add tests for cluster config set and reset
bba2d66 
Signed-off-by: Varsha Rao <varao@redhat.com>
@varshar16 varshar16 force-pushed the wip-nfs-update-user-config branch from ca18d0f to bba2d66 Compare August 10, 2020 10:56
@varshar16 varshar16 marked this pull request as ready for review August 10, 2020 10:57
@varshar16 varshar16 requested a review from batrick August 10, 2020 10:57
@varshar16
Copy link
Contributor Author

varshar16 commented Aug 10, 2020

With Rook (and I think cephadm) I am thinking about the workflow that will be used programmatically by the orchestrator. In my mind, thinking of Rook, it looks something like below.

1. Rook needs to be able to set up NFS such that it connects to the Ceph cluster with some sort of minimal config

2. Rook can programmatically apply additional user-defined configs to NFS in such a way that does not overwrite the minimal configs needed to connect NFS to Ceph as done in step 1.
   2.a. Rook will likely wish to use some pre-existing mechanism to accomplish 2 above, and it's very helpful if that mechanism can be used by users manually in the interim period before it is implemented in Rook. It seems like your idea of an override file here would meet this desire, but it needn't be the only way.

The biggest question I am left with after looking at this draft PR is whether the user must re-specify the minimal connection configs from my item 1 in order to apply config changes or whether applying config changes will "merge" with the Rook-applied minimal configs.

User will not be required to respecify the default/minimal config again. As Ganesha uses the last loaded config block.

@varshar16
Copy link
Contributor Author

varshar16 commented Aug 11, 2020

@batrick Tests have passed: http://qa-proxy.ceph.com/teuthology/varsha-2020-08-10_15:58:10-rados-wip-nfs-update-user-config-distro-basic-smithi/

@batrick batrick merged commit dc06096 into ceph:master Aug 11, 2020
@varshar16 varshar16 deleted the wip-nfs-update-user-config branch August 12, 2020 04:42
@smithfarm smithfarm mentioned this pull request Aug 14, 2020
Merged
@sebastian-philipp sebastian-philipp mentioned this pull request Aug 21, 2020
Merged
@asettle asettle mentioned this pull request Sep 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@batrick batrick batrick approved these changes

@mgfritch mgfritch Awaiting requested review from mgfritch

@sebastian-philipp sebastian-philipp Awaiting requested review from sebastian-philipp

@ajarr ajarr Awaiting requested review from ajarr

@jtlayton jtlayton Awaiting requested review from jtlayton

Assignees

No one assigned

Labels

cephadm cephfs Ceph File System feature needs-qa wip-pdonnell-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@varshar16 @sebastian-philipp @BlaineEXE @batrick @mgfritch