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

Switch base docker image from golang-alpine to ubuntu:20.04 #3882

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

jkneubuh
Copy link
Contributor

Type of change

  • Bug fix

Description

This PR switches the Fabric base docker images from golang-alpine to ubuntu:20.04. The upgrade is necessary as the golang-alpine libc runtimes (musl) are incompatible with the requirements on multi-arch runtimes.

The motivations for this upgrade are described in detail in Discussion #3876 and comments in DRAFT PR #3877

Additional details

Related issues

@jkneubuh jkneubuh requested a review from a team as a code owner December 21, 2022 15:00
@jkneubuh
Copy link
Contributor Author

@denyeart @C0rWin @pfi79 @davidkel @lehors - final stake / compromise for arm64 support.

images/ccenv/Dockerfile Outdated Show resolved Hide resolved
images/tools/Dockerfile Outdated Show resolved Hide resolved

# Disable cgo when building in the container. This will create a static binary, which can be
# copied and run in the base alpine container without the libc/musl runtime.
ENV CGO_ENABLED 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove static compilation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The static link in docker was only added to work around the SIGSEGVs in alpine. Now with the build in Docker and with the matching libc runtime FROM ubuntu, the static build is no longer required.

Copy link
Contributor

Choose a reason for hiding this comment

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

Compiling binary statically has another side effect of having all dependencies compiled into binary, ensuring the resulting executable is assembled and equipped with all dependencies it was compiled with, ensuring consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@C0rWin are you proposing that we change the make target to generate a static executable? I thought we had settled on the use of Docker to establish consistency across runtimes. Recall that the above update ENV CGO_ENABLED 0 was new behavior introduced in the release 2.5 alpha builds for alpine - the behavior in this PR reverts to the same mechanisms we've been using in Fabric since ~2017, with the primary difference that the base Docker image is now derived from ubuntu. Running with the ubuntu / gnu libc seems to have resolved all of the errors encountered with alpine's musl runtime.

Also:

  • Formal release binaries are static, generated with the golang cross-compiler.

  • Changing the default make target to a static build will cause major issues for the pkcs11 runtimes.

@jkneubuh
Copy link
Contributor Author

Looks like ubuntu:20.04 has been on a tighter diet than debian:slim. This is on the arm64 - also on amd64 the debian images are a few MB larger.

jkneubuh@gala fabric % docker images 
debian    stable-slim    86f9b934c377   22 hours ago    74.3MB
ubuntu    20.04          43ed104e759f   12 days ago     65.7MB
alpine    latest         d3156fec8bcb   4 weeks ago     7.46MB

Signed-off-by: Josh Kneubuhl <jkneubuh@us.ibm.com>
Copy link
Contributor

@denyeart denyeart left a comment

Choose a reason for hiding this comment

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

Looks like there is general agreement in the Discussion on the approach. Let's give it a try!

@denyeart denyeart merged commit 114695a into hyperledger:release-2.5 Jan 4, 2023
@denyeart
Copy link
Contributor

denyeart commented Jan 4, 2023

@Mergifyio backport main

@mergify
Copy link

mergify bot commented Jan 4, 2023

backport main

✅ Backports have been created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants