KubeNow
KubeNow copied to clipboard
Dockerfile best practices need to be applied
Docker file install needs to be splitted in multiple commands. As it is now it is hard to read. https://github.com/kubenow/KubeNow/blob/master/Dockerfile#L26-L61
Yes but otherwise it creates more layers and get substantially larger... In the future Travis might support the new (currently experimental) command docker --squash
Docker squash is now in Travis: https://github.com/travis-ci/travis-ci/issues/7380
Unfortunately it is still part of the "experimental" features not enabled on travis - I already did try to enable it with env-vars but never got it to work.
Is there any thread we can follow, so we kwon when it will become available?
Well, if you want to have your stuff readable and in one RUN
, simply follow the dockerfile best practices. There is no need to split it and squash it.
RUN true \
&& command \
&& apt install -y \
aIndented \
bAnd \
cSORTED \
dPackage \
eList \
&& command \
&& true
- Group (read: indent) the commands correct.
- Pack the arguments into a new line (and if the arguments are a set, sort it!)
- I usually add
true
as the first and last command. This allows me to insert commands at the beginning/end without touching the previous first/last command and not polluting the git history
The stuff with the
`# this is a comment`
is genius!
I'm submitting a PR. Give me some time pls.
That's a good remark 🙂
So, I also reviewed this part of the dockerfile (But I'm not touching these with the PR):
- Also, there should be an
export DEBIAN_FRONTEND=noninteractive
in everyRUN
statement involving apt. (Do not export it in anENV
). - The multiple
ENV
s should also be grouped together. - If you install
apt-transport-https
, I think you should also add the repository withhttps
instead ofhttp
. - As you're downloading from
https
, it might be also good to make sure, thatca-certificates
package is installed and up to date.
@mcapuccini Thanks for merging #312. But KIM the other points, I also have noted in my previous comment here.
Yep, reopening the issue and renaming it 🙂