Skip to content

Refine Dockerfiles#250

Merged
jchristgit merged 6 commits into
masterfrom
container-refinement
Sep 20, 2019
Merged

Refine Dockerfiles#250
jchristgit merged 6 commits into
masterfrom
container-refinement

Conversation

@scragly
Copy link
Copy Markdown
Contributor

@scragly scragly commented Sep 17, 2019

I've done some refinement for the Dockerfiles. This has resulted in the following improvements:

  • Build time is reduced from 1:08 to 0:50 when tested on my machine.
  • Image size is reduced from 549.2 MB to 466.2MB
  • Image size is further reduced to 440.2MB thanks to @MarkKoz's suggestion of disabling Pip cache.

Change Details

  • Dockerfile.local has been changed to local.Dockerfile for better filetype detection.

  • Changed from interactive adduser command that resulted in logged warnings to the non-interactive useradd command.

    • --system makes it a homeless non-login user.
    • --shell has been set to avoid possible security issues.
  • uwsgi is now installed from the system package repository, as it is already built and actively updated, using the exact same version we did.

    • Since it's already built, previous build packages aren't installed.
    • Since build packages aren't installed, purging is removed (but as a note, this didn't reduce the container size anyway).
  • install_packages is used instead of apt-get as it does cleanup, handles any prompts and doesn't install recommends automatically.

  • COPY for Pipfiles and project files are now done at the same time.

  • RUN commands have been tidied to use shorter references as the entrypoints are confirmed to be for the same locations.

  • pipenv install has been moved to same RUN as setuptools

Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

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

Looks good. None of the Python dependencies need to be built with GCC, right?

Comment thread docker/app/local.Dockerfile
@scragly
Copy link
Copy Markdown
Contributor Author

scragly commented Sep 17, 2019

Correct, it was only uwsgi. I was sure to actually run the resulting containers to ensure things still worked just in case I was mistaken beforehand.

@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Sep 17, 2019

I think doing ENV PIP_NO_CACHE_DIR=false would be helpful since that'd also affect the dependencies pipenv installs, right? Could throw in a couple more to perhaps make logs nicer too:

ENV PIP_NO_CACHE_DIR=false \
    PIPENV_HIDE_EMOJIS=1 \
    PIPENV_NOSPIN=1

@scragly scragly force-pushed the container-refinement branch from d2e1b65 to 9ce9b95 Compare September 17, 2019 19:48
@scragly
Copy link
Copy Markdown
Contributor Author

scragly commented Sep 17, 2019

@MarkKoz Added the env vars suggested. It hasn't impacted size or build time, and confirmed it doesn't have any issues building and running the container with them.

Edit: I looked at wrong image details for size. It dropped to 440.2MB 🎉

@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Sep 17, 2019

It's 440 MB if the cache is disabled, 466 MB if it is enabled. Are you sure you built the right Dockerfile when testing that? I noticed you only set those environment variables for local.Dockerfile

@scragly
Copy link
Copy Markdown
Contributor Author

scragly commented Sep 17, 2019

Yep, built the correct one, but just didn't look at the right image when checking the size. You're right, it's decreased again, which is great news.

I'll add them to the other container also.

@scragly scragly requested a review from lemonsaurus September 19, 2019 16:49
@jchristgit jchristgit self-requested a review September 20, 2019 21:39
@jchristgit jchristgit self-assigned this Sep 20, 2019
@jchristgit jchristgit added the area: backend Related to internal functionality and utilities label Sep 20, 2019
Copy link
Copy Markdown
Contributor

@jchristgit jchristgit left a comment

Choose a reason for hiding this comment

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

rock solid

@jchristgit jchristgit merged commit 56576b7 into master Sep 20, 2019
@jchristgit
Copy link
Copy Markdown
Contributor

Thanks!

@jchristgit jchristgit deleted the container-refinement branch September 20, 2019 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Related to internal functionality and utilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants