Re: Can not start a script with systemd

[Greg Wooledge <greg@wooledge.org>]

On Sun, Mar 13, 2022 at 01:44:23PM +0200, Alexis Grigoriou wrote:
>  The script to start alienarena-ded:
> 
>    #! /bin/sh
>    killall alienarena-ded
>    sleep 5
>    cd /home/aaserver/aa
>    ./alienarena-ded +set game arena +exec ctfir.cfg
>    exit 0

There are two or three problems with this.  I'll start with the most
obvious and important ones first:

1) You didn't check the result of cd.  If the cd fails, then you might
   end up running the wrong command.  Of course, it's not *likely* that
   there is an alienarena-ded command in another directory, so maybe
   you'll be fortunate enough that the command following cd will also
   fail.  But there's no reason to assume it.

   cd /home/aaserver/aa || exit 1

2) You didn't "exec" the alienarena-ded command.  This means your script
   forks alienarena-ded as a child process, and then both the script and
   the alienarena-ded hang around in memory together, as separate PIDs.

   exec ./alienarena-ded +set game arena +exec ctfir.cfg

Note: in order for this to work correctly, the alienarena-ded should
run as a simple foreground task.  It must not fork and abandon a child
process ("self-daemonizing" or "self-backgrounding").  If it does this
by default, look for an option to stop it from doing this.  If it has
no such options, then the entire remainder of this email is incorrect,
and you'll have to rewrite everything for a Type=forking service.

And the less obvious problem:

3) You appear to be relying on the killall command to clean up previous
   instances of the alienarena-ded command.  Don't do this.  Write
   everything correctly, and then let systemd manage it all.

In fact, you don't need this wrapper script at all.  Everything it does
should be handled by a properly written systemd unit.

But for now, let's just keep the wrapper script, and have the systemd
unit invoke the wrapper script, which will then invoke the alienarena-ded
program.

Here's the corrected wrapper script:

=============================================================
#!/bin/sh
cd /home/aaserver/aa || exit 1
exec ./alienarena-ded +set game arena +exec ctfir.cfg
=============================================================

> And the unit file:
> 
>    [Unit]
>    Description=Alien Arena Dedicated Server
> 
>    [Service]
>    Type=exec
>    User=aaserver
>    ExecStart=/home/aaserver/aas.sh
>    ExecStop=/usr/bin/killall alienarena-ded
>    ExecReload=/home/aaserver/aas.sh
> 
>    [Install]
>    WantedBy=multi-user.target

You have Type=exec but the way you wrote your original wrapper script
(with the forked-not-exec'ed alienarena-ded) should actually have been
Type=forking.

With the corrected wrapper script, and assuming the alienarena-ded
program doesn't self-background, we can now use Type=exec.  We also
don't need the ExecStop= line or the ExecReload= line.  By omitting
the ExecStop= you're telling systemd to just send a SIGTERM, which is
what we want.  You don't need the killall any more, because now that
the alienarena-ded process is a direct child of systemd, systemd can
manage it normally.

You also don't want ExecReload= here.  You don't have a "configuration
reload" feature to invoke.  All you're doing there is stopping (badly!)
and then restarting the original program.  That's not really a reload;
it's a restart.

So, here's our new unit file:

=============================================================
[Unit]
Description=Alien Arena Dedicated Server

[Service]
Type=exec
User=aaserver
ExecStart=/home/aaserver/aas.sh

[Install]
WantedBy=multi-user.target
=============================================================

Closing notes: your wrapper script is doing its own "cd" (change
directory) command.  That's not wrong.  But it would be better if the
working directory were specified in the systemd unit file, along with
the User= directive.  That would keep all of the fundamental setup
in one place.  So, something like this:

=============================================================
[Service]
Type=exec
User=aaserver
WorkingDirectory=/home/aaserver/aa
ExecStart=/home/aaserver/aas.sh
=============================================================

You could also leave the cd command in the wrapper script, as a bit
of redundant protection.

Or, if you want to get rid of the wrapper script entirely:

=============================================================
[Unit]
Description=Alien Arena Dedicated Server

[Service]
Type=exec
User=aaserver
WorkingDirectory=/home/aaserver/aa
ExecStart=/home/aaserver/aa/alienarena-ded +set game arena +exec ctfir.cfg

[Install]
WantedBy=multi-user.target
=============================================================