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

Use healthcheck instead of wait-for #125

Merged
merged 1 commit into from
May 24, 2024
Merged

Use healthcheck instead of wait-for #125

merged 1 commit into from
May 24, 2024

Conversation

aelkiss
Copy link
Member

@aelkiss aelkiss commented May 23, 2024

Also, remove extra docker-compose.test.yml

@aelkiss aelkiss requested a review from mwarin May 23, 2024 20:50
Also, remove extra docker-compose.test.yml
@aelkiss
Copy link
Member Author

aelkiss commented May 23, 2024

@mwarin I think all the race condition issues we were seeing were in feed-internal, but this still seems to work insofar as tests are passing.

Copy link
Contributor

@mwarin mwarin left a comment

Choose a reason for hiding this comment

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

Works & looks good.
Approve!

@@ -71,16 +111,25 @@ services:
MINIO_ACCESS_KEY: TESTINGACCESSKEY
MINIO_SECRET_KEY: testingsecretkey
command: server /data
healthcheck:
<<: *healthcheck-defaults
Copy link
Contributor

Choose a reason for hiding this comment

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

Could minio and pushgateway health checks use the same tool (curl/get) to call the thing they are calling?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sadly, no, because the health check runs in the containers, and those containers are built off of different base images and include different things prom/pushgateway is built off of busybox and minio/minio appears to be built off of some kind of minimal RHEL thing.

aelkiss@byrd:~$ docker run --rm -ti --entrypoint /bin/bash minio/minio
bash-5.1# curl
curl: try 'curl --help' for more information
bash-5.1# wget
bash: wget: command not found
aelkiss@byrd:~$ docker run --rm -ti --entrypoint /bin/sh prom/pushgateway
/pushgateway $ curl
/bin/sh: curl: not found
/pushgateway $ wget
BusyBox v1.36.1 (2023-12-04 22:44:12 UTC) multi-call binary.

Usage: wget [-cqS] [--spider] [-O FILE] [-o LOGFILE] [--header STR]
...

@aelkiss aelkiss merged commit 68ac0de into main May 24, 2024
1 check passed
@aelkiss aelkiss deleted the remove-wait-for branch May 24, 2024 19:37
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.

2 participants