Skip to content

mgr/volumes/nfs: remove 'ganesha-' prefix from cluster id#38510

Merged
batrick merged 2 commits intoceph:masterfrom
varshar16:wip-nfs-remove-ganesha-prefix
Jan 8, 2021
Merged

mgr/volumes/nfs: remove 'ganesha-' prefix from cluster id#38510
batrick merged 2 commits intoceph:masterfrom
varshar16:wip-nfs-remove-ganesha-prefix

Conversation

@varshar16
Copy link
Contributor

@varshar16 varshar16 commented Dec 9, 2020 •
edited
Loading

Orchestrator defines service name as "<service_type>.<service_id>". The ganesha
common config object name in orchestrator is "conf-<service_name>". Volume's
nfs plugin deploys nfs-ganesha clusters with 'ganesha' prefixed to cluster id
and common config object. It can cause unecessary issues with rook and cephadm
nfs deployments. So let's remove the prefix.

Fixes: https://tracker.ceph.com/issues/48514
Signed-off-by: Varsha Rao varao@redhat.com

Note: Not supporting backward compatibility

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

@varshar16 varshar16 requested review from a team December 9, 2020 17:08
batrick
batrick reviewed Dec 9, 2020
View reviewed changes
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

I'm okay with this change but what about making it backwards compatible for folks who setup clusters on Octopus?

varshar16 added a commit to varshar16/rook that referenced this pull request Dec 10, 2020
@varshar16
ceph: remove 'ganesha-' from nfs-ganesha config object name
f5527af 
This PR[1] in volume nfs plugin will remove it too. Since it can cause
inconsistencies.

[1] ceph/ceph#38510
Signed-off-by: Varsha Rao <varao@redhat.com>
@varshar16
Copy link
Contributor Author

varshar16 commented Dec 16, 2020

I'm okay with this change but what about making it backwards compatible for folks who setup clusters on Octopus?

The way namespace is set, makes backward compatibility complicated.

@varshar16 varshar16 requested a review from batrick December 16, 2020 15:46
@varshar16
Copy link
Contributor Author

varshar16 commented Dec 16, 2020

jenkins retest this please

varshar16 added a commit to varshar16/rook that referenced this pull request Dec 16, 2020
@varshar16
ceph: remove 'ganesha-' from nfs-ganesha config object name
d7accc1 
This PR[1] in volume nfs plugin will remove it too. Since it can cause
inconsistencies.

[1] ceph/ceph#38510
Signed-off-by: Varsha Rao <varao@redhat.com>
@varshar16 varshar16 mentioned this pull request Dec 16, 2020
Merged
10 tasks
varshar16 added a commit to varshar16/rook that referenced this pull request Dec 17, 2020
@varshar16
ceph: remove 'ganesha-' from nfs-ganesha config object name
dd33814 
PR[1] in volume nfs plugin will remove it too. Since it can cause
inconsistencies.

This change will not affect exports created using dashboard. As it looks for
ganesha config object just by 'conf-' [2].  Exports cannot be created by using
volume/nfs plugin in Octopus version. Because the ceph rook module is broken.

[1] ceph/ceph#38510
[2] https://github.com/ceph/ceph/blob/octopus/src/pybind/mgr/dashboard/services/ganesha.py#L1111

Signed-off-by: Varsha Rao <varao@redhat.com>
varshar16 added a commit to varshar16/rook that referenced this pull request Dec 17, 2020
@varshar16
ceph: remove 'ganesha-' from nfs-ganesha config object name
87d8efb 
PR[1] in volume nfs plugin will remove it too. Since it can cause
inconsistencies.

This change will not affect exports created using dashboard. As it looks for
ganesha config object just by 'conf-' [2].  Exports cannot be created by using
volume/nfs plugin in Octopus version. Because the ceph rook module is broken.

[1] ceph/ceph#38510

[2] https://github.com/ceph/ceph/blob/octopus/src/pybind/mgr/dashboard/services/ganesha.py#L1111

Signed-off-by: Varsha Rao <varao@redhat.com>
varshar16 added a commit to varshar16/rook that referenced this pull request Dec 18, 2020
@varshar16
ceph: remove 'ganesha-' from nfs-ganesha config object name
9530807 
PR[1] in volume nfs plugin will remove it too. Since it can cause
inconsistencies.

This change will not affect exports created using dashboard. As it looks for
ganesha config object just by 'conf-' [2].  Exports cannot be created by using
volume/nfs plugin in Octopus version. Because the ceph rook module is broken.

[1] ceph/ceph#38510

[2] https://github.com/ceph/ceph/blob/octopus/src/pybind/mgr/dashboard/services/ganesha.py#L1111

Signed-off-by: Varsha Rao <varao@redhat.com>
@batrick
Copy link
Member

batrick commented Dec 22, 2020

I'm okay with this change but what about making it backwards compatible for folks who setup clusters on Octopus?

The way namespace is set, makes backward compatibility complicated.

I think the most basic form of backwards compatibility is to just load userconf-ganesha-{user_id} if userconf-{user_id} does not exist? Then always save the new name (and leave the old object behind). Is it not that simple? The daemon names is indeed a little more work?

@BlaineEXE BlaineEXE mentioned this pull request Jan 4, 2021
Closed
@varshar16 varshar16 force-pushed the wip-nfs-remove-ganesha-prefix branch from 744947b to abd6894 Compare January 5, 2021 11:29
@varshar16
Copy link
Contributor Author

varshar16 commented Jan 5, 2021 •
edited
Loading

I'm okay with this change but what about making it backwards compatible for folks who setup clusters on Octopus?

The way namespace is set, makes backward compatibility complicated.

I think the most basic form of backwards compatibility is to just load userconf-ganesha-{user_id} if userconf-{user_id} does not exist? Then always save the new name (and leave the old object behind). Is it not that simple? The daemon names is indeed a little more work?

As per discussion change to config object is not required to maintain backward compatibility. Rather will require changes to namespace.

batrick
batrick requested changes Jan 7, 2021
View reviewed changes
@varshar16
mgr/volumes/nfs: remove 'ganesha-' prefix from cluster id
24b70c5 
Orchestrator defines service name as "<service_type>.<service_id>". The ganesha
common config object name in orchestrator is "conf-<service_name>". Volume's
nfs plugin deploys nfs-ganesha clusters with 'ganesha' prefixed to cluster id
and common config object.  It can cause unecessary issues in rook and cephadm.
So let's remove the prefix.

Fixes: https://tracker.ceph.com/issues/48514
Signed-off-by: Varsha Rao <varao@redhat.com>
@varshar16
PendingReleaseNotes: Update about volume/nfs cluster id changes
ef3660b 
Signed-off-by: Varsha Rao <varao@redhat.com>
@varshar16 varshar16 force-pushed the wip-nfs-remove-ganesha-prefix branch from abd6894 to ef3660b Compare January 8, 2021 07:09
@varshar16 varshar16 requested a review from batrick January 8, 2021 07:10
@varshar16
Copy link
Contributor Author

varshar16 commented Jan 8, 2021

Tests Passed: http://qa-proxy.ceph.com/teuthology/varsha-2021-01-08_10:15:12-rados-wip-nfs-remove-ganesha-prefix-distro-basic-smithi/

@batrick batrick merged commit 71c7315 into ceph:master Jan 8, 2021
@varshar16 varshar16 deleted the wip-nfs-remove-ganesha-prefix branch January 11, 2021 08:58
varshar16 added a commit to varshar16/ceph that referenced this pull request Feb 3, 2021
@varshar16
mgr/test_orchestrator: Update nfs service id
3bd8d8e 
The nfs-ganesha cluster name was recently updated in this PR[1]. Update the nfs
service id to maintain uniformity.

[1] ceph#38510

Fixes: https://tracker.ceph.com/issues/49121
Signed-off-by: Varsha Rao <varao@redhat.com>
varshar16 added a commit to varshar16/ceph that referenced this pull request Feb 3, 2021
@varshar16
mgr/test_orchestrator: Update nfs service id
f0969ce 
The nfs-ganesha cluster name was recently updated in this PR[1]. Update the nfs
service id to maintain uniformity.

[1] ceph#38510

Fixes: https://tracker.ceph.com/issues/49121
Signed-off-by: Varsha Rao <varao@redhat.com>
singuliere pushed a commit to singuliere/ceph that referenced this pull request Mar 9, 2021
@varshar16 @singuliere
mgr/test_orchestrator: Update nfs service id
d6dbb16 
The nfs-ganesha cluster name was recently updated in this PR[1]. Update the nfs
service id to maintain uniformity.

[1] ceph#38510

Fixes: https://tracker.ceph.com/issues/49121
Signed-off-by: Varsha Rao <varao@redhat.com>
(cherry picked from commit f0969ce)
singuliere pushed a commit to singuliere/ceph that referenced this pull request Mar 9, 2021
@varshar16 @singuliere
mgr/test_orchestrator: Update nfs service id
1213c5c 
The nfs-ganesha cluster name was recently updated in this PR[1]. Update the nfs
service id to maintain uniformity.

[1] ceph#38510

Fixes: https://tracker.ceph.com/issues/49121
Signed-off-by: Varsha Rao <varao@redhat.com>
(cherry picked from commit f0969ce)
myoungwon pushed a commit to myoungwon/ceph that referenced this pull request Mar 29, 2021
@varshar16 @myoungwon
mgr/test_orchestrator: Update nfs service id
9b5ffcb 
The nfs-ganesha cluster name was recently updated in this PR[1]. Update the nfs
service id to maintain uniformity.

[1] ceph#38510

Fixes: https://tracker.ceph.com/issues/49121
Signed-off-by: Varsha Rao <varao@redhat.com>
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

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

Assignees

No one assigned

Labels

bug-fix cephfs Ceph File System documentation needs-review nfs orchestrator pybind tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@varshar16 @batrick