Repository navigation
Add Docker healthcheck for Flux - #24
Conversation
10k11
left a comment
There was a problem hiding this comment.
Thanks a lot for the PR, and welcome! 🎉 This is a clean, focused change and the README note is a nice touch.
There is one problem I'd like to fix before merging: this healthcheck will report unhealthy in private mode.
Compose defaults to private mode (FLUX_ACCESS_PASSWORD set, FLUX_PUBLIC_OPEN=0). In server.mjs the HTTP Basic gate (around lines 766-787) runs before the /api/health route, and the healthcheck sends no credentials. It gets a 401, so !response.ok exits 1 and the container never becomes healthy. That misses the issue's "shows healthy after startup" goal for the default setup. (It would pass in public mode, which is probably how it was tested.)
Suggested fix, still Dockerfile-only: send Basic auth from the container's own environment when a password is set. As a bonus, this can also require ready: true, since /api/health returns 200 even when yt-dlp or FFmpeg is missing:
HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=3 CMD ["node","-e","const h={};if(process.env.FLUX_ACCESS_PASSWORD)h.authorization='Basic '+Buffer.from((process.env.FLUX_ACCESS_USERNAME||'flux')+':'+process.env.FLUX_ACCESS_PASSWORD).toString('base64');fetch('http://127.0.0.1:3000/api/health',{headers:h}).then(r=>r.json().then(j=>process.exit(r.ok&&j.ready?0:1))).catch(()=>process.exit(1))"]Please verify both modes with docker compose up -d --build and docker compose ps:
- with
FLUX_ACCESS_PASSWORDset, and - with
FLUX_PUBLIC_OPEN=1and no password.
Both should show healthy. Once that's done I'll approve the CI run and merge. Thanks again!
|
Addressed the review feedback. The healthcheck now sends Basic auth from FLUX_ACCESS_USERNAME/FLUX_ACCESS_PASSWORD when configured and requires /api/health to return ready: true. Verified Docker images in both private-password and public-open modes; both reached healthy after startup. |
10k11
left a comment
There was a problem hiding this comment.
Thanks, that's exactly the fix. Approving. I'll merge once CI is green.
Summary
HEALTHCHECKusing Node's built-infetchagainst/api/healthhealthyin the Deployment sectionValidation
npm run checkgit diff --checkdocker build --tag flux-healthcheck-test .Closes #19