build: Optimize Dockerfile #3606
Conversation
|
|
theProf
force-pushed
the
build/docker-optimization
branch
from
eaaa986 to
fb04428
Compare
|
|
||
| #This port will be used by gunicorn. | ||
| EXPOSE 8080 | ||
| ENV DOCKER=true |
There was a problem hiding this comment.
Where is this env used, and for what purpose? didn't find it searching the source
|
hi, thanks, i would like to optizmize the build process. To be honest I don't really have dev and production dependencies well sorted, I just add them to docker whenever something fails to build and the mess has been growing for years. The vue-3 branch ist the one where all development is happening right now and its probably best to optimize that one from the beginning on so testing can happen gradually while the alpha tests start and nothing on the existing pipeline breaks. One issue I have had for years is building for the different architectures. With the vue-3 branch I dropped support for arm/v7 because it was a pain to maintain and would just break every few month. Still there are architecture related issues for other arm build as well which sometimes cause the x86 build, which is the most important one, to fail. Properly separating these two would be great but I think it might require different tags on docker hub, not sure. Feel free to play around and ask if you have any questions, would be great if someone could sort this mess. |
no worries, understood. I'll help get them sorted
Glad I picked the right branch to base from!
Where is the best place to have discussions around the existing pipelines and how they are used? I'll have some general questions on how/where things are used as I get up to speed and wouldn't want to pollute this PR thread is something like an Issue or Discussion with a label would be better for more QA/advice/back and forth?
It can be tough! Especially if you have any dependencies on libc, then you start getting into glibc (common default of Ubuntu for example) vs musl libc (default of Alpine). I'd be happy to help sort out the multi-arch building. One question I did have is: what is the process for getting the development environment setup from scratch? I wasn't sure if everything is supposed to be in the Dockerfile after building/running, or if the Dockerfile is just the backend and vue3 needs to be started separately, or if TandoorRecipes/open-tandoor-data needed to be pulled in as well, etc. If you have any sort of "minimum commands to run" to get started, it would be greatly appreciated. |
theProf
force-pushed
the
build/docker-optimization
branch
from
18ebcd6 to
523a9de
Compare
| @@ -1,44 +1,94 @@ | |||
| FROM python:3.13-alpine3.21 | |||
| FROM node:20.19-alpine AS node-deps | |||
There was a problem hiding this comment.
what version(s) of node should this be built with?
There was a problem hiding this comment.
i dont really care, I usually use some LTS version for years and once it runs out upgrade. I think I have tried everything with 22 on some machine so that should work
| ENV PYTHONUNBUFFERED 1 | ||
| # TODO: use --frozen-lockfile | ||
| # RUN yarn install --frozen-lockfile | ||
| RUN yarn install |
There was a problem hiding this comment.
Adding back --frozen-lockfile will be a requirement before merge, but uncommenting for now while cleaning up and moving forward
| @@ -35,5 +35,6 @@ | |||
| "vite": "^5.4.7", | |||
| "vite-plugin-vuetify": "^2.0.4", | |||
| "vue-tsc": "^2.0.26" | |||
| } | |||
| }, | |||
| "packageManager": "[email protected]" | |||
There was a problem hiding this comment.
what version of yarn do you want to support? it feels like this should be a monorepo, since vue3 is referencing vue, which would require upwards of yarn v1 (current latest at [email protected]). even if not wanting to use yarn workspaces, upgrading from v1 would give more reliability/performance surrounding node modules
There was a problem hiding this comment.
sure, feel free to upgrade. As with node I have never tought about this version. The vue folder will disappear at some point, just left it in because the old stuff is not yet cleaned out.
clone the repo, install yarn/pip requirements, start the yarn dev server and start the django dev server, run migrations. That should be all thats needed to get a hot reloading dev setup to work.
feel free to open an issue, I get notified so I will respond there. Alternatively I could offer a quick discord call to explain stuff. Generally speaking the whole system is just "what worked" for the last years, I have tried a few times to optimize for different things but overall there isn't much reason for any particular decision besides that it worked, so feel free to change whatever you want, it just needs to work :) |
That is a very kind offer, I will happily take you up on this!
Also did so here (and added link in the PR description), but a realtime chat may be more efficient upfront |
There was a problem hiding this comment.
Great work @theProf <3
Did a quick review with some other newish Docker syntax, nice use of the new --link already!
Feel free to ping for any questions or a final review when you are done. I now just did a quick review in the Github interface no local checkout.
It would IMO also be nice to setup an unprivilged user and switching to that. Quick copy paste to setup a 1000:1000 user and group without useradd or adduser (and remembering the different syntaxes hehe):
FROM alpine as builder
RUN <<PASSWD cat > /etc_passwd && <<GROUP cat > /etc_group
app:x:1000:1000:app:/:
PASSWD
app:x:1000:app
GROUP
FROM alpine
COPY --from=builder /etc_group /etc/group
COPY --from=builder /etc_passwd /etc/passwd
USER app| ENV PYTHONUNBUFFERED=1 | ||
|
|
||
| # Install all dependencies. | ||
| RUN apk add --no-cache \ |
There was a problem hiding this comment.
| RUN apk add --no-cache \ | |
| RUN --mount=type=cache,target=/etc/apk/cache \ | |
| apk add \ |
make use of the 'newish' run volumes to cache the packages inbetween builds. Mostly an easy speedup for local dev.
|
|
||
| COPY --link requirements.txt ./ | ||
|
|
||
| RUN <<EOF |
There was a problem hiding this comment.
| RUN <<EOF | |
| RUN --mount=type=cache,target=/etc/apk/cache --mount=type=cache,target=/root/.cache <<EOF |
/root/.cache is where the pip cache lives
| # remove Development dependencies from requirements.txt | ||
| sed -i '/# Development/,$d' requirements.txt | ||
|
|
||
| apk add --no-cache --virtual .build-deps \ |
There was a problem hiding this comment.
| apk add --no-cache --virtual .build-deps \ | |
| apk add --virtual .build-deps \ |
There was a problem hiding this comment.
related to the caching in the RUN command (proposed change)
| curl https://sh.rustup.rs -sSf | sh -s -- -y | ||
| fi | ||
|
|
||
| venv/bin/pip install -r requirements.txt --no-cache-dir |
There was a problem hiding this comment.
| venv/bin/pip install -r requirements.txt --no-cache-dir | |
| venv/bin/pip install -r requirements.txt |
related to the caching in the RUN command (proposed change)
| RUN <<EOF | ||
| /opt/recipes/venv/bin/python version.py | ||
| # delete git repositories to reduce image size | ||
| find . -type d -name ".git" | xargs rm -rf |
There was a problem hiding this comment.
I would add .git in the .dockerignore file instead of removing it. This is kinda meh since this only removes it from the final image the .git is already in the COPY layer which would be avoided when using the dockerignore.
There was a problem hiding this comment.
I agree. Removing files added in an earlier layer doesn't actually remove them from the image. The files are still there even if they aren't visible, so it doesn't reduce the container image's size.
| venv/bin/pip install setuptools_rust==1.10.2 | ||
|
|
||
| if [ $(apk --print-arch) = "aarch64" ]; then | ||
| curl https://sh.rustup.rs -sSf | sh -s -- -y |
There was a problem hiding this comment.
Couldn't this be replaced by a simple apk add rust?
The package is available:
https://pkgs.alpinelinux.org/packages?name=rust&branch=v3.21&repo=&arch=aarch64&origin=&flagged=&maintainer=
Then it is nicely within the virtual-deps as well.
Thanks for the review! I'm glad I kept this as a Draft PR as I feel it best to split it. I've realized there are 2 goals and want to try my best not to mix them (as it will end up being a large and difficult to review PR):
I mention as I'm 100% on board for things like RUN mount caches (have it in a local branch), but that falls under 2 and will have impacts to the build pipeline as well (to actually utilize the cache). The reason I want to focus on 1 is to gain clarity on things like: https://github.com/TandoorRecipes/recipes/pull/3606/files#r2033544477
I thought the same! I then found out there may be git submodules used by the pipelines such as here. And that's the kind of thing I'd like to move into the Dockerfile as a source of truth on all dependencies and build steps. Once the Dockerfile is organized and a reliable source of truth, it will be much easier to add optimizations with confidence. |
Yes I think that is a good call, might still be nice to keep the Then the Dockerfile is a given and the second PR could focus fully on optimizing the pipelines. That would also give a nice insight in the performance gain.
IMO the non root running of the image would be part of this. At this moment you can't run this image on Kubernetes where Pod Security is enforced.
Interesting one, it seems that there previously where two tags (normal and open-data), for the 2.0.0 release the open-data workflow has been removed: Maybe @vabene1111 has something else in mind to import the open data (maybe in the new interface?) instead of releasing two containers. Otherwise I would make it an |
|
thanks, I definitely already have no idea what you are talking about but it seems you two understand each other 😂 regarding the open data/submodules: tandoor supports plugins. This is not really documented and not used by anyone other than me to add the open data project to the hosted instance and to add some hosted specific code to that. In the future I would like the abiliy to install these plugins at runtime because its annoying to keep multiple pipelines working for them, thats why I removed them for the tandoor 2 branch. The problem is that I am not yet sure that I will be able to do that. Ideally there would be an interface where you add a git url, clone the repo, yarn build runs in the background and then the new modules are available. At the same time during development I want them to be in the same structure to make it faster. I would say for now leave this out, I will need to check if runtime installing of plugins works and we will see. Obviously if any of you are interested in building that support (or at least making sure the required dependencies will be available in the image) that would be great. One more information regarding the volumes: The only reason I use a mix of volumes and bind mounts is to provide the default nginx config. I have always kept nginx seperate from the base image because I thought people should have the freedom to choose their webserver and because I thought bundling nginx might be a bad idea. That said it has caused many people trouble over the years and I think there are many instances running without the nginx which is not good and can even break things from time to time, so I would not be opposed to including nginx directly in the image, depending on what you two think is best practice. I am happy to get your input on this matter |
I'm in that situation too, trying (and so far failing) to run everything under the restricted Pod Security Standard. So I'd very much appreciate the non-root user. @wilmardo actually, a user account doesn't even need to exist in the container image. |
|
Thank you @theProf ! 😄 I think those are good improvements to the clarity of the container build process. |
|
Sorry I haven't been responsive, had some personal life going on. Great to see all the activity and feedback! I'll take a crack at these (and creating a checklist to split things up) tomorrow morning. |
theProf
force-pushed
the
build/docker-optimization
branch
from
523a9de to
457d44d
Compare
|
Took a bit longer as I had to get reacquainted with the code. Only pushed a rebase from feature/vue3. But putting update in the issue |
|
@theProf this has probably been closed by accident when the branch got deleted. Would you mind reopening? |
thanks for the nudge. initially thought it was closed out intentionally. Have had some personal life events so haven't been so active. I'll open this back up and likely stack some changes with @wilmardo PRs ref |
Note
This PR has been moved to #3919
Description
Refactor the Dockerfile for readability, while optimizing for a slimmer final image by reducing the layers and stripping the build dependencies from the production image
Background
I wanted to start contributing to issues, but got side tracked during setup. This is a work in progress as it will require more testing. The image itself relies on external dependencies not included in the Dockerfile (looking at the CI pipelines) and since I haven't got the dev environment up and running yet, unsure which dependencies are solely for building and which are required for production, but err-ed on the side of caution rather than optimization.
Comparison of sizes
Questions
I didn't see anywhere that was tagged with contribution / discussion around DevOps, CI/CD, building, etc, but if I missed it please feel free to point me in the correct direction!
EDIT: Created an Issue for larger discussion that can be found here