search

LMS-Community / slimserver-platforms

Platform-specific build tools for Lyrion Music Server
48 stars 41 forks source link

Debian: Add a systemd service file for LMS #18

Closed mw9 closed 3 years ago

mw9 commented 3 years ago

Summary

As discussed in the forum thread "systemd service file for LMS": https://forums.slimdevices.com/showthread.php?113114-systemd-service-file-for-LMS

This proposed change adds a systemd unit file to the Debian installation package, and will stand in place of the "auto-generated" unit file that systemd currently creates. The "auto-generated" unit file simply calls the existing SysV init script.

Main features:

I have removed a "Service reload" ('HUP' to LMS) action from the draft unit file that I posted in the forum thread. I am not sure that the function that it supplies is useful. It can be added at a later date if it is, indeed, useful.

I have left a complete set of comments in the unit file itself, which should help to explain why each option is set the way it is, so I won't repeat them here. I'd propose leaving them in there to assist knowledgeable users, but perhaps it is too wordy.

I have successfully built a Debian package on both Debian 9 and Debian 10, and I have installed the resulting package successfully on Debian 7 through 9. It should work equally well on any Debian derived system, with or without systemd, including Ubuntu and Devuan (a non-systemd port of Debian).

So I think it will prove itself in the real world of LMS users, but I shall not be surprised if the real world of LMS users does generate a surprise or two.

Commits

There are three commits:

Debian: Add systemd service unit

Adds boiler plate changes to Debian rules, config file, and the unit file itself. These are as described in the Debian wiki: https://wiki.debian.org/Teams/pkg-systemd/Packaging

Debian: Add systemd service unit - support older systems without 'init-system-helpers'

The boiler plate rules may or may not generate a dependency on a relatively new package, init-system-helpers. This will break older Debian systems if the dependency is not removed, and there still appear to be plenty of users whose systems fall into this category. Installation will proceed successfully without it, although a "deb-systemd-helper: not found" message will be logged. Such users need to upgrade to a more modern system to remove it.

Debian: Add systemd service unit - add explanatory comments to LMS default file

This is not essential, but, I think helpful. It just adds additional explanatory comments in the existing LMS default file /etc/default/logitechmediaserver.

However, we should be aware that anyone who has edited his LMS default file away from the previously installed version will get the usual Debian "Configuration file has been modified" interactive prompt during installation. And that may trigger requests for help in the forum. I would hope not, but it might.

sciurius commented 3 years ago

LMS user is hardcoded to 'squeezeboxserver'

For access purposes I run my LMS as an 'ordinary' user. It would be nice if the LMS user can be set in the /etc/default/logitechmediaserver .

mw9 commented 3 years ago

For access purposes I run my LMS as an 'ordinary' user. It would be nice if the LMS user can be set in the /etc/default/logitechmediaserver .

With the proposed unit file, a knowledgeable user can override the user setting (or any others), using systemctl edit. Or just by copying the unit file into /etc/systemd/system and editing to suit.

The existing Init script reads and executes the default file, and then uses start-stop-daemon to set the user. As far as I can see, the Debian package has used this method "for ever".

A systemd unit file does not offer that kind of flexibility. In an attempt to maintain some kind of compatibility with the use of the existing default file, the proposed unit file reads it and sets environment variables to achieve a similar effect. But this approach cannot work with systemd's USER setting in the unit file.

The unit file could choose to honour the SLIMUSER setting in the default file by passing that as an argument to LMS, with LMS launched as root. LMS would then take on the obligation to change the effective user/group after it has launched. I don't think that this code path has been used for many years (on Debian), and I don't have the competence to judge its effectiveness. So I took the cowardly approach and avoided it.

Perhaps there is a better way of launching LMS from a systemd unit file. For example, by using a "pre-launch" script/program to do the job. But that would add additional complexity.

sciurius commented 3 years ago

With the proposed unit file, a knowledgeable user can override the user setting (or any others),

Which is what I did a long time ago ;).

If there is no need for a more generic solution let's leave it as is. No problem.

tomscytale commented 3 years ago

Nice!

mw9 commented 3 years ago

Does this support non-systemd versions of debian or ubuntu?

Yes.

Devuan: I don't have a Devuan system, but it is based on Debian, which supports both sysvinit and systemd, so it should just work.

Pre 2015: Tested on Debian 7 & Debian 6. There's a bit of chatter during installation, but installation succeeds. See comment in second commit.

But needs testing in the Real World to confirm that practice matches theory.

michaelherger commented 3 years ago

@mw9 - thanks for this PR. I'm a bit puzzled by how this would become active, used. You say

squeezeboxserver_safe goes..

But I don't see any related change. How would I tell debpkg to use this init script instead of the old one?

tomscytale commented 3 years ago

do you have a copy of the .deb available somewhere I could download? would save me a few minutes building it.

tomscytale commented 3 years ago

@mherger is there a list of supported versions of debian, ubuntu, red hat, centos?

Or is it more that a valiant attempt is made to support running on any reasonably sane system with perl >= 5.10 ?

mw9 commented 3 years ago

@mw9 - thanks for this PR. I'm a bit puzzled by how this would become active, used. You say

squeezeboxserver_safe goes..

But I don't see any related change. How would I tell debpkg to use this init script instead of the old one?

You need tell it nothing.

The secret sauce is in how systemd operates:

This change installs a unit file for systemd to use.

A no systemd here system will not concern itself with the installed unit file, and will run the init script in the usual way.

I hope that helps.

mw9 commented 3 years ago

do you have a copy of the .deb available somewhere I could download? would save me a few minutes building it.

Not that would be useful to you. (A somewhat patched armel version of LMS that is not fit for public viewing). Testing the build on systems other than my own would be valuable in proving correct operation.

michaelherger commented 3 years ago

You need tell it nothing.

The secret sauce is in how systemd operates:

So you're saying that if I merged this PR (and it was not broken), things should "just work"? 8.2 is in dev, I could give it a try and let the brave people using 8.2 report issues 😁.

tomscytale commented 3 years ago

I've tested this on a Devuan 3 (Beowulf) x64 system (equivelant to Debian 10 Buster).

Everything works fine.

It's running on a tiny Linode instance. I'll leave it up for another few days (at a total cost of less than a dollar).

If any of you want to check it out pm me your ssh pubkey.

I think we can now be pretty confident that this will now work on the bulk of debian based systems.

So what testing is still necessary?

This of course is all x86_64.

x86_32 is close enough that it should be covered.

ARM... harder to find cloud-based ARM systems to test on. Plenty of RPi's about tho.

I'd suggest

Test matrix

Distro init manager version status
Debian systemd 10 tested OK
9 tested OK
8 tested OK
sysvinit 7
6
5
4 not supported
Devuan sysvinit 3 tested OK
2
Ubuntu systemd 20.10
20.04LTS
19.10
...
upstart/sysv 15.04
...
mw9 commented 3 years ago

You need tell it nothing. The secret sauce is in how systemd operates:

So you're saying that if I merged this PR (and it was not broken), things should "just work"? 8.2 is in dev, I could give it a try and let the brave people using 8.2 report issues 😁.

That is the "design intent". People for whom it won't just work are those who have set SLIMUSER.

It might be worth waiting on @tomscytale, he was planning to set up a few test rigs.

mw9 commented 3 years ago

It might be worth waiting on @tomscytale, he was planning to set up a few test rigs.

Ah, I see that he has.

I can report that it works on armel based Debian 6 through 10. I don't plan to make a Debian 5 for testing. I am not aware of any architectural dependencies/nuances that would get in the way.

mherger commented 3 years ago

Thank you very much guys! Let's give this a try.

tomscytale commented 3 years ago

ubuntu 20.10 looking good

there's a very high probability that this will just work everywhere now.

I will try to test on a few other systems over the coming days - in particular my RPi running aarch64 Ubuntu 20.10

mw9 commented 3 years ago

Thank you very much guys! Let's give this a try.

The new nightly arm deb looks good. Doubtless the other debs will be good as well.

clivem commented 3 years ago

Tested on: RPiOS (Buster 10) armhf Ubuntu 20.04LTS aarch64