moodle-docker
moodle-docker copied to clipboard
moodlehq
Implement healthcheck for app development container
Following up from #126, we need to bump the version of the compose config to support the healthcheck option. This should be a safe upgrade given that version 2.3 has also been around for a long time.
I'd say we shoudl bump to a much more recent version if we're going to go to the effort of bumping at all.
I'd say we should bump to a much more recent version if we're going to go to the effort of bumping at all.
Isn't the point to bump the version to support the healthcheck directive?
In which case bumping to the minimum version you can to support this directive seems like the right approach to maintain maximum compatibility/least breakage for developers?
Isn't the point to bump the version to support the healthcheck directive?
In which case bumping to the minimum version you can to support this directive seems like the right approach to maintain maximum compatibility/least breakage for developers?
The issue is that all yml files have to have the same version number. If developers have local customisations then they will need to update these files too. I feel it is better to make the change once, rather than when we want to add another new feature which has actually existed for many years.
Equally increasing to a much newer version also allows our users to make use of other new features in Docker, even if we don't use them internally ourselves. We have been preventing them using healthcheck files as we have been providing such an old version of the compose spec.
Version 2.3 was compatible with Docker Engine 17.06.0, released in 2017 (https://docs.docker.com/compose/compose-file/). and with a matching docker-compose release.
Updating to a newer version should be relatively low impact as even the latest release (3.8) is for Docker 19.03, released in March 2019 - nearly two years ago. In addition docker-compose now comes bundled with Docker Desktop on both Mac, and Windows.
I would argue that a 2-year old version of docker should be fine in terms of least breakage, and will actually giver greater compatability in the sense of allowing use of other new features.
Hi Everyone, I was w/ @danpoltawski position 'till the comment of @andrewnicols, pointing out when 3.8 has been supported by Docker i.e. since more than 1 year. Even though 19.03.0 has been actually released on 2019-07-22, a bit later than March, I'm prone to concur with @andrewnicols proposal i.e. 3.8. Developers could easily align their dev resources to a recent docker version, wondering just their CIs if any.
At the end my little +1 to bumping to 3.8.
HTH, Matteo
Note: I'm not necessarily saying 3.8, just a more recent version. 3.8 is the most recent version and has been out for > 18 months though, so it is actually a reasonable candidate.
Just to support this further, I wanted to add a health check to an image a while back when I was doing some testing but was unable to use the feature as we only had version 2 in the core yml, so I didn't bother. The same can apply to other features too.
I don't mind either way, but once we reach a consensus let me know so that I update the PR.
For the records... I was looking in #160 (database health-checks and dependencies) about when it was possible to use the condition element in the depends_on specification of a service... because there was some confusion, with people saying that it was removed in composer v3... others saying that no, it was back... so I started to read and found:
-
the version element in the compose is 100% optional, more yet, it's deprecated right now. And no implementation should use that version to validate schemas or anything.
-
it was agreed about not to remove any feature ever, so, at some point in 3.x series (3.8, available since July 2019, as commented above - with engine 19.03.x), all the features existing in 2.x that were removed in 3.0 were, finally, put back. And the policy is to, always, keep all the old features working. (hence, my desired "depends_on -> condition is back).
Docker Compose 1.27.0+ (September 2020, maybe not old enough) implements the format defined by the Compose Specification. Previous Docker Compose versions have support for several Compose file formats – 2, 2.x, and 3.x. The Compose specification is an unified 2.x and 3.x file format, aggregating properties across these formats.
So, maybe, just maybe... we can start getting rid of that version in all the files and, when something doesn't work for somebody because it's using super old versions... simply it will fail... and ask for upgrade should be the reply. As far as we don't run on the edge/very latest features... it should be ok and easy enough for everybody.
And health-checks are here since long ago (v2), so not edge at all.
Just IMO, ciao :-)
@andrewnicols Ok, I'll update this PR but it may take a while because I'll wait until I finish what I'm doing in #156.
the version element in the compose is 100% optional, more yet, it's deprecated right now. And no implementation should use that version to validate schemas or anything.
@stronk7 I just tried removing the version and I got an error. Looking further into this the link you shared is from compose-spec.io, but docker-compose doesn't seem to follow that spec because looking at their docs it says "Version 1. This is specified by omitting a version key at the root of the YAML.". So removing it would mean using version 1, not the latest.
So I guess we're back to decide which version we want to use? As I said before, I don't really mind which version we use as long as the healthcheck option is available, so once we reach a consensus I will update the PR.