Docker & CI build: improve speed by leveraging caching #4545
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
derhuerst
commented
Apr 21, 2022
| @@ -3,6 +3,8 @@ on: | |||
| push: | |||
| branches: | |||
| - v2 | |||
| # todo: remove | |||
There was a problem hiding this comment.
This needs to be removed before merging!
derhuerst
force-pushed
the
improve-ci-speed
branch
21 times, most recently
from
5023ad2
to
0352591
Compare
derhuerst
commented
Apr 21, 2022
.github/workflows/dev-pipeline.yml
Outdated
| publish-npm: | ||
| if: github.ref == 'refs/heads/v2' | ||
| # todo: re-add this before merging! | ||
| # if: github.ref == 'refs/heads/v2' |
There was a problem hiding this comment.
This needs to be removed before merging!
.github/workflows/dev-pipeline.yml
Outdated
| PUBLISHED_PACKAGES: ${{ steps.log-parse.outputs.published-packages }} | ||
| shell: bash | ||
| # todo: re-add this before merging! | ||
| # - name: Publish shared components to npm |
There was a problem hiding this comment.
This needs to be removed before merging!
derhuerst
force-pushed
the
improve-ci-speed
branch
7 times, most recently
from
c228f58
to
93620c3
Compare
- use 1.4 syntax to support `COPY --link` commands - set NODE_ENV=production - remove $WORK build arg
…ching - If e.g. `test` has been added in a command/step, removing it in a subsequent command/step does not have a positive effect on the image size. Rather, we add it .dockerignore to stop adding it in the first place. - node_modules/.cache & /tmp/Relay* are populated by Webpack plugins.
app/server.js reads .entrypoints.main.assets in order to generate an appropriate manifest and `preload` tags, but it doesn't need the rest. This is why we turn all stats off via `all: false` and then selectively turn `entrypoints` on. Before, stats.json was 82mb, now it is ~9kb.
Even if a commit has changed `yarn.lock`, we can use an old `.yarn/cache` directory, so that we don't have to fetch 99% of all dependencies again.
This reverts 32bbfa8, which reverts 3f6e105, which apparently failed because of an unrelated problem[1]. [1] https://github.com/HSLdevcom/digitransit-ui/actions/runs/4762670497/jobs/8469259999
This probably improves favicons-webpack-plugin's performance, as node_modules won't be resolved for resolving the path.
This speeds up the webpack build within the Docker image build significantly.
derhuerst
force-pushed
the
improve-ci-speed
branch
from
b17bb1f
to
abfbfd8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Still missing:
Mount.yarn/cacheintonode-modulesthe Docker build.node-modulesstage, and check if it is actually faster than building from scratch. – much faster, I didn't expect thatyarn buildis a lot slower than before. 🤔 ( ~5m before, 10-20m after) – due to the upgradedfavicons-webpack-plugin--immutabletoyarn installcauses the build to fail, as currently both onv3andimprove-ci-speed, the checked-inyarn.lockdoes not reflect the workspace packages' versions. cc @vesameskanenProposed Changes
This PR changes the
Dockerfile& GitHub Actions dev pipeline to be faster.While an un-cached (different
yarn.lockand/or differentdigitransit-*/**and/or different commit hash) build is slightly slower, a cached one is is much faster.Review
For an unknown reason,
[email protected]crashed during the Docker build, andfavicons-webpack-plugin@^4froze, so I usedfavicons-webpack-plugin@^3.When looking at the changes, I recommend to review each commit individually, as I tried to make them as atomic and sensible as possible. Some of them also contain explanations.
Unfortunately, because the GitHub Actions system seems to lack support for YAML anchors, I added a lot of duplicated code in
.github/workflows/dev-pipeline.yml. The benefit of the repeated "use cache if exists, otherwise rebuild" patterns is that each step is effectively idempotent – it works regardless of wether the cache is available and if a previous step populated it –, with the downside being very repetitive code. Once YAML anchors are supported, we could DRY it a lot.