-
Notifications
You must be signed in to change notification settings - Fork 66
added healthcheck in Dockerfile #1192
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,9 @@ FROM amazonlinux:2023 AS base | |
ENV NODE_VERSION=24.4.0 | ||
|
||
RUN yum update -y && \ | ||
yum install -y tar xz openssl && \ | ||
# Amazon Linux 2023 base image is minimal and does not include curl by default, | ||
# so we explicitly install curl here to ensure availability. | ||
yum install -y tar xz openssl curl && \ | ||
ARCH=$(uname -m) && \ | ||
if [ "$ARCH" = "x86_64" ]; then NODE_ARCH="x64"; \ | ||
elif [ "$ARCH" = "aarch64" ]; then NODE_ARCH="arm64"; \ | ||
|
@@ -38,6 +40,7 @@ ENV GRAPH_EXP_ENV_ROOT_FOLDER=${GRAPH_EXP_ENV_ROOT_FOLDER:-/explorer} | |
|
||
ENV PROXY_SERVER_HTTP_PORT=${NEPTUNE_NOTEBOOK:+9250} | ||
ENV PROXY_SERVER_HTTP_PORT=${PROXY_SERVER_HTTP_PORT:-80} | ||
ENV PROXY_SERVER_HTTPS_PORT=443 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to define the HTTPS default port here. It will be defaulted to 443 if not set explicitly |
||
|
||
ENV LOG_STYLE=${NEPTUNE_NOTEBOOK:+cloudwatch} | ||
ENV LOG_STYLE=${LOG_STYLE:-default} | ||
|
@@ -54,7 +57,12 @@ RUN pnpm install && \ | |
chmod a+x ./process-environment.sh && \ | ||
chmod a+x ./docker-entrypoint.sh | ||
|
||
# Expose ports for HTTP, HTTPS, and Neptune Notebook proxy | ||
EXPOSE 443 | ||
EXPOSE 80 | ||
EXPOSE 9250 | ||
|
||
HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ | ||
CMD curl -fsSL "http://localhost:${PROXY_SERVER_HTTP_PORT}/status" || exit 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still does not take in to account the different deployment scenarios I pointed out last time. Perhaps I did not give enough information about the issue. Administrators of Graph Explorer have flexible deployment options. They can:
This means the curl command you have above will only work in the HTTP scenario. All the other scenarios will break and report unhealthy to Docker. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the healthcheck, shall we mention the URL in the Dockerfile and also add it to .env so it can be configured? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The user can already configure it, but it is a collection of options mapped together. I tried seeing what the LLMs would recommend, and they offered a decent approach. Please don't take this as the solution. You'll need to test this out in all the different scenarios to prove it works properly. I'm already a bit suspect of the check against # Dynamic healthcheck URL construction
HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \
CMD protocol=$([ "$PROXY_SERVER_HTTPS_CONNECTION" = "true" ] && echo "https" || echo "http"); \
port=$([ "$PROXY_SERVER_HTTPS_CONNECTION" = "true" ] && echo "$PROXY_SERVER_HTTPS_PORT" || echo "$PROXY_SERVER_HTTP_PORT"); \
base_path=$([ "$NEPTUNE_NOTEBOOK" = "true" ] && echo "/proxy/9250" || echo ""); \
curl -f ${protocol}://localhost:${port}${base_path}/status || exit 1 |
||
|
||
ENTRYPOINT ["./docker-entrypoint.sh"] |
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.
I did a quick test to see if curl was available in the base image. It seems like it is.
Tested it with:
docker build -t curl-test . docker run curl-test
Which produces the output:
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.
don't need to install curl manually anymore