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

Statically build using musl toolchain and target alpine #303

Merged
merged 6 commits into from
Jan 28, 2022

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Jan 25, 2022

Main changes:

  • Statically build controller using musl toolchain.
  • Simplifies Dockerfile by leveraging the musl toolchain static libraries provided by golang-with-libgit2@libgit2-1.1.1-4.
  • Target alpine as final base image.
  • Add file containing attributions for libraries that are statically built into controller.
  • Add make verify.

Although the controller is statically built, we are targeting alpine to help users debugging the image if they so need/wish.

@pjbgf pjbgf changed the title WIP: Use debian:bookworm-slim as base image Use debian:bookworm-slim as base image Jan 25, 2022
@pjbgf pjbgf changed the title Use debian:bookworm-slim as base image Statically build using musl toolchain and target alpine Jan 26, 2022
@pjbgf
Copy link
Member Author

pjbgf commented Jan 26, 2022

Needs to be updated with source controller once fluxcd/source-controller#558 is merged.

@pjbgf pjbgf force-pushed the bookworm branch 8 times, most recently from 83aa0e4 to e6978bb Compare January 26, 2022 15:49
Copy link
Member

@relu relu left a comment

Choose a reason for hiding this comment

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

Looks solid to me. ✔️

# - Use read-only bind instead of copying go source files.
# - Cache go packages.
RUN --mount=target=. \
--mount=type=cache,target=/root/.cache/go-build \
Copy link
Member

Choose a reason for hiding this comment

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

This works fine on localhost but doesn't help much in CI because the build cache is not something you can easily restore. See this issue for context.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you @relu, I will take a look and check ways to improve it. 👍

@pjbgf pjbgf force-pushed the bookworm branch 5 times, most recently from 10b9850 to 622f3ee Compare January 27, 2022 13:08
@pjbgf pjbgf force-pushed the bookworm branch 5 times, most recently from 81e6fbf to c70ec1d Compare January 27, 2022 20:31
Paulo Gomes added 5 commits January 28, 2022 09:35
This aligns with the final image used by source controller.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @pjbgf 🏅

I've tested this on several EKS ARM64 clusters where the previous release was failing 90% of the time with ✗ ImageUpdateAutomation reconciliation failed: 'unable to clone: SSH could not read data: Error waiting on socket'. After using a build from this PR all errors were gone 🎉

Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Updated the branch protection rules so this one can pass, thanks @pjbgf 🙇

@hiddeco hiddeco merged commit 643b9c7 into fluxcd:main Jan 28, 2022
@hiddeco hiddeco added the area/ci CI related issues and pull requests label Jan 28, 2022
@pjbgf pjbgf deleted the bookworm branch January 28, 2022 15:06
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci CI related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants