Skip to content
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

Add Docker support #438

Merged
merged 33 commits into from
Apr 3, 2021
Merged

Add Docker support #438

merged 33 commits into from
Apr 3, 2021

Conversation

aperture147
Copy link

@aperture147 aperture147 commented Jan 30, 2021

Things has been made so far:

  • A Dockerfile based on the latest LTS ubuntu:focal
  • A Dockerfile based on older LTS ubuntu:bionic
  • A simple docker-compose.yml file
  • Modified README.md, docker-compose deployment example added

@aperture147
Copy link
Author

aperture147 commented Feb 2, 2021

@RElesgoe Just updated Readme.md and docker-compose.yml, d2cs and d2dbs added. Please check this out. Btw have the mysql8 support added?

… Hub Pull Request Limits

Add some git options to decrease the clone time
fix git clone directory to pvpgn-server
@aperture147
Copy link
Author

aperture147 commented Feb 3, 2021

Some changes were made:

@RElesgoe
Copy link
Member

RElesgoe commented Feb 8, 2021

When running one of the example commands,

docker build . --build-arg with_d2cs=true --build-arg with_bnetd=false --build-arg with_mysql=false --build-arg with_sqlite3=true --build-arg git_branch=develop -t pvpgn-server:d2cs-sqlite

The build errors with:

Cloning into 'pvpgn-server'...
error: pathspec 'develop' did not match any file(s) known to git
The command '/bin/sh -c apt-get update &&     apt-get install -y --no-install-recommends         git         netbase         ca-certificates         gcc g++         libc6-dev libc6         libc++-dev libc++1          zlib1g-dev zlib1g         libcurl4-openssl-dev libcurl4         cmake make         $(if ${with_mysql}; then echo "libmysqlclient-dev libmysqlclient21"; fi)         $(if ${with_sqlite3}; then echo "libsqlite3-dev libsqlite3-0"; fi)         $(if ${with_pgsql}; then echo "libpq-dev libpq5"; fi)         $(if ${with_odbc}; then echo "unixodbc-dev libodbc1"; fi)         $(if ${with_lua}; then echo "liblua5.1-0-dev liblua5.1-0"; fi) &&     git clone --depth=1 ${git_repo} pvpgn-server &&     cd pvpgn-server &&     git checkout ${git_branch} &&     cmake -G "Unix Makefiles" -S./ -B./build           -DWITH_BNETD=${with_bnetd}           -DWITH_D2CS=${with_d2cs}           -DWITH_D2DBS=${with_d2dbs}           -DWITH_LUA=${with_lua}           -DWITH_MYSQL=${with_mysql}           -DWITH_SQLITE3=${with_sqlite3}           -DWITH_PGSQL=${with_pgsql}           -DWITH_ODBC=${with_odbc} &&     cd build && make -j$(nproc) && make install &&     apt-get autoremove --purge -y         git         gcc g++         libc6-dev         libc++-dev         zlib1g-dev         libcurl4-openssl-dev         cmake make         $(if ${with_mysql}; then echo "libmysqlclient-dev "; fi)         $(if ${with_sqlite3}; then echo "libsqlite3-dev "; fi)         $(if ${with_pgsql}; then echo "libpq-dev "; fi)         $(if ${with_pgsql}; then echo "unixodbc-dev"; fi)         $(if ${with_lua}; then echo "liblua5.1-0-dev"; fi) &&     rm -rf /var/lib/apt/lists/* /build/pvpgn-server &&     apt-get clean' returned a non-zero code: 1

@aperture147
Copy link
Author

When running one of the example commands,

docker build . --build-arg with_d2cs=true --build-arg with_bnetd=false --build-arg with_mysql=false --build-arg with_sqlite3=true --build-arg git_branch=develop -t pvpgn-server:d2cs-sqlite

The build errors with:

Cloning into 'pvpgn-server'...
error: pathspec 'develop' did not match any file(s) known to git
The command '/bin/sh -c apt-get update &&     apt-get install -y --no-install-recommends         git         netbase         ca-certificates         gcc g++         libc6-dev libc6         libc++-dev libc++1          zlib1g-dev zlib1g         libcurl4-openssl-dev libcurl4         cmake make         $(if ${with_mysql}; then echo "libmysqlclient-dev libmysqlclient21"; fi)         $(if ${with_sqlite3}; then echo "libsqlite3-dev libsqlite3-0"; fi)         $(if ${with_pgsql}; then echo "libpq-dev libpq5"; fi)         $(if ${with_odbc}; then echo "unixodbc-dev libodbc1"; fi)         $(if ${with_lua}; then echo "liblua5.1-0-dev liblua5.1-0"; fi) &&     git clone --depth=1 ${git_repo} pvpgn-server &&     cd pvpgn-server &&     git checkout ${git_branch} &&     cmake -G "Unix Makefiles" -S./ -B./build           -DWITH_BNETD=${with_bnetd}           -DWITH_D2CS=${with_d2cs}           -DWITH_D2DBS=${with_d2dbs}           -DWITH_LUA=${with_lua}           -DWITH_MYSQL=${with_mysql}           -DWITH_SQLITE3=${with_sqlite3}           -DWITH_PGSQL=${with_pgsql}           -DWITH_ODBC=${with_odbc} &&     cd build && make -j$(nproc) && make install &&     apt-get autoremove --purge -y         git         gcc g++         libc6-dev         libc++-dev         zlib1g-dev         libcurl4-openssl-dev         cmake make         $(if ${with_mysql}; then echo "libmysqlclient-dev "; fi)         $(if ${with_sqlite3}; then echo "libsqlite3-dev "; fi)         $(if ${with_pgsql}; then echo "libpq-dev "; fi)         $(if ${with_pgsql}; then echo "unixodbc-dev"; fi)         $(if ${with_lua}; then echo "liblua5.1-0-dev"; fi) &&     rm -rf /var/lib/apt/lists/* /build/pvpgn-server &&     apt-get clean' returned a non-zero code: 1

Oh sorry, I forgot to commit some changes. Btw I'm on a Lunar New Year vacation and don't bring any computer with. I will fix this problem as soon as possible

will not work with depth=1
@aperture147
Copy link
Author

@RElesgoe the problem is now fixed.

ARG with_sqlite3=false
ARG with_pgsql=false
ARG with_odbc=false
ARG with_lua=false

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should lua support enable as default ?

Copy link
Author

@aperture147 aperture147 Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my build, enable LUA will cause pvpgn server to crash everytime an user connect Not anymore if you use develop branch. The crash is not related to Lua since it happens in any build configuration if you build on branch master
Btw LUA is a experimental feature and an average server admin will not gain any benefit from that so I don't think we should enable this by default.

Copy link
Author

@aperture147 aperture147 Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should lua support enable as default ?

Sorry, for the false claim, server crashing is not related to Lua since it happens on any build configuration if you build this image on branch master. I've just created a issue for this

@RElesgoe
Copy link
Member

Switch base image from default ubuntu to quay.io/aperture147/ubuntu to mitigate the impact of Docker Hub Pull Request Limits which can leads to failing CI build. This is an automated build from a synced-fork aperture147/docker-brew-ubuntu-core.

I prefer using the official Ubuntu image from the default container repository. If a user is experiencing rate limiting, they should modify the Dockerfile on their machine to avoid rate limiting.

After running

docker run -v /tmp/conf:/tmp/conf --rm --entrypoint cp pvpgn-server:bnetd-mysql /usr/local/etc/pvpgn/* /tmp/conf

I receive the following error:

cp: -r not specified; omitting directory '/usr/local/etc/pvpgn/i18n'

I receive similar errors when running

docker run -v /tmp/assets:/tmp/assets --rm --entrypoint cp pvpgn-server:bnetd-mysql /usr/local/var/pvpgn/* /tmp/assets

I also noticed that one of the commands specifies the mysql:8.0.23 image. Can we use mysql:8.0 or mysql:8 instead?

After running

docker run -d --name pvpgn-d2cs --restart unless-stopped -p 6113:6113 -p 6113:6113/udp -v /tmp/conf:/usr/local/etc/pvpgn -v /tmp/assets:/usr/local/var/pvpgn pvpgn-server:d2cs-mysql

I receive the following error:

docker: Error response from daemon: OCI runtime create failed: container_linux.go:370: starting container process caused: exec: "bnetd": executable file not found in $PATH: unknown.

When checking the logs using

docker logs -f --tail 1000 pvpgn-bnetd

All I get is:

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
You are currently running PvPGN 1.99.7.2.1-PRO
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
If you need support:
 * Create an issue at https://github.com/pvpgn/pvpgn-server

Server is now running.

Because bnetd is running in the foreground.

@aperture147
Copy link
Author

aperture147 commented Feb 25, 2021

About changing the base image to official ubuntu:

Changed. According to this document, the rate limit is 100 pull per 6 hours so this might not affect this project's CI/CD

About getting error while copying base config files from the image:

Fixed. I forgot to add -r

About changing mysql version to 8.0 or 8:

In my opinion, locking mysql (or any image) to a specific version is a best practice, this might help us to figure the "best-known version" and make the debugging process easier. Changing mysql to latest or 8 will pull the latest version (currently 8.0 but could be 8.1 or 8.2 in the future) with (maybe) some breaking changes. Although mysql is pretty stable and libmysqlclient supports old version pretty well but to keep the best practice, I will only change the mysql version tag to 8.0.

About running specific build of d2cs and d2dbs cause error due to bnetd is missing:

This is my fault to hard-coded the CMD in the Dockerfile to bnetd -f. It's hard to figure out which server type the operator want to build, they can build an image which supports any combination of bnetd, d2cs and d2dbs so writing a script to predict the CMD command is not possible. My solution is adding another SERVER_TYPE env var to the image (which is set default to bnetd) and modify the CMD to $SERVER_TYPE -f. Anytime an operator wants to spin up a server, they have to choose an appropriate server type between between bnetd, d2cs and d2dbs. I've already push this change, do you have any better solution?

@RElesgoe
Copy link
Member

Do you guys use anything like discord, whatsapp or even IRC so we can contact more convenient.

#botdev at Discord - BNETDocs

@RElesgoe RElesgoe added this to the 1.99.8.0.0 milestone Mar 3, 2021
@cen1 cen1 self-requested a review March 7, 2021 20:41
@cen1 cen1 self-assigned this Mar 7, 2021
Copy link
Collaborator

@cen1 cen1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I run with docker run pvpgn-server:bnetd-mysql I cannot kill the container with ctrl+c. Something catches the signal?

README_DOCKER.md Outdated Show resolved Hide resolved
README_DOCKER.md Show resolved Hide resolved
README_DOCKER.md Outdated Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
@cen1
Copy link
Collaborator

cen1 commented Mar 8, 2021

Dockerfile is good, small fix to compose and mostly readme fixes which I documented above. After next iteration I will also retest on Mac and Windows since it should work just fine in theory.

Best regards.

@aperture147
Copy link
Author

If I run with docker run pvpgn-server:bnetd-mysql I cannot kill the container with ctrl+c. Something catches the signal?

I'm running it as bnetd -D, so it's the one which catching the stop signal should be the bnetd itself. The default signal which docker sends is SIGTERM, after a few seconds or you hit the ctrl + c again then it will send SIGKILL. I don't know what is the meaning of "killing" the container, is it gracefully stop the container or make it stop immediately?

@aperture147
Copy link
Author

Dockerfile is good, small fix to compose and mostly readme fixes which I documented above. After next iteration I will also retest on Mac and Windows since it should work just fine in theory.

Best regards.

Thanks! I've resolved all of the issues mentioned. This build is tested on Linux and I'm running it on my server which has approx 200 - 300 unique users daily (most of them play DotA with my hosted ghost++).

If this build works on linux then 99% it will run on windows and macos too since docker on those system is running on a VM. If there are errors, most of the time it would be networking misconfiguration.

@cen1
Copy link
Collaborator

cen1 commented Mar 11, 2021

Left a couple more comments. Just need to test Windows and we are almost there.

@RElesgoe RElesgoe self-requested a review March 17, 2021 20:11
@aperture147 aperture147 reopened this Mar 18, 2021
@aperture147
Copy link
Author

Finished!

@cen1 cen1 reopened this Mar 20, 2021
@cen1
Copy link
Collaborator

cen1 commented Mar 20, 2021

@aperture147 please check the 4 pending comments above. The mysql config string is the important one, the rest I can close myself.

@aperture147
Copy link
Author

@aperture147 please check the 4 pending comments above. The mysql config string is the important one, the rest I can close myself.

But I don't see any new comment :( Resolved all 5 issues above. Am I missing anything?
image

Dockerfile Outdated Show resolved Hide resolved
README_DOCKER.md Show resolved Hide resolved
bionic.dockerfile Outdated Show resolved Hide resolved
README_DOCKER.md Show resolved Hide resolved
@cen1
Copy link
Collaborator

cen1 commented Mar 23, 2021

@aperture147 please check the 4 pending comments above. The mysql config string is the important one, the rest I can close myself.

But I don't see any new comment :( Resolved all 5 issues above. Am I missing anything?
image

My bad, I forgot to actually submit the review. :insert facepalm:

@aperture147
Copy link
Author

just resolved your issue above. Please check!

@cen1
Copy link
Collaborator

cen1 commented Apr 3, 2021

I had some issues with modern firewalld blocking internal docker container comms but I will patch the readme mysql with that info and fix the language a little bit. All seems to work fine, I made it to the login.

@cen1 cen1 merged commit 3242818 into pvpgn:develop Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants