mariadb-alpine icon indicating copy to clipboard operation
mariadb-alpine copied to clipboard

feat: use mariadb client for healthchecks

Open jbergstroem opened this issue 2 years ago • 2 comments

Instead of grepping for an existing daemon, we now run a query similar to how mysqladmin does it (connects to server/db and runs select 1).

Todo

  • [x] review how others deal with then case of setting up a database with user/pass/root pass but then remove from a startup (conclusion: pass a mysql client config)
  • [ ] tests (painful! takes ~15s before it responds as healthy via docker inspect --format "{{json .State.Health }}" - I could just lean on similar query / logic)
  • [ ] add documentation about best practices in production environments

jbergstroem avatar Mar 08 '23 14:03 jbergstroem

Findings:

  • mariadb (auth ignored / assumed): https://github.com/MariaDB/mariadb-docker/blob/master/11.0/healthcheck.sh#L52-L54
  • mysql: does not provide healthcheck

jbergstroem avatar Mar 08 '23 14:03 jbergstroem

this makes this image deviate from the original mariadb one a bit too much for me

This is what the original does:

$ mariadb -h localhost --protocol tcp -e 'select 1'

If you set up this repo using the PR and remove your environment variables you'd see the same result.

Edit: if you check the original issue about adding this functionality to MariaDB; they are talking about the same thing / what I am trying to do as default: https://github.com/MariaDB/mariadb-docker/issues/94

The cleanest approach to me from a production standpoint is to initialize your container, no longer pass credentials in your compose, then mount a config that contains authentication in a [mariadb-client] directive. Even with that, what I'm suggesting in this PR would work just fine (no variables are set -> just connect without passing credentials, reading from config instead).

I would add it to the documentation / best practices. That would be a great way to unit test this as well.

What do you think?

jbergstroem avatar Mar 09 '23 20:03 jbergstroem