-
Notifications
You must be signed in to change notification settings - Fork 241
Conversation
Can one of the admins verify this patch? |
4 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Thanks for your interest and the PR! The trouble with this approach is that curl requests will fail as x-pack (inc. security) is included in the Elasticsearch image and thus those API calls require authentication. So for that to work we'd need to tweak the default x-pack configuration and allow anonymous access specifically to a chosen health check endpoint (e.g. Additionally, for every new feature added, we need acceptance tests, so we'll need to add python tests to ensure this is working as expected. Incidentally, I just tried |
You're right about the authorization issue with xpack. I did not face it because I was just overriding the configuration with one that doesn't use xpack. What do you suggest as next steps for this PR? |
host="$(hostname -i || echo '127.0.0.1')" | ||
|
||
# TODO with xpack, the below curl will fail with "unauthorized" error if xpack is enabled | ||
if health="$(curl -fsSL "http://$host:9200/_cat/health?h=status")"; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even if you get a 401/403 response code, this means container is healthy
you could call curl -I -X "http://$host:9200"
and parse response code
Closing this based on the argumentation in #60 |
This PR adds support for healthcheck in the
docker-compose
file.ATM, your repo's tester has a built-in
wait for health check
intests/es_acceptance.py
in functionwait_for_cluster_health
. Using docker's healthcheck, other services can benefit from this without having to re-code await for health check
function in each. On the down-side, this requires upgrading theyml
version from2.0
to2.1
, and hence upgradingdocker-compose
anddocker
to versions supporting itdocker info
output: (1.13.1) http://paste.ubuntu.com/24039459/docker-compose
version:docker-compose version 1.11.1, build 7c5d5e4
Maybe creating a separate branch for this would be convenient?
Note that I ran
make
for testing and it failed with aconnection failed
error, but then again I ranmake
on master, and it also failed.