Skip to content
This repository has been archived by the owner on Feb 14, 2023. It is now read-only.

Suggestions to improve your Dockerfile #1

Open
ramonvanstijn opened this issue Feb 23, 2018 · 1 comment
Open

Suggestions to improve your Dockerfile #1

ramonvanstijn opened this issue Feb 23, 2018 · 1 comment

Comments

@ramonvanstijn
Copy link

Hi,

The Dockerfile can be improved in a couple of areas:

  1. Implement the best practice for writing Dockerfiles for running apt-get. The best practice says to always combine RUN apt-get update with apt-get install in the same RUN statement. For the full explanation see https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run
    For example:
RUN apt-get update && apt-get install -y  \
		wget \
	&& rm -rf /var/lib/apt/lists/*
  1. Add node_modules to .dockerignore. If you do the instruction RUN rm -rf ./node_modules; can be removed.
  2. Remove the instruction RUN cd /app;, you already used WORKDIR "/app" so you are already in /app
  3. Don't use semi-colons at the end of an instruction
  4. I wouldn't use COPY . /app, it copies everything in the current directory to /app (except what is in .dockerignore of course). The contents of /app with the current Dockerfile looks like this:
root@fa0fc7257bd6:/app# ls -latr
total 384
drwxr-xr-x   3 root root   4096 Feb 23 07:45 views
drwxr-xr-x   2 root root   4096 Feb 23 07:45 test
drwxr-xr-x   5 root root   4096 Feb 23 07:45 server
drwxr-xr-x   5 root root   4096 Feb 23 07:45 public
-rw-r--r--   1 root root   1135 Feb 23 07:45 package.json
-rw-r--r--   1 root root 266356 Feb 23 07:45 package-lock.json
drwxr-xr-x   4 root root   4096 Feb 23 07:45 multiarch-manifests
-rw-r--r--   1 root root    155 Feb 23 07:45 manifest.yml
-rw-r--r--   1 root root   2752 Feb 23 07:45 idt.js
-rw-r--r--   1 root root    672 Feb 23 07:45 gulpfile.js
drwxr-xr-x   2 root root   4096 Feb 23 07:45 docker-8
drwxr-xr-x   2 root root   4096 Feb 23 07:45 docker-6
-rw-r--r--   1 root root    556 Feb 23 07:45 cli-config.yml
drwxr-xr-x   3 root root   4096 Feb 23 07:45 chart
-rw-r--r--   1 root root   6712 Feb 23 07:45 README.md
-rw-r--r--   1 root root  13317 Feb 23 07:45 NOTICES
-rw-r--r--   1 root root  11351 Feb 23 07:45 LICENSE
-rw-r--r--   1 root root    147 Feb 23 07:45 Jenkinsfile
-rw-r--r--   1 root root    394 Feb 23 07:45 Dockerfile
-rw-r--r--   1 root root    109 Feb 23 07:45 .gitignore
drwxr-xr-x   7 root root   4096 Feb 23 07:45 .git
-rw-r--r--   1 root root      5 Feb 23 07:45 .dockerignore
drwxr-xr-x 133 root root   4096 Feb 23 07:48 node_modules
drwxr-xr-x   1 root root   4096 Feb 23 07:48 .
drwxr-xr-x   1 root root   4096 Feb 23 07:48 ..

In my view a lot of stuff that should not be in the /app directory. Of course you can update .dockerignore but it might be a better idea to create a separate folder for your code and copy that folder and individual files to /app instead.

@a-roberts
Copy link
Contributor

Thanks for the suggestion, @jkingoliver FYI - it'd be useful to incorporate this feedback into our Node.js starters and potentially to update this project with the above pointers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants