Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: add support for docker healthcheck to grafana/mimir image #9035

Closed
wants to merge 1 commit into from
Closed

build: add support for docker healthcheck to grafana/mimir image #9035

wants to merge 1 commit into from

Conversation

DeadNews
Copy link

What this PR does

Add support for docker healthcheck to grafana/mimir image.

Like loki does:

FROM gcr.io/distroless/base-nossl:debug

COPY --from=build /src/loki/cmd/loki/loki /usr/bin/loki
COPY cmd/loki/loki-docker-config.yaml /etc/loki/local-config.yaml

SHELL [ "/busybox/sh", "-c" ]

RUN addgroup -g 10001 -S loki && \
    adduser -u 10001 -S loki -G loki
RUN mkdir -p /loki/rules && \
    mkdir -p /loki/rules-temp && \
    chown -R loki:loki /etc/loki /loki && \
    ln -s /busybox/sh /bin/sh

debug tag is used to have busybox available in the image.

Which issue(s) this PR fixes or relates to

Fixes #9034

Checklist

  • [n/a] Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [n/a] about-versioning.md updated with experimental features.

@DeadNews DeadNews requested review from tacole02 and a team as code owners August 18, 2024 21:21
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@charleskorn charleskorn left a comment

Choose a reason for hiding this comment

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

Could you please provide more information about why this is necessary? (ie. why do you want to use Docker's healthcheck feature?)

We intentionally chose to use distroless images to minimise the number of dependencies in the image, particularly tools like shells and HTTP clients that tend to have vulnerabilities or could be used to pivot from another vulnerability.

@DeadNews
Copy link
Author

Could you please provide more information about why this is necessary? (ie. why do you want to use Docker's healthcheck feature?)

Orchestration tools / load balancers can use the healthcheck feature to determine when a container is ready to accept traffic. running container != healthy container.

We intentionally chose to use distroless images to minimise the number of dependencies in the image, particularly tools like shells and HTTP clients that tend to have vulnerabilities or could be used to pivot from another vulnerability.

It is the same distroless image, but version with busybox available in the image (+1 Mb on-disk size).

It is possible to use the internal commands to check the health of the service, as described in the alternatives.
Or make wget available as loki, grafana, others and this PR do.

@charleskorn
Copy link
Contributor

Which orchestration tool or load balancer are you using @DeadNews? Most support a simple HTTP healthcheck, and this is what I'd encourage you to use.

Mimir already has a /ready endpoint for this exact purpose.

@DeadNews
Copy link
Author

Which orchestration tool or load balancer are you using @DeadNews? Most support a simple HTTP healthcheck, and this is what I'd encourage you to use.

Mimir already has a /ready endpoint for this exact purpose.

You can read from here and below. It's the exact same questions. I referenced it in the issue.

To answer the question: docker + traefik for example. docker itself does not have a mechanism for health probes over http. Tool for healthcheck should be available in the image.
And without healthcheck, healthy/unhealthy statuses are not available for monitoring.

@@ -69,6 +69,7 @@
* [ENHANCEMENT] Make `-query-frontend.additional-query-queue-dimensions-enabled` and `-query-scheduler.additional-query-queue-dimensions-enabled` non-operational flags in preparation for removal. #8984
* [ENHANCEMENT] Add a new ingester endpoint to prepare instances to downscale. #8956
* [ENHANCEMENT] Query-scheduler: Add `query-scheduler.prioritize-query-components` which, when enabled, will primarily prioritize dequeuing fairly across queue components, and secondarily prioritize dequeuing fairly across tenants. When disabled, tenant fairness is primarily prioritized. `query-scheduler.use-multi-algorithm-query-queue` must be enabled in order to use this flag. #9016
* [ENHANCEMENT] Build: `grafana/mimir` docker image is now support docker `healthcheck`. #9034
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* [ENHANCEMENT] Build: `grafana/mimir` docker image is now support docker `healthcheck`. #9034
* [ENHANCEMENT] Build: `grafana/mimir` docker image supports docker `healthcheck`. #9034

@56quarters
Copy link
Contributor

Sorry @DeadNews, we're not interested in adding any dependencies to the distroless image. If this is required for your use case, it should easy enough to build a custom Mimir image.

@56quarters 56quarters closed this Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add support for docker healthcheck to grafana/mimir image
5 participants