feat: optimize image size with multi-staged dockerfile #3313
Conversation
Varun-Kolanu
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe changes involve modifying the Dockerfile to implement a multi-stage build process. The first stage, named Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3313--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Dockerfile (1)
12-27: Well-structured development stage with room for minor improvement.The second stage, "dev", is implemented effectively:
- Consistent use of the node:18-alpine base image.
- Efficient copying of node_modules from the deps stage, avoiding temporary caches.
- Proper setup of the development environment with correct port exposure and NODE_ENV setting.
These changes successfully address the objectives outlined in issue #3223.
Consider adding a comment before the COPY command on line 20 to clarify its purpose:
COPY --from=deps /async/node_modules ./node_modules + # Copy application files COPY . .This addition would improve readability and make the purpose of each COPY command more explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- Dockerfile (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
Dockerfile (2)
3-11: Excellent implementation of the dependency installation stage!The first stage, named "deps", is well-structured and follows best practices:
- Using a lightweight alpine base image.
- Copying only package files before running npm install, which leverages Docker's layer caching.
- Separating dependency installation from the main build, which optimizes build time and image size.
These changes align well with the PR objectives of optimizing the Docker image size and build speed.
1-27: Excellent implementation of a multi-stage Dockerfile for optimized development.This PR successfully transforms the Dockerfile into a multi-stage build, addressing the objectives outlined in issue #3223. The key improvements are:
- Separation of dependency installation and application build stages.
- Efficient use of Docker layer caching.
- Reduction of the final image size by excluding unnecessary files and caches.
These changes should result in faster builds and a significantly smaller Docker image (reported 350 MB decrease), enhancing the development workflow.
To confirm the image size reduction, please run the following script:
This script will build the image and output its size. Please compare this with the previous image size to confirm the reported 350 MB reduction.
Varun-Kolanu
force-pushed
the
docker-multistaged
branch
from
2bff358
to
6f3fef9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3313 +/- ##
=======================================
Coverage 49.14% 49.14%
=======================================
Files 21 21
Lines 647 647
=======================================
Hits 318 318
Misses 329 329 ☔ View full report in Codecov by Sentry. |
Varun-Kolanu
force-pushed
the
docker-multistaged
branch
from
6f3fef9
to
029ea19
Compare
Varun-Kolanu
force-pushed
the
docker-multistaged
branch
from
029ea19
to
79c67ea
Compare
|
Using docker as a dev container orchestrator requires the npm cache to speed up node_module downloads in case they purge the previous ones |
|
@Varun-Kolanu I think @anshgoyalevil has a valid point. Moreover, we don't see any need to optimize the image since it's not a big issue for any developer. Thanks for creating the PR, but we won't be merging this PR. |
Description
Related issue(s)
Fixes #3223
This PR successfully fixes this issue doing the intended changes. I tried so much to further reduce the size of image but concluded (in my opinion) that this may be the optimal size for the development docker image.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation