-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Suggestions to improve Dockerfile - reduce image size #4986
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
Comments
The main issues we have with the docker setup is that it isn’t that easy to get started IHMO. As the cloud, config and all env are provided a posteriori, and possible with more package to install from npm/yarn, the base image as it is, isn’t mega useful. Would you have an idea on how to solve that? ie: making a base image useful enough that you can start quickly AND fully enjoy when you have complex config/cloud code |
I’m also thinking using with k8s, can we provide an easy to use pod template or helm that ‘scales’. Otherwise it’s easy to package a node app into a docker image, so we should encourage people to build a node app, and then use docker for their deployment |
I just stated using the docker image. But I do have some experience with Docker/K8s, so if I can understand the problem better, I could help. |
I mean that as there are two main ways to use parse-server with docker.
The benefits of 1 are that the whole thing is Docker oriented. No need to know about node runtimes etc... The benefits of 2 is that it’s node JS oriented and easy to run locally. Do you see what I mean? My ‘issue’ at this time is that the two options are totally valid. I tend to go to option 2 so Docker is not ‘required’ to run the projects locally. As the 2 options are totally valid, it’s ‘hard’ for someone to pick the right one, OR to properly explain it, or to make an image that is very very useful for everyone. |
I haven't been able to get #1 to work with our cloud code. I haven't put too much effort into it, but I did try just dockerizing the full repo. As I recall, I wasn't able to get my cloud code running. so far I have gone with #2 as @flovilmart suggests. |
I am interested in figuring out a Kubernetes recipe for parse-server deployment. |
I'm actually having trouble installing the npm. bcrypt needs to build for some reason, and I have python 3.x instead of 2.7. Once I solved that, it says I'm missing Xcode build tools... This is exactly why docker is great - no need to worry about environment setup. |
I can help with multi state docker build, docker-compose and CI, though I have experience with Jenkins, not TravisCI |
@barakbd you’ll Likely need Xcode installed and the command line tools. (Which can be installed through https://github.com/KrauseFx/xcode-install/blob/master/README.md) |
I think that for our on-boarding documentation, the docker solution should be a workflow that uses the prebuilt parseplatform/parse-server docker. I played around with this a bit over the last few days and made more successful progress than I have to date. I set up three containers: mongo, parse-server, and parse-dashboard. In the case of the dashboard, I mounted a volume with the dashboard config in it. In the case of parse server, I created a volume with cloud code in it and passed the rest of the config via command line arguments to docker run. I put a package.json in the cloud directory with lodash as a dependency and used lodash. Here's the cloud code which all worked as expected (I could verify the lodash use in the dashboard and I could see the 'hi mom' in the docker logs):
So this to me is a good proof of concept that docker images can work and is reasonable. The next step for me is to try and put it all together in k8s to see how easy it is to get running locally in minikube. @barakbd. Do you want to open a pr with your docker file changes? I haven't tested with those yet, but I will. |
I want to test first. Do you know if the docker image is built from an express app that imports parse-server or is it pure parse-server? |
@barakbd the docker image that we auto build is using the parse-server CLI so it's creating it's own express server. Which is basically:
|
By the way, this is a cleaner way for gitignore/dockerignore - https://stackoverflow.com/questions/987142/make-gitignore-ignore-everything-except-a-few-files
|
Just to confirm why I would like to reduce the image size:
Docker image size is 897 mb (docker images)
Am I correct? |
Do we need really to rewrite that now? What's the benefit?
This makes sense as we're building from a 'fat source' BUT, it also has some dependencies. If by inadvertence the dev packages are copied into the docker image, this would explain why it's so big. |
Also the base image is plain node. We should use "FROM node:carbon" which is smaller. I just forked and and will try to build locally with an updated Dockerfile. |
I’m not convinced in the utility of tini / pid 1 reaping problem. The server doesn’t spawn any child processes, and should not spawn any. I don’t expect the process to restart on crashes, this should be handled externally, through health checks. |
If your entrypoint is calling "node" directly, then no need. But if you are starting "node" through "npm", which we should not, then you should have tini. The improvement to the ignore file is not urgent, but good to note. So for building locally I need to fork the project, clone master locally, make changes to Dockerfile, and run docker build? |
There are other issues using npm, as the entrypoint, as improper forwarding of certain signals to the node process. So always we should use node and not tini. |
It doesn't seem that TravisCI is building and pushing to DockerHub. Is this correct? Are you doing this manually? |
everything is done automatically with docker hub, yes.
Well, that you should know, what does your docker file look like? |
I successfully built a smaller image using the multi-stage Dockerfile below. Without adding python, make and g++, which are required for bcrypt 3.x, image size is 167 mb. stage 3 - test - is commented out because tests were failing, specfically the pretest script which runs lint.
|
Do you have more info? Yes it’s likely you’ll need to copy the built JS, and not work with the base JS from the src folder as no node runtime understands the source. As for a target user, how would he use it? How is it better/easier than what we have now? I seems very complex, and the last thing I want to add is complexity in both maintenance and usage. |
This is standard now, as multi-stage builds ensures unit tests pass before image is built. See the first 2 links I have put in the top of the Dokcerfile. |
Well, you need the dev dependencies to run the tests. As 1: you need to build the lib folder and 2: you need jasmine et al. |
Stage 3 which runs the tests has all those. Please see the Dockerfile. Stage 3 starts From dependencies, so the base image for stage 3 has all those. |
The target user didn't need the Dockerfile. It is just for building a smaller, more reliable image which the target user can enjoy. |
Dependencies is npm install —only-production |
Look at the last line. It runs npm install again, without prod flag. The logic for this is in the article from Codefresh. |
Can you show me where you build before testing in the travis.yaml for? |
Well, hahaha :) it’s not because someone write something in an article that it works. I find it odd to run 2 npm installs. The toolchain and environment is completely different from tests to production. If anything, the test image should mount the lib folder from the prod image, not the opposite |
The build of the lib folder is done on npm install. |
Obviously. This why I am trying to understand where you build before testing and which files/folders are needed for prod. |
I don't see it in postinstall script or postinstall.js. what an I missing? |
That's a bad idea, because you're twisting the node project in order to make it run in docker.
Why do you care? It's built on docker hub. Seems to be a premature optimizaation which leads to bad things. Also: I though the goal was to:
It seems that the current solution you're exploring, while perhaps going there, isn't bringing us any closer to the goal. |
there is prepare and post install: https://github.com/parse-community/parse-server/blob/master/package.json#L84 AND perhpas the package.json docs may help you as well: https://docs.npmjs.com/files/package.json#devdependencies |
Have you looked at the error that I shared from the test stage? The test stage does have the /lib directory as a full npm install is run previously (stage 2). I tested npm install, and since there is a build step, 2 npm installs are required. The devDependencies add about 300 mb. When npm install --production is run, the lib directory is not created (obviously).
|
You are probably missing Many files, have a look at the package.json and the files directive that declares the files that need to be copied over when distributing. Now, the order of operations don’t make any sense as no tests are running. It would make more sense to run install —prod in the base image What i really don’t like with this approach of copying things over is that if requires manual maintenance if we add more things. |
Also, in the sense of ‘all articles may not be right’ moving the node_modules folder is unnecessary, you can use |
I found the error with the test stage. Apparently flow-bin from npm doesn't work in alpine version. Solution is here (posted 7 days ago): facebook/flow#3649.
However I cannot run
This is exactly what the dockerfile is doing (see above)
Yes, but then you would also have to rm -rf src/ and other items to reduce image size. In any case it is a manual process. I am testing an updated Dockerfile. Do we want to create it with bcrypt version 3.x? according to npm (https://www.npmjs.com/package/bcrypt) this would require node version 10. Is there any issue in using node version 10 as the base image? |
This seems to be overcomplicated / overengineered for me at this point. Also, npm should run prepare etc... if it isn’t that doesn’t fall into the ‘expected behaviours’, isn’t the system supposed to finally run the process as an unprivileged user? Do you have any docs supporting the claims that running as ‘root’ npm won’t run the normal flow? Node 10 is not supported nor recommended for productions. AND I don’t want to maintain / support many docker images. Ideally I want to minimize the amount of support. Feel free to open a Pr when ready |
The src folder is 1.2Mb... Which is pale in comparison to the node_modules folder... |
Docker -node best practices - non-root user npm scripts not running in docker as root: |
This is probably why the current docker image uses:
I've built the image based upon our current one:
With minimal changes:
The final size is: 378MB which is not bad and actually very simple to maintain, understand and run. This still doesn't help with usability in the context of compose or k8s which is for me the main issue. As you originally suggested, a slimmed down image should be used (:YAY:) Can we move forward with that and not keep going with nit picking megabytes? Closing the issue pending as all required elements are available for a PR. |
OK. I will create a PR soon. |
@acinader Can you have a look at the following repo? It is a docker-compose file that runs mongo and parse-server. mongo-3.6 | 2018-09-12T23:45:00.583+0000 I ACCESS [conn1] SCRAM-SHA-1 authentication failed for admin on undefined from client 172.25.0.3:45108 ; UserNotFound: Could not find user admin@undefined
mongo-3.6 | 2018-09-12T23:45:00.593+0000 I NETWORK [conn1] end connection 172.25.0.3:45108 (0 connections now open)
parse-server | warn: Unable to ensure uniqueness for usernames: MongoError: Authentication failed.
parse-server | at /parse-server/node_modules/mongodb-core/lib/connection/pool.js:581:63
parse-server | at authenticateStragglers (/parse-server/node_modules/mongodb-core/lib/connection/pool.js:504:16)
parse-server | at Connection.messageHandler (/parse-server/node_modules/mongodb-core/lib/connection/pool.js:540:5)
parse-server | at emitMessageHandler (/parse-server/node_modules/mongodb-core/lib/connection/connection.js:310:10)
parse-server | at Socket.<anonymous> (/parse-server/node_modules/mongodb-core/lib/connection/connection.js:453:17)
parse-server | at emitOne (events.js:116:13)
parse-server | at Socket.emit (events.js:211:7)
parse-server | at addChunk (_stream_readable.js:263:12)
parse-server | at readableAddChunk (_stream_readable.js:250:11)
parse-server | at Socket.Readable.push (_stream_readable.js:208:10) Here is my repo with the updated dockerfile to build parse-server using multi-stage: |
In your env file it seems that the mongodb url is malformed. Unless ‘mongo’ is a valid host. |
In the Dockerfile "mongo" is the name of the service. Actually there is no need to even specify a port so it could even be: The only issue with using mongo container is trying to figure out how to create a db on docker run. I am able to connect the the db using a mongo client from the host. |
Yes. You need a specific database to connect to. As all the examples show in the tests, ci and documentations. |
@barakbd i have no experience with docker composer, so I took a look, but not sure what I am looking at. Just fyi. As @flovilmart points out, your error is indicative of not being able to connect to the database. |
Is your feature request related to a problem? Please describe.
Use node:carbon-alpine base image to reduce image size (unless you you really need the full size node image)
Volumes:
No need to mkdir + VOLUME. Also, it is possible to specify multi in one line
https://docs.docker.com/engine/reference/builder/#volume
Non-root user:
https://github.com/nodejs/docker-node/blob/master/docs/BestPractices.md#non-root-user
Entrypoint - use node and not npm
https://github.com/nodejs/docker-node/blob/master/docs/BestPractices.md#docker-run
Multi stage Dockerfile - reduce image size by building final image without devDependencies:
Docker multi stage - https://docs.docker.com/develop/develop-images/multistage-build/
TravisCI supported - Upgrade Docker for multi-stage build support travis-ci/travis-ci#8181
Suggestted Dockerfile
The text was updated successfully, but these errors were encountered: