-
Notifications
You must be signed in to change notification settings - Fork 86
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
[PR] Resized the sizes of final images for frontend and backend below 150MB #33
base: main
Are you sure you want to change the base?
[PR] Resized the sizes of final images for frontend and backend below 150MB #33
Conversation
@pasanchamikara please sign the CLA , then we will evaluate |
@kalinga777 done! |
Great common optimizations for a docker build. Maybe switch to caddy for the serving to get easy automatic HTTPS? |
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.
I left some comments with minor change requests.
|
||
COPY --from=builder /usr/src/app/build/package*.json . | ||
COPY --from=builder /usr/src/app/build dist/ | ||
COPY --from=builder /usr/src/app/.env . |
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.
.env file must be ignored and not added to the build. Injecting the .env must be a deployment task.
# FIRST STAGE: Install Dependencies | ||
|
||
# Use the official Node.js 18-alpine3.20 image as a base | ||
FROM node:18 as builder |
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.
Why not use the same FROM node:18-alpine3.20 as build
? Also, the naming convention is not consistent, in one image the first stage is named build and another is builder. See worklenz-frontend/Dockerfile:2
of your PR.
@@ -6,7 +6,8 @@ services: | |||
dockerfile: Dockerfile | |||
container_name: worklenz_frontend | |||
ports: | |||
- "4200:4200" | |||
- "80:80" | |||
- "44:443" |
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.
Your nginx configuration is not using port 443. Maybe you miss the nginx configuration for https? If this is not the case, remove the unused port.
This PR would bring the final image sizes of the docker build below 150MB, making it easier (Saves bandwidth) for any K8s pod or a docker run Instance to re-instantiate the application.
The approach of multi-layered docker images are followed.
Main Changes
builder
, there are some alpine linux issues which forced us to make use of the same base image which used to work. Somehow, still the backend image could be resolved to 143MBs.