Skip to content

Conversation

@lorenjohnson
Copy link
Contributor

@lorenjohnson lorenjohnson commented Aug 20, 2024

Takes all in-line healthchecks specifics and puts them in a healthcheck.sh for each image. Not applying by default in image (i.e. as a HEALTHCHECK in the Dockerfile) to keep things flexible, but gives the responsibility for how to healthcheck back to the image--which is what Docker recommends. Also removing this specific from Deploy removes one more possible change vector in the Deploy configuration, keeping things easier to maintain and upgrade for users over time. Also has the benefit of making a standard healthcheck option available for the images for use cases outside of Deploy.

In a related move this adds a new IS_JOBRUNNER option the Wikibase image instead of changing the command/entrypoint script. This allows the same health check script to be pointed which now will also healthcheck jobrunner when used in that mode.

This PR is setup with atomic conventional commits and would be best to do a regular merge vs squash.

@lorenjohnson lorenjohnson requested a review from a team August 20, 2024 12:05
@lorenjohnson lorenjohnson force-pushed the healthchecks-to-dockerfiles branch 3 times, most recently from ccd31f5 to 2e419f1 Compare August 20, 2024 17:08
@lorenjohnson lorenjohnson force-pushed the main branch 4 times, most recently from e17ecc8 to 58a3ece Compare October 7, 2024 11:29
Copy link
Contributor

@rti rti left a comment

Choose a reason for hiding this comment

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

Sorry for the late review. Nice effort. I like the cleanup and agree with the motivation.

Due to the long wait, merge conflicts appeared. Do you mind fixing them?

@lorenjohnson lorenjohnson force-pushed the healthchecks-to-dockerfiles branch from 8ed8556 to 2e419f1 Compare November 7, 2024 13:13
@lorenjohnson lorenjohnson force-pushed the healthchecks-to-dockerfiles branch from 2e419f1 to 9004bdb Compare November 7, 2024 14:19
@lorenjohnson
Copy link
Contributor Author

Due to the long way, merge conflicts appeared. Do you mind fixing them?

Got it, re-applied on the current main.

@lorenjohnson lorenjohnson requested a review from rti November 7, 2024 14:21
@rti
Copy link
Contributor

rti commented Nov 13, 2024

This PR is setup with atomic conventional commits and would be best to do a regular merge vs squash.

Maybe we can now squash it again?

@lorenjohnson lorenjohnson marked this pull request as draft March 25, 2025 13:12
@lorenjohnson lorenjohnson marked this pull request as ready for review July 15, 2025 04:59
@lorenjohnson lorenjohnson requested a review from a team July 16, 2025 14:07
@lorenjohnson lorenjohnson requested a review from rti July 21, 2025 17:01
rti added a commit that referenced this pull request Jul 23, 2025
rti added a commit that referenced this pull request Jul 24, 2025
* feat: setup wikibase manifest external service mappings

* fix: tests

* feat: SELinux compatibility, configure Traefik via file (#862)

* Volume Flags

* wip: static traefik config

* wip

* wip: static config

* style: lint

* fix: traefik config override in tests

* chore: cleanup, remove ping for now

* Privilege

* fix: selinux label for runners workspace volume

---------

Co-authored-by: ricki-jay-wmde <[email protected]>

* chore: remove manifest config from deploy

* feat: add IS_JOBRUNNER like in #755

* chore: move manifest external service setting to wikibase image

* test: new manifest values

* style: lint

* chore: remove IS_JOBRUNNER again, not yet required

---------

Co-authored-by: ricki-jay-wmde <[email protected]>
@lorenjohnson lorenjohnson changed the title Add optional healthcheck scripts to WBS images proposal: Add optional healthcheck scripts to WBS images Aug 14, 2025
@rti
Copy link
Contributor

rti commented Sep 24, 2025

I like the idea to integrate the health checks in the images. I think this is the right place to put them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants