Skip to content

Fix Docker setup - #98

Open
mateusdeap wants to merge 7 commits into
mainfrom
fix-docker-setup
Open

Fix Docker setup#98
mateusdeap wants to merge 7 commits into
mainfrom
fix-docker-setup

Conversation

@mateusdeap

Copy link
Copy Markdown
Member

What is this PR:

  • Bug fix
  • Feature
  • Chore

Description:
This updates the docker setup for the audit app. Changes docker-compose.yml to build web_next and adds several improvements that help avoid issues with changing dependencies, undesired cache busting when changing application code and also implements caching when running bundle install

The changes that are actually needed are only the ones configuring web_next to get built from the Dockerfile and removing the version pinning on the bundle install command. The rest is just improvements that I'm suggesting here

How has this been tested?

  • Automated tests
  • Manual tests

What manual tests have been run?
Delete all your local images of for the audit app and attempt to setup the app as in the README.

The rest are standard docker features that require no testing, as long as images build successfully and docker compose up spins up both containers without issues.

This just ensures BuildKit uses the latest version of Dockerfile syntax
This lets us drop the options in the command and also guarantees these
options are set when running bundler commands inside the container.
Using an absolute path for BUNDLE_GEMFILE also guarantees bundler
commands work properly in subdirectories inside the container
This prevents cache busting the entrypoint copy on changes to the
application code.
service_started doesn't prevent race conditions since the web service
may have started and db operations might still be working. Rails
provides an `/up` endpoint which guarantees the _app_ is up. Only after
that is it safe to run the entrypoint script
Attempting to reuse the web image presumes it already exists locally.
Furthermore, it's better to have them build separately since their
dependencies can be different
Comment thread Dockerfile
# Remove this block (and Gemfile.next / Gemfile.next.lock) once the upgrade lands.
COPY Gemfile.next Gemfile.next.lock ./
RUN BUNDLE_GEMFILE=Gemfile.next bundle _2.2.21_ install --jobs=4 --retry=3
RUN --mount=type=cache,target=/usr/local/bundle/cache,sharing=locked \

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.

this Dockerfile is installing both current and next gems but we don't share the image between the web and web-next services

we can instead use a build ARG and only install the gems for that service to speed this up, check the "Dual-Boot Rails" section in this blogpost https://www.fastruby.io/blog/dual-boot-and-docker.html

Comment thread Dockerfile

COPY docker/entrypoint.sh /usr/local/bin/entrypoint.sh
RUN chmod +x /usr/local/bin/entrypoint.sh
COPY . .

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.

I think we can skip copying all the code inside the container since we are using a mounted volume anyway that replaces that with the actual code?

Comment thread Dockerfile
BUNDLE_JOBS=4 \
BUNDLE_RETRY=3

COPY Gemfile Gemfile.lock .ruby-version ./

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.

do we need the .ruby-version file here? the version is already defined by the base image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants