azerothcore-wotlk
azerothcore-wotlk copied to clipboard
feat(Docker): Clean up build and runtime
I can be contacted on discord as mynameismeat#2486. Please feel free to reach out/tag me in a channel if you have any questions, comments, concerns.
Goals
- Smaller docker containers
- Less extraneous tooling
- Merge various build targets into single target per component
- Utilize multi-stage builds to ensure contents of containers are only runtime dependencies
- remove "prod" container distinction
- Enable users to get started with minimal effort
- wrap the worldserver in a way that safely runs
server shutdown
when the container is shutdown - Allow for creation/management of an "admin account"
- Maintain compatibility with documentation and
acore-docker
within reason. - Use docker's built-in caching for ccache management
- Remove the need (and the ability :p) from the user to access the console.
Changes Proposed:
- This is a large PR with many files changed.
- Files changed and description of changes involved
-
apps/docker/Dockerfile
- Remove "prod" distinction
- utilize multi-stage container builds
- Split the
ac-dev-server
out into its' own dockerfile
-
docker-compose.yml
- Remove "prod" distinction
-
apps/docker/docker-cmd.ts
- Remove "prod" distinction
-
apps/docker/client-data.sh
- Script to install client data
- function extracted out of
apps/installer/includes/functions.sh
-
apps/docker/entrypoint.sh
- Starts azerothcore components
- starts worldserver wrapper if desired
- The wrapper script can create a user on startup and runs
server shutdown 5s 0
in the case of a SIGTERM
- The wrapper script can create a user on startup and runs
- templates out config files
- There was a few ways I wanted to do this, but to avoid spooky action at a distance it just templates the
.conf
in place.
- There was a few ways I wanted to do this, but to avoid spooky action at a distance it just templates the
-
env/docker/etc/*.dockerdist
- Template out MySQL connection
- disable logging to a file. Docker handles logging, logging to a file as well doesn't really do anything for us
-
deps/boost/CMakeLists.txt
- Allow boost to be statically linked if
DOCKER
env var is defined. - Boost libraries (even non
-dev
versions) took a large amount of space in the final libraries
- Allow boost to be statically linked if
-
apps/compiler/includes/functions.sh
- Revise for params that can include spaces
-
apps/bash_shared_includes.sh
- Allow Deno to be option with
SKIP_INSTALL_DENO
env var
- Allow Deno to be option with
-
Issues Addressed:
- I don't think it counts if I created the issue, but #14644
- There's a wrapper that creates an admin user, similar outcome to #14774 but not exactly the same
- Can replace #14914
SOURCE:
Tests Performed:
- Able to build, login, and play without issues. Modules seem to work without issues.
How to Test the Changes:
- Destroy previous tooling with
docker compose --profile app --profile prod down -v --remove-orphans
- Start AzerothCore via command
docker compose --profile app up --build
- Observe new account being created in docker logs
- Login with creds
admin
admin
and confirm functionality - Run ctrl+c to stop the server. Quickly, check in game message logs showing the shutdown
- Install a module (
git clone https://github.com/azerothcore/mod-transmog --depth 1 modules/mod-transmog
) - Rebuild with the command from
1
, it should be similar in speed to builds with ccache - Confirm module works
Known Issues and TODO List:
- the mysql client needs to be added to the runtime container, and it adds more space is needed imo. It'd be nice if we can ditch it entirely since
libmysqlclient
is needed anyway - wrapping the worldserver isn't ideal. It'd be fantastic if we could
- create a user off of worldserver environment variables natively
- Safely shutdown the server on SIGTERM/SIGINT
- Console access isn't necessarily available to the user. I personally don't think this is an issue, but I can see how it'd get in the way. Might be worth setting up SOAP or RA access by default.
- A health check would be great. Since I'm already using
expect
for wrapping the worldserver, it might not be too much of a stretch for it to access telnet and runserver info
or something
How to Test AzerothCore PRs
When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].
You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:
http://www.azerothcore.org/wiki/How-to-test-a-PR
REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).
For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.
I'm gonna review this soon, please wait for it
Ran into a small issue with config management. will need to work out but otherwise this should be fine for review.
Why has the prod stage been removed from the Dockerfile?
I've sent you a friend request on Discord to speed up the information gathering about this PR.
I've taken the process for dev, local, and prod images and refactored both build stages into a single setup.
Having all of the components of azerothcore, and then the dev, local, and prod images for each just increases confusion.
I've taken the process for dev, local, and prod images and refactored both build stages into a single setup.
Having all of the components of azerothcore, and then the dev, local, and prod images for each just increases confusion.
The idea behind was to make sure that the images remains small and without extra files/libs when not needed. Prod for instance, the image should not include all the libraries to compile
In the new Dockerfile, that's what happening in the build
stage:
https://github.com/azerothcore/azerothcore-wotlk/blob/97382f05548f40f8f1cf16b7c5ae1b76dcbbe912/apps/docker/Dockerfile#L53-L113
The binaries are copied out of this build stage for the final build targets, and the var
directory is cached to allow for ccache to persist properly.
If the operator wants to re-compile, they can run another docker compose --profile up -d --build
(or ./acore.sh docker build && ./acore.sh docker start:d
) regardless of if they're in a dev or prod setup.
Prod for instance, the image should not include all the libraries to compile
The current images don't abide by this. For both a "prod" or "dev" setup in the current docker builds with the local
images, the servicebase
stage installs source libraries as well as the entire /data
directory, making the resulting images roughly 1.6GB. The non-local images copy over the right programs, so add on ~800MB for the worldserver and much less for everything else.
The images created by the dockerfile total to these sizes:
$ docker image ls | grep acore | awk '{print $1, $7}' | column -t
acore/ac-wotlk-worldserver 826MB
acore/ac-wotlk-db-import 197MB
acore/ac-wotlk-authserver 205MB
acore/ac-wotlk-client-data 93.9MB
@Yehonal friendly reminder to review this when you can.
I'm sorry but this PR can't be merged if the ac-dev-server is removed, this is a feature documented here:
https://www.azerothcore.org/wiki/install-with-docker#how-to-use-the-dev-container
My largest reservation on the code for devcontainer is that I don't like the code specifically concerned with development to be so closely integrated with something I expect to run in production. If I want to edit something in the compose file for the server I run, I have to scroll through what is essentially noise to reach the parts I'm looking for.
My understanding is that it's relatively common convention to have the .devcontainer directory contain its' own compose file and Dockerfile. Can we reach a compromise where the dev server's docker setup lives in .devcontainer, and the repo's main compose file and dockerfile are for running and building the application in a non-dev environment?
My largest reservation on the code for devcontainer is that I don't like the code specifically concerned with development to be so closely integrated with something I expect to run in production. If I want to edit something in the compose file for the server I run, I have to scroll through what is essentially noise to reach the parts I'm looking for.
My understanding is that it's relatively common convention to have the .devcontainer directory contain its' own compose file and Dockerfile. Can we reach a compromise where the dev server's docker setup lives in .devcontainer, and the repo's main compose file and dockerfile are for running and building the application in a non-dev environment?
The reasons why I would keep everything in one single file are the following:
-
Easier to maintain - you update the "skeleton" stage once. We should avoid adding too many places where to maintain the OS configurations and keep it centralized. It's ok if we can't use the acore.sh dashboard for caching purposes (although we might find a solution for it as well), but at least let's try not to add even more code duplication
-
Easier to test the compilation in our CI and use the acore-dashboard to build and run the dev container without adding complexity. I personally use the dev container both with vscode and intellij (https://www.jetbrains.com/help/idea/docker.html). If we put the dockerfile into the vscode folder then we remove the possibility to use the same dockerfile/docker-compose container for other IDEs/dev environments.
To solve the "Dockerfile mess" I would just suggest to put everything regarding the dev part at the bottom...e.g.:
- skeleton
- build
- runtime
- authserver # These 3 get binaries from the build step
- worldserver
- db-import
- client-data
- dev
Where dev inherits from the skeleton + it adds what is missing to have a complete dev environment
Same thing for the docker-compose, where the dev container can be specified at the end and it just reuses the production db. I do not see the need for a different database, and if someone needs that, it's always possible to create the docker-compose.override.yml
file
Any news here? Do you have ETA? Need further info/help from me?
Sorry, I've been poking it here and there but haven't gotten a commit it until just now.
Easier to maintain - you update the "skeleton" stage once. We should avoid adding too many places where to maintain the OS configurations and keep it centralized. It's ok if we can't use the acore.sh dashboard for caching purposes (although we might find a solution for it as well), but at least let's try not to add even more code duplication
I'm sorry to say that I disagree. I don't think that having a main Dockerfile for the actual components and a separate Dockerfile given to contributors if they'd like to use devcontainer is too much of an extra burden on maintaining them. If the devcontainer Dockerfile was more complex it'd be a harder choice for me, but as it stands I prefer to have the mental separation of the Dockerfiles over strict de-deduplication.
If we put the dockerfile into the vscode folder then we remove the possibility to use the same dockerfile/docker-compose container for other IDEs/dev environments.
This is incorrect. I can run docker compose -f .devcontainer/docker-compose.yml run ac-dev-server bash
for the same experience. I don't use VSCode or Jetbrains IDE's (typically stick to vim) but I imagine as long as the user can configure a compose file they should be good to go regardless of environment.
Easier to test the compilation in our CI and use the acore-dashboard to build and run the dev container without adding complexity.
I haven't messed with it in a while but if I remember correctly, I was able to cut out a non-insignificant portion of the GHA workflows, but I can definitely revisit those and make sure they're not doing anything crazy.
the dev container can be specified at the end and it just reuses the production db. I do not see the need for a different database
My point of view for this is that it's not that there's a different database, it's that either docker-compose.yml files have the services (just mysql, in this case) to host the game server.
@Yehonal can you please check this again?
Closing as I don't see a high chance of this getting merged.
I may revisit this in the future, though it'd probably be much better as a collection of smaller PR's rather than one big one.