Feature: Add condition to depends_on and healthcheck for mysql

[mateuszmrw]

By default the depends_on condition only guarantees that the "mysql" container will be built before the "webserver" container. This PR adds an health check and condition to the "mysql" container that makes sure that the "webserver" will be started only after the mysql database is ready to allow incoming connections.

[simjanos-dev]

Thank you!

I think we have code in the entrypoint.sh to make sure that the database server is running. If I remember correctly, this was the thing we tried before, but for some reason it did not work.

@sergiolaverde0 Can you please tell me if I should merge this in? And thank you too for helping out a TON with docker!

[sergiolaverde0]

Yes this is what we originally tried back in #2 but when starting from a fresh install MySQL would initialize a default database and refuse connecting to the webserver if it took too long, making the first run fail occasionally and forcing a restart to fix it.

So following the recommendation in the MySQL page at DockerHub, in #71 we made the webserver retry if the connection was refused and we never had issues with it again.

Merging this doesn't break anything, but doesn't fix anything either and might make startup marginally slower in faster devices.

[mateuszmrw]

So looking at the old PR you used:

       test: ["CMD", "mysqladmin", "ping", "-h", "localhost"]

This causes an issue in which the health check returns true even if in the case when the server is not ready: via documentation: [mysqladmin](https://dev.mysql.com/doc/refman/8.0/en/mysqladmin.html

Check whether the server is available. The return status from [mysqladmin](https://dev.mysql.com/doc/refman/8.0/en/mysqladmin.html) is 0 if the server is running, 1 if it is not. This is 0 even in case of an error such as Access denied, because this means that the server is running but refused the connection, which is different from the server not running.

Ping can return true when the container is not ready due to command above returning the Access denied:

sh-5.1# mysqladmin ping -h localhost
mysqladmin: connect to server at 'localhost' failed
error: 'Access denied for user 'root'@'localhost' (using password: NO)'

The command in the PR passes the root user and password to health check resulting in the correct response:

sh-5.1# mysqladmin ping -u root -h localhost -p
Enter password: 
mysqld is alive

Merging this PR would remove the need of the entrypoint.sh running the retry logic as the webserver container would not be able to start before the mysql one.

However, if it deemed not necessary I don't think there is a point in merging it.

[mateuszmrw]

I think if the issue with the fresh install could be solved by the health check command being changed to "SELECT 1" or something similar.

My main issue is that currently when you run docker compose almost every time you will see issues in the logs regarding connection to the database, when the application starts as the retry logic inside entrypoint.sh usually fails to connect to db at least once.

This would remove the error logs and make the start of the application smoother.

[sergiolaverde0]

You are entirely right about the logs getting cleaner, I didn't consider that. Let me get home and test it just to be sure, but this could be a nice feature.

[sergiolaverde0]

Tested and works like a charm, not a single error in the logs :)

[simjanos-dev]

Merging this PR would remove the need of the entrypoint.sh running the retry logic as the webserver container would not be able to start before the mysql one.

I think we should do that in the next update just to be sure.

Tested and works like a charm, not a single error in the logs :)

Does this mean that there's no issue with startup speed or anything else, and I'm free to merge this in?

Thank you so much!

[sergiolaverde0]

Does this mean that there's no issue with startup speed or anything else?

The Laravel container will wait a little bit more to start but will actually connect to the database on the first try, so total startup time remains the same, with the advantage of no errors in the log (for me it didn't even create a log file).

[simjanos-dev]

I merge this in. Thank you for the PR!