Implement healthcheck for app development container #131
Conversation
NoelDeMartin
force-pushed
the
app-healthcheck
branch
from
77ab671 to
df3f51d
Compare
|
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. |
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, At the end my little +1 to bumping to 3.8. HTH, |
|
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. |
Basically does the same than old trvis runs, with some differences: - I've grouped the jobs into 3 categories (phpunit, behat, app). - Each one has own setup/teardown/test shell script, instead of the previous "mammoth" setup and test scripts, that I've left unmodified - Aiming to get the travis CI runs disabled because they are super slow (we only have 2 concurrent jobs allowed) and, while running, all the other moodlehq repos have to wait. - I'll create an issue to remove travis support once this is accepted and working ok. - Only change that I had to perform to existing stuff is raising the harcoded sleep of 5 seconds to 10 seconds because I was getting some random failures with MySQL, needed more time to startup. - We should move to proper healthchecks for database containers soon, also for the app containers (moodlehq#131)
Basically does the same than old Travis runs, with some differences: - I've grouped the jobs into 3 categories (phpunit, behat, app). - Each one has own setup/teardown/test shell script, instead of the previous "mammoth" setup and test scripts, that I've left unmodified - Aiming to get the travis CI runs disabled because they are super slow (we only have 2 concurrent jobs allowed) and, while running, all the other moodlehq repos have to wait. - I'll create an issue to remove travis support once this is accepted and working ok. - Only change that I had to perform to existing stuff is raising the hardcoded sleep of 5 seconds to 10 seconds because I was getting some random failures with MySQL, needing more time to start. - We should move to proper health-checks for database containers soon, also for the app containers (moodlehq#131)
Basically does the same than old Travis runs, with some differences: - I've grouped the jobs into 3 categories (phpunit, behat, app). - Each one has own setup/teardown/test shell script, instead of the previous "mammoth" setup and test scripts, that I've left unmodified - Aiming to get the travis CI runs disabled because they are super slow (we only have 2 concurrent jobs allowed) and, while running, all the other moodlehq repos have to wait. - I'll create an issue to remove travis support once this is accepted and working ok. - Only change that I had to perform to existing stuff is raising the hardcoded sleep of 5 seconds to 10 seconds because I was getting some random failures with MySQL, needing more time to start. - We should move to proper health-checks for database (moodlehq#160) containers soon, much like app containers (moodlehq#131).
Basically does the same than old Travis runs, with some differences: - I've grouped the jobs into 3 categories (phpunit, behat, app). - Each one has own setup/teardown/test shell script, instead of the previous "mammoth" setup and test scripts, that I've left unmodified - Aiming to get the travis CI runs disabled because they are super slow (we only have 2 concurrent jobs allowed) and, while running, all the other moodlehq repos have to wait. - I'll create an issue to remove travis support once this is accepted and working ok. - Only change that I had to perform to existing stuff is raising the hardcoded sleep of 5 seconds to 10 seconds because I was getting some random failures with MySQL, needing more time to start. - We should move to proper health-checks for database (moodlehq#160) containers soon, much like app containers (moodlehq#131).
Basically does the same than old Travis runs, with some differences: - I've grouped the jobs into 3 categories (phpunit, behat, app). - Each one has own setup/teardown/test shell script, instead of the previous "mammoth" setup and test scripts, that I've left unmodified - Aiming to get the travis CI runs disabled because they are super slow (we only have 2 concurrent jobs allowed) and, while running, all the other moodlehq repos have to wait. - I'll create an issue to remove travis support once this is accepted and working ok. - Only change that I had to perform to existing stuff is raising the hardcoded sleep of 5 seconds to 10 seconds because I was getting some random failures with MySQL, needing more time to start. - We should move to proper health-checks for database (moodlehq#160) containers soon, much like app containers (moodlehq#131).
|
For the records... I was looking in #160 (database health-checks and dependencies) about when it was possible to use the
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 :-) |
|
@NoelDeMartin , do you want to Make It So! |
|
@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. |
@stronk7 I just tried removing the 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 |
Following up from #126, we need to bump the version of the compose config to support the
healthcheckoption. This should be a safe upgrade given that version 2.3 has also been around for a long time.