Skip to content

[18.09 backport] use official shellcheck 0.6.0, and don't patch Dockerfiles in CI - #1776

Merged
andrewhsu merged 3 commits into
docker:18.09from
thaJeztah:18.09_backport_upgrade_shellcheck_0.6.0
Mar 28, 2019
Merged

[18.09 backport] use official shellcheck 0.6.0, and don't patch Dockerfiles in CI#1776
andrewhsu merged 3 commits into
docker:18.09from
thaJeztah:18.09_backport_upgrade_shellcheck_0.6.0

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

backports for the 18.09 branch of:

thought it'd be good to keep this branch a bit up-to-date for CI

When building the Dockerfiles for development, those images are mainly used to
create a reproducible build-environment. The source code is bind-mounted into
the image at runtime; there is no need to create an image with the actual
source code, and copying the source code into the image would lead to a new
image being created for each code-change (possibly leading up to many "dangling"
images for previous code-changes).
However, when building (and using) the development images in CI, bind-mounting
is not an option, because the daemon is running remotely.
To make this work, the circle-ci script patched the Dockerfiles when CI is run;
adding a `COPY` to the respective Dockerfiles.
Patching Dockerfiles is not really a "best practice" and, even though the source
code does not and up in the image, the source would still be _sent_ to the daemon
for each build (unless BuildKit is used).
This patch updates the makefiles, circle-ci script, and Dockerfiles;
- When building the Dockerfiles locally, pipe the Dockerfile through stdin.
Doing so, prevents the build-context from being sent to the daemon. This speeds
up the build, and doesn't fill up the Docker "temp" directory with content that's
not used
- Now that no content is sent, add the COPY instructions to the Dockerfiles, and
remove the code in the circle-ci script to "live patch" the Dockerfiles.
Before this patch is applied (with cache):
```
$ time make -f docker.Makefile build_shell_validate_image
docker build -t docker-cli-shell-validate -f ./dockerfiles/Dockerfile.shellcheck .
Sending build context to Docker daemon 41MB
Step 1/2 : FROM debian:stretch-slim
...
Successfully built 81e14e8ad856
Successfully tagged docker-cli-shell-validate:latest
2.75 real 0.45 user 0.56 sys
```
After this patch is applied (with cache)::
```
$ time make -f docker.Makefile build_shell_validate_image
cat ./dockerfiles/Dockerfile.shellcheck | docker build -t docker-cli-shell-validate -
Sending build context to Docker daemon 2.048kB
Step 1/2 : FROM debian:stretch-slim
...
Successfully built 81e14e8ad856
Successfully tagged docker-cli-shell-validate:latest
0.33 real 0.07 user 0.08 sys
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 166856a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This patch switches the shellcheck image to use the official image
from Docker Hub.
Note that this does not yet update shellcheck to the latest version (v0.5.x);
Shellcheck v0.4.7 added some new checks, which makes CI currently fail, so will
be done in a follow-up PR. Instead, the v0.4.6 version is used in this PR, which
is closest to the same version as was installed in the image before this change;
```
docker run --rm docker-cli-shell-validate shellcheck --version
ShellCheck - shell script analysis tool
version: 0.4.4
license: GNU General Public License, version 3
website: http://www.shellcheck.net
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 388646e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit ff107b3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

@andrewhsuandrewhsu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@andrewhsu
andrewhsu merged commit 207ff08 into docker:18.09Mar 28, 2019
@thaJeztah
thaJeztah deleted the 18.09_backport_upgrade_shellcheck_0.6.0 branch March 28, 2019 20:11
Sign up for freeto 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.

3 participants

@thaJeztah@andrewhsu@GordonTheTurtle