Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix caching being busted by moving .profile update to end of build phase #842

Merged

Conversation

flybayer
Copy link
Contributor

Successor to #840

As first discussed in Discord, this node_modules path addition in the install phase is invalidating the docker cache every single time for us in CI, causing npm modules to always be installed even when optimizing only_include_files.

Here you can see two different build runs where RUN printf '\nPATH=/app/node_modules/.bin:$PATH' >> /root/.profile invalidates the cache.

1:04:26 PM: #6 [stage-0  2/16] WORKDIR /app/
1:04:26 PM: #6 sha256:4b5e767673d5546e1b805d90dc48abd7b28b9f26e46273af3af21220c4df0b85
1:04:26 PM: #6 CACHED
1:04:26 PM: 
1:04:26 PM: #9 [stage-0  4/16] RUN nix-env -if .nixpacks/nixpkgs-293a28df6d7ff3dec1e61e37cc4ee6e6c0fb0847.nix && nix-collect-garbage -d
1:04:26 PM: #9 sha256:e48ef8aadb2a73954263ab6faf873f16c8501cf662a4147318b1736222ba099d
1:04:26 PM: #9 pulling sha256:b65bcf19d1445822c0d6f15ea82c9ed82ac1d903cfd6c1284ba7b2409a092845
1:04:26 PM: #9 pulling sha256:eacfd968fa4e8b968bd5881543c7a84ca7350e3638e9ff1550e219639c68823f
1:04:26 PM: #9 pulling sha256:88eeafe67020271b9d01ee5450f7ddac5d7485a4f1a4f0b3083cf14d0c0e3bb6
1:04:26 PM: #9 pulling sha256:b65bcf19d1445822c0d6f15ea82c9ed82ac1d903cfd6c1284ba7b2409a092845 1.0s done
1:04:26 PM: #9 pulling sha256:012ca49434089586bd23bb8e180e7a4d81bb5957d69b8a538c811820ea7c360d
1:04:28 PM: #9 pulling sha256:eacfd968fa4e8b968bd5881543c7a84ca7350e3638e9ff1550e219639c68823f 1.0s done
1:04:28 PM: #9 pulling sha256:88eeafe67020271b9d01ee5450f7ddac5d7485a4f1a4f0b3083cf14d0c0e3bb6 1.1s done
1:04:28 PM: #9 pulling sha256:012ca49434089586bd23bb8e180e7a4d81bb5957d69b8a538c811820ea7c360d 0.1s done
1:04:28 PM: #9 pulling sha256:8ed289fc5fdae9a2f05c803eb73a1239e6baa29eaaf49416ee06f6b761706359 0.1s done
1:04:28 PM: #9 pulling sha256:6e53634a717d9bc3590c2532cf9f6454db4e46db16b71431d27d92dc69931a10
1:04:30 PM: #9 pulling sha256:6e53634a717d9bc3590c2532cf9f6454db4e46db16b71431d27d92dc69931a10 3.7s done
1:04:36 PM: #9 CACHED
1:04:36 PM: 
1:04:36 PM: #10 [stage-0  5/16] RUN printf '\nPATH=/app/node_modules/.bin:$PATH' >> /root/.profile
1:04:36 PM: #10 sha256:c765a3a9df262b682adb82c261b51d1702c1afd4b71bffec13a65b41cd16b55f
1:04:38 PM: #10 DONE 3.1s
1:04:38 PM: 
1:04:38 PM: #11 [stage-0  6/16] COPY .npmrc /app/.npmrc
1:04:38 PM: #11 sha256:f3d28b6d933fdd404bf7c2d6d94b6a91c0bb0a2af85410d536f23b66c1fe297d
1:04:38 PM: #11 DONE 0.1s

--- 

12:55:47 PM: #6 [stage-0  2/16] WORKDIR /app/
12:55:47 PM: #6 sha256:4b5e767673d5546e1b805d90dc48abd7b28b9f26e46273af3af21220c4df0b85
12:55:47 PM: #6 CACHED
12:55:47 PM: 
12:55:47 PM: #8 [stage-0  3/16] COPY .nixpacks/nixpkgs-293a28df6d7ff3dec1e61e37cc4ee6e6c0fb0847.nix .nixpacks/nixpkgs-293a28df6d7ff3dec1e61e37cc4ee6e6c0fb0847.nix
12:55:47 PM: #8 sha256:96642b880335fd870dd59479723e60ae30ffa3216a103a807848de2c8dd8c3af
12:55:47 PM: #8 CACHED
12:55:47 PM: 
12:55:47 PM: #9 [stage-0  4/16] RUN nix-env -if .nixpacks/nixpkgs-293a28df6d7ff3dec1e61e37cc4ee6e6c0fb0847.nix && nix-collect-garbage -d
12:55:47 PM: #9 sha256:e46612b80c0168bc4c2f0d3c2ef872f614dc13541401c7cc238d5b448bc7a7a7
12:55:47 PM: #9 CACHED
12:55:47 PM: 
12:55:47 PM: #10 [stage-0  5/16] RUN printf '\nPATH=/app/node_modules/.bin:$PATH' >> /root/.profile
12:55:47 PM: #10 sha256:7837f6161528cfa2afe9dfa3189061392f9e28bfc2013dcf2b5b8200eb4e2f76
12:55:47 PM: #10 DONE 0.8s
12:55:47 PM: 
12:55:47 PM: #11 [stage-0  6/16] COPY .npmrc /app/.npmrc
12:55:47 PM: #11 sha256:d409936d66d70833358ca72dcd4c4efd0798a204f2eb5055c53a06c881a1a2b7
12:55:47 PM: #11 DONE 0.1s

Since RUN commands are not supported to break caching, it seems maybe it's a bug in docker?

This Fix

Since .profile is used for interactive shells, and that will only happen after start, we can move those to the end of the build phase, where busted. caching is no longer a big issue.

This PR moves all of the .profile path updates to the end of the build phase, and before the start phase.

It separates the ENV and the RUN commands for a given path update.

Before:

ENV PATH $HOME/.rbenv/bin:$PATH
RUN printf '\nPATH=$HOME/.rbenv/bin:$PATH' >> /root/.profile

After:

ENV PATH $HOME/.rbenv/bin:$PATH

// .. rest of install phase and build phase

RUN printf '\nPATH=$HOME/.rbenv/bin:$PATH' >> /root/.profile

I had to make one small tweak to the ruby install step to keep it working with this change.

Here's a full generated dockerfile for ruby + node app:

FROM ghcr.io/railwayapp/nixpacks:ubuntu-1679356985

ENTRYPOINT ["/bin/bash", "-l", "-c"]
WORKDIR /app/


COPY .nixpacks/nixpkgs-293a28df6d7ff3dec1e61e37cc4ee6e6c0fb0847.nix .nixpacks/nixpkgs-293a28df6d7ff3dec1e61e37cc4ee6e6c0fb0847.nix
RUN nix-env -if .nixpacks/nixpkgs-293a28df6d7ff3dec1e61e37cc4ee6e6c0fb0847.nix && nix-collect-garbage -d
RUN apt-get update && apt-get install -y --no-install-recommends procps libpq-dev libicu-dev git curl autoconf bison build-essential libssl-dev libyaml-dev libreadline6-dev zlib1g-dev libncurses5-dev libffi-dev libgdbm6 libgdbm-dev libdb-dev

ARG BUNDLE_GEMFILE GEM_HOME GEM_PATH MALLOC_ARENA_MAX NIXPACKS_METADATA RAILS_LOG_TO_STDOUT RAILS_SERVE_STATIC_FILES
ENV BUNDLE_GEMFILE=$BUNDLE_GEMFILE GEM_HOME=$GEM_HOME GEM_PATH=$GEM_PATH MALLOC_ARENA_MAX=$MALLOC_ARENA_MAX NIXPACKS_METADATA=$NIXPACKS_METADATA RAILS_LOG_TO_STDOUT=$RAILS_LOG_TO_STDOUT RAILS_SERVE_STATIC_FILES=$RAILS_SERVE_STATIC_FILES

# node:setup phase
# noop

# setup phase
ENV PATH $HOME/.rbenv/bin:$PATH
RUN  curl -fsSL https://github.com/rbenv/rbenv-installer/raw/HEAD/bin/rbenv-installer | bash -s stable && printf '\neval "$(~/.rbenv/bin/rbenv init -)"' >> /root/.profile && . /root/.profile && rbenv install 3.2.1 && rbenv global 3.2.1 && gem install bundler:2.4.8

# install phase
ENV PATH /usr/local/rvm/rubies/3.2.1/bin:/usr/local/rvm/gems/3.2.1/bin:/usr/local/rvm/gems/3.2.1@global/bin:$PATH
COPY . /app/.
RUN --mount=type=cache,id=muk66F5hTK4-/root/bundle/cache,target=/root/.bundle/cache bundle install

# node:install phase
ENV PATH /app/node_modules/.bin:$PATH
COPY . /app/.
RUN --mount=type=cache,id=muk66F5hTK4-/usr/local/share/cache/yarn/v6,target=/usr/local/share/.cache/yarn/v6 yarn install --frozen-lockfile

# build phase
COPY . /app/.
RUN  bundle exec rake assets:precompile

# release phase
COPY . /app/.
RUN  bundle exec rails db:migrate; bundle exec rails db:seed


RUN printf '\nPATH=$HOME/.rbenv/bin:$PATH' >> /root/.profile
RUN printf '\nPATH=/usr/local/rvm/rubies/3.2.1/bin:/usr/local/rvm/gems/3.2.1/bin:/usr/local/rvm/gems/3.2.1@global/bin:$PATH' >> /root/.profile
RUN printf '\nPATH=/app/node_modules/.bin:$PATH' >> /root/.profile



# start
COPY . /app
CMD ["bundle exec puma -t 5:5 -p ${PORT:-3000} -e ${RACK_ENV:-development}"]

@coffee-cup coffee-cup added the release/minor Author minor release label Mar 25, 2023
Copy link
Contributor

@coffee-cup coffee-cup left a comment

Choose a reason for hiding this comment

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

Works great! Thanks 😄
I'm still super confused why that RUN command was busting the cache in the first place though

@@ -111,7 +111,7 @@ impl RubyProvider {

setup.add_cmd(format!(
"curl -fsSL https://github.com/rbenv/rbenv-installer/raw/HEAD/bin/rbenv-installer | bash -s stable \
&& printf '\\neval \"$(rbenv init -)\"' >> /root/.profile \
&& printf '\\neval \"$(~/.rbenv/bin/rbenv init -)\"' >> /root/.profile \
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this have to change as well? Looks fine, just curious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently this line is launching an interactive shell, so the the updated PATH is not available there yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/minor Author minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants