azerothcore-wotlk icon indicating copy to clipboard operation
azerothcore-wotlk copied to clipboard

feat(Docker): Clean up build and runtime

Open michaeldelago opened this issue 2 years ago • 10 comments

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
      • 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.
    • 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
    • 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

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:

  1. Destroy previous tooling with docker compose --profile app --profile prod down -v --remove-orphans
  2. Start AzerothCore via command docker compose --profile app up --build
  3. Observe new account being created in docker logs
  4. Login with creds admin admin and confirm functionality
  5. Run ctrl+c to stop the server. Quickly, check in game message logs showing the shutdown
  6. Install a module (git clone https://github.com/azerothcore/mod-transmog --depth 1 modules/mod-transmog)
  7. Rebuild with the command from 1, it should be similar in speed to builds with ccache
  8. 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 run server 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.

michaeldelago avatar Feb 13 '23 14:02 michaeldelago

I'm gonna review this soon, please wait for it

Yehonal avatar Feb 18 '23 08:02 Yehonal

Ran into a small issue with config management. will need to work out but otherwise this should be fine for review.

michaeldelago avatar Feb 26 '23 16:02 michaeldelago

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.

Yehonal avatar Mar 04 '23 21:03 Yehonal

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.

michaeldelago avatar Mar 05 '23 14:03 michaeldelago

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

Yehonal avatar Mar 05 '23 19:03 Yehonal

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

michaeldelago avatar Mar 05 '23 20:03 michaeldelago

@Yehonal friendly reminder to review this when you can.

55Honey avatar Apr 26 '23 13:04 55Honey

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

Yehonal avatar May 15 '23 08:05 Yehonal

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?

michaeldelago avatar May 16 '23 02:05 michaeldelago

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:

  1. 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

  2. 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

Yehonal avatar May 17 '23 06:05 Yehonal

Any news here? Do you have ETA? Need further info/help from me?

Yehonal avatar May 20 '23 19:05 Yehonal

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.

michaeldelago avatar May 21 '23 04:05 michaeldelago

@Yehonal can you please check this again?

FrancescoBorzi avatar Jun 29 '23 14:06 FrancescoBorzi

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.

michaeldelago avatar Jul 22 '23 22:07 michaeldelago