-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
file: Add support for named MDS metadata pool names #15056
file: Add support for named MDS metadata pool names #15056
Conversation
dcb071e
to
ec854ba
Compare
pkg/operator/ceph/file/filesystem.go
Outdated
metadataPool.Application = cephfsApplication | ||
if metadataPool.Name == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, the data pool names always have the filesystem name prefix and the customization of the pool name is only after the prefix (see generateDataPoolNames()
). For consistency, seems like the metadata pool name should also have the filesystem name prefix. Or is there any need to not use the filesystem name as a prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right here. The documentation has resulted in a misunderstanding as it says [...] a pool name can be specified with the name field to override the default generated name; see more below.
. But further below it describes that the pool name is generated anyway of what is specified. It only appends it to the filesystem name combined with an -
.
Do you see any possibility and does it sound reasonable for your to add a flag to really be able to specify the pool names manually? I could add a flag like generatePoolNames
default to true
to pools
[1].
[1] https://rook.github.io/docs/rook/latest-release/CRDs/Shared-Filesystem/ceph-filesystem-crd/#pools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea behind this request is that I come with migration of exisiting resources in mind. Ceph documents that MDS uses pool names like cephfs_metadata
and cephfs_data
. Currently there is no way in Rook to use these kinds of names as they always get prefixed including -
in it's name.
See https://docs.ceph.com/en/latest/cephfs/createfs/#creating-pools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you say more about your migration? Rook has never had the goal to take over a ceph cluster. There are too many different assumptions in the deployments of the components such as mons, osds, the naming convention of pools, etc.
If we need to ability to set any pool name for cephfs that will be easy enough if it's needed, I'm more concerned about the attempt to migrate which is a bigger goal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The request is coming from #14971. A third-party tool provides support on doing an migration with minimal downtime just touching Ceph daemons and replacing them with Rook based ones. A blocker we had is that most traditional Ceph clusters use e.g. cephfs_metadata
as documented upstream.
Ceph configuration already stored at e.g. "ceph-mon" daemons are not touched and do work after migration as long as Rook does not try to change these settings. We haven't encountered severe issues in the migration process but our migration scenario required had been pretty straight forward so far.
I've added an boolean now to decide if metadata and data pool names are used as is or default behavior is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting to hear the migration is actually working. At some point, more comments on #14971 would be helpful to understand these steps more, or perhaps it's already documented in that rookify project.
ec854ba
to
ca3ad08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good, I just reran one of the CI tests to confirm if it was intermittent before merging...
pkg/operator/ceph/file/filesystem.go
Outdated
metadataPool.Application = cephfsApplication | ||
if metadataPool.Name == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting to hear the migration is actually working. At some point, more comments on #14971 would be helpful to understand these steps more, or perhaps it's already documented in that rookify project.
8c9341f
to
e93a7cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing...Please also add mention of the new setting in ceph-filesystem-crd.md
cc0894f
to
33bb7e1
Compare
This commit adds optional support to specify the MDS metadata pool name. It defaults to `<fsName>-metadata` as current implementation expects but allows customization if needed e.g. if exisiting naming conventions used `<fsName>_metadata`. Additionally an "preservePoolNames" boolean has been added to indicate that no generated pool names should be used. Signed-off-by: Tobias Wolf <wolf@b1-systems.de>
33bb7e1
to
675678f
Compare
file: Add support for named MDS metadata pool names (backport #15056)
file: Add support for named MDS metadata pool names (backport #15056)
This commit adds optional support to specify the MDS metadata pool name. It defaults to
<fsName>-metadata
as current implementation expects but allows customization if needed e.g. if exisiting naming conventions used<fsName>_metadata
.Checklist: