-
Notifications
You must be signed in to change notification settings - Fork 6
Moves build tasks from build.sh to Dockerfile (#2223) #2244
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: main
Are you sure you want to change the base?
Conversation
ysbaddaden
left a comment
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.
To answer your questions:
The task can run as root. Docker makes it a nightmare to try to use another user. Still, a good practice is to try to run the eventual command as non-root... but we don't do it (yet, probably something to consider).
Using system packages is fine 👍
Yes, we'd like to keep mix deps.get --only prod. Looking at mix.exs and package.json it seem that the elixir dependencies with an impact on the frontend in production are installed.
To further improve:
Ideally, we'd like to avoid having yarnpkg and more importantly the node_modules in the resulting image, we only need the built assets, and node_modules grows very big.
Sadly we can't just delete them, because a Docker image is made of all the intermediary layers; it's not a copy of the end image state. Each command in the Dockerfile is a layer, and each command is responsible to cleanup, hence the RUN apt update && apt install && apt clean && rm command for example, solely to avoid artifacts.
This is where multistage images are useful. In this Dockerfile, we take advantage of it to have a single Dockerfile to build a dev image (as requested by target: dev in docker compose), with just the bare system, enough for local development, that we segue into a release image to build the actual image to deploy.
The advantage of multistage is that we can build something, then start again from another image, and copy the bits we want from it in the final image. There is an example in Citta's Dockerfile: we compile using a crystal image, then start the actual release image using a bare alpine system and copy the built binaries.
I think you can intertwine multiple stages?
Sorry for the long description, but you said you're not very familiar with Docker, so I went in verbose mode 🙇
NOTE: now that I think about it, I'm wondering if we need the elixir deps or even the Elixir image at all after we compile? and... apparently we don't but let's save that for later.
Dockerfile
Outdated
|
|
||
| RUN apt -q update && \ | ||
| apt -q install -y default-mysql-client inotify-tools festival && \ | ||
| apt -q install -y default-mysql-client inotify-tools festival yarnpkg && \ |
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.
We're not using yarn in the development image (since we're running a webpack service in the docker-compose.yml). And we definitely not need it in the production one.
Dockerfile
Outdated
| RUN yarnpkg install --no-progress | ||
| RUN yarnpkg deploy |
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.
We should add yet another stage in the file, using the same image we're running for the webpack service right now, and build the UI as we're now doing in the build.sh file - but in this Dockerfile, so we can then COPY --from=webpack those resulting files.
|
As an update to this, I have a multi-stage version where elixir builds a binary and the node vm builds a frontend asset bundle separately from the final image, but I was getting crashes and other issues from the erlang binary that I haven't figured out yet. I might also try building the final image including the erlang runtime components to avoid that issue. |
|
The updated patch uses a separate image for building the frontend assets. I couldn't quite get a release image without the erlang runtime to work but this goes part of the way. It also looks like I might need to copy the |
ysbaddaden
left a comment
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.
@nthiad Nice, we're almost there!
The only issue is setting MIX_ENV=prod for the dev build. Maybe we could squeeze a deps stage? For example:
dev -> deps -> js
-> release
So that dev would only have the bare erlang image (where we can install, update or remove deps at will), but allow js and release to have the erlang deps installed once.
And a nitpick: maybe rename js as frontend?
Here is a first attempt at #2223.
I tried to make this minimal but I have some questions:
mix deps.getuse--only prod? I took it out to matchbuild.shbut perhaps that is wrong here.I didn't remove build.sh entirely in this patch since it has other tasks like
dockerBuildAndPushbut perhaps this could be removed if somebody who knows more about this can comment.