From 7cb259d6925f3ad03ed53e48d9a16fb4875f3d14 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Wed, 15 Jan 2025 15:46:53 +0100 Subject: [PATCH] [CI] Refactor yarn-install composite action (#9613) # Introduction Unless I'm mistaken in the existing `yarn-install` composite action `dependencies` are cached twice. ## Cached through `setup-node` As we're providing `yarn` value to cache inputs. Setup node will cache the global `.yarn` ```yml - name: Setup Node.js and get yarn cache uses: actions/setup-node@v3 with: node-version: 18 cache: yarn ``` ## Programmatic `node_modules` caching We even manually still cache `node_modules` folder using: ```yml - name: Cache node modules id: node-modules-cache uses: actions/cache@v3 with: path: | node_modules packages/*/node_modules key: root-node_modules-${{ hashFiles('yarn.lock') }} restore-keys: root-node_modules- ``` By the way it seems like that the `yarn.lock` file is useless as the restore-key pattern omits it This could, unless I'm mistaken, leads to invalid cache restore ## Runtime The current infra results in the following install deps logs: ```sh Prepare all required actions Getting action download info Download action repository 'actions/setup-node@v3' (SHA:1a444[2](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:2)cacd436585916779262731d5b162bc6ec7) Download action repository 'actions/cache@v[3](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:3)' (SHA:f4b3439a656ba812b8cb417d2d49f9c810103092) Run ./.github/workflows/actions/yarn-install Run actions/setup-node@v3 Found in cache @ /opt/hostedtoolcache/node/18.20.5/x6[4](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:4) Environment details /usr/local/bin/yarn --version 4.4.0 /usr/local/bin/yarn config get cacheFolder /home/runner/.yarn/berry/cache /usr/local/bin/yarn config get enableGlobalCache true Cache Size: ~421 MB (441369819 B) /usr/bin/tar -xf /home/runner/work/_temp/883eae34-9b21-4684-96[5](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:5)8-cdb937f62965/cache.tzst -P -C /home/runner/work/twenty/twenty --use-compress-program unzstd Cache restored successfully Cache restored from key: node-cache-Linux-yarn-a8c18b6600f1bc685e60192f39a0a0c5c51b918829090129d3787302789547fc Run actions/cache@v3 Cache Size: ~506 MB (530305961 B) /usr/bin/tar -xf /home/runner/work/_temp/becd3767-ac80-4ca9-8d[21](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:23)-93fa61e3070c/cache.tzst -P -C /home/runner/work/twenty/twenty --use-compress-program unzstd Cache restored successfully Cache restored from key: root-node_modules-a8c18b6600f1bc685e60192f39a0a0c5c51b918829090129d378730[27](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:30)89547fc ``` Where we can see in details that a cache is hit twice. ``` Cache Size: ~421 MB (441369819 B) Cache Size: ~506 MB (530305961 B) ``` ## Suggestion We should stick to only one deps caching mechanism. Also caching the node_modules folder is not [recommended](https://github.com/actions/cache/blob/611465405cc89ab98caca2c42504d9289fb5f22e/examples.md#node---npm). That's why I would factorize our caching to the `setup-node` scope only. ## The cache-primary-key crafting Within the cache-primary-key we will inject the following metrics: - Node version, as we're caching `node_modules` directly in this way to avoid cache desync in case we upgrade node. - Hash of the lockfile, in this way if the lockfile comes to mutate we will re-install the deps from scratch, in this way no need to programmatically listen for `changed-files` on the `package.json` and the `lockfile` - Strict installation with `check-cache` and `enableHardenedMode` to true and obviously the `--immutable` flag ( note adding the `--immutable-cache` by principle even if on paper is overkill as a cache restoration and install should never occurs ) --- .../actions/yarn-install/action.yaml | 54 +++++++++++++------ 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/.github/workflows/actions/yarn-install/action.yaml b/.github/workflows/actions/yarn-install/action.yaml index d17134a61..0aff7fe55 100644 --- a/.github/workflows/actions/yarn-install/action.yaml +++ b/.github/workflows/actions/yarn-install/action.yaml @@ -1,23 +1,43 @@ name: Yarn Install +inputs: + node-version: + required: false + default: '18' runs: using: "composite" steps: - - name: Setup Node.js and get yarn cache - uses: actions/setup-node@v3 - with: - node-version: "18" - cache: yarn - - name: Cache node modules - id: node-modules-cache - uses: actions/cache@v3 - with: - path: | - node_modules - packages/*/node_modules - key: root-node_modules-${{ hashFiles('yarn.lock') }} - restore-keys: root-node_modules- - - name: Install Dependencies + - name: Cache primary key builder + id: globals shell: bash - run: yarn --immutable --check-cache - if: steps.node-modules-cache.outputs.cache-hit != 'true' \ No newline at end of file + run: | + echo "ACTION_SHELL=bash" >> "${GITHUB_OUTPUT}" + echo "CACHE_KEY_PREFIX=node_modules-cache-node-${{ inputs.node-version }}-${{ hashFiles('yarn.lock') }}" >> "${GITHUB_OUTPUT}" + echo 'PATH_TO_CACHE<> $GITHUB_OUTPUT + echo "node_modules" >> $GITHUB_OUTPUT + echo "packages/*/node_modules" >> $GITHUB_OUTPUT + echo 'EOF' >> $GITHUB_OUTPUT + - name: Setup Node.js and get yarn cache + uses: actions/setup-node@v4 + with: + node-version: ${{ inputs.node-version }} + - name: Restore node_modules + id: cache-node-modules + uses: actions/cache/restore@v4 + with: + key: ${{ steps.globals.outputs.CACHE_KEY_PREFIX }}-${{github.sha}} + restore-keys: ${{ steps.globals.outputs.CACHE_KEY_PREFIX }}- + path: ${{ steps.globals.outputs.PATH_TO_CACHE }} + - name: Install Dependencies + if: ${{ steps.cache-node-modules.outputs.cache-hit != 'true' && steps.cache-node-modules.outputs.cache-matched-key == '' }} + shell: ${{ steps.globals.outputs.ACTION_SHELL }} + run: | + yarn config set enableHardenedMode true + yarn --immutable --immutable-cache --check-cache + - name: Save cache + if: ${{ steps.cache-node-modules.outputs.cache-hit != 'true' && steps.cache-node-modules.outputs.cache-matched-key == '' }} + uses: actions/cache/save@v4 + with: + key: ${{ steps.cache-node-modules.outputs.cache-primary-key }} + path: ${{ steps.globals.outputs.PATH_TO_CACHE }} + \ No newline at end of file