Infinidoge / nix-minecraft

An attempt to better support Minecraft-related content for the Nix ecosystem
MIT License
245 stars 26 forks source link

Replace tmux with stdin systemd sockets #41

Closed Misterio77 closed 1 year ago

Misterio77 commented 1 year ago

To make reviewing this easier, see this diff: https://github.com/Misterio77/nix-minecraft/compare/60bf3412edbb562da2d253fd5159d985f4e849a1...master


This PR, that builds on the previous ones (#38, #39, #40), replaces tmux with a systemd stdin socket. With it, you can easily run commands like so:

echo "say Hi" > /run/minecraft/foobar.stdin

We now use this to run "stop" (or any custom command) on ExecStop.

To complement extraReload, I've added more extra* options for different parts of the lifecycle: extra{Post,Pre}{Start,Stop} (preStop and postStart can run console commands, awesome!).

The tests are also expanded, as I mentioned on the testing PR.

Misterio77 commented 1 year ago

I want to, soon(ish), add a script (automatically added to minecraft's PATH) that makes this easier and also echoes the output back.

This is what I currently have:

server="$1"
socket="/run/minecraft-server/$1.stdin"
shift

timestamp="$(date +%s)"
echo "$@" > "$socket"

# Get console output since before running the command, and keep following for 500ms
timeout --preserve-status 0.5 journalctl \
    -u "minecraft-server-$server.service" \
    --since "@$timestamp" \
    --follow

Works like so:

$ mcrun survival say foo bar
jun 09 17:01:46 celaeno minecraft-server[675194]: [17:01:46 INFO]: [Not Secure] [Server] foo bar

$ mcrun survival list
jun 09 17:01:51 celaeno minecraft-server[675194]: [17:01:51 INFO]: There are 0 of a max of 20 players online:

$ mcrun proxy lpv help
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: [LP] Running LuckPerms v5.4.64.
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv user <user>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv group <group>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv track <track>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv log
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv sync
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv info
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv editor [type]
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv verbose <on|record|off|upload> [filter]
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv tree [scope] [player]
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv search <permission>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv networksync
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv import <file>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv export <file>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv reloadconfig
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv bulkupdate
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv translations
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv creategroup <group>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv deletegroup <group>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv listgroups
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv createtrack <track>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv deletetrack <track>
jun 09 17:13:41 celaeno velocity[684812]: [17:13:41 INFO]: > /lpv listtracks
Misterio77 commented 1 year ago

I remembered you were working on something similar. I'd argue, though, that we should phase out tmux support - as it's kinda brittle and does not integrate well with systemd.

Infinidoge commented 1 year ago

A prerequisite to any change in how the server is accessed is that it must be configurable.

First and foremost, this is a breaking change, and given that it is based in the module, there should be a way code-wise to mark it as deprecated while not immediately breaking.

Additionally, there are cases where having access to a full terminal is desirable, as opposed to needing to send commands individually, and I would prefer to continue to allow that to be used.

My end-goal would be to replace tmux with the ability to choose between systemd sockets and something like abduco or similar, which would accomplish the same thing as tmux without being nearly as heavy weight.

Misterio77 commented 1 year ago

Thanks for the feedback! That's understandable, I'll look into making both be supported.

I will do the same as you did and add a serverType option, but I think tmux and socket is enough (as socket works for all simple usecases).

Would it be okay to make stateVersion < 23.11 default to tmux and >= 23.11 default to socket? Perhaps with a warning if you're not setting it explicitly?

Infinidoge commented 1 year ago

The stateVersion check sounds fine to me. A warning if not set explicitly is a good idea. (I feel like NixOS itself has a warning for it, but I don't remember.)

Infinidoge commented 1 year ago

I would recommend cherry picking the commits onto their own branch and retargeting the PR, as opposed to pulling from master, so that we don't have GHA commits in the diff

Infinidoge commented 1 year ago

Please cherry pick and retarget!

Misterio77 commented 1 year ago

Sorry for taking so long! Working on it now