-
-
Notifications
You must be signed in to change notification settings - Fork 304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: Add pre and post update, start, backup, and shutdown discord hooks. #172
Feat: Add pre and post update, start, backup, and shutdown discord hooks. #172
Conversation
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.
Not a good idea to run eval $var
in init.sh as that will be ran as root.
I don't recommend running the anything before the shutdown command. If for some reason eval $PRE_SHUTDOWN_HOOK
is frozen then your server will not save.
I recommend moving the post shutdown statement to the end of start.sh. Not sure how you could safety run the pre shutdown safely.
Well these are optional and would allow others to extend the container as they want. I understand that this should be only for advanced users, maybe I can make a note of that in the readme. Unfortunately palworld is currently not running as a service so its difficult for me to grab an accurate status. This could be another enhancement. When I place the hook at the end of start.sh I am unable to send a message && sleep 60 seconds, as the term_handler will have already run the rcon shutdown before anyone is notified. 2024-01-28T19:35:29.369379798Z *****SIGTERM BEGIN*****
2024-01-28T19:35:29.395356988Z Complete Save
2024-01-28T19:35:29.425128085Z The server will shut down in 1 seconds. Please prepare to exit the game.
2024-01-28T19:35:33.034047136Z dlmopen steamservice.so failed: steamservice.so: cannot open shared object file: No such file or directory
2024-01-28T19:35:33.852180758Z Shutdown handler: cleanup.
2024-01-28T19:35:33.875764572Z *****HI START.SH END*****
2024-01-28T19:35:34.427063991Z *****SIGTERM END*****
2024-01-28T19:35:34.427682703Z *****HI INIT.SH END***** If the concern is security I could make this only for the discord hooks for now, as this is the most valuable for me right now: DISCORD_WEBHOOK="https://discord.com/api/webhooks/$DISCORD_WEBHOOK_ID"
if [ -n "$DISCORD_WEBHOOK" ]; then
echo "DISCORD_PRE_INIT_JSON: $DISCORD_PRE_INIT_JSON"
curl -sfSL -H "Content-Type: application/json" -d "$DISCORD_PRE_INIT_JSON" "$DISCORD_WEBHOOK"'
fi I can also move the hooks to another sh and have it run by steam user only: # run as steam user
su steam -c ./hook.sh discord_hook $DISCORD_JSON $DISCORD_WEBHOOK & Open to more suggestions as well. |
tbh, I really like the dedicated discord hooks idea, keeps it more simple and I can imagine that a lot of people would like such a feature |
I think a Discord notifier is a great idea and I'm eager to use it myself. Term is meant to catch stopping the container so it must shutdown quickly. It's about preserving the save data. Recommend timeout for curl: If you're only doing discord then you need at least these environmental vars
The messages as environmental variables are a nice touch although I'm not sure how necessary they are. |
@v4de is this PR still active? |
hey @thijsvanloef, ill work on this today. I have been busy with other work. |
@v4de no problem at all! Was just curious, take your time :) |
@Dashboy1998 We could make some standard json formatting which will reduce the amount of variables. Currently I have been using a dansgaming lamball icon and some basic colors and messages: Not sure how to give the ability to choose to have some messages and not others. Will just add to all the places I believe helpful for notifications right now. |
I think the message variables are a good idea although I would specify default values for the messages, timeout, and even user. Still not sure how to get a good shutdown warning other than a custom shutdown script otherwise I worry about people changing the NICE_SHUTDOWN_TIME but not the stop_grace_period. |
Hi @v4de, Thanks for the work! will look at it tonight! |
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.
Assuming your game is update to date then the order update completed message may be displayed before the server is started message.
If there is an update then the order is correct. It's just a timing issue in the script.
I am getting no messages for running backup.
I am getting no messages for running update.
Please fix all the spell check errors for shell SC2086.
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.
I really appreciate you taking the time for this PR.
I do however have one big issue with the discord.sh script.
Currently this script is designed with human interaction in mind, I don't believe that it should have been designed this way. It makes the code quite messy to read, as there is a lot of repeating lines in it, for example:
/home/steam/server/discord.sh -i "{$DISCORD_WEBHOOK_ID}" -c "{$DISCORD_CONNECT_TIMEOUT}" -M "{$DISCORD_MAX_TIMEOUT}" -m "Unable to delete old backups, OLD_BACKUP_DAYS is empty." -l "warn"
I much rather would like to see the following in the code:
/home/steam/server/discord.sh "Unable to delete old backups, OLD_BACKUP_DAYS is empty." "warn"
@thijsvanloef Changed the syntax for the discord.sh to make it less messy. I wanted to expose the script some how, but agree no good way to do that right now. Also agree with moving it out of usr/bin. Sorry for all the linting changes, will try to keep that very minimal in the future. |
Replying to @Dashboy1998 :
I have seen this also in my testing, I am not sure why it was happening so quick, I removed the success from running in the background anymore.
This is interesting, I was seeing backup messages in my testing. Let me test this again, maybe I changed something after I tested.
I think @thijsvanloef assisted with the linting, I also updated some when changing the command. |
README.md
Outdated
@@ -184,6 +184,14 @@ It is highly recommended you set the following environment values before startin | |||
| AUTO_REBOOT_CRON_EXPRESSION | Setting affects frequency of automatic updates. | 0 0 \* \* \* | Needs a Cron-Expression - See [Configuring Automatic Backups with Cron](#configuring-automatic-reboots-with-cron) | | |||
| AUTO_REBOOT_ENABLED | Enables automatic reboots | false | true/false | | |||
| AUTO_REBOOT_WARN_MINUTES | How long to wait to reboot the server, after the player were informed. | 5 | !0 | | |||
| DISCORD_WEBHOOK_URL | Discord webhook url found after creating a webhook on a discord server | https://discord.com/api/webhooks/<webhook_id> | | |
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.
| DISCORD_WEBHOOK_URL | Discord webhook url found after creating a webhook on a discord server | https://discord.com/api/webhooks/<webhook_id> | | | |
| DISCORD_WEBHOOK_URL | Discord webhook url found after creating a webhook on a discord server | 'https://discord.com/api/webhooks/<webhook_id>' | | |
Looks like you need to put the url inside a code block.
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.
awesome that fixed it!
I suspect the issue is one curl command beating the other. Not sure how to prevent that other than not running it in the background like you changed it or going to the complicated process of making a FIFO for the discord messages.
Strange. Here's my docker compose services:
palworld:
image: thijsvanloef/palworld-server-docker:test
build:
dockerfile: Dockerfile
restart: unless-stopped
container_name: palworld-server-test
stop_grace_period: 30s # Set to however long you are willing to wait for the container to gracefully stop
ports:
- 8212:8211/udp
- 27015:27015/udp # Required if you want your server to show up in the community servers tab
environment:
- PUID=1000
- PGID=1000
- PORT=8211 # Optional but recommended
- PLAYERS=16 # Optional but recommended
- SERVER_PASSWORD=worldofpals # Optional but recommended
- MULTITHREADING=true
- RCON_ENABLED=true
- RCON_PORT=25575
- TZ=UTC
- ADMIN_PASSWORD=adminPasswordHere
- COMMUNITY=false # Enable this if you want your server to show up in the community servers tab, USE WITH SERVER_PASSWORD!
- SERVER_NAME=World of Pals
- SERVER_DESCRIPTION=palworld-server-docker by Thijs van Loef
- DISCORD_WEBHOOK_ID=1234/abde
volumes:
- ./palworld:/palworld/
|
Ok its working for me now, all the conditions were still checking for ID. Pull latest and use Saving Backup:
I do not have an update, so was not able to test updates, I can add a message for already up to date, but that might be too spammy with autoupdate crons now being a thing.
|
I agree repeatedly stating an update is available is excessive. Take a look at #209 to see how to trick the update script into updating. |
Cool that worked and got the update warning! |
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.
Looks good to me.
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.
made a small change in the text, but LGTM
Feat: Add pre and post update, start, backup, and shutdown discord hooks.
Context
Choices
Test instructions
Tested discord webhooks
docker build . -t v4de/palworld-server-docker:0.1
docker compose up -d --no-deps --build
docker exec palworld-server backup
docker compose down
Tested backup after shutdown
- POST_SHUTDOWN_HOOK=backup
docker compose up -d
docker compose down
Checklist before requesting a review