-
Notifications
You must be signed in to change notification settings - Fork 160
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
Refactor file structure, enhance backup manager functionality and fix permissions issues #160
Conversation
…ix permissions issues" Details: - Restructured the file structure for better organization - Refactored code to best practices and to standardize it - Improved output of logs for readability (with colors too!) - Enhanced backup manager to accept arguments for multiple backup operations - Added server backup create, list and clean operations (more to come) - Complemented @jammsen backup retention policy - Added ability to run backup manager as a pseudo binary (symlynk to /usr/local/bin/) - Also to run rcon protected by the env var (if set to true, runs normally) - Also other commands, but now they do nothing (just to keep the structure) - Removed VOLUME and USER directives from Dockerfile to fix permissions issues - Still retain main process as steam user for obvious security reasons - Added a new entrypoint script to handle permissions and launch servermanager.sh as steam user - Updated README.md to reflect changes and improve usability
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.
Added my thoughts and comments.
…(old README_ENV.md now in new docs directory)
Fixed typos in code
Fixed bad env and local vars fixed wrapper for backup manager Fixed counts in clean and list backup functions
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.
Fixed all changes requested in review with @jammsen
7165f17
to
5938e69
Compare
Organized default.env for better readability Added aliases for install.sh webhook operations
5938e69
to
43b4145
Compare
Some explanations on some things:
Any questions just ask 😉 |
Added error checking for tar command
Add check for rcon and for webhook on function call to make easier to call functions using them
As discussed #179 continues this PR and was just merged to develop. Closing this as agreed. |
Enhancements and Refactoring
Summary
This PR introduces several enhancements and changes aimed at improving the organization, readability, and functionality of the project. It includes restructuring the file structure, refactoring the code, improving log output, enhancing the backup manager, and updating the Dockerfile and README.md.
Motivation
The motivation behind these changes were to enhance the project's already great structure and functionality. I wanted to do linting on the code to ensure it adheres to best practices and standardization, allowing for the addition of the flow to check shell linting.
I also wanted to improve the log output for better readability, hence the new log messages (still to improve).
Also, to enhance the backup manager to accept arguments for multiple backup operations.
And to fix the permissions issues with the Dockerfile and the volumes, leading to users having to run
chmod
orchown
on the host. (Effectively achieving what I wanted in #147)Changes
VOLUME
andUSER
directives from Dockerfile to fix permissions issues.rcon.yaml
to be enforced ifRCON_EBABLED=true
andRCON_PORT=<rcon_port_number>
andADMIN_PASSWORD=<your_admin_pw>
all exist and are set, independently of the value inSERVER_SETTING_MODE
.Testing
The changes were validated by running the project locally multiple times with different setups of docker compose, volumes and named volumes and verifying permissions and correct ownership of the main script running on the image.
The refactored code was tested with ShellCheck.
The improved log output was checked for better readability on both live logging and inspection.
The enhanced backup manager was tested with multiple backup operations, during multiple stages of the container, to ensure the best usage and minimize risk scenarios.
The updated Dockerfile was used to build a Docker image and run a container to confirm the fixed permissions issues.
The new entrypoint script was executed to verify it handles permissions and launches servermanager.sh as the steam user.
The updated README.md was reviewed to ensure it accurately reflects the changes and improves usability.
Additional Information
These changes collectively aim to improve the project's overall quality and usability.
Also complete/fix #147.
Fixes #156.
Fixes #117.