Skip to content
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

Samba: add clustering functionality. #27300

Closed
wants to merge 2 commits into from

Conversation

andry-dev
Copy link
Contributor

@andry-dev andry-dev commented Dec 20, 2020

I waited for Samba 4.X to get merged to create this PR. I didn't want to interfere with the original one because it was pretty important.

This PR enables the cluster module (ctdb) and the GlusterFS VFS:

  • The GlusterFS VFS (/usr/lib/samba/glusterfs_vfs.so) is needed to improve I/O performance with GlusterFS volumes. This is enabled automatically if waf can find the Gluster libraries, so to make it work it's only needed to add glusterfs-devel in the makedepends for samba and that's it.
  • The ctdb module is needed to keep track of TDB across various nodes in a cluster, so that a share can be exposed as a single IP and resume operations in case of node failures. This is generally used in conjunction with Gluster, Ceph or other distributed filesystems.

Enabling both modules doesn't break existing users since these features are opt-in: for the Gluster VFS you specifically need to enable it in a share definition (see vfs_glusterfs(8)); for ctdb you need to enable and configure the daemon ctdbd.

EDIT: So it seems that glusterfs doesn't work on -musl, can I enable it only for glibc builds?

@ericonr
Copy link
Member

ericonr commented Dec 20, 2020

EDIT: So it seems that glusterfs doesn't work on -musl, can I enable it only for glibc builds?

Yes, but I would make it a build option that gets added to build_options_default if XBPS_TARGET_LIBC is glibc.

common/shlibs Outdated Show resolved Hide resolved
srcpkgs/samba/template Outdated Show resolved Hide resolved
srcpkgs/samba/template Outdated Show resolved Hide resolved

vmove usr/libexec/ctdb
vmove usr/share/ctdb
vmove etc/ctdb
Copy link
Member

Choose a reason for hiding this comment

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

Is it appropriate for package updates to clobber everything in /etc/ctdb, or should some of the files in /etc/ctdb be marked as conf_files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The files under /etc/ctdb, other than ctdb.conf and nodes, are scripts used for administering ctdb.

Now that I think about it, those scripts should probably go under mutable_files.

Copy link
Member

Choose a reason for hiding this comment

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

Even if they are scripts, if you expect the user to modify them and you don't want to clobber them with every update, use conf_files instead of mutable_files. Files listed in mutable_files are still replaced during upgrades and removed when a package is removed, but XBPS ignores changes to the files when evaluating package integrity. Files listed in conf_files are removed or replaced if and only if their contents have not been changed by the user since installation time.

srcpkgs/samba/template Show resolved Hide resolved
@ahesford
Copy link
Member

Also, please incorporate ahesford@6efd1c0. I noticed some typos in the _privlib list and was able to drop a few more private shared libs. I've tested your changes (with my suggestions) against my cleanup and everything seems to build.

You should not need to modify common/shlibs when you adopt my suggestions; xbps-src warns about some unlisted libraries but, since they are intended to be private, they do not need to be described in the master list. The other private libraries are only included to satisfy interdependencies between the various samba subpackages.

@andry-dev
Copy link
Contributor Author

Thank you for your feedback. Right now I'm not at home; I'll integrate your changes ASAP.

Just to get the last part right: I'll remove all my changes to common/shlibs, cherry-pick the commit from your repo and rebase my changes on top of that one, right?

@ahesford
Copy link
Member

Yes, and don't forget to add ctdb-event-client to the _privlibs list, which means you won't need it in common/shlibs and you can drop the vmove usr/lib/libctdb-event-client-samba4.so in your new sub package as well.

@andry-dev andry-dev force-pushed the samba-gluster branch 2 times, most recently from 4ea2a31 to 9988c3c Compare December 20, 2020 22:42
ahesford pushed a commit to ahesford/void-packages that referenced this pull request Dec 21, 2020
Also remove a few more private libraries that can be built in.

Co-authored-by: andry-dev <andry-dev@users.noreply.github.com>
Co-authored-by: Andrew J. Hesford <ajh@sideband.org>

Closes: void-linux#27300 [via git-merge-pr]
ahesford pushed a commit to ahesford/void-packages that referenced this pull request Dec 21, 2020
Also remove a few more private libraries that can be built in.

Co-authored-by: andry-dev <andry-dev@users.noreply.github.com>
Co-authored-by: Andrew J. Hesford <ajh@sideband.org>

Closes: void-linux#27300 [via git-merge-pr]
@ahesford
Copy link
Member

I've done a little more digging into this and have made changes in my branch to address the following concerns:

  1. The ctdb package should probably be called samba-ctdb. I wondered whether ctdb served a purpose as a standalone clustered database, but as upstream seems to think its sole purpose is to be tightly coupled with Samba, the name should probably reflect that it's a subcomponent.

  2. Building with GlusterFS adds a hard dependency on libglusterfs for the main samba package. The only place this appears is in the glusterfs.so VFS module, so we can put this module and its man page into a separate subpackage that will allow people to avoid the dependency without requiring a custom build.

  3. After review, I don't think conf_files or mutable_files are appropriate for contents of /etc/ctdb. The functions file seems to be sourced from a lot of script not intended to be edited by users (specifically, those in /usr/share) and probably should be considered non-editable. The events contents are symlinks, which don't work properly conf_files. I think the best option we have for these files is noextract rules if users want to override the default set of event scripts.

  4. Some executables in /usr/bin related to CTDB were still left behind in the main samba package.

Please take a look at commit ahesford/void-packages@d700385 and, if you have no objections, I'll merge that one.

@andry-dev
Copy link
Contributor Author

I looked at your commit and it seems fine to me. You can merge that one and close this PR.

Thank you for your time and patience!

@ahesford ahesford closed this in da2cea5 Dec 21, 2020
@andry-dev andry-dev deleted the samba-gluster branch December 21, 2020 13:16
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants