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

Rework docker build process #2163

Merged
merged 5 commits into from
Jun 22, 2017
Merged

Rework docker build process #2163

merged 5 commits into from
Jun 22, 2017

Conversation

Diokuz
Copy link
Contributor

@Diokuz Diokuz commented Jun 13, 2017

DO NOT MERGE

What was done

Rework dockerfile

  1. Copy only production needed stuff
  2. Install only production packages
  3. Remove all caches
  4. Move some packages to devDeps

The result is: decrease image size from ~400 mb to ~100 mb. 4x.

Requirements

  1. This pr requires some changes on CI servers: .npmrc-docker file with auth token. So, for now it must not be merger. See readme.md in this pr for details.
  2. Docker 1.13+ (2016 december version or higher). No need for --squash now.

@Diokuz
Copy link
Contributor Author

Diokuz commented Jun 13, 2017

I suddenly realized, that production packages are only koa and maybe superagent?.. anyway, 90% of "production" packages are actually not since we have no server side render.

I think that also will eliminate the need of auth. Gonna figure that out and rework pr.

@Diokuz
Copy link
Contributor Author

Diokuz commented Jun 13, 2017

Looks like moving deps decreased the size even further: to 100 mb, which is 4x times smaller! It'll be even better (I think ~80 mb) when yarn will fix its --production issue.

Also, because of yarn it is impossible to get rid of authToken, even if wings now in devDeps. yarn --production always tries to resolve all packages. Issue.

@Diokuz Diokuz force-pushed the squash branch 2 times, most recently from d1696df to 08538a0 Compare June 13, 2017 23:52
@narma
Copy link
Contributor

narma commented Jun 16, 2017

LGTM

@@ -1,7 +1,19 @@
FROM mhart/alpine-node:7.10.0
FROM mhart/alpine-node:8.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

If we update docker image base version here, can we do it for all our node projects?

@mbektimirov
Copy link
Contributor

@jmataya Can you create new npm user to use its token for CI builds?

COPY ./package.json /ashes/package.json
COPY ./yarn.lock /ashes/yarn.lock
COPY ./.npmrc-docker /ashes/.npmrc
COPY ./src/opt /ashes/src/opt
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest using one COPY command for all files to reduce number of layers in the image.

@mbektimirov mbektimirov merged commit 618f2b1 into master Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants