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

update dockerfile and use openssl #6828

Merged
merged 4 commits into from
Jan 16, 2020
Merged

update dockerfile and use openssl #6828

merged 4 commits into from
Jan 16, 2020

Conversation

Stebalien
Copy link
Member

This PR:

  • Updates docker deps (go -> 1.13.5, tini -> 0.18.0)
  • Builds go-ipfs using openssl. We don't distribute go-ipfs with OpenSSL support by default to avoid depending on external libraries. However, we completely control the docker environment so there's no reason not to build with OpenSSL support. This significantly reduces CPU usage.
  • Explicitly set the permissions on start_ipfs so we don't inherit the defaults set by the host OS.

@Stebalien Stebalien requested a review from mburns January 16, 2020 01:19
LABEL maintainer="Steven Allen <steven@stebalien.com>"

# Install deps
RUN apt-get update && apt-get install -y \
Copy link
Member

Choose a reason for hiding this comment

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

Note: I think moving this to the first line means we'll have to explicitly run with --no-cache=true when we want to re-update those dependencies. The layer cache uses the string value of the RUN command to see if already has a layer for that.

aside from the ADD and COPY commands, cache checking does not look at the files in the container to determine a cache match. For example, when processing a RUN apt-get -y update command the files updated in the container are not examined to determine if a cache hit exists. In that case just the command string itself is used to find a match.

https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#leverage-build-cache

Copy link
Member Author

Choose a reason for hiding this comment

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

That's probably for the best, actually. When rebuilding, we usually just want to update ipfs itself.

Copy link
Member

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

👍 ran it locally, it still blends.

@Stebalien Stebalien merged commit 7e5dc58 into master Jan 16, 2020
@Stebalien Stebalien deleted the feat/improve-docker branch January 16, 2020 15:06
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.

2 participants