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

Examples: Add healthckecks in docker example #7731

Merged
merged 3 commits into from
Nov 23, 2022

Conversation

bitkill
Copy link
Contributor

@bitkill bitkill commented Nov 21, 2022

What this PR does / why we need it:
Adds healthchecks for the loki docker compose example, this makes the getting started example more complete,
and can be now used with the --wait command.

Special notes for your reviewer:
Use the following commands to test:

cd examples/getting-started
docker compose up -d --wait
docker compose ps

Checklist

  • Reviewed the CONTRIBUTING.md guide
  • CHANGELOG.md updated

@CLAassistant
Copy link

CLAassistant commented Nov 21, 2022

CLA assistant check
All committers have signed the CLA.

@bitkill bitkill marked this pull request as ready for review November 21, 2022 01:19
@bitkill bitkill requested a review from a team as a code owner November 21, 2022 01:19
@bitkill bitkill changed the title Add healthckecks in docker example Examples: Add healthckecks in docker example Nov 21, 2022
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-target-branch/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@kavirajk
Copy link
Contributor

@bitkill thanks for the PR. Two clarifications.

  1. After reading the PR description, still I'm failing to understand what exactly is the problem we are trying to fix with these changes? Can you clarify?
  2. How do you come up with retry and timeout values?

@bitkill
Copy link
Contributor Author

bitkill commented Nov 21, 2022

1. After reading the PR description, still I'm failing to understand what exactly is the problem we are trying to fix with these changes? Can you clarify?

I wanted to be able to have a better base for testing loki locally, and waiting for the systems to be healthy is a good way to automate some tests that I am running.
In the middle of a bash script, I need to be able to launch the docker-compose and wait for it to be all working, to then start to generate logs and send them to loki (without failure).
This also helps me to write ECS containerDefinitions and load balancer healthchecks faster.

2. How do you come up with `retry` and `timeout` values?

These were a bit arbitrary, just tried to make the initial wait time (for all services to be healthy) fast, so it wouldn't take a long time just because we are waiting 30s between checks.

@bitkill
Copy link
Contributor Author

bitkill commented Nov 22, 2022

@kavirajk forgot to tag you in the response yesterday. response above ☝️

Copy link
Contributor

@kavirajk kavirajk left a comment

Choose a reason for hiding this comment

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

LGTM. thanks for the clarification!

@kavirajk kavirajk merged commit 1ae1318 into grafana:main Nov 23, 2022
Abuelodelanada pushed a commit to canonical/loki that referenced this pull request Dec 1, 2022
**What this PR does / why we need it**:
Adds healthchecks for the loki `docker compose` example, this makes the
getting started example more complete,
and can be now used with the `--wait` command.

**Special notes for your reviewer**:
Use the following commands to test:
```sh
cd examples/getting-started
docker compose up -d --wait
docker compose ps
```

**Checklist**
- [x] Reviewed the `CONTRIBUTING.md` guide
- [x] `CHANGELOG.md` updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants