feat(deploy): add healthchecks to docker, docker compose, podman quadlet, and helm chart#1137
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds and standardizes HTTP health checks across deployment artifacts: Dockerfile ENV/HEALTHCHECK added; Podman quadlet HealthCmd switched to spider mode; Helm values switched probes to HTTP and added startupProbe with template support; Compose grimmory healthcheck removed and left as commented templates. ChangesUnified Healthcheck updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/compose/docker-compose.yml`:
- Around line 36-42: Update grimmory-docs/docs/installation.md to reflect the
Compose file's new healthcheck policy: note that the docker-compose.yml now only
includes a commented "override-only" healthcheck block (the healthcheck:
test/interval/retries/start_period/timeout stanza) and that the project uses the
new probe-style readiness/liveness mechanism instead of the old active
wget-based check; replace the old active compose healthcheck example/command
with the commented override example and add a short example showing how to
supply an override healthcheck or configure the new probe style (mention the
healthcheck stanza and the probe endpoints used by the service).
In `@deploy/helm/grimmory/values.yaml`:
- Around line 94-109: Probes currently hardcode port 6060 in the httpGet blocks
causing desync with .Values.service.port; update the livenessProbe,
readinessProbe (and the first httpGet block) to reference the container named
port "http" instead of the numeric 6060 (i.e., set httpGet.port to the string
"http") so probes follow the container port mapping tied to {{
.Values.service.port }}.
In `@Dockerfile`:
- Around line 119-120: The HEALTHCHECK currently hardcodes port 6060; update the
Dockerfile HEALTHCHECK command to use the BOOKLORE_PORT environment variable
instead so runtime overrides are honored: modify the HEALTHCHECK line (the
existing HEALTHCHECK --interval=... CMD ...) to reference $BOOKLORE_PORT (or
${BOOKLORE_PORT}) in the URL
(http://localhost:$BOOKLORE_PORT/api/v1/healthcheck) and ensure BOOKLORE_PORT is
declared via ENV or passed at runtime so the health probe targets the correct
port.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1b361cbe-1c2c-49f5-94cd-606bf1fac81a
📒 Files selected for processing (5)
Dockerfiledeploy/compose/docker-compose.ymldeploy/helm/grimmory/templates/deployment.yamldeploy/helm/grimmory/values.yamldeploy/podman/quadlet/grimmory.container
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Suite / Backend Tests
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
deploy/podman/quadlet/grimmory.containerdeploy/compose/docker-compose.ymldeploy/helm/grimmory/values.yamlDockerfiledeploy/helm/grimmory/templates/deployment.yaml
**/Dockerfile*
⚙️ CodeRabbit configuration file
**/Dockerfile*: Dockerfile review:
- Prefer multi-stage builds (builder + runtime).
- Use non-root USER in the final stage.
- Pin base image versions (e.g., eclipse-temurin:25-jre-noble).
- Flag secrets or credentials embedded in the image.
- Ensure --enable-preview JVM flag is carried into ENTRYPOINT/CMD if required at runtime.
Files:
Dockerfile
🔀 Multi-repo context grimmory-tools/grimmory-docs
[::grimmory-tools/grimmory-docs::] docs/installation.md:113-118
- Documentation includes a docker-compose healthcheck example using the old command:
test: ["CMD-SHELL", "wget -q -O - http://localhost:6060/api/v1/healthcheck || exit 1"]
interval: 10s, timeout: 2s, retries: 5, start_period: 10s - PR changes the global healthcheck to use
wget -q --spider ...and comments out the composehealthcheck. The docs were not updated to reflect the new--spiderform nor the commented-out/removed compose healthcheck.
🔇 Additional comments (2)
deploy/podman/quadlet/grimmory.container (1)
23-23: Good alignment with image-level behavior.Using
wget -q --spiderhere is consistent with the Docker healthcheck strategy and avoids noisy response-body output.deploy/helm/grimmory/templates/deployment.yaml (1)
78-81: Startup probe templating is correctly wired.The conditional
startupProberender path is consistent with existing probe blocks and integrates cleanly.
thibaultamartin
force-pushed
the
support-healthchecks
branch
from
9e47295 to
bed83fc
Compare
thibaultamartin
force-pushed
the
support-healthchecks
branch
from
bed83fc to
488c6e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/compose/docker-compose.yml`:
- Line 38: The commented wget healthcheck line uses "wget --spider" without the
quiet flag; update that commented template to use "wget -q --spider" so it
matches the Dockerfile HEALTHCHECK and quadlet HealthCmd and avoids emitting
connection progress to stderr when users copy the block.
In `@deploy/helm/grimmory/values.yaml`:
- Around line 93-98: The startupProbe timeout is likely too short for a JVM
Spring Boot app running Flyway migrations; increase the probe budget by raising
startupProbe.failureThreshold from 5 to a larger value (e.g., 10 or 20) and/or
adjust startupProbe.periodSeconds if needed so the container gets ~300–600s to
start; update the startupProbe block (inspect the startupProbe,
failureThreshold, periodSeconds and httpGet.path /api/v1/healthcheck entries) to
use the new values so slow startups on resource-constrained nodes won’t trigger
premature restarts.
- Around line 92-110: The probes (startupProbe, livenessProbe, readinessProbe)
lack timeoutSeconds and thus use the 1s Kubernetes default which is too short
for a JVM Spring Boot app; update each probe block (startupProbe, livenessProbe,
readinessProbe) to add timeoutSeconds: 5 so HTTP GET `/api/v1/healthcheck` has a
5-second timeout to avoid spurious failures under GC/CPU pressure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56c461a6-37c9-45ce-beb7-5da97838bc7f
📒 Files selected for processing (4)
deploy/compose/docker-compose.ymldeploy/helm/grimmory/templates/deployment.yamldeploy/helm/grimmory/values.yamldeploy/podman/quadlet/grimmory.container
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Frontend Lint Threshold Check
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
deploy/helm/grimmory/templates/deployment.yamldeploy/helm/grimmory/values.yamldeploy/podman/quadlet/grimmory.containerdeploy/compose/docker-compose.yml
🔀 Multi-repo context grimmory-tools/grimmory-docs
grimmory-tools/grimmory-docs
- docs/installation.md contains an active Docker Compose healthcheck example that uses the old wget form (prints body):
- healthcheck block and example present around lines ~113–118 and the compose snippet earlier in the file. [::grimmory-tools/grimmory-docs::docs/installation.md:113-118]
Relevance: Documentation still shows the previous command (wget -q -O - http://localhost:6060/api/v1/healthcheck) and an enabled compose healthcheck, which conflicts with the PR changes that switch to wget --spider and comment out the compose healthcheck.
🔇 Additional comments (2)
deploy/podman/quadlet/grimmory.container (1)
23-23: LGTM!Switching to
wget -q --spidercorrectly performs a no-output HEAD-style probe, eliminating the JSON body from container logs.deploy/helm/grimmory/templates/deployment.yaml (1)
78-81: LGTM!The
startupProbeconditional block is consistent with howlivenessProbeandreadinessProbeare already handled, and correct placement before those probes ensures Kubernetes disables them until startup succeeds.
thibaultamartin
force-pushed
the
support-healthchecks
branch
from
488c6e8 to
ac89fdd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
deploy/helm/grimmory/values.yaml (1)
93-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
startupProbewindow of 150 s is likely too short for a JVM + Spring Boot 4 + Flyway app.
failureThreshold: 5×periodSeconds: 30= 150 seconds before Kubernetes kills and restarts the container. This was flagged in a prior review but remains unchanged.The official Kubernetes example for slow-starting applications uses
failureThreshold: 30withperiodSeconds: 10, giving the container a maximum of 5 minutes (300 s) to finish startup. JVM applications are notorious for slow startup times, and this app also runs Flyway DB migrations after the init container unblocks. A safe starting point recommended for Spring Boot services isfailureThreshold: 30withperiodSeconds: 10andtimeoutSeconds: 5, allowing up to 5 minutes for a cold start.Consider at minimum
failureThreshold: 10(300 s) or higher for resource-constrained nodes.🛡️ Proposed fix
startupProbe: httpGet: path: /api/v1/healthcheck port: http - failureThreshold: 5 - periodSeconds: 30 + failureThreshold: 10 + periodSeconds: 30 timeoutSeconds: 5🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/grimmory/values.yaml` around lines 93 - 99, The startupProbe timings are too short for a JVM + Spring Boot 4 + Flyway app: update the startupProbe settings in the values.yaml (the startupProbe block containing httpGet, failureThreshold, periodSeconds, timeoutSeconds) to allow a longer cold-start window — set failureThreshold: 30 and periodSeconds: 10 with timeoutSeconds: 5 (or at minimum increase failureThreshold to 10 if you need a smaller change) so Kubernetes will allow up to ~5 minutes for startup before restarting the container.deploy/compose/docker-compose.yml (1)
36-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLinked-repo docs still reference the old healthcheck command.
According to the linked repository context,
grimmory-tools/grimmory-docs/docs/installation.md(around lines 113–118) still showswget -q -O - http://localhost:6060/api/v1/healthcheckand an active (non-commented) healthcheck block, which conflicts with both the switch to--spiderand the compose healthcheck now being commented out. Users following the installation docs would get divergent behavior.Please verify whether the grimmory-docs update landed — or open a follow-up PR in that repo to replace the old command with the new
--spiderform and reflect the "commented override" pattern introduced here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/compose/docker-compose.yml` around lines 36 - 46, Docs and compose disagree on the healthcheck: update the documentation in grimmory-docs/docs/installation.md to match the docker-compose.yml change by replacing the old wget command `wget -q -O - http://localhost:6060/api/v1/healthcheck` with the new `wget -q --spider http://localhost:6060/api/v1/healthcheck` and also change the example healthcheck block in the docs to show the commented-override pattern (i.e., show the healthcheck block commented out with the same commented fields as in docker-compose.yml); if the docs repo has not been updated here, open a follow-up PR in that repo applying these exact edits and mention this compose change so users see consistent instructions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/grimmory/values.yaml`:
- Around line 107-113: The readinessProbe is pointed at /api/v1/healthcheck
which always returns 200 from HealthcheckController and won't fail when the DB
is down; update the readiness probe to use an endpoint that reflects DB
readiness (either enable and point to Spring Boot Actuator
/actuator/health/readiness which includes DB indicators, or modify
HealthcheckController's /api/v1/healthcheck to perform a simple DB connectivity
check such as a JDBC ping/SELECT 1 and return non-200 when the DB is
unreachable) and change the readinessProbe target accordingly.
---
Duplicate comments:
In `@deploy/compose/docker-compose.yml`:
- Around line 36-46: Docs and compose disagree on the healthcheck: update the
documentation in grimmory-docs/docs/installation.md to match the
docker-compose.yml change by replacing the old wget command `wget -q -O -
http://localhost:6060/api/v1/healthcheck` with the new `wget -q --spider
http://localhost:6060/api/v1/healthcheck` and also change the example
healthcheck block in the docs to show the commented-override pattern (i.e., show
the healthcheck block commented out with the same commented fields as in
docker-compose.yml); if the docs repo has not been updated here, open a
follow-up PR in that repo applying these exact edits and mention this compose
change so users see consistent instructions.
In `@deploy/helm/grimmory/values.yaml`:
- Around line 93-99: The startupProbe timings are too short for a JVM + Spring
Boot 4 + Flyway app: update the startupProbe settings in the values.yaml (the
startupProbe block containing httpGet, failureThreshold, periodSeconds,
timeoutSeconds) to allow a longer cold-start window — set failureThreshold: 30
and periodSeconds: 10 with timeoutSeconds: 5 (or at minimum increase
failureThreshold to 10 if you need a smaller change) so Kubernetes will allow up
to ~5 minutes for startup before restarting the container.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: f45d20e9-384e-4b0d-8ff0-d7df46a354bc
📒 Files selected for processing (4)
deploy/compose/docker-compose.ymldeploy/helm/grimmory/templates/deployment.yamldeploy/helm/grimmory/values.yamldeploy/podman/quadlet/grimmory.container
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Frontend Lint Threshold Check
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
deploy/helm/grimmory/templates/deployment.yamldeploy/podman/quadlet/grimmory.containerdeploy/helm/grimmory/values.yamldeploy/compose/docker-compose.yml
🔀 Multi-repo context grimmory-tools/grimmory-docs
grimmory-tools/grimmory-docs
- docs/installation.md contains an active Docker Compose healthcheck example that uses the old wget form (prints body):
- healthcheck block and example present around lines ~113–118 and the compose snippet earlier in the file. [::grimmory-tools/grimmory-docs::docs/installation.md:113-118]
Relevance: Documentation still shows the previous command (wget -q -O - http://localhost:6060/api/v1/healthcheck) and an enabled compose healthcheck, which conflicts with the PR changes that switch to wget --spider and comment out the compose healthcheck.
🔇 Additional comments (2)
deploy/podman/quadlet/grimmory.container (1)
23-23: LGTM — consistent and correct.
wget -q --spidermatches the DockerfileHEALTHCHECKand the docker-compose override template, making the probe behavior uniform across all runtimes.deploy/helm/grimmory/templates/deployment.yaml (1)
78-81: LGTM — correct and idiomatic Helm.The
{{- with .Values.startupProbe }}/toYaml . | nindent 12pattern is identical to the existinglivenessProbeandreadinessProbeblocks, the indentation is correct, and the conditional correctly allows users to disable the probe by nulling out the value in their override.
|
Does this need to be updated in the readme and the doc page as well? |
|
I'm not sure this needs to be updated in the README or docs, since this makes the Maybe it would make sense to drop it from the examples to keep the docker-compose minimal, since both point to the more complete version in the repo? |
|
I might be overengineering it, but I think it would also make sense to make the CI open a PR against |
Unless we start seeing a lotta changes, overhead could outweigh it. Not sure. |
|
Doing one last test actually - then I'll merge. |
| EXPOSE ${BOOKLORE_PORT} | ||
|
|
||
| HEALTHCHECK --interval=60s --timeout=10s --start-period=60s --retries=5 \ | ||
| CMD wget -q --spider http://localhost:${BOOKLORE_PORT}/api/v1/healthcheck |
There was a problem hiding this comment.
If we want to use the var here I think it needs to have
ENV BOOKLORE_PORT=${BOOKLORE_PORT}
Otherwise we never reach a healthy state seen with docker container ls
thibaultamartin
force-pushed
the
support-healthchecks
branch
from
ac89fdd to
f461025
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/podman/quadlet/grimmory.container`:
- Line 23: HealthCmd currently hardcodes port 6060 causing mismatched health
checks when BOOKLORE_PORT is changed; change the Quadlet HealthCmd to use the
shell form so the container shell expands the environment variable (use
${BOOKLORE_PORT} in the URL) by switching to a CMD-SHELL style HealthCmd that
executes wget -q --spider http://localhost:${BOOKLORE_PORT}/api/v1/healthcheck;
ensure the BOOKLORE_PORT env var (already set in the Dockerfile ENV) is present
in the container environment so the shell expansion works at health-check time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3f8e753-b78b-4b16-812a-da8424c03db4
📒 Files selected for processing (5)
Dockerfiledeploy/compose/docker-compose.ymldeploy/helm/grimmory/templates/deployment.yamldeploy/helm/grimmory/values.yamldeploy/podman/quadlet/grimmory.container
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
deploy/podman/quadlet/grimmory.containerdeploy/helm/grimmory/templates/deployment.yamldeploy/compose/docker-compose.ymldeploy/helm/grimmory/values.yamlDockerfile
**/Dockerfile*
⚙️ CodeRabbit configuration file
**/Dockerfile*: Dockerfile review:
- Prefer multi-stage builds (builder + runtime).
- Use non-root USER in the final stage.
- Pin base image versions (e.g., eclipse-temurin:25-jre-noble).
- Flag secrets or credentials embedded in the image.
- Ensure --enable-preview JVM flag is carried into ENTRYPOINT/CMD if required at runtime.
Files:
Dockerfile
🔀 Multi-repo context grimmory-tools/grimmory-docs
Linked repositories findings
grimmory-tools/grimmory-docs
- docs/installation.md: contains the Docker Compose example for the grimmory service that exposes BOOKLORE_PORT and shows the compose snippet. [::grimmory-tools/grimmory-docs::docs/installation.md:73]
- docs/installation.md: the compose snippet includes a healthcheck block for the mariadb service (not for grimmory) at the shown healthcheck lines. [::grimmory-tools/grimmory-docs::docs/installation.md:113]
- Multiple docs reference docker-compose and BOOKLORE_PORT (e.g., docs/getting-started.md and others), so changing the compose example or its healthcheck behavior in the main repo could warrant a docs update across these pages. [::grimmory-tools/grimmory-docs::docs/getting-started.md:38]
Conclusion: grimmory-docs contains docker-compose examples and references to BOOKLORE_PORT; review whether the PR's change (switching to wget --spider, commenting out grimmory healthcheck, and making healthcheck optional) should be reflected in these docs.
🔇 Additional comments (4)
Dockerfile (1)
117-121: Looks good — Docker healthcheck now correctly follows configurable port.Using
BOOKLORE_PORTin the probe command addresses the prior unhealthy-state issue when port is overridden.deploy/compose/docker-compose.yml (1)
36-46: Compose override-only healthcheck pattern is clean and clear.The commented override and disable examples are well-scoped and align with fallback-to-image healthcheck behavior.
deploy/helm/grimmory/templates/deployment.yaml (1)
78-81: Good Helm templating change for startup probe support.Conditional rendering is implemented correctly and keeps chart behavior configurable via values.
deploy/helm/grimmory/values.yaml (1)
92-113: Probe defaults are solid and internally consistent.Using HTTP checks on named port
httpwith explicit timeout values is a strong default for this chart.
|
The point raised by CodeRabbit about the podman quadlet seems valid to me. I'm running Fedora Silverblue, which makes everything docker a pain in the backside but everything podman much easier to test. I'll have a look and test it properly for the quadlet in the European evening. Next time I'll touch the Dockerfile or docker-compose I'll also spin up a VM to test them properly 🫡 |
3 participants
Description
This PR adds healthchecks in the OCI image and the various deployments that rely on it, namely docker-compose, podman quadlets, and helm chart.
It doesn't fine tune default values and reuses the ones in the pre-existing docker-compose.
Linked Issue
Fixes #1134
Changes
wget -q --spider http://localhost:6060/api/v1/healthcheckto avoid littering the logs with jsonHEALTHCHECKinstruction inDockerfilehealthcheck:override indocker-compose.yamlHealthCmdcommand inpodman/quadlet/grimmory.containerfor consistencystartupProbe,livenessProbe, andreadinessProbein the helm chart (both invalues.yamlandtemplates/deployment.yaml)Manual Testing Steps
Admittedly, I only tested the helm chart deployment, without rebuilding the Docker image.
Screenshots (Optional)
Additional Context (Optional)
AI Disclosure
None
Checklist
just ui checkandjust api check.Summary by CodeRabbit