ci: Simplify the caching strategy and always check the hash-sum

Signed-off-by: Rémy Coutable <remy@rymai.me>

.gitlab/ci/caching.gitlab-ci.yml

@@ -23,20 +23,17 @@ cache-workhorse:
 .cache-assets-base:
   extends:
     - .compile-assets-base
+    - .ruby-node-cache
     - .caching:rules:cache-assets
   stage: prepare
   variables:
     WEBPACK_REPORT: "false"
   script:
     - !reference [.yarn-install, script]
-    - export GITLAB_ASSETS_HASH=$(bundle exec rake gitlab:assets:hash_sum | tee assets-hash.txt)
+    - export GITLAB_ASSETS_HASH=$(bundle exec rake gitlab:assets:hash_sum | tee cached-assets-hash.txt)
     - source scripts/gitlab_component_helpers.sh
     - 'gitlab_assets_archive_doesnt_exist || { echoinfo "INFO: Exiting early as package exists."; exit 0; }'
-    # If we still use the legacy cache, we don't want to create a package from it as we don't check the sha sum in gitlab:assets:compile_with_new_strategy.
-    # This line can be removed once we stop downloading the legacy cache.
-    - rm -rf public/assets
-    - run_timed_command "bin/rake gitlab:assets:compile_with_new_strategy"
-    - run_timed_command "scripts/clean-old-cached-assets"
+    - run_timed_command "bin/rake gitlab:assets:compile"
     - run_timed_command "create_gitlab_assets_package"
     - run_timed_command "upload_gitlab_assets_package"
 

.gitlab/ci/frontend.gitlab-ci.yml

@@ -24,17 +24,15 @@
   stage: prepare
   script:
     - !reference [.yarn-install, script]
+    - export GITLAB_ASSETS_HASH=$(bin/rake gitlab:assets:hash_sum)
+    - 'echo "CACHE_ASSETS_AS_PACKAGE: ${CACHE_ASSETS_AS_PACKAGE}"'
+    # The new strategy to cache assets as generic packages is experimental and can be disabled by removing the `CACHE_ASSETS_AS_PACKAGE` variable
     - |
-      # The new strategy to cache assets as generic packages is experimental and can be disabled by removing the `CACHE_ASSETS_AS_PACKAGE` variable
       if [[ "${CACHE_ASSETS_AS_PACKAGE}" == "true" ]]; then
-        export GITLAB_ASSETS_HASH=$(bundle exec rake gitlab:assets:hash_sum | tee assets-hash.txt)
         source scripts/gitlab_component_helpers.sh
-        run_timed_command "download_and_extract_gitlab_assets" || true
-        run_timed_command "bin/rake gitlab:assets:compile_with_new_strategy"
-      else
-        run_timed_command "bin/rake gitlab:assets:compile"
+        gitlab_assets_archive_doesnt_exist || run_timed_command "download_and_extract_gitlab_assets"
       fi
-    - run_timed_command "scripts/clean-old-cached-assets"
+    - run_timed_command "bin/rake gitlab:assets:compile"
 
 compile-production-assets:
   extends:
@@ -80,6 +78,9 @@ update-assets-compile-production-cache:
     - .assets-compile-cache-push
     - .shared:rules:update-cache
   stage: prepare
+  script:
+    - !reference [compile-production-assets, script]
+    - echo -n "${GITLAB_ASSETS_HASH}" > "cached-assets-hash.txt"
   artifacts: {}  # This job's purpose is only to update the cache.
 
 update-assets-compile-test-cache:
@@ -88,8 +89,12 @@ update-assets-compile-test-cache:
     - .assets-compile-cache-push
     - .shared:rules:update-cache
   stage: prepare
+  script:
+    - !reference [compile-test-assets, script]
+    - echo -n "${GITLAB_ASSETS_HASH}" > "cached-assets-hash.txt"
   artifacts: {}  # This job's purpose is only to update the cache.
 
+# TODO: Remove this as it's duplicating update-assets-compile-*-cache
 update-yarn-cache:
   extends:
     - .default-retry

.gitlab/ci/global.gitlab-ci.yml

@@ -79,10 +79,10 @@
   policy: push  # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up.
 
 .assets-cache: &assets-cache
-  key: "assets-debian-${DEBIAN_VERSION}-ruby-${RUBY_VERSION}-node-${NODE_ENV}-v3"
+  key: "assets-debian-${DEBIAN_VERSION}-ruby-${RUBY_VERSION}-node-${NODE_ENV}-v4"
   # This list should match GITLAB_ASSETS_PATHS_LIST from scripts/gitlab_component_helpers.sh
   paths:
-    - assets-hash.txt
+    - cached-assets-hash.txt
     - app/assets/javascripts/locale/**/app.js
     - public/assets/
     - tmp/cache/assets/sprockets/
@@ -176,7 +176,7 @@
   cache:
     - *ruby-gems-cache
 
-.danger-review-cache:
+.ruby-node-cache:
   cache:
     - *ruby-gems-cache
     - *node-modules-cache
@@ -201,6 +201,7 @@
   cache:
     - *node-modules-cache
 
+# TODO: Remove this as it's duplicating .assets-compile-cache-push
 .yarn-cache-push:
   cache:
     - *node-modules-cache-push

.gitlab/ci/review.gitlab-ci.yml

@@ -72,7 +72,7 @@ review-app-test-results:
 danger-review:
   extends:
     - .default-retry
-    - .danger-review-cache
+    - .ruby-node-cache
     - .review:rules:danger
   stage: test
   needs: []

doc/development/pipelines.md

@@ -733,7 +733,7 @@ This works well for the following reasons:
    - `.static-analysis-cache`
    - `.rubocop-cache`
    - `.coverage-cache`
-   - `.danger-review-cache`
+   - `.ruby-node-cache`
    - `.qa-cache`
    - `.yarn-cache`
    - `.assets-compile-cache` (the key includes `${NODE_ENV}` so it's actually two different caches).
@@ -800,7 +800,7 @@ needed in the GitLab test suite (under `app/assets/javascripts/locale/**/app.js`
 and `tmp/cache/vue-loader/`).
 
 - If the package URL returns a 404:
-   1. It runs `bin/rake gitlab:assets:compile_with_new_strategy`, so that the GitLab assets are compiled.
+   1. It runs `bin/rake gitlab:assets:compile`, so that the GitLab assets are compiled.
    1. It then creates an archive which contains the assets and upload it [as a generic package](https://gitlab.com/gitlab-org/gitlab/-/packages/).
 - Otherwise, if the package already exists, it exits the job successfully.
 
@@ -809,7 +809,7 @@ and `compile-production-assets` jobs to:
 
 1. First download the GitLab assets generic package build and uploaded by `cache-assets:*`.
 1. If the package is retrieved successfully, assets aren't compiled.
-1. If the package URL returns a 404, the behavior doesn't change compared to the current one: the GitLab assets are compiled as part of `bin/rake gitlab:assets:compile_with_new_strategy`.
+1. If the package URL returns a 404, the behavior doesn't change compared to the current one: the GitLab assets are compiled as part of `bin/rake gitlab:assets:compile`.
 
 NOTE:
 The version of the package is the assets folders hash sum.

lib/tasks/gitlab/assets.rake

@@ -21,9 +21,24 @@ module Tasks
       EXCLUDE_PATTERNS = %w[
         app/assets/javascripts/locale/**/app.js
       ].freeze
-      MASTER_SHA256_HASH_FILE = 'master-assets-hash.txt'
-      HEAD_SHA256_HASH_FILE = 'assets-hash.txt'
       PUBLIC_ASSETS_DIR = 'public/assets'
+      HEAD_ASSETS_SHA256_HASH_ENV = 'GITLAB_ASSETS_HASH'
+      CACHED_ASSETS_SHA256_HASH_FILE = 'cached-assets-hash.txt'
+
+      def self.master_assets_sha256
+        @master_assets_sha256 ||=
+          if File.exist?(Tasks::Gitlab::Assets::CACHED_ASSETS_SHA256_HASH_FILE)
+            File.read(Tasks::Gitlab::Assets::CACHED_ASSETS_SHA256_HASH_FILE)
+          else
+            'missing!'
+          end
+      end
+
+      def self.head_assets_sha256
+        @head_assets_sha256 ||= ENV.fetch(Tasks::Gitlab::Assets::HEAD_ASSETS_SHA256_HASH_ENV) do
+          Tasks::Gitlab::Assets.sha256_of_assets_impacting_compilation(verbose: false)
+        end
+      end
 
       def self.sha256_of_assets_impacting_compilation(verbose: true)
         start_time = Time.now
@@ -58,45 +73,21 @@ namespace :gitlab do
   namespace :assets do
     desc 'GitLab | Assets | Return the hash sum of all frontend assets'
     task :hash_sum do
-      head_assets_sha256 = Tasks::Gitlab::Assets.sha256_of_assets_impacting_compilation(verbose: false)
-      puts head_assets_sha256
+      print Tasks::Gitlab::Assets.sha256_of_assets_impacting_compilation(verbose: false)
     end
 
     desc 'GitLab | Assets | Compile all frontend assets'
     task :compile do
       require_dependency 'gitlab/task_helpers'
 
-      %w[
-        gitlab:assets:compile_if_needed_with_old_strategy
-        gitlab:assets:fix_urls
-        gitlab:assets:check_page_bundle_mixins_css_for_sideeffects
-      ].each(&::Gitlab::TaskHelpers.method(:invoke_and_time_task))
-    end
-
-    desc 'GitLab | Assets | Compile all assets'
-    task :compile_if_needed_with_old_strategy do
-      # The hash file is stored as assets-hash.txt in the cache, so we rename it before generating the one for HEAD.
-      FileUtils.mv(Tasks::Gitlab::Assets::HEAD_SHA256_HASH_FILE, Tasks::Gitlab::Assets::MASTER_SHA256_HASH_FILE, force: true)
-
-      master_assets_sha256 =
-        if File.exist?(Tasks::Gitlab::Assets::MASTER_SHA256_HASH_FILE)
-          File.read(Tasks::Gitlab::Assets::MASTER_SHA256_HASH_FILE)
-        else
-          'missing!'
-        end
-
-      head_assets_sha256 = Tasks::Gitlab::Assets.sha256_of_assets_impacting_compilation.tap do |sha256|
-        File.write(Tasks::Gitlab::Assets::HEAD_SHA256_HASH_FILE, sha256)
-      end
-
-      puts "Assets SHA256 for `master`: #{master_assets_sha256}"
-      puts "Assets SHA256 for `HEAD`: #{head_assets_sha256}"
-
-      public_assets_dir_exists = Dir.exist?(Tasks::Gitlab::Assets::PUBLIC_ASSETS_DIR)
+      puts "Assets SHA256 for `master`: #{Tasks::Gitlab::Assets.master_assets_sha256.inspect}"
+      puts "Assets SHA256 for `HEAD`: #{Tasks::Gitlab::Assets.head_assets_sha256.inspect}"
 
-      if head_assets_sha256 != master_assets_sha256 || !public_assets_dir_exists
-        FileUtils.rm_r(Tasks::Gitlab::Assets::PUBLIC_ASSETS_DIR) if public_assets_dir_exists
+      if Tasks::Gitlab::Assets.head_assets_sha256 != Tasks::Gitlab::Assets.master_assets_sha256
+        FileUtils.rm_r(Tasks::Gitlab::Assets::PUBLIC_ASSETS_DIR) if Dir.exist?(Tasks::Gitlab::Assets::PUBLIC_ASSETS_DIR)
 
+        # gettext:po_to_json needs to run before rake:assets:precompile because
+        # app/assets/javascripts/locale/**/app.js are pre-compiled by Sprockets
         Gitlab::TaskHelpers.invoke_and_time_task('gettext:po_to_json')
         Gitlab::TaskHelpers.invoke_and_time_task('rake:assets:precompile')
 
@@ -111,31 +102,9 @@ namespace :gitlab do
 
         puts "Written webpack stdout log to #{log_path}" if log_path
         puts "You can inspect the webpack log here: #{ENV['CI_JOB_URL']}/artifacts/file/#{log_path}" if log_path && ENV['CI_JOB_URL']
-      end
-    end
-
-    # With this new strategy, we don't have to run `gettext:po_to_json` prior to check the assets hash sum
-    desc 'GitLab | Assets | Compile all frontend assets'
-    task :compile_with_new_strategy do
-      require_dependency 'gitlab/task_helpers'
-
-      %w[
-        gitlab:assets:compile_if_needed_with_new_strategy
-        gitlab:assets:check_page_bundle_mixins_css_for_sideeffects
-      ].each(&::Gitlab::TaskHelpers.method(:invoke_and_time_task))
-    end
-
-    desc 'GitLab | Assets | Compile all assets'
-    task :compile_if_needed_with_new_strategy do
-      unless Dir.exist?(Tasks::Gitlab::Assets::PUBLIC_ASSETS_DIR)
-        Gitlab::TaskHelpers.invoke_and_time_task('gettext:po_to_json')
-        Gitlab::TaskHelpers.invoke_and_time_task('rake:assets:precompile')
-
-        unless system('yarn webpack')
-          abort 'Error: Unable to compile webpack production bundle.'.color(:red)
-        end
 
         Gitlab::TaskHelpers.invoke_and_time_task('gitlab:assets:fix_urls')
+        Gitlab::TaskHelpers.invoke_and_time_task('gitlab:assets:check_page_bundle_mixins_css_for_sideeffects')
       end
     end
 

scripts/clean-old-cached-assets

-#!/usr/bin/env bash
-
-# Clean up cached files that are older than 4 days
-find tmp/cache/assets/sprockets/ -type f -mtime +4 -execdir rm -- "{}" \;
-find tmp/cache/webpack-dlls/ -maxdepth 1 -type d -mtime +4 -exec rm -rf -- "{}" \;
-
-du -d 0 -h tmp/cache/assets/sprockets | cut -f1 | xargs -I % echo "tmp/cache/assets/sprockets/ is currently %"
-du -d 0 -h tmp/cache/webpack-dlls | cut -f1 | xargs -I % echo "tmp/cache/webpack-dlls is currently %"

scripts/gitlab_component_helpers.sh

@@ -39,7 +39,7 @@ export GITLAB_WORKHORSE_PACKAGE="workhorse-${GITLAB_WORKHORSE_TREE}.tar.gz"
 export GITLAB_WORKHORSE_PACKAGE_URL="${API_PACKAGES_BASE_URL}/${GITLAB_WORKHORSE_FOLDER}/${GITLAB_WORKHORSE_TREE}/${GITLAB_WORKHORSE_PACKAGE}"
 
 # Assets constants
-export GITLAB_ASSETS_PATHS_LIST="assets-hash.txt app/assets/javascripts/locale/**/app.js public/assets/ tmp/cache/assets/sprockets/ tmp/cache/babel-loader/ tmp/cache/vue-loader/"
+export GITLAB_ASSETS_PATHS_LIST="cached-assets-hash.txt app/assets/javascripts/locale/**/app.js public/assets/ tmp/cache/assets/sprockets/ tmp/cache/babel-loader/ tmp/cache/vue-loader/"
 
 export GITLAB_EDITION="ee"
 if [[ "${FOSS_ONLY:-no}" = "1" ]] || [[ "${CI_PROJECT_NAME}" = "gitlab-foss" ]]; then