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

Dockerfile improvements #2876

Merged
merged 6 commits into from
Sep 28, 2022
Merged

Dockerfile improvements #2876

merged 6 commits into from
Sep 28, 2022

Conversation

niccoloraspa
Copy link
Member

Closes:

What is the purpose of the change

This PR improves our Dockerfile by:

  • Populating the .dockerignore with more folders.
  • Reducing the number of layers
  • Fixing a bug: $WASMVM was hardcoded for the checksum

Now builds are significantly faster due to better use of layer caching and previous work by @ValarDragon on the go build go mod download cache.

I have also added the relevant commands in the Makefile:

  • make docker-build builds local docker image using the default distroless runner image
  • make docker-build-distroless same as above
  • make docker-build-alpine builds local docker image using the alpine runner image
  • make docker-build-nonroot builds local docker image using the distroless-nonroot runner image

Brief Changelog

  • Reduce the number of layers in Dockerfile
  • Extend .dockerignore

Testing and Verifying

  1. Build image:
docker build -t osmosis:local .
  1. Run image:
docker run -ti osmosis:local version

You should see an empty line as output because there is no version set.
To set a version:

docker build \
  --build-arg GIT_VERSION="my-version" \
  --build-arg GIT_COMMIT="my-commit" \
  -t osmosis:local 
  . 

To get the correct values programmatically:

VERSION=$(echo $(shell git describe --tags) | sed 's/^v//')
COMMIT= $(git log -1 --format='%H')

docker build \
  --build-arg GIT_VERSION=$VERSION \
  --build-arg GIT_COMMIT=$COMMIT \
  -t osmosis:local 
  . 

The --build-arg shouldn't be needed for building an image for testing purposes.

Here I'm trying to be more verbose in explaining the internals

or more simply:

  1. Build:
make docker-build
  1. Run:
docker run -ti osmosis:local version

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
Copy link
Member Author

Only issue missing in epic #1646 is #1683

Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

Great work. Thank you @niccoloraspa . LGTM once comments are addressed

.dockerignore Show resolved Hide resolved
.dockerignore Show resolved Hide resolved
--build-arg GIT_COMMIT=$(COMMIT) \
-f Dockerfile .

docker-build-nonroot:
Copy link
Member

Choose a reason for hiding this comment

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

Does docker-build-debug need to be updated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but not because of these changes.
There is no longer a "debug" osmosis image.

I have opened this issue to remove it: #2853

niccoloraspa and others added 3 commits September 28, 2022 09:18
Co-authored-by: Roman <roman@osmosis.team>
Co-authored-by: Roman <roman@osmosis.team>
@ValarDragon ValarDragon merged commit a39c9d6 into main Sep 28, 2022
@ValarDragon ValarDragon deleted the nicco/dockerfile-improvements branch September 28, 2022 08:26
@niccoloraspa niccoloraspa added the A:backport/v12.x backport patches to v12.x branch label Sep 28, 2022
mergify bot pushed a commit that referenced this pull request Sep 28, 2022
* Replace hardcoded version in checksum verification and merge cosmwasm layers

* Avoid copying .git folder

* Add relevant commands to Makefile

* Update .dockerignore

Co-authored-by: Roman <roman@osmosis.team>

* Update .dockerignore

Co-authored-by: Roman <roman@osmosis.team>

Co-authored-by: Roman <roman@osmosis.team>
(cherry picked from commit a39c9d6)
niccoloraspa added a commit that referenced this pull request Sep 28, 2022
* Replace hardcoded version in checksum verification and merge cosmwasm layers

* Avoid copying .git folder

* Add relevant commands to Makefile

* Update .dockerignore

Co-authored-by: Roman <roman@osmosis.team>

* Update .dockerignore

Co-authored-by: Roman <roman@osmosis.team>

Co-authored-by: Roman <roman@osmosis.team>
(cherry picked from commit a39c9d6)

Co-authored-by: Niccolo Raspa <6024049+niccoloraspa@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v12.x backport patches to v12.x branch T:build T:CI
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants