From c58e0ac3a1f7b0edfa19d4cefaba2fd69d332397 Mon Sep 17 00:00:00 2001 From: Stan Hu <stanhu@gmail.com> Date: Tue, 15 Nov 2022 22:46:22 +0000 Subject: [PATCH] Revert assets image hash optimization This reverts merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/103247 because CNG and Omnibus depend on pulling `registry.gitlab.com/gitlab-org/gitlab/gitlab-assets-ee:master`, and they are not prepared to use the new assets hash. --- .gitlab/ci/build-images.gitlab-ci.yml | 11 +-- .gitlab/ci/frontend.gitlab-ci.yml | 5 +- .../ci/package-and-test/main.gitlab-ci.yml | 53 +++++-------- .gitlab/ci/qa.gitlab-ci.yml | 2 - .gitlab/ci/review-apps/main.gitlab-ci.yml | 16 ++-- .gitlab/ci/review.gitlab-ci.yml | 2 - Dockerfile.assets | 2 +- scripts/build_assets_image | 78 ++++--------------- scripts/trigger-build.rb | 3 +- scripts/utils.sh | 17 +--- spec/scripts/trigger-build_spec.rb | 22 ++++++ 11 files changed, 71 insertions(+), 140 deletions(-) diff --git a/.gitlab/ci/build-images.gitlab-ci.yml b/.gitlab/ci/build-images.gitlab-ci.yml index bb18ba12c4b07..3c7056a92c1c8 100644 --- a/.gitlab/ci/build-images.gitlab-ci.yml +++ b/.gitlab/ci/build-images.gitlab-ci.yml @@ -18,7 +18,7 @@ build-qa-image: - ./scripts/build_qa_image # This image is used by: -# - The `CNG` downstream pipelines (we pass the image tag via the `review-build-cng` job): https://gitlab.com/gitlab-org/gitlab/-/blob/c34e0834b01cd45c1f69a01b5e38dd6bc505f903/.gitlab/ci/review-apps/main.gitlab-ci.yml#L69 +# - The `CNG` pipelines (via the `review-build-cng` job): https://gitlab.com/gitlab-org/build/CNG/-/blob/cfc67136d711e1c8c409bf8e57427a644393da2f/.gitlab-ci.yml#L335 # - The `omnibus-gitlab` pipelines (via the `e2e:package-and-test` job): https://gitlab.com/gitlab-org/omnibus-gitlab/-/blob/dfd1ad475868fc84e91ab7b5706aa03e46dc3a86/.gitlab-ci.yml#L130 build-assets-image: extends: @@ -27,10 +27,7 @@ build-assets-image: stage: build-images needs: ["compile-production-assets"] script: + # TODO: Change the image tag to be the MD5 of assets files and skip image building if the image exists + # We'll also need to pass GITLAB_ASSETS_TAG to the trigerred omnibus-gitlab pipeline similarly to how we do it for trigerred CNG pipelines + # https://gitlab.com/gitlab-org/gitlab/issues/208389 - run_timed_command "scripts/build_assets_image" - artifacts: - expire_in: 7 days - paths: - # The `cached-assets-hash.txt` file is used in `review-build-cng-env` (`.gitlab/ci/review-apps/main.gitlab-ci.yml`) - # to pass the assets image tag to the CNG downstream pipeline. - - cached-assets-hash.txt diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 4d120de277a2b..6be77fe52c814 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -28,7 +28,6 @@ fi fi - assets_compile_script - - echo -n "${GITLAB_ASSETS_HASH}" > "cached-assets-hash.txt" compile-production-assets: extends: @@ -44,7 +43,6 @@ compile-production-assets: # These assets are used in multiple locations: # - in `build-assets-image` job to create assets image for packaging systems # - GitLab UI for integration tests: https://gitlab.com/gitlab-org/gitlab-ui/-/blob/e88493b3c855aea30bf60baee692a64606b0eb1e/.storybook/preview-head.pug#L1 - - cached-assets-hash.txt - public/assets/ - "${WEBPACK_COMPILE_LOG_PATH}" when: always @@ -75,6 +73,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: diff --git a/.gitlab/ci/package-and-test/main.gitlab-ci.yml b/.gitlab/ci/package-and-test/main.gitlab-ci.yml index 9d07cadc04cc0..f0bf79f009d2b 100644 --- a/.gitlab/ci/package-and-test/main.gitlab-ci.yml +++ b/.gitlab/ci/package-and-test/main.gitlab-ci.yml @@ -38,6 +38,23 @@ stages: extends: - .gitlab-qa-install +.omnibus-env: + variables: + BUILD_ENV: build.env + script: + - | + SECURITY_SOURCES=$([[ ! "$CI_PROJECT_NAMESPACE" =~ ^gitlab-org\/security ]] || echo "true") + echo "SECURITY_SOURCES=${SECURITY_SOURCES:-false}" > $BUILD_ENV + echo "OMNIBUS_GITLAB_CACHE_UPDATE=${OMNIBUS_GITLAB_CACHE_UPDATE:-false}" >> $BUILD_ENV + for version_file in *_VERSION; do echo "$version_file=$(cat $version_file)" >> $BUILD_ENV; done + echo "OMNIBUS_GITLAB_RUBY3_BUILD=${OMNIBUS_GITLAB_RUBY3_BUILD:-false}" >> $BUILD_ENV + echo "OMNIBUS_GITLAB_CACHE_EDITION=${OMNIBUS_GITLAB_CACHE_EDITION:-GITLAB}" >> $BUILD_ENV + echo "Built environment file for omnibus build:" + cat $BUILD_ENV + artifacts: + reports: + dotenv: $BUILD_ENV + .update-script: script: - export QA_COMMAND="bundle exec gitlab-qa Test::Omnibus::UpdateFromPrevious $RELEASE $GITLAB_VERSION $UPDATE_TYPE -- $QA_RSPEC_TAGS $RSPEC_REPORT_OPTS" @@ -91,42 +108,9 @@ dont-interrupt-me: trigger-omnibus-env: extends: + - .omnibus-env - .rules:omnibus-build stage: .pre - needs: - # We need this job because we need its `cached-assets-hash.txt` artifact, so that we can pass the assets image tag to the downstream omnibus-gitlab pipeline. - - pipeline: $PARENT_PIPELINE_ID - job: build-assets-image - variables: - BUILD_ENV: build.env - before_script: - - | - # This is duplicating the function from `scripts/utils.sh` since that file can be included in other projects. - function assets_image_tag() { - local cache_assets_hash_file="cached-assets-hash.txt" - - if [[ -n "${CI_COMMIT_TAG}" ]]; then - echo -n "${CI_COMMIT_REF_NAME}" - elif [[ -f "${cache_assets_hash_file}" ]]; then - echo -n "assets-hash-$(cat ${cache_assets_hash_file} | cut -c1-10)" - else - echo -n "${CI_COMMIT_SHA}" - fi - } - script: - - | - SECURITY_SOURCES=$([[ ! "$CI_PROJECT_NAMESPACE" =~ ^gitlab-org\/security ]] || echo "true") - echo "SECURITY_SOURCES=${SECURITY_SOURCES:-false}" > $BUILD_ENV - echo "OMNIBUS_GITLAB_CACHE_UPDATE=${OMNIBUS_GITLAB_CACHE_UPDATE:-false}" >> $BUILD_ENV - for version_file in *_VERSION; do echo "$version_file=$(cat $version_file)" >> $BUILD_ENV; done - echo "OMNIBUS_GITLAB_RUBY3_BUILD=${OMNIBUS_GITLAB_RUBY3_BUILD:-false}" >> $BUILD_ENV - echo "OMNIBUS_GITLAB_CACHE_EDITION=${OMNIBUS_GITLAB_CACHE_EDITION:-GITLAB}" >> $BUILD_ENV - echo "GITLAB_ASSETS_TAG=$(assets_image_tag)" >> $BUILD_ENV - echo "Built environment file for omnibus build:" - cat $BUILD_ENV - artifacts: - reports: - dotenv: $BUILD_ENV trigger-omnibus: extends: .rules:omnibus-build @@ -144,7 +128,6 @@ trigger-omnibus: GITLAB_SHELL_VERSION: $GITLAB_SHELL_VERSION GITLAB_WORKHORSE_VERSION: $GITLAB_WORKHORSE_VERSION GITLAB_VERSION: $CI_COMMIT_SHA - GITLAB_ASSETS_TAG: $GITLAB_ASSETS_TAG IMAGE_TAG: $CI_COMMIT_SHA TOP_UPSTREAM_SOURCE_PROJECT: $CI_PROJECT_PATH SECURITY_SOURCES: $SECURITY_SOURCES diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index f6668d7864ed7..8740a5fe17dfe 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -74,8 +74,6 @@ e2e:package-and-test: - build-qa-image - e2e-test-pipeline-generate variables: - # This is needed by `trigger-omnibus-env` (`.gitlab/ci/package-and-test/main.gitlab-ci.yml`). - PARENT_PIPELINE_ID: $CI_PIPELINE_ID SKIP_MESSAGE: Skipping package-and-test due to mr containing only quarantine changes! RELEASE: "${REGISTRY_HOST}/${REGISTRY_GROUP}/build/omnibus-gitlab-mirror/gitlab-ee:${CI_COMMIT_SHA}" GITLAB_QA_IMAGE: "${CI_REGISTRY_IMAGE}/gitlab-ee-qa:${CI_COMMIT_SHA}" diff --git a/.gitlab/ci/review-apps/main.gitlab-ci.yml b/.gitlab/ci/review-apps/main.gitlab-ci.yml index 8b49327b9f48f..0c3fd847c9914 100644 --- a/.gitlab/ci/review-apps/main.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/main.gitlab-ci.yml @@ -34,24 +34,18 @@ review-build-cng-env: - .review:rules:review-build-cng image: ${GITLAB_DEPENDENCY_PROXY_ADDRESS}ruby:3.0-alpine3.13 stage: prepare - needs: - # We need this job because we need its `cached-assets-hash.txt` artifact, so that we can pass the assets image tag to the downstream CNG pipeline. - - pipeline: $PARENT_PIPELINE_ID - job: build-assets-image - variables: - BUILD_ENV: build.env + needs: [] before_script: - source ./scripts/utils.sh - install_gitlab_gem script: - - 'ruby -r./scripts/trigger-build.rb -e "puts Trigger.variables_for_env_file(Trigger::CNG.new.variables)" > $BUILD_ENV' - - echo "GITLAB_ASSETS_TAG=$(assets_image_tag)" >> $BUILD_ENV - - cat $BUILD_ENV + - 'ruby -r./scripts/trigger-build.rb -e "puts Trigger.variables_for_env_file(Trigger::CNG.new.variables)" > build.env' + - cat build.env artifacts: reports: - dotenv: $BUILD_ENV + dotenv: build.env paths: - - $BUILD_ENV + - build.env expire_in: 7 days when: always diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index b185e8deb7066..35df4de65139d 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -29,8 +29,6 @@ start-review-app-pipeline: # They need to be explicitly passed on to the child pipeline. # https://docs.gitlab.com/ee/ci/pipelines/multi_project_pipelines.html#pass-cicd-variables-to-a-downstream-pipeline-by-using-the-variables-keyword variables: - # This is needed by `review-build-cng-env` (`.gitlab/ci/review-apps/main.gitlab-ci.yml`). - PARENT_PIPELINE_ID: $CI_PIPELINE_ID SCHEDULE_TYPE: $SCHEDULE_TYPE DAST_RUN: $DAST_RUN SKIP_MESSAGE: Skipping review-app due to mr containing only quarantine changes! diff --git a/Dockerfile.assets b/Dockerfile.assets index ba69a614e888f..403d16cc4ab1b 100644 --- a/Dockerfile.assets +++ b/Dockerfile.assets @@ -1,4 +1,4 @@ # Simple container to store assets for later use FROM scratch -COPY public/assets /assets/ +ADD public/assets /assets/ CMD /bin/true diff --git a/scripts/build_assets_image b/scripts/build_assets_image index 7482a170fe707..8aa6526061ada 100755 --- a/scripts/build_assets_image +++ b/scripts/build_assets_image @@ -1,82 +1,36 @@ -#!/bin/sh - -. scripts/utils.sh - # Exit early if we don't want to build the image -if [ "${BUILD_ASSETS_IMAGE}" != "true" ] +if [[ "${BUILD_ASSETS_IMAGE}" != "true" ]] then exit 0 fi -get_repository_id() { - repository_name="${1}" - repositories_url="${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/registry/repositories" - - curl --header "PRIVATE-TOKEN: ${PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE}" "${repositories_url}" | jq "map(select(.name == \"${repository_name}\")) | .[0].id" -} - # Generate the image name based on the project this is being run in ASSETS_IMAGE_NAME="gitlab-assets-ce" - # `dev.gitlab-org` still has gitlab-ee. -if [ "${CI_PROJECT_NAME}" = "gitlab" ] || [ "${CI_PROJECT_NAME}" = "gitlab-ee" ] +if [[ "${CI_PROJECT_NAME}" == "gitlab" ]] || [[ "${CI_PROJECT_NAME}" == "gitlab-ee" ]] then ASSETS_IMAGE_NAME="gitlab-assets-ee" fi -ASSETS_IMAGE_PATH="${CI_REGISTRY}/${CI_PROJECT_PATH}/${ASSETS_IMAGE_NAME}" -COMMIT_ASSETS_HASH_TAG="$(assets_image_tag)" -COMMIT_ASSETS_HASH_DESTINATION="${ASSETS_IMAGE_PATH}:${COMMIT_ASSETS_HASH_TAG}" +ASSETS_IMAGE_PATH=${CI_REGISTRY}/${CI_PROJECT_PATH}/${ASSETS_IMAGE_NAME} -DESTINATIONS="--destination=${COMMIT_ASSETS_HASH_DESTINATION}" - -SKIP_ASSETS_IMAGE_BUILDING_IF_ALREADY_EXIST="true" - -# Also tag the image with GitLab version, if running on a tag pipeline -# (and thus skip the performance optimization in that case), for back-compatibility. -if [ -n "${CI_COMMIT_TAG}" ]; then - COMMIT_REF_NAME_DESTINATION="${ASSETS_IMAGE_PATH}:${CI_COMMIT_REF_NAME}" - DESTINATIONS="$DESTINATIONS --destination=$COMMIT_REF_NAME_DESTINATION" - SKIP_ASSETS_IMAGE_BUILDING_IF_ALREADY_EXIST="false" -fi +mkdir -p assets_container.build/public +cp -r public/assets assets_container.build/public/ +cp Dockerfile.assets assets_container.build/ -# The auto-deploy branch process still fetch assets image tagged with $CI_COMMIT_SHA, -# so we need to push the image with it (and thus skip the performance optimization in that case), -# for back-compatibility. -if echo "${CI_COMMIT_BRANCH}" | grep -Eq "^[0-9]+-[0-9]+-auto-deploy-[0-9]+$"; then - COMMIT_SHA_DESTINATION=${ASSETS_IMAGE_PATH}:${CI_COMMIT_SHA} - DESTINATIONS="$DESTINATIONS --destination=$COMMIT_SHA_DESTINATION" - SKIP_ASSETS_IMAGE_BUILDING_IF_ALREADY_EXIST="false" -fi +COMMIT_REF_SLUG_DESTINATION=${ASSETS_IMAGE_PATH}:${CI_COMMIT_REF_SLUG} -if [ "${SKIP_ASSETS_IMAGE_BUILDING_IF_ALREADY_EXIST}" = "true" ] && [ -n "${PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE}" ]; then - echoinfo "Checking if the ${COMMIT_ASSETS_HASH_DESTINATION} image exists..." - repository_id=$(get_repository_id "${ASSETS_IMAGE_NAME}") +COMMIT_SHA_DESTINATION=${ASSETS_IMAGE_PATH}:${CI_COMMIT_SHA} +COMMIT_REF_NAME_DESTINATION=${ASSETS_IMAGE_PATH}:${CI_COMMIT_REF_NAME} - if [ -n "${repository_id}" ]; then - api_image_url="${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/registry/repositories/${repository_id}/tags/${COMMIT_ASSETS_HASH_TAG}" - echoinfo "api_image_url: ${api_image_url}" +DESTINATIONS="--destination=$COMMIT_REF_SLUG_DESTINATION --destination=$COMMIT_SHA_DESTINATION" - if test_url "${api_image_url}" "--header \"PRIVATE-TOKEN: ${PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE}\""; then - echosuccess "Image ${COMMIT_ASSETS_HASH_DESTINATION} already exists, no need to rebuild it." - exit 0 - else - echoinfo "Image ${COMMIT_ASSETS_HASH_DESTINATION} doesn't exist, we'll need to build it." - fi - else - echoerr "Repository ID couldn't be found for the '${ASSETS_IMAGE_NAME}' image!" - fi -else - echoinfo "The 'PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE' variable is not present, so we cannot check if the image already exists." +# Also tag the image with GitLab version, if running on a tag pipeline, so +# other projects can simply use that instead of computing the slug. +if [ -n "$CI_COMMIT_TAG" ]; then + DESTINATIONS="$DESTINATIONS --destination=$COMMIT_REF_NAME_DESTINATION" fi -mkdir -p assets_container.build/public -cp -r public/assets assets_container.build/public/ -cp Dockerfile.assets assets_container.build/ - -echo "Building assets image for destinations: ${DESTINATIONS}" +echo "building assets image for destinations: $DESTINATIONS" -/kaniko/executor \ - --context="assets_container.build" \ - --dockerfile="assets_container.build/Dockerfile.assets" \ - ${DESTINATIONS} +/kaniko/executor --context=assets_container.build --dockerfile=assets_container.build/Dockerfile.assets $DESTINATIONS diff --git a/scripts/trigger-build.rb b/scripts/trigger-build.rb index 8dfab8dd2ebba..897ca9f473e32 100755 --- a/scripts/trigger-build.rb +++ b/scripts/trigger-build.rb @@ -160,8 +160,6 @@ def version_file_variables end class CNG < Base - ASSETS_HASH = "cached-assets-hash.txt" - def variables # Delete variables that aren't useful when using native triggers. super.tap do |hash| @@ -189,6 +187,7 @@ def extra_variables "TRIGGER_BRANCH" => ref, "GITLAB_VERSION" => ENV['CI_COMMIT_SHA'], "GITLAB_TAG" => ENV['CI_COMMIT_TAG'], # Always set a value, even an empty string, so that the downstream pipeline can correctly check it. + "GITLAB_ASSETS_TAG" => ENV['CI_COMMIT_TAG'] ? ENV['CI_COMMIT_REF_NAME'] : ENV['CI_COMMIT_SHA'], "FORCE_RAILS_IMAGE_BUILDS" => 'true', "CE_PIPELINE" => Trigger.ee? ? nil : "true", # Always set a value, even an empty string, so that the downstream pipeline can correctly check it. "EE_PIPELINE" => Trigger.ee? ? "true" : nil # Always set a value, even an empty string, so that the downstream pipeline can correctly check it. diff --git a/scripts/utils.sh b/scripts/utils.sh index 378f492a8bfd2..50ca7f558f6ac 100644 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -15,11 +15,9 @@ function retry() { function test_url() { local url="${1}" - local curl_args="${2}" local status - local cmd="curl ${curl_args} --output /dev/null -L -s -w ''%{http_code}'' \"${url}\"" - status=$(eval "${cmd}") + status=$(curl --output /dev/null -L -s -w ''%{http_code}'' "${url}") if [[ $status == "200" ]]; then return 0 @@ -205,16 +203,3 @@ function danger_as_local() { # We need to base SHA to help danger determine the base commit for this shallow clone. bundle exec danger dry_run --fail-on-errors=true --verbose --base="${CI_MERGE_REQUEST_DIFF_BASE_SHA}" --head="${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA:-$CI_COMMIT_SHA}" --dangerfile="${DANGER_DANGERFILE:-Dangerfile}" } - -# We're inlining this function in `.gitlab/ci/package-and-test/main.gitlab-ci.yml` since this file can be included in other projects. -function assets_image_tag() { - local cache_assets_hash_file="cached-assets-hash.txt" - - if [[ -n "${CI_COMMIT_TAG}" ]]; then - echo -n "${CI_COMMIT_REF_NAME}" - elif [[ -f "${cache_assets_hash_file}" ]]; then - echo -n "assets-hash-$(cat ${cache_assets_hash_file} | cut -c1-10)" - else - echo -n "${CI_COMMIT_SHA}" - fi -} diff --git a/spec/scripts/trigger-build_spec.rb b/spec/scripts/trigger-build_spec.rb index ebf05167428b6..9032ba85b9fca 100644 --- a/spec/scripts/trigger-build_spec.rb +++ b/spec/scripts/trigger-build_spec.rb @@ -319,6 +319,28 @@ def ref_param_name end end + describe "GITLAB_ASSETS_TAG" do + context 'when CI_COMMIT_TAG is set' do + before do + stub_env('CI_COMMIT_TAG', 'v1.0') + end + + it 'sets GITLAB_ASSETS_TAG to CI_COMMIT_REF_NAME' do + expect(subject.variables['GITLAB_ASSETS_TAG']).to eq(env['CI_COMMIT_REF_NAME']) + end + end + + context 'when CI_COMMIT_TAG is nil' do + before do + stub_env('CI_COMMIT_TAG', nil) + end + + it 'sets GITLAB_ASSETS_TAG to CI_COMMIT_SHA' do + expect(subject.variables['GITLAB_ASSETS_TAG']).to eq(env['CI_COMMIT_SHA']) + end + end + end + describe "CE_PIPELINE" do context 'when Trigger.ee? is true' do before do -- GitLab