-
Notifications
You must be signed in to change notification settings - Fork 2
Dockerfile: migrated to dumb-init for fast container signals #20
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: master
Are you sure you want to change the base?
Conversation
WalkthroughSwitch Docker base image to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Docker as Docker daemon
participant Init as /usr/bin/dumb-init
participant App as node (dist/index.js)
Docker->>Init: start container (ENTRYPOINT)
Init-->>Docker: reap signals / forward signals
Init->>App: exec node dist/index.js (CMD)
App-->>Init: respond / exit
Init-->>Docker: exit status forwarded
note right of Init #CFE9F8: dumb-init added to manage\nsignal handling and reaping
sequenceDiagram
autonumber
participant GHAction as GitHub Actions
participant GHCR as ghcr.io
GHAction->>GHCR: authenticate using GITHUB_TOKEN (repository_owner)
GHAction->>GHCR: build and push image f213/mock-openai-api
GHCR-->>GHAction: push result / image digest
note right of GHCR #F0F4C3: Registry switched from Docker Hub to GHCR
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Dockerfile (3)
2-2
: Base image bump looks good; consider digest pin and clarifying stage.
- Nice move to
node:22-alpine3.21
for musl-based small images.- For reproducibility, consider pinning by digest.
- The header says “Build stage” but there’s only one stage. Either rename the comment or convert to a true multi-stage build to drop dev deps from the runtime image.
Apply if you just want to clarify intent:
-# Build stage +# Base image FROM node:22-alpine3.21If you prefer multi-stage (optional skeleton):
-FROM node:22-alpine3.21 +FROM node:22-alpine3.21 AS builder ... -RUN npm i +RUN npm ci ... RUN npm run build ... -EXPOSE 3000 -ENV NODE_ENV=production -ENV PORT=3000 -ENV HOST=0.0.0.0 -ENV VERBOSE=true -ENTRYPOINT ["/usr/bin/dumb-init", "--"] -# Start the application with verbose logging enabled -CMD ["node", "dist/index.js"] +FROM node:22-alpine3.21 AS runner +WORKDIR /app +# Only runtime deps +COPY package*.json ./ +RUN npm ci --omit=dev --ignore-scripts +COPY --from=builder /app/dist ./dist +EXPOSE 3000 +ENV NODE_ENV=production +ENV PORT=3000 +ENV HOST=0.0.0.0 +ENV VERBOSE=true +ENTRYPOINT ["/usr/bin/dumb-init", "--"] +CMD ["node", "dist/index.js"]
5-6
: Fix comment typo and validate package pin.
- “Install pages” → “Install dumb-init” (typo).
- Version pin
dumb-init==1.2.5-r3
is good for repeatability but can break if the repo rotates; ensure it exists for alpine3.21.Apply the typo fix:
-# Install pages +# Install dumb-init (for proper signal handling as PID 1) RUN apk add --no-cache dumb-init==1.2.5-r3Please verify the pin matches the base repo by running locally:
docker run --rm node:22-alpine3.21 sh -lc 'apk add --no-cache dumb-init==1.2.5-r3 && dumb-init --version'
32-35
: ENTRYPOINT via dumb-init: good; add STOPSIGNAL and note behavior change.
- Good use of exec-form with dumb-init to forward signals and reap zombies.
- Recommend declaring STOPSIGNAL to align with graceful shutdown.
- ENTRYPOINT changes container UX (overriding CMD no longer bypasses dumb-init). Document in README or release notes.
Apply:
+STOPSIGNAL SIGTERM ENTRYPOINT ["/usr/bin/dumb-init", "--"]
If you prefer not to bake init into the image, an alternative is advising
docker run --init …
or Composeinit: true
in ops docs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Dockerfile
(2 hunks)
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docker-publish.yml (1)
65-73
: Remove or disable stale Docker Hub description step
TheUpdate Docker Hub description
step still depends on Docker Hub credentials and uses${{ env.IMAGE_NAME }}
(now pointing to GHCR), causing failures post-migration. Choose one:
Option A — Guard and point to an explicit Docker Hub repo:
- name: Update Docker Hub description - if: github.event_name != 'pull_request' && github.ref == 'refs/heads/main' + name: Update Docker Hub description + if: github.event_name != 'pull_request' && github.ref == 'refs/heads/main' && env.DOCKERHUB_REPOSITORY != '' uses: peter-evans/dockerhub-description@v4 with: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} - repository: ${{ env.IMAGE_NAME }} + repository: ${{ env.DOCKERHUB_REPOSITORY }} readme-filepath: ./README.mdAnd add at top-level
env:
:DOCKERHUB_REPOSITORY: "" # e.g., "username/repo" if you still maintain Docker HubOption B — Remove this step entirely if Docker Hub is no longer used.
🧹 Nitpick comments (2)
.github/workflows/docker-publish.yml (2)
16-17
: Parameterize image name to avoid hardcoding owner/repo.Hardcoding
f213/mock-openai-api
makes forks/org renames brittle. Prefer deriving from the repository.Apply:
- REGISTRY: ghcr.io - IMAGE_NAME: f213/mock-openai-api + REGISTRY: ghcr.io + IMAGE_NAME: ${{ github.repository }}
33-39
: Step name and auth now target GHCR, not Docker Hub.Rename for clarity; config is correct for GHCR with
GITHUB_TOKEN
andpackages: write
.- - name: Log in to Docker Hub + - name: Log in to GHCR if: github.event_name != 'pull_request' uses: docker/login-action@v3 with: registry: ${{ env.REGISTRY }} username: ${{ github.repository_owner }} password: ${{ secrets.GITHUB_TOKEN }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/docker-publish.yml
(3 hunks)
Current container implemntation takes 10 secs to stop after pressing Ctrl-C. This happens because of node.js not handling docker signals.
Summary by CodeRabbit