-
Notifications
You must be signed in to change notification settings - Fork 31
Overhaul the Docker setup #117
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
Conversation
This patch rewrites the Docker setup used to create a deployable
container running a production LNT server.
- Split the Docker image into two, a basic image with LNT installed
and an image with the actual production server as an entry point.
- Relocate all of the Docker-related files under docker/.
- Document the various Docker-related files.
- Improve input validation in the docker entrypoint.
- Using proper Docker secrets to transmit sensitive information to
the LNT webserver entry point and the Postgres database.
- Update to Postgres 18. We might as well use the latest version
available since we're standing this up from scratch.
With this setup, I am able to spin up a local LNT server instance with:
docker compose --file docker/compose.yaml --env-file <secrets> up
An example of a secrets file would be
LNT_DB_PASSWORD=foo
LNT_AUTH_TOKEN=bar
docker/lnt.dockerfile
Outdated
| COPY . /var/tmp/lnt | ||
| WORKDIR /var/tmp/lnt |
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.
Can we avoid copying the entire source directory and only copy the required files to avoid clobbering the layers? This was originally done in 2d87edd, it helps keep image sizes small and reduces the cost of container registries over time etc.
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.
Ah, good point. Actually, I don't think we need anything except the installed Python package and the entrypoints, right? So we should be able to copy the sources during the build phase but not have them in the resulting image.
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.
Let me know what you think of the new approach with --mount=type=bind. I'm (obviously) learning Docker so I hope my usage is alright, but IIUC this should result in the smallest possible image.
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.
@lukel97 and I discussed this now at the LLVM Dev Meeting and basically we'd like to move the installation of dependencies into a separate step that runs before the installation of LNT itself. This allows caching the dependencies when only LNT itself changes.
This can be done by copying requirements.txt into the Docker image and getting rid of . in requirements.txt. Then, pip3 install -r requirements.txt will install only the dependencies, without requiring the actual sources in the Docker image.
This will be pursued in a separate PR.
docker/lnt.dockerfile
Outdated
| # Install LNT itself | ||
| COPY . /var/tmp/lnt | ||
| WORKDIR /var/tmp/lnt | ||
| RUN pip3 install -r requirements.server.txt && apk --purge del .build-deps |
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.
Should the client image be installing requirements.client.txt?
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.
This is actually creating an image that can run a server, I didn't think we wanted to produce an image that contains just the client, but I guess it makes some sense? In that case, I would suggest we produce three images:
llvm-lnt-clientllvm-lnt-serverllvm-lnt-server-prod
The idea is basically that we produce base images that just contain (the various flavours of) LNT, and then on top of that we build an image that has the actual configuration to run in production (the docker entry point, etc).
Let me know if you think we should be building three images, or something else.
1c00ef8 to
1bd2b5d
Compare
|
@lukel97 I removed the usage of Docker secrets from this PR. However, we get the following warnings when building the Docker image: We'll need to figure out how to best use Docker Secrets while still providing a readily-usable Docker image in a separate PR. |
lukel97
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.
LGTM, thanks
docker/compose.yaml
Outdated
|
|
||
| db: | ||
| container_name: dbserver | ||
| image: docker.io/postgres:18-alpine |
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.
Can you land the postgres upgrade in a separate commit just in case the upgrade causes any issues and we want to revert?
This patch rewrites the Docker setup used to create a deployable container running a production LNT server.
With this setup, I am able to spin up a local LNT server instance with:
An example of a secrets file would be