-
-
Notifications
You must be signed in to change notification settings - Fork 2k
dockerfile: compile openssl instead of using the one bundled on the crystal alpine image. #5441
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: master
Are you sure you want to change the base?
Conversation
…rystal alpine image.
Wow this is very cool. I would be interested in the ARM64 version :) |
There it is, feel free to test it |
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.
Pull Request Overview
This PR addresses a memory leak issue by compiling OpenSSL from source instead of using the bundled version in the Crystal Alpine image. The changes implement custom OpenSSL compilation to potentially fix memory leak issues documented in issue #1438.
- Adds OpenSSL 3.5.2 compilation from source with checksum verification
- Removes dependency on Alpine's bundled OpenSSL packages
- Updates build process to use the custom-compiled OpenSSL via PKG_CONFIG_PATH
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
docker/Dockerfile | Implements OpenSSL source compilation and updates Crystal build to use custom OpenSSL |
docker/Dockerfile.arm64 | Applies the same OpenSSL compilation changes for ARM64 architecture |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
I wonder if compiling OpenSSL ourselves is really the better option compared to just switching the image to be based off of Debian or something |
Yeah it's most likely a better idea. Though we should see if it doesn't increase the final docker image size by too much. I found this docker image which offer amd64 and arm64 with either debian or ubuntu: https://hub.docker.com/r/84codes/crystal/tags. Found it here: crystal-lang/distribution-scripts#125 (comment). They also have .deb files: https://packagecloud.io/84codes/crystal Could finally unify our Dockerfile for both cpu architectures. |
84codes is also one of Crystal's main sponsors so it should be stable for the foreseeable future too. Though the Debian images still rely on a static Crystal compiler compiled on Alpine... https://github.com/84codes/crystal-packages/blob/main/debian-static/Dockerfile so I wonder if that can potentially cause this leak to resurface. To my knowledge it shouldn't but this leak is also really strange. |
I have not personally found anything else distributing debian crystal with arm64 support. Everything else is only for amd64. But indeed it would be better to have full glibc. I wonder if their .deb is full glibc or also based on alpine: https://packagecloud.io/84codes/crystal |
I'll take some time to make a Debian based Dockerfile and see how well it works, I'm also worried about the final image size, but if is not that much, that will be fine. The current image seems to weight about ~75MB |
It now weights 225MB, too much :/ There is 84codes/crystal-packages#35, so I will try to use LibreSSL instead on my Invidious instance and see how's the memory usage |
I have squeezed 20MB by replacing your apt by. So I got 204MB.
I used dive to check what's using the most of MB. It's the base system and the apt-get install.
200MB uncompressed it's not that bad to be fair. |
I'm now testing LibreSSL on my instance without memory limits to see how's the memory usage ;) |
It also leaks, it used up to 1.63GB on an Invidious process that is just used to receive feed updates from Youtube PubSub. With self compiling OpenSSL it never used more than 400MB |
Which flags do you use when compiling openssl? |
Just |
What a about using a static glibc binary with alpine as the final image? RUN --mount=type=cache,target=/root/.cache/crystal if [[ "${release}" == 1 ]] ; then \
crystal build ./src/invidious.cr \
--release \
--warnings all \
--static \
--cross-compile; \
else \
crystal build ./src/invidious.cr \
--warnings all \
--static \
--cross-compile; \
fi
RUN cc ./invidious.o -o ./invidious \
-rdynamic -static \
-Wl,--gc-sections \
-lyaml -lxml2 -licuuc -licudata -lstdc++ -lgcc -lz -llzma \
-lsqlite3 -lssl -lcrypto -lpcre2-8 -lm -lpthread -ldl \
-L/usr/lib/crystal -lgc I added RUN mkdir -p /usr/lib/ssl/
RUN ln -s /etc/ssl/certs/ca-certificates.crt /usr/lib/ssl/cert.pem Complete DockerfileFROM 84codes/crystal:1.16.3-debian-12 AS builder
RUN apt update && \
apt install -y --no-install-recommends --no-install-suggests \
libsqlite3-dev libyaml-dev libssl-dev \
libxml2-dev liblzma-dev \
libicu-dev libstdc++-12-dev
ARG release
WORKDIR /invidious
COPY ./shard.yml ./shard.yml
COPY ./shard.lock ./shard.lock
RUN shards install --production
COPY ./src/ ./src/
# TODO: .git folder is required for building – this is destructive.
# See definition of CURRENT_BRANCH, CURRENT_COMMIT and CURRENT_VERSION.
COPY ./.git/ ./.git/
# Required for fetching player dependencies
COPY ./scripts/ ./scripts/
COPY ./assets/ ./assets/
COPY ./videojs-dependencies.yml ./videojs-dependencies.yml
RUN crystal spec --warnings all \
--link-flags "-lxml2 -llzma"
# Use bash to avoid syntax error on Debian (/bin/sh: 1: [[: not found)
SHELL ["bash","-c"]
RUN --mount=type=cache,target=/root/.cache/crystal if [[ "${release}" == 1 ]] ; then \
crystal build ./src/invidious.cr \
--release \
--warnings all \
--static \
--cross-compile; \
else \
crystal build ./src/invidious.cr \
--warnings all \
--static \
--cross-compile; \
fi
RUN cc ./invidious.o -o ./invidious \
-rdynamic -static \
-Wl,--gc-sections \
-lyaml -lxml2 -licuuc -licudata -lstdc++ -lgcc -lz -llzma \
-lsqlite3 -lssl -lcrypto -lpcre2-8 -lm -lpthread -ldl \
-L/usr/lib/crystal -lgc
RUN ls -l ./invidious
RUN ldd ./invidious || true
FROM alpine:3.22
RUN apk add --no-cache rsvg-convert ttf-opensans tini tzdata
WORKDIR /invidious
RUN addgroup -g 1000 -S invidious && \
adduser -u 1000 -S invidious -G invidious
COPY --chown=invidious ./config/config.* ./config/
RUN mv -n config/config.example.yml config/config.yml
RUN sed -i 's/host: \(127.0.0.1\|localhost\)/host: invidious-db/' config/config.yml
COPY ./config/sql/ ./config/sql/
COPY ./locales/ ./locales/
COPY --from=builder /invidious/assets ./assets/
COPY --from=builder /invidious/invidious .
RUN chmod o+rX -R ./assets ./config ./locales
RUN mkdir -p /usr/lib/ssl/
RUN ln -s /etc/ssl/certs/ca-certificates.crt /usr/lib/ssl/cert.pem
EXPOSE 3000
USER invidious
ENTRYPOINT ["/sbin/tini", "--"]
CMD [ "/invidious/invidious" ] ps. docker run --rm --entrypoint bash 84codes/crystal:1.16.3-debian-12 -c 'ldd $(which crystal shards)' ldd result
The debian-static is probably for building .deb packages for "Any Deb based distributions" as on https://packagecloud.io/84codes/crystal. Edit: |
Maybe a stupid question: has anyone tried to incrementally add the configure options from Alpine's APKBUILD file, to determine which one is in cause and so be able to upstream a fix? |
For that we would need to create a POC Crystal program to test it's memory usage against different flags, because it takes around ~30 minutes (or less, depends of the traffic of the instance) to make a single Invidious process go beyond 1GB of memory. Since I applied this on my instance, the memory of my 4 processes has never used more than 1GB of memory constantly ![]() |
May fix #1438
Supersede, close #5336
This has been running on my instance for a long time now and the memory usage has been reduced significantly as it can be seen here: #1438 (comment)
Although it still increases slowly for some reason, is way better than letting it crash by out of memory.
The memory drops are from Docker daemon restarts while I was configuring some things, Invidious didn't crash by OOM and I didn't restart it manually either
Since this graphs are from my fork that has some added things that upstream Invidious doesn't have, this may completely fix the memory leak, but I'm not really sure.
I haven't tested arm64 at all so I didn't make any changes to docker/Dockerfile.arm64