Page MenuHomeWildfire Games

lobby bots systemd service
Needs ReviewPublic

Authored by elexis on Nov 1 2018, 12:53 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Since lobby bots should be services that are automatically restarted if the system restarts, and since the target platform is a debian based server,
there must be a systemd service to achieve that.

Since leper does not grant Wildfire Games to release the script under GPL 2v+, we need a replacement.
The new script should be more versatile, cleaner and maintained.

Test Plan

Check that the code has the maximum degree of versatility, the minimum of code, hardcodings and assumptions.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6421
Build 10637: Vulcan BuildJenkins
Build 10636: arc lint + arc unit

Event Timeline

elexis created this revision.Nov 1 2018, 12:53 AM
Vulcan added a subscriber: Vulcan.Nov 1 2018, 12:58 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/759/display/redirect

elexis planned changes to this revision.Nov 1 2018, 6:38 PM

README.md update after D1659

source/tools/XpartaMuPP/systemd/pyrogenesis-lobbybots.sh
33

should depend on room

98

this command can fail, making the script exit early. but abort wasnt really planned for in that case.

smiley added a subscriber: smiley.Nov 1 2018, 6:57 PM
smiley added inline comments.
source/tools/XpartaMuPP/systemd/pyrogenesis-lobbybots.sh
43

Is the situation where bots operate in different rooms foreseeable?

shookees added inline comments.
source/tools/XpartaMuPP/systemd/pyrogenesis-lobbybots.service
13

was stop intended for reload (while script itself contains instructions for restart)

source/tools/XpartaMuPP/systemd/pyrogenesis-lobbybots.sh
44

maybe use env variable instead of relying on changing after deployment (implying this is not the actual password)

elexis edited the summary of this revision. (Show Details)Nov 3 2018, 2:05 PM
elexis added inline comments.
source/tools/XpartaMuPP/systemd/pyrogenesis-lobbybots.service
13

good catch

source/tools/XpartaMuPP/systemd/pyrogenesis-lobbybots.sh
43

It's even backseeable. There is one set of bots per room. One gamelist per room. One rating database per room (that might not be ideal but it's the current state). The bot scripts can currently join only one room. The client code must also match the bot backend code, so different scripts per room too.

44

This fake array is the configuration. Perhaps it could go into separate files.

This is the actual password. The password also shows up in the process list, since it's passed via commandline. Perhaps that may be changed in the future.

smiley added inline comments.Nov 3 2018, 2:10 PM
source/tools/XpartaMuPP/systemd/pyrogenesis-lobbybots.sh
43

I meant it as more like ratings bot and gamelist bot being in different rooms. In which case, its not necessary to have two room variables.

Why to use systemd services to call sysvinit init scripts? Why not just using systemd services? That'd make everything easier.

Why to use systemd services to call sysvinit init scripts? Why not just using systemd services? That'd make everything easier.

At least with the approach of all bots started from one script, there needs to be one script regardless, i.e. no gain to make it incompatible with sysvinit.

In D1659#65912, @elexis wrote:

The pid/logfile is handled by systemd.
There is one logfile and pidfile per bot, at least one bot per room, but one systemd service handling all bots.
I don't see a better place to put them rather than the directory for that room for that bot, do you know one?
(Every bot instance needs an own code copy to provide the correct version for everyroom).

That sounds just wrong:

  • By default systemd handles the location of logs and pids on its own. No need to pollute the directory containing the code with it.
  • There should be two different systemd service files, one for XpartaMuPP and one for EcheLOn and as many instances of each service, as there are lobbies (different code location should also be no issue with this approach). See e.g. https://coreos.com/os/docs/latest/getting-started-with-systemd.html#instantiated-units

I agree it would be nicer to have systemd take care of pid and logfiles.

It's indeed possible to restart all instances of a systemd service with one command. (Still two restart commands unless adding an alias.)
I'm not sure if it is really easier if we by hand have to create a new script file per bot per room storing the login and a symlink for the unit/service file for every bot/room instance.
And what would the ExecStop command be if we don't capture the PID? (running grep on ps aux?)

source/tools/XpartaMuPP/systemd/pyrogenesis-lobbybots.sh
43

It should be possible to specify any combination of rooms with any amount of bots per individual room.

Notice the code below has no knowledge of which botscripts exist, so one can add the moderation bot, the donation bot and the historical quote bot and whatever too, optional per each room.

As already mentioned I don't see a benefit in using a sysvinit script. I'd just use one systemd service template for each bot and one systemd service instance per bot instance.
Such a systemd template would look like:

[Unit]
Description=XPartaMuPP Pyrogenesis/0 A.D. Lobby Bot %i
Documentation=https://trac.wildfiregames.com/
After=network.target ejabberd.service

[Service]
Type=simple
User=nobody
EnvironmentFile=/etc/0ad/xpartamupp-%i
ExecStart=python3 /code/directory/xpartamupp-%i/XpartaMuPP.py \
          --login ${LOGIN} --password ${PASSWORD} --nickname ${NICKNAME} --domain ${HOST} \
          --room ${ROOM} -d
Restart=on-failure

# Hardening
NoNewPrivileges=yes
PrivateDevices=yes
ProtectKernelTunables=yes
ProtectKernelModules=yes
PrivateUsers=yes
PrivateTmp=yes
ProtectSystem=yes

[Install]
WantedBy=multi-user.target

I would place that template in /etc/systemd/system/xpartamupp@.service and create an instance per lobby by doing systemctl enable xpartamupp@a23.service. This instance (a23) would take the code from /code/directory/xpartamupp-a23/ and configuration from /etc/0ad/xpartamupp-a23. The configuration file would be a simple file with a key-value pair per line:

LOGIN=xpartamupp
PASSWORD=password
HOST=localhost
NICKNAME=WFGBot
ROOM=arena23

That unit file isn't perfect yet (especially there is way more you can do for hardening), but it works for me locally and provides so many advantages over the systemd-and-sysvinit-approach that I'm too lazy to even try to list them all. πŸ˜„

Looks good, wanna commandeer and add the README entry?
Also would be good to support arguments that only work for some versions (for example disable TLS and the rating bot are recent arguments).

elexis updated this revision to Diff 8428.Jun 11 2019, 3:10 PM

Dunedans script, modified, extended README, updated ejabberd format of the ipstamp module, deployed at the lobby server.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1688/display/redirect

wraitii added a subscriber: wraitii.EditedJun 11 2019, 3:45 PM

If this is deployed on the lobby server, shouldn't we merge it ASAP and then possibly open other diffs to improve it?

If this is deployed on the lobby server, shouldn't we merge it ASAP and then possibly open other diffs to improve it?

For that I would have to make a decision I don't like.

What is more urgent from a WFG code point of view are the unit motion issues. I'm not sure if asking for more bugfixes will result in less bugs however.

source/tools/lobbybots/mod_ipstamp/src/mod_ipstamp.erl
39 β†—(On Diff #8428)

new in ejabberd 18.03

40 β†—(On Diff #8428)
In D1661#81903, @elexis wrote:

If this is deployed on the lobby server, shouldn't we merge it ASAP and then possibly open other diffs to improve it?

For that I would have to make a decision I don't like.

What decision is that?

elexis updated this revision to Diff 8442.Jun 12 2019, 12:32 AM

Add the backup scripts based on those deployed by Itms and user1

elexis updated this revision to Diff 8443.Jun 12 2019, 12:34 AM

Remove dead line from backup-ratings.sh

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1696/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1697/display/redirect

elexis added inline comments.Jun 15 2019, 2:11 PM
source/tools/lobbybots/backup/backup-users.sh
7 β†—(On Diff #8443)

BUPTMP is only the intermediate
the process returns before the new file is created
an indefinite waiting loop for the file to vanish sounds ugly
sleep 45; is a working hack, that I guess breaks for huge databases and is too slow for the average case, and ugly as it's not conditional
Took a quick look at the ejabberd code, but didn't follow where the second process is launched, or what else happens.

elexis updated this revision to Diff 8474.Jun 15 2019, 2:23 PM

Fix BUPTMP

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1719/display/redirect

elexis updated this revision to Diff 8479.Jun 15 2019, 3:59 PM

Use /etc/crontab instead of crontab -e, this way being able to restrict the user access of the backup scripts, and remove the PATH variable.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1724/display/redirect

Stan added a subscriber: Stan.Jun 15 2019, 4:03 PM
Stan added inline comments.
source/tools/lobbybots/README.md
376 β†—(On Diff #8479)

Maybe the logs location should depend on the version as well ?

561 β†—(On Diff #8479)

Were the command reversed ?

elexis added inline comments.Jun 15 2019, 4:14 PM
source/tools/lobbybots/README.md
376 β†—(On Diff #8479)

There is a subfolder per room and a room per version.

561 β†—(On Diff #8479)

No