Add systemd instantiated service file and per-session environment files #4
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| minecraftd-backup.service | ||
| minecraftd-backup.timer | ||
| minecraftd-backup@.service | ||
| minecraftd-backup@.timer | ||
| minecraftd-init@.service | ||
| minecraftd.conf | ||
| minecraftd.service | ||
| minecraftd.sh | ||
| minecraftd.sysusers | ||
| minecraftd.tmpfiles | ||
| minecraftd@.service |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| SHELL = /bin/sh | ||
| INSTALL = install | ||
| INSTALL_PROGRAM = $(INSTALL) -m755 | ||
| INSTALL_DIR = $(INSTALL) -d -m755 | ||
| INSTALL_DATA = $(INSTALL) -m644 | ||
| confdir = /etc/conf.d | ||
| prefix = /usr | ||
|
|
@@ -10,27 +11,39 @@ datarootdir = $(prefix)/share | |
| mandir = $(prefix)/share/man | ||
| man1dir = $(mandir)/man1 | ||
|
|
||
| SOURCES = $(wildcard *.in) | ||
| OBJECTS = $(SOURCES:.in=) | ||
|
|
||
| GAME = minecraft | ||
| INAME = minecraftd | ||
| SERVER_ROOT = /srv/$(GAME) | ||
| CONFIG_PATH = /etc/conf.d/$(GAME) | ||
| SYSCONFDIR = /etc | ||
| INSTANCE_CONFIG_DIR = $(SYSCONFDIR)/$(GAME) | ||
|
Owner
There was a problem hiding this comment. What do you think about merging Specifically what do you think about /etc/minecraftd and /etc/minecraftd.d (respectively with INAME and config dir)? IMHO this would be a neat solution. |
||
| BACKUP_DEST = $(SERVER_ROOT)/backup | ||
| LIBRARY_PATH = $(libdir)/$(GAME) | ||
| BACKUP_PATHS = world | ||
| BACKUP_FLAGS = -z | ||
| KEEP_BACKUPS = 10 | ||
| GAME_USER = $(GAME) | ||
| SERVER_MEMORY_INITIAL = 512 | ||
| SERVER_MEMORY_MAXIMUM = 1024 | ||
|
Comment on lines
+29
to
+30
Owner
There was a problem hiding this comment. These flags are not meaningful for servers which do not allow for configuring the memory via the command line.
Author
There was a problem hiding this comment. I'll need to look into this further. You mentioned cuberite which I am not familiar with.
Owner
There was a problem hiding this comment. Yes all java based implementations should honor this flag or more precisely, it will be enforced by the JVM. However, cuberite is a C++ rewrite and does not care about any of this. |
||
| MAIN_EXECUTABLE = minecraft_server.jar | ||
| ifeq ($(MAIN_EXECUTABLE),$(MAIN_EXECUTABLE:/%=)) | ||
| MAIN_EXECUTABLE_ABSOLUTE = $(SERVER_ROOT)/$(MAIN_EXECUTABLE) | ||
| else | ||
| MAIN_EXECUTABLE_ABSOLUTE = $(MAIN_EXECUTABLE) | ||
| endif | ||
Edenhofer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| TMUX_SOCKET_DIR = /run/$(GAME)/tmux | ||
| SESSION_NAME = $(GAME) | ||
| SERVER_START_CMD = java -Xms@SERVER_MEMORY_INITIAL@M -Xmx@SERVER_MEMORY_MAXIMUM@M -jar @MAIN_EXECUTABLE@ nogui | ||
|
Owner
There was a problem hiding this comment. This does not work. You may not use EDIT: I just saw that you actually account for this later but that just feels very wrong on so many levels. Please don't do that!
Author
There was a problem hiding this comment. Yeah, while it does work, I'm open to any better solution. My goal here is to allow users to customize the memory allocated to the server without overriding the entire JVM command line, should the packager need to add/change/remove other JVM arguments. Do you have any recommendations for how this could be accomplished? I'm okay with returning to the use of
Owner
There was a problem hiding this comment. The goal is clear but I don't like adding any java specific options. The only viable solution to me seems to alter all of I am afraid you have to jog my memory a little about your proposal regarding |
||
| SERVER_START_SUCCESS = done | ||
| IDLE_SERVER = false | ||
| IDLE_SESSION_NAME = idle_server_@SESSION_NAME@ | ||
|
Owner
There was a problem hiding this comment. See above. I am fine with removing this variable completely and simply derive it from |
||
| GAME_PORT = 25565 | ||
| CHECK_PLAYER_TIME = 30 | ||
| IDLE_IF_TIME = 1200 | ||
| GAME_COMMAND_DUMP = $(TMUX_SOCKET_DIR)/@INAME@_@SESSION_NAME@_command_dump.txt | ||
|
Owner
There was a problem hiding this comment. See above. Again I am fine with removing this variable from the configuration munging and hardcode its dependency on |
||
|
|
||
| .MAIN = all | ||
|
|
||
|
|
@@ -40,12 +53,20 @@ define replace_all | |
| -e 's#@INAME@#$(INAME)#g' \ | ||
| -e 's#@GAME@#$(GAME)#g' \ | ||
| -e 's#@SERVER_ROOT@#$(SERVER_ROOT)#g' \ | ||
| -e 's#@CONFIG_PATH@#$(CONFIG_PATH)#g' \ | ||
| -e 's#@LIBRARY_PATH@#$(LIBRARY_PATH)#g' \ | ||
| -e 's#@SYSCONFDIR@#$(SYSCONFDIR)#g' \ | ||
| -e 's#@INSTANCE_CONFIG_DIR@#$(INSTANCE_CONFIG_DIR)#g' \ | ||
| -e 's#@BACKUP_DEST@#$(BACKUP_DEST)#g' \ | ||
| -e 's#@BACKUP_PATHS@#$(BACKUP_PATHS)#g' \ | ||
| -e 's#@BACKUP_FLAGS@#$(BACKUP_FLAGS)#g' \ | ||
| -e 's#@KEEP_BACKUPS@#$(KEEP_BACKUPS)#g' \ | ||
| -e 's#@GAME_USER@#$(GAME_USER)#g' \ | ||
| -e 's#@MAIN_EXECUTABLE@#$(MAIN_EXECUTABLE)#g' \ | ||
| -e 's#@MAIN_EXECUTABLE_ABSOLUTE@#$(MAIN_EXECUTABLE_ABSOLUTE)#g' \ | ||
| -e 's#@SERVER_MEMORY_INITIAL@#$(SERVER_MEMORY_INITIAL)#g' \ | ||
| -e 's#@SERVER_MEMORY_MAXIMUM@#$(SERVER_MEMORY_MAXIMUM)#g' \ | ||
| -e 's#@TMUX_SOCKET_DIR@#$(TMUX_SOCKET_DIR)#g' \ | ||
| -e 's#@SESSION_NAME@#$(SESSION_NAME)#g' \ | ||
| -e 's#@SERVER_START_CMD@#$(SERVER_START_CMD)#g' \ | ||
| -e 's#@SERVER_START_SUCCESS@#$(SERVER_START_SUCCESS)#g' \ | ||
|
|
@@ -88,12 +109,18 @@ maintainer-clean: clean | |
|
|
||
| install: | ||
| $(INSTALL_PROGRAM) -D minecraftd.sh "$(DESTDIR)$(bindir)/$(INAME)" | ||
| $(INSTALL_DIR) "$(DESTDIR)$(LIBRARY_PATH)" | ||
| $(INSTALL_DATA) -D argparse.sh "$(DESTDIR)$(LIBRARY_PATH)/argparse.sh" | ||
| $(INSTALL_DATA) -D minecraftd.conf "$(DESTDIR)$(confdir)/$(GAME)" | ||
| $(INSTALL_DATA) -D minecraftd.service "$(DESTDIR)$(libdir)/systemd/system/$(INAME).service" | ||
| $(INSTALL_DATA) -D minecraftd-backup.service "$(DESTDIR)$(libdir)/systemd/system/$(INAME)-backup.service" | ||
| $(INSTALL_DATA) -D minecraftd-backup.timer "$(DESTDIR)$(libdir)/systemd/system/$(INAME)-backup.timer" | ||
| $(INSTALL_DATA) -D minecraftd@.service "$(DESTDIR)$(libdir)/systemd/system/$(INAME)@.service" | ||
| $(INSTALL_DATA) -D minecraftd-init@.service "$(DESTDIR)$(libdir)/systemd/system/$(INAME)-init@.service" | ||
| $(INSTALL_DATA) -D minecraftd-backup@.service "$(DESTDIR)$(libdir)/systemd/system/$(INAME)-backup@.service" | ||
| $(INSTALL_DATA) -D minecraftd-backup@.timer "$(DESTDIR)$(libdir)/systemd/system/$(INAME)-backup@.timer" | ||
| $(INSTALL_DATA) -D minecraftd.sysusers "$(DESTDIR)$(libdir)/sysusers.d/$(INAME).conf" | ||
| $(INSTALL_DATA) -D minecraftd.tmpfiles "$(DESTDIR)$(libdir)/tmpfiles.d/$(INAME).conf" | ||
|
|
||
| uninstall: | ||
| rm -f "$(bindir)/$(INAME)" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| #!/bin/bash | ||
|
Owner
There was a problem hiding this comment. Please just inline this script. It is pretty fancy and not specifically related to the minecraft server. However, in its current form it still couples tightly to it. Furthermore, I really like that the server is just a single bash script without any additional library files.
Author
There was a problem hiding this comment. This file adds a lot of LOC and I don't feel that it's tightly coupled. Can you share why you said it couples tightly to the launcher script? With the launcher script adding a lot of LOC in this change, do you think it makes sense to go the opposite direction, and break out the launcher script into individual files? It's not yet at the point where it really needs that, but if it keeps growing that will eventually be a tech debt item and maybe it could be better to just get that out of the way now. Completely up to you though.
Owner
There was a problem hiding this comment.
I would much rather have one larger script than many small libraries stuck together (at least in bash). The scoping in bash is a mess and I find it easier to grasp larger bash scripts. I don't think we have hit the point yet that the script has become unwieldy. |
||
|
|
||
| # shell script library to parse and validate command line arguments, and generate the --help text. | ||
|
|
||
| set -u | ||
|
|
||
| re_subcmd_vn='^[a-z0-9]+:[A-Za-z0-9_]+$' | ||
| COMMAND= | ||
|
|
||
| declare -A args args_help subcmds | ||
|
|
||
| add_arg() { | ||
| local short="$1" | ||
| local long="$2" | ||
| local varname="$3" | ||
| local help="$4" | ||
| local required="${5:-false}" | ||
| [ -v 6 ] && local default="$6" | ||
|
Owner
There was a problem hiding this comment. indentation, plus missing double brackets |
||
| if [[ "$varname" =~ $re_subcmd_vn ]]; then | ||
| local subc _vn | ||
| IFS=":" read subc _vn <<< "$varname" | ||
| [[ -n "${subcmds[$subc]}" ]] | ||
|
Owner
There was a problem hiding this comment. Is this an implicit test that this subcommand is defined? If so please terminate explicitly here instead of relying on |
||
| fi | ||
| args[$varname]="$short $long $required" | ||
| args_help[$varname]="$help" | ||
| # init the global | ||
| [[ -v default ]] && declare -g ${varname#*:}="${default}" | ||
| } | ||
|
|
||
| add_subcommand() { | ||
| local subc="$1" | ||
| local help="$2" | ||
|
|
||
| subcmds[$subc]="$help" | ||
| } | ||
|
|
||
| usage() { | ||
| # help function. generates usage instructions | ||
| local short long required help | ||
| local n_subcmds=${#subcmds[@]} | ||
| local ofd=1 | ||
| [[ -v 1 ]] && ofd=2 | ||
| local cols=80 | ||
| [[ -t $ofd ]] && cols=$(tput cols) | ||
| ( | ||
| if [[ -v 1 ]]; then | ||
| echo "ERROR: $1" | ||
| echo "" | ||
| fi | ||
| if [[ -v DESCRIPTION ]]; then | ||
| echo -e "$DESCRIPTION" | ||
| echo "" | ||
| fi | ||
| if [[ $n_subcmds < 1 ]]; then | ||
| echo "Usage: $0 [options]" | ||
| echo "" | ||
| echo "Valid options are:" | ||
| else | ||
| echo "Usage: $0 [global options] COMMAND [command-specific options]" | ||
| echo "" | ||
| echo "Global options:" | ||
| fi | ||
| for varname in ${!args[@]}; do | ||
| [[ $varname =~ $re_subcmd_vn ]] && continue | ||
| IFS=" " read short long required <<< "${args[$varname]}" | ||
| help="${args_help[$varname]}" | ||
| printf " -%s, --%-16s %s (required: %s)\n" "$short" "$long" "$help" "$required" | ||
| done | ||
|
|
||
| if [[ $n_subcmds > 0 ]]; then | ||
| echo "" | ||
| echo "Valid commands:" | ||
| for subcmd in ${!subcmds[@]}; do | ||
| printf " %-12s %s\n" "$subcmd" "${subcmds[$subcmd]}" | ||
| done | ||
|
|
||
| for subcmd in ${!subcmds[@]}; do | ||
| echo "" | ||
| echo "Options for command \"$subcmd\":" | ||
| local n=0 | ||
| for varname in ${!args[@]}; do | ||
| [[ "${varname%:*}" == "$subcmd" ]] || continue | ||
| ((n++)) | ||
| IFS=" " read short long required <<< "${args[$varname]}" | ||
| help="${args_help[$varname]}" | ||
| printf " -%s, --%-16s %s (required: %s)\n" "$short" "$long" "$help" "$required" | ||
| done | ||
| [[ $n == 0 ]] && echo " (None)" | ||
| done | ||
| echo "" | ||
|
|
||
| [[ -v COPYRIGHT ]] && echo -e "$COPYRIGHT" | ||
| fi | ||
| ) | fold -w "$cols" -s >&$ofd | ||
| exit 1 | ||
| } | ||
|
|
||
| parse_args() { | ||
| local short long required longprefix found | ||
| local n_subcmds=${#subcmds[@]} | ||
| declare -a bareargs reqargs | ||
| while [[ -v 1 ]]; do | ||
| case "$1" in | ||
| --help|-h) | ||
| usage ;; | ||
| -*) | ||
| # parse as named option | ||
| found=false | ||
| for varname in ${!args[@]}; do | ||
| IFS=" " read short long required <<< "${args[$varname]}" | ||
| if [[ $varname =~ $re_subcmd_vn ]]; then | ||
| # subcommand option | ||
| local vsubc _vn | ||
| IFS=":" read vsubc _vn <<< "$varname" | ||
| [[ "$vsubc" = "$COMMAND" ]] || continue | ||
| varname=$_vn | ||
| fi | ||
| longprefix="--$long=" | ||
| case "$1" in | ||
| -$short|--$long) | ||
| [[ -v 2 ]] || usage "value for argument $1 may not be omitted" | ||
| declare -g $varname="$2" | ||
| found=true | ||
| shift | ||
| ;; | ||
| $longprefix*) | ||
| found=true | ||
| declare -g $varname=${1:${#longprefix}} | ||
| ;; | ||
| esac | ||
| done | ||
| [[ "$found" == "true" ]] || usage "Unknown option: $1" | ||
| ;; | ||
| *) | ||
| if [[ $n_subcmds > 0 && $COMMAND == "" ]]; then | ||
| COMMAND="$1" | ||
| [[ -v subcmds[$COMMAND] ]] || usage "Undefined command: $COMMAND" | ||
| else | ||
| bareargs+=("$1") | ||
| fi | ||
| ;; | ||
| esac | ||
| shift | ||
| done | ||
|
|
||
| [[ $n_subcmds > 0 && $COMMAND == "" ]] && usage "No command was given." | ||
|
|
||
| # build list of missing required args | ||
| for varname in ${!args[@]}; do | ||
| IFS=" " read short long required <<< "${args[$varname]}" | ||
| [[ "$required" == "true" ]] || continue | ||
| if [[ $varname =~ $re_subcmd_vn ]]; then | ||
| # subcommand option | ||
| local vsubc _vn | ||
| IFS=":" read vsubc _vn <<< "$varname" | ||
| [[ "$vsubc" = "$COMMAND" ]] || continue | ||
| [[ -v $_vn ]] && continue | ||
| reqargs+=("$varname") | ||
| else | ||
| [[ -v $varname ]] && continue | ||
| reqargs+=("$varname") | ||
| fi | ||
| done | ||
| # process bareword args | ||
| while [[ -v bareargs[0] && -v reqargs[0] ]]; do | ||
| declare -g ${reqargs[0]#*:}="${bareargs[0]}" | ||
| bareargs=("${bareargs[@]:1}") | ||
| reqargs=("${reqargs[@]:1}") | ||
| done | ||
|
|
||
| # if there's any bareword arguments left, we are out of ideas for what to do with them, so fail. | ||
| [[ -v bareargs[0] ]] && usage "Unknown argument: ${bareargs[0]}" | ||
|
|
||
| # enforce required args | ||
| for varname in ${!args[@]}; do | ||
| IFS=" " read short long required <<< "${args[$varname]}" | ||
| [[ "$required" == "true" ]] || continue | ||
| if [[ $varname =~ $re_subcmd_vn ]]; then | ||
| # subcommand arg | ||
| local vsubc _vn | ||
| IFS=":" read vsubc _vn <<< "$varname" | ||
| [[ "$vsubc" == "$COMMAND" ]] || continue | ||
| [[ ! -v $_vn ]] && usage "Option is required for command $vsubc but not set: $long" | ||
| else | ||
| # global arg | ||
| [[ ! -v $varname ]] && usage "Required option or positional argument not set: $long" | ||
| fi | ||
| done | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| [Unit] | ||
| Description=@GAME@ Server World Backup | ||
| After=local-fs.target | ||
|
|
||
| [Service] | ||
| Type=oneshot | ||
| Environment=SERVER_ROOT=@SERVER_ROOT@/servers/%i | ||
| Environment=BACKUP_DEST=@SERVER_ROOT@/servers/%i/backup | ||
| Environment=SESSION_NAME=@GAME@-%i | ||
| EnvironmentFile=-@INSTANCE_CONFIG_DIR@/%i | ||
| ExecStart=/usr/bin/@INAME@ backup | ||
| User=@GAME_USER@ | ||
| Group=@GAME_USER@ | ||
|
|
||
| [Install] | ||
| WantedBy=multi-user.target |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| [Unit] | ||
| Description=Daily @GAME@ Server Backup | ||
|
|
||
| [Timer] | ||
| OnCalendar=daily | ||
| AccuracySec=5min | ||
| Persistent=true | ||
|
|
||
| [Install] | ||
| WantedBy=multi-user.target |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| [Unit] | ||
|
Owner
There was a problem hiding this comment. Is this service file really necessary? It runs exactly once and is not needed afterwards, right?
Author
There was a problem hiding this comment. I tried everything, but unfortunately, systemd's sandboxing affects all Instead of this approach, we can update the launcher script to exit with an error after instructing the user to run the
Owner
There was a problem hiding this comment. Personally, I would much rather print an error and require user intervention than have a systemd service lying around that runs exactly once. |
||
| Description=Create server directories for @GAME@ %i instance | ||
| After=local-fs.target network.target multi-user.target | ||
|
|
||
| [Service] | ||
| Type=oneshot | ||
| RemainAfterExit=no | ||
| # create the "servers" directory followed by the instance directory | ||
| ExecStart=/usr/bin/@INAME@ -i %i init | ||
|
|
||
| [Install] | ||
| WantedBy=multi-user.target | ||
|
Owner
There was a problem hiding this comment. Missing new line at end of file |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you swap this line with the one below, you can use
SYSCONFDIRinCONFIG_PATH.