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

Fix reproducible linux builds #2427

Merged
merged 3 commits into from
Aug 18, 2022
Merged

Fix reproducible linux builds #2427

merged 3 commits into from
Aug 18, 2022

Conversation

niccoloraspa
Copy link
Member

@niccoloraspa niccoloraspa commented Aug 17, 2022

Closes: #2362

What is the purpose of the change

This PR:

  • Improves the main Dockerfile
  • Fixes reproducible builds in the Makefile

🐳 General Dockerfile improvements:

  • Move up the layer to download go dependencies
  • Automatically detect CosmWasm version and architecture to download the correct libwasm
  • Explicitly use the go build... command
  • Generalize runner images

🛠 Reproducible build idea:

  • Compile the binary (statically linked with CosmWasm) while building the container image
  • cp the binary out of the container and place it in the build folder

These simple changes drastically simplify the build process, and they allow very nice improvements:

  1. osmobuilder is now deprecated as the functionality is now integrated in the main Dockerfile

Removing osmobuilder would requires:

  • Changing the state-compatibility-check to build the binary using make build-reproducible-amd64
  • Update release docs
  1. Deprecating osmobuilder allows to deprecated the whole contrib/ folder, that can now be removed.

We would need to move the scripts

  1. We can now backport this build process to all vX branches to have consistent way to build the osmosisd binary across all versions (only difference would be the CosmWasm line)

  2. The generalization of the runner image allows to create different flavour of the images starting from the same Dockerfile. This can be used to:

  • Create different flavours of the osmosis Docker image (distroless, nonroot, alpine)
  • Reduce the number of Dockerfile used internally (ref: Found a bug #1679)

I will create issues for every point.

Brief Changelog

  • Improve Dockerfile
  • Fix commands in Makefile to create reproducible builds for linux

Testing and Verifying

You can build the binaries with:

make build-reproducible
# Inspected build folder 
ls build
# osmosisd-linux-amd64 osmosisd-linux-arm64

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
  • How is the feature or change documented? not applicable

@niccoloraspa niccoloraspa requested a review from a team August 17, 2022 10:42
@czarcas7ic
Copy link
Member

Looks like we will need to modify e2e tests for this change, will look into it

@niccoloraspa
Copy link
Member Author

Thanks @czarcas7ic

Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

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

Seems like its good to go now, just had to specify start command in e2e now that we took it out form the Dockerfile (I think that was the right move FWIW)

Comment on lines +16 to +17
COPY go.mod go.mod
COPY go.sum go.sum
Copy link
Member

Choose a reason for hiding this comment

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

Consider doing this in one command to minimize the number of layers. As per:
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#minimize-the-number-of-layers

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I'add in the next PR I'm making so we don't have to review again the changes

@mattverse mattverse merged commit c83936c into main Aug 18, 2022
@mattverse mattverse deleted the fix/reproducible-linux-builds branch August 18, 2022 10:27
Comment on lines +37 to +50
VERSION=$(echo $(git describe --tags) | sed 's/^v//') && \
COMMIT=$(git log -1 --format='%H') && \
go build \
-mod=readonly \
-tags "netgo,ledger,muslc" \
-ldflags "-X github.com/cosmos/cosmos-sdk/version.Name="osmosis" \
-X github.com/cosmos/cosmos-sdk/version.AppName="osmosisd" \
-X github.com/cosmos/cosmos-sdk/version.Version=$VERSION \
-X github.com/cosmos/cosmos-sdk/version.Commit=$COMMIT \
-X github.com/cosmos/cosmos-sdk/version.BuildTags='netgo,ledger,muslc' \
-w -s -linkmode=external -extldflags '-Wl,-z,muldefs -static'" \
-trimpath \
-o /osmosis/build/ \
./...
Copy link
Member

Choose a reason for hiding this comment

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

Is there a make command we should be running, instead of copying this here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if there is already a make target that will generate the go build command with all these flags.

We could for sure write one but avoiding using the Makefile in the Dockerfile was intentional. It makes the image creation more transparent and immediate.

@p0mvn p0mvn added the A:backport/v11.x backport patches to v11.x branch label Aug 18, 2022
mergify bot pushed a commit that referenced this pull request Aug 18, 2022
* Avoid hardcoding CosmWasm version

* Improve Dockerfile and enable reproducible builds

* add start cmd to e2e test

Co-authored-by: Adam Tucker <adam@osmosis.team>
(cherry picked from commit c83936c)

# Conflicts:
#	Dockerfile
#	tests/e2e/containers/containers.go
niccoloraspa added a commit that referenced this pull request Aug 18, 2022
* Fix reproducible linux builds  (#2427)

* Avoid hardcoding CosmWasm version

* Improve Dockerfile and enable reproducible builds

* add start cmd to e2e test

Co-authored-by: Adam Tucker <adam@osmosis.team>
(cherry picked from commit c83936c)

# Conflicts:
#	Dockerfile
#	tests/e2e/containers/containers.go

* Fix conflicts in Dockerfile

* Remove unconsistent file

* Replace command to get cosmwasm version

* add fix

Co-authored-by: Niccolo Raspa <6024049+niccoloraspa@users.noreply.github.com>
Co-authored-by: Niccolo Raspa <nraspa@live.it>
Co-authored-by: Adam Tucker <adam@osmosis.team>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v11.x backport patches to v11.x branch T:build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add make command for building binary w/ osmobuilder
5 participants