Skip to content

Conversation

sawyerh
Copy link
Contributor

@sawyerh sawyerh commented Apr 26, 2024

Ticket

Resolves #305

Changes

  • Set HOSTNAME for the Docker container. This fixes an issue where the healthcheck fails after upgrading to Next.js 13.4.13+

Context for reviewers

This was copied from the Next.js Docker example. I tried a few other formats, but none of those worked.

Testing

Tested on the Platform test app

CleanShot 2024-04-26 at 10 03 31@2x

@sawyerh sawyerh requested review from rocketnova and a team April 26, 2024 17:05
Copy link

Coverage report for app

St.
Category Percentage Covered / Total
🟢 Statements 93.1% 81/87
🟢 Branches 82.35% 14/17
🟢 Functions 93.33% 14/15
🟢 Lines 93.59% 73/78

Test suite run success

16 tests passing in 5 suites.

Report generated by 🧪jest coverage report action from a3cb946

Copy link
Contributor

@rocketnova rocketnova left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for trying the different formats and linking to the next.js Dockerfile. I'm surprised the other formats didn't work, but there shouldn't be anything inherently wrong with this approach.

@sawyerh sawyerh merged commit b3404b5 into main Apr 26, 2024
@sawyerh sawyerh deleted the sawyerh/305-fix-health branch April 26, 2024 17:23
@lorenyu
Copy link
Contributor

lorenyu commented Apr 29, 2024

@sawyerh I think CMD ["HOSTNAME=0.0.0.0", "node", "server.js"] doesn't work because the exec form of CMD (["executable", "arg1", "arg2"]) invokes the executable directly without first invoking a shell, whereas the shell form (executable arg1 arg2) invokes a shell which then invokes the executable. The HOSTNAME=0.0.0.0 isn't an executable so it can't be run, it's something that shells understand.

I'm surprised why the ENV HOSTNAME 0.0.0.0 doesn't work.

As an aside, it's considered best practice to use the ENV NAME=VAR form (with equal sign) than the ENV NAME VAR form without equal sign. See the following snippet from Docker docs:

This syntax does not allow for multiple environment-variables to be set in a single ENV instruction, and can be confusing. For example, the following sets a single environment variable (ONE) with value "TWO= THREE=world":
[ENV](https://docs.docker.com/reference/dockerfile/#env) ONE TWO= THREE=world
The alternative syntax is supported for backward compatibility, but discouraged for the reasons outlined above, and may be removed in a future release.

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.

Docker health check fails after Next.js 13.4.13
3 participants