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

Use RuntimeDirectory in systemd service file #1759

Open
knight-of-ni opened this issue Nov 16, 2024 · 14 comments
Open

Use RuntimeDirectory in systemd service file #1759

knight-of-ni opened this issue Nov 16, 2024 · 14 comments

Comments

@knight-of-ni
Copy link
Contributor

knight-of-ni commented Nov 16, 2024

For daemons that need a pre-existing runtime folder to store pid or socket files, the traditional systemd way to do that is to create a tmpfiles.d config file. When netatalk is installed by the package manager, a config file is placed into the proper location and tmpfiles.d is restarted to pickup the new changes. I'm sure you are familiar with the process.

For the case of rpm packaging, that meant I had to create and manage that tmpfile.d config separately and add hooks into the rpm %post to get tmpfiles.d to pickup the change when the netatalk rpm is installed or updated. This works, but it turns out there is a simpler way.

If one adds the RuntimeDirectory parameter directly to the systemd service file, then none of the extra tmpfiles.d stuff is needed.

For the moment, I'm injecting this line into the service file when the rpm is built:
https://src.fedoraproject.org/rpms/netatalk/blob/rawhide/f/netatalk.spec#_160

Perhaps we could add support for this directly into the netatalk project.
This method should work for any systemd linux distro. The only catch is, the path must be relative to the distro's /run or /var/run folder. Consequently, we can't directly use the netatalk with-lockfile-path meson build flag, because it is an absolute path.

Since this is the first project I've worked with that uses meson, I haven't figured out the "meson" way to strip components from a path string. Perhaps I can just do this directly with python, since that seems to be what meson is based on.

@knight-of-ni knight-of-ni changed the title Use RuntimeDirectory in systemd dervice file Use RuntimeDirectory in systemd service file Nov 16, 2024
@rdmark
Copy link
Member

rdmark commented Nov 16, 2024

If this is the standard systemd way of doing it, then I'm all for introducing this line into the standard systemd unit templates.

Do you know what exactly systemd does with this information, though? Netatalk daemons manages their own pid/lockfiles internally.

Regarding working with file system paths, the Meson fs object comes in handy. e.g. fs.parent() fs.name() fs.stem().

https://mesonbuild.com/Fs-module.html

@rdmark
Copy link
Member

rdmark commented Nov 16, 2024

It's also worth noting that the papd and atalkd daemons have lockfiles that are defined in C code rather than through the Meson build system (for legacy reasons.)

See: #1368

a2boot, macipgw, and timelord don't use lockfiles.

@singular0
Copy link
Contributor

Do you know what exactly systemd does with this information, though? Netatalk daemons manages their own pid/lockfiles internally.

@rdmark My understanding is that systemd will just makes and sets ownership of RuntimeDirectory upon service start and removes it with all the content when service terminates. Otherwise it is up to the service to create its pid/lock files there.

@singular0
Copy link
Contributor

I haven't figured out the "meson" way to strip components from a path string.

@knight-of-ni Do I understand correctly that you intend to take the last part of with-lockfile-path build flag and use it as RuntimeDirectory?

@rdmark rdmark self-assigned this Nov 20, 2024
@rdmark
Copy link
Member

rdmark commented Nov 20, 2024

@knight-of-ni Do you want to take a shot at a PR?

The substitution of the netatalk(afpd) lockfile should be straight forward to achieve with Meson.

@rdmark rdmark added this to the release-4.0 milestone Nov 20, 2024
@knight-of-ni
Copy link
Contributor Author

Netatalk creates the the lockfile, but it does not create the path the lockfile should live in. For the case of fedora/redhat that path is /run/lock/netatalk. If the path does not exist then the netatalk service fails to start.

If other distros just put the lockfile directly into /run/lock then this issue would not appear. For curiosity, I'll look at what Debian does.

Back in the sys v init days, one had write shell scipt in the init file to manage any folders that need to pre-exist prior to startup. Nowadays, systemd has built-in tools to handle this. tmpfiles.d can do it, but it's much more simple to just set the Runtime directory flag in the systemd service file.

@knight-of-ni Do I understand correctly that you intend to take the last part of with-lockfile-path build flag and use it as RuntimeDirectory?

Yes, that is my initial thought.

I've gotten distracted on other tasks for the moment and will return to this shortly

@singular0
Copy link
Contributor

Wouldn't using different parts of the same with-lockfile-path depending on init style introduce some unintuitive ambiguity? Maybe it would be better to introduce separate flag (something like systemd-workdir-name) and ignore with-lockfile-path for systemd altogether?

@rdmark
Copy link
Member

rdmark commented Nov 23, 2024

FYI I plan to tag the next netatalk bugfix release this weekend.

@knight-of-ni
Copy link
Contributor Author

Thanks for the heads up. Due to travels this weekend and the then holiday travel next week, please move forward as needed.

@rdmark
Copy link
Member

rdmark commented Nov 23, 2024

@singular0 I hear the unintuitive argument. From a user's perspective, this behavior would be entirely hidden away. At the same time, I think with-lockfile-path would have to be in sync with systemd-workdir-name since the former decides where afpd will put its lockfile, and the latter arranges the location for said lockfile. If I understand this mechanism correctly. :)

Maybe a better long-term approach would be split with-lockfile-path up into lockfile-prefix and lockfile-path for instance, where the former would be something like /run and the latter lock/netatalk which are then concatenated by Meson.

Thoughts?

@singular0
Copy link
Contributor

@rdmark Yes, lockfile-prefix + lockfile-path sounds to be logical and clean solution to me.

@rdmark
Copy link
Member

rdmark commented Nov 24, 2024

One thing to figure out is how to detect the default lockfile-prefix on a particular host OS. I haven't seen /run or /var/run being exposed by either of the default path variables.

We roll our own defaults right now, but I don't know if this is correct.

if lockfile_path != ''
    lockfile += lockfile_path
elif host_os == 'freebsd'
    lockfile += '/var/spool/lock/netatalk'
elif host_os in ['netbsd', 'openbsd', 'darwin']
    lockfile += '/var/run/netatalk.pid'
elif host_os == 'linux'
    lockfile += '/var/lock/netatalk'
else
    lockfile += '/var/spool/locks/netatalk'
endif

@knight-of-ni
Copy link
Contributor Author

knight-of-ni commented Nov 26, 2024

I had a moment of opportunity to dig into this a littler further. Here is what I came up with:
knight-of-ni@e7b75eb

That diff is against 4.0.6, which is why I have not made it a PR. I can rebase this against the main branch if we decide to move forward.

This creates a new runtime directory build option.

If neither runtime dir nor lockfile path are passed to meson, then meson will auto-select a runtimedir of "netatalk" and a lockfile path of /run/netatalk/netatalk.pid, but only for systemd linux distros.

As far as I know, if the linux distro uses systemd, then the runtime directory prefix is going to be /run. Note that /var/run might exist, but that is a symlink back to /run. This is the case for f41, el8, and debian 12 anyway.

I found another github project that makes the same assumption. It even uses meson:
https://github.com/fwupd/fwupd/blob/main/data/motd/meson.build#L5

I did not want to deal with a situation where runtimedir was set, but the lockfile path was not, so I flagged that as an error.

The irony of this proposed change is it will replace just a single line of code in my rpm specfile. Now that I can see what it would take to implement upstream, I am on the fence whether it is worth it. I guess it would come down to whether it will have any benefit to other distros.

I looked at debian, arch, and opensuse. None of them today try to create a subfolder under the runtime directory for the lockfile.

@rdmark rdmark removed their assignment Nov 26, 2024
@rdmark
Copy link
Member

rdmark commented Nov 27, 2024

It is a fair amount of added complexity, but I'm supportive if it facilitates downstream packaging in the long run. I would want to make sure other systemd distros get any unexpected side effects of course. This might be something for the future 4.1 release series given the potential regression scope?

In addition to atalkd, papd also has a lockfile that should be managed...

@rdmark rdmark removed this from the release-4.0 milestone Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants