Add a healthcheck. #142
Add a healthcheck. #142
Conversation
Dockerfile
Outdated
| @@ -3,6 +3,9 @@ ARG NGINX_NAME="nginx-${NGINX_VERSION}" | |||
|
|
|||
| FROM debian AS build | |||
|
|
|||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=60s \ | |||
There was a problem hiding this comment.
@holdenk this is in the wrong place... 😸 Because this Dockerfile uses a multi-stage build, this HEALTHCHECK belongs after FROM nginx (not the FROM debian AS build where it is currently proposed).
There was a problem hiding this comment.
Ah gotcha, yeah a halth check in the build container is probably not very useful.
Co-authored-by: Michael Vorburger ⛑️ <[email protected]> Signed-off-by: Holden Karau <[email protected]>
| @@ -85,6 +85,9 @@ RUN echo "Cloning nginx and building $NGINX_VERSION (rev $NGINX_COMMIT from '$NG | |||
|
|
|||
| FROM nginx:${NGINX_VERSION} | |||
|
|
|||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=60s \ | |||
There was a problem hiding this comment.
I'm wondering what a suitable interval could be... probably something LOWER than the Healthcheck that the Orchestrator runs against Nodes? (I don't know what intervals those are set to; see my question yesterday on Slack.) I'm currently using 7s on my livenessProbe, but chose that somewhat arbitrarily. The 30s seems relatively high, to me; but I'm sure more knowledgeable reviewers might have some thoughts about this.
| @@ -85,6 +85,9 @@ RUN echo "Cloning nginx and building $NGINX_VERSION (rev $NGINX_COMMIT from '$NG | |||
|
|
|||
| FROM nginx:${NGINX_VERSION} | |||
|
|
|||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=60s \ | |||
| CMD (curl -f http://localhost/ipfs/QmXjYBY478Cno4jzdCcPy4NcJYFrwHZ51xaCP8vUwN9MGm/) || exit 1 | |||
There was a problem hiding this comment.
Wait, does this actually work? If I run curl -f http://192.168.1.xxx:31080/ipfs/QmXjYBY478Cno4jzdCcPy4NcJYFrwHZ51xaCP8vUwN9MGm/ from CLI, it has exit code 23, see https://everything.curl.dev/usingcurl/returns... which would fail this HEALTCHECK, I think.
There was a problem hiding this comment.
Weird when I run this inside of the container what I see is:
root@DEN:/usr/src/app# curl -f http://localhost/ipfs/QmXjYBY478Cno4jzdCcPy4NcJYFrwHZ51xaCP8vUwN9MGm/
<html>
<head><title>301 Moved Permanently</title></head>
<body>
<center><h1>301 Moved Permanently</h1></center>
<hr><center>nginx</center>
</body>
</html>
root@DEN:/usr/src/app# echo $?
0
There was a problem hiding this comment.
Duh, yeah, please ignore that... sorry. But while looking into this, I've noticed that as-is it would actually pass (exit code 0) whether or not it "really worked" (e.g. even if it's a wrong IPFS CID, or there is a problem to resolve it, it' still always just 301). I've had a closer look at curl options, what do you think about we do the following:
curl --fail --silent --output /dev/null --location --insecure http://localhost/ipfs/QmXjYBY478Cno4jzdCcPy4NcJYFrwHZ51xaCP8vUwN9MGm
The --fail is (your) -f just perhaps more explicit in the long form.
The --silent --output /dev/null are with an eye towards #114.
The --location --insecure follows that 301 redirect - and lets use "test" that it "really worked".
There was a problem hiding this comment.
| CMD (curl -f http://localhost/ipfs/QmXjYBY478Cno4jzdCcPy4NcJYFrwHZ51xaCP8vUwN9MGm/) || exit 1 | |
| CMD (curl --fail --silent --output /dev/null --location --insecure http://localhost/ipfs/QmXjYBY478Cno4jzdCcPy4NcJYFrwHZ51xaCP8vUwN9MGm/) || exit 1 |
@holdenk LGTY?
There was a problem hiding this comment.
@holdenk do you want to click Accept / Apply for this?
See #124
We can expand on the healthcheck to do more than just check the localhost HTTP but figured this was a good start.