Skip to content
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

chore: add alpine based docker images #39

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

pimlie
Copy link
Contributor

@pimlie pimlie commented Jun 16, 2024

As mentioned in my previous PR #31, using alpine as base reduces the image size significantly so I took another go at that.

$ docker images mollysocket
REPOSITORY                    TAG       IMAGE ID       CREATED        SIZE
mollysocket                   latest    0cf19f7f68a7   9 minutes ago  21.2MB
ghcr.io/mollyim/mollysocket   latest    e656277398f2   2 weeks ago    92.2MB

Also resolves #37

-- edit --
To clarify, while working on the prev pr I wasn't aware that Alpine already provided statically linked packages for ssl & sqlite. So during the build of mollysocket those libraries couldn't be found, and I thought we had to build ssl/sqlite from source. But after looking a bit more I found that Alpine already includes openssl-libs-static and sqlite-static packages which makes the build 'just' work.

@p1gp1g
Copy link
Member

p1gp1g commented Aug 17, 2024

This will break the CI. But we can do alpine, alpine-x and alpine-x.y tags

@pimlie
Copy link
Contributor Author

pimlie commented Sep 15, 2024

It seems that it's more common to add a suffix instead of a prefix, see f.e. https://hub.docker.com/_/nginx

Unfortunately I'm unsure how I can test whether the images tags are correct, can't get act to work. It might also be nice to add tags to the debian image with a debian suffix so the latest image would have tags like 1.4.1, 1.4, 1, latest, 1.4.1-debian, 1.4-debian, 1-debian, latest-debian.
Not sure how to do that though, maybe just add another step like meta-debian and then concat the tags in the Build and push step?

@p1gp1g
Copy link
Member

p1gp1g commented Sep 19, 2024

It seems that it's more common to add a suffix instead of a prefix, see f.e. https://hub.docker.com/_/nginx

Then going with a suffix is fine 👍

Unfortunately I'm unsure how I can test whether the images tags are correct, can't get act to work.

An easy way is to push tag to your own github repo

It might also be nice to add tags to the debian image with a debian suffix so the latest image would have tags like 1.4.1, 1.4, 1, latest, 1.4.1-debian, 1.4-debian, 1-debian, latest-debian. Not sure how to do that though, maybe just add another step like meta-debian and then concat the tags in the Build and push step?

A solution would be to do it in the meta step. This would probably be better for the -alpine flavor too (so we are sure it wont override :latest :major etc.

          tags: |
            type=schedule
            type=raw,value=latest,enable={{is_default_branch}}
            type=semver,pattern={{version}},value=${{ steps.checkout.outputs.tag }}
            type=semver,pattern={{major}}.{{minor}},value=${{ steps.checkout.outputs.tag }}
            type=semver,pattern={{major}},value=${{ steps.checkout.outputs.tag }}
            type=semver,pattern={{version}}-debian,value=${{ steps.checkout.outputs.tag }}
            type=semver,pattern={{major}}.{{minor}}-debian,value=${{ steps.checkout.outputs.tag }}
            type=semver,pattern={{major}}-debian,value=${{ steps.checkout.outputs.tag }}

I don't know if it possible to run in parallel -debian and -alpine using functions built-in the docker/build-push-action. Else we should use an action matrix or use a 2nd action. If we don't the action will take ages to run

Copy link
Contributor Author

@pimlie pimlie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@p1gp1g Sorry for the bunch of commits, had to debug here. Found a SO that mentioned you could just add a pull_request: event trigger to a workflow to test the builds :)

Let me know if you have further remarks!


RUN apk add --no-cache musl-dev openssl-dev openssl-libs-static sqlite-dev sqlite-static

# First build dependencies, this should cache a dependency layer which
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that although this comment is intrinsically correct, github actions require additional configuration to make docker layer caching work during github workflows.

Not sure if you are interested in that because of the comment about nightly builds to resolve CVE's in the base images.

# only needs to be refreshed when Cargo.(lock|toml) is updated
COPY Cargo.lock Cargo.toml ./
RUN mkdir src && echo "fn main() { panic!(\"why am i running?\") }" > src/main.rs
RUN cargo build --release --locked
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note I added --locked here, this does ofc mean that any dependency update would need to be manually committed first.

Personally I feel explicitly updating dependencies is always better, especially because the current build workflows don't have a test stage first. For easier maintenance it could require an additional github workflow to help maintaining dependency updates (fe using https://github.com/marketplace/actions/dependencies-autoupdate)

@pimlie pimlie changed the title chore: use alpine as base for build and runtime containers chore: add alpine based docker images Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Use alpine linux instead of debian
2 participants