From cbbb373a4cc56a9d78e7ff2477363f1c3abad88e Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 15 Oct 2025 15:34:25 +0200 Subject: [PATCH 01/20] Rename files if necessary after remote to local artifact migration Changelog: changed --- .../base_migrater.rb | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb index 0484957a6feda1..81601d85dc353f 100644 --- a/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb @@ -30,13 +30,31 @@ def batch_size def migrate(items, store) items.find_each(batch_size: batch_size) do |item| # rubocop:disable CodeReuse/ActiveRecord item.file.migrate!(store) - + fix_file_path_if_necessary(item) if store == ObjectStorage::Store::LOCAL log_success(item, store) rescue StandardError => e log_error(e, item) end end + def fix_file_path_if_necessary(artifact) + return if artifact.file_final_path.blank? + + desired_file_name = artifact.attributes['file'] + final_file_dir = File.dirname(artifact.file.path) + remote_file_name = File.basename(artifact.file_final_path) + + return if desired_file_name == remote_file_name + return if File.exist?(File.join(final_file_dir, desired_file_name)) + + to_be_renamed_file = File.join(final_file_dir, remote_file_name) + return unless File.exist?(to_be_renamed_file) + + final_file_name = File.join(final_file_dir, desired_file_name) + log_success(artifact, "Fixed file path for #{final_file_name}") + File.rename(to_be_renamed_file, final_file_name) + end + def log_success(item, store) logger.info("Transferred #{item.class.name} ID #{item.id} with size #{item.size} to #{storage_label(store)} storage") end -- GitLab From dd4e410f6780b3e1b7bc6d9d7235ccd1fdc8c32e Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 15 Oct 2025 15:41:01 +0200 Subject: [PATCH 02/20] Move logging statement after rename for possible error handling --- lib/gitlab/local_and_remote_storage_migration/base_migrater.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb index 81601d85dc353f..1465d7176841c0 100644 --- a/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb @@ -51,8 +51,8 @@ def fix_file_path_if_necessary(artifact) return unless File.exist?(to_be_renamed_file) final_file_name = File.join(final_file_dir, desired_file_name) - log_success(artifact, "Fixed file path for #{final_file_name}") File.rename(to_be_renamed_file, final_file_name) + log_success(artifact, "Fixed file path for #{final_file_name}") end def log_success(item, store) -- GitLab From 7cb19e3f5457742a9d38f53023676455f99b14a3 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 16 Oct 2025 14:46:48 +0200 Subject: [PATCH 03/20] Move artifact migration code to artifact migrater --- .../artifact_migrater.rb | 28 +++++++++++++++++++ .../base_migrater.rb | 20 +------------ 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb index b25305382b2dee..afdca82bb38e31 100644 --- a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb @@ -5,6 +5,34 @@ module LocalAndRemoteStorageMigration class ArtifactMigrater < Gitlab::LocalAndRemoteStorageMigration::BaseMigrater private + def migrate(items, store) + items.find_each(batch_size: batch_size) do |item| # rubocop:disable CodeReuse/ActiveRecord -- not an active record model + item.file.migrate!(store) + fix_file_path_if_necessary(item) if store == ObjectStorage::Store::LOCAL + log_success(item, store) + rescue StandardError => e + log_error(e, item) + end + end + + def fix_file_path_if_necessary(artifact) + return if artifact.file_final_path.blank? + + desired_file_name = artifact.attributes['file'] + final_file_dir = File.dirname(artifact.file.path) + remote_file_name = File.basename(artifact.file_final_path) + + return if desired_file_name == remote_file_name + return if File.exist?(File.join(final_file_dir, desired_file_name)) + + to_be_renamed_file = File.join(final_file_dir, remote_file_name) + return unless File.exist?(to_be_renamed_file) + + final_file_name = File.join(final_file_dir, desired_file_name) + File.rename(to_be_renamed_file, final_file_name) + log_success(artifact, "Fixed file path for #{final_file_name}") + end + def items_with_files_stored_locally ::Ci::JobArtifact.with_files_stored_locally end diff --git a/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb index 1465d7176841c0..0484957a6feda1 100644 --- a/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb @@ -30,31 +30,13 @@ def batch_size def migrate(items, store) items.find_each(batch_size: batch_size) do |item| # rubocop:disable CodeReuse/ActiveRecord item.file.migrate!(store) - fix_file_path_if_necessary(item) if store == ObjectStorage::Store::LOCAL + log_success(item, store) rescue StandardError => e log_error(e, item) end end - def fix_file_path_if_necessary(artifact) - return if artifact.file_final_path.blank? - - desired_file_name = artifact.attributes['file'] - final_file_dir = File.dirname(artifact.file.path) - remote_file_name = File.basename(artifact.file_final_path) - - return if desired_file_name == remote_file_name - return if File.exist?(File.join(final_file_dir, desired_file_name)) - - to_be_renamed_file = File.join(final_file_dir, remote_file_name) - return unless File.exist?(to_be_renamed_file) - - final_file_name = File.join(final_file_dir, desired_file_name) - File.rename(to_be_renamed_file, final_file_name) - log_success(artifact, "Fixed file path for #{final_file_name}") - end - def log_success(item, store) logger.info("Transferred #{item.class.name} ID #{item.id} with size #{item.size} to #{storage_label(store)} storage") end -- GitLab From 5ae27bd92af5fa313e33961efd77bc74ed3b929d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 16 Oct 2025 16:17:33 +0200 Subject: [PATCH 04/20] Move file path fixer to its own helper --- .../artifact_migrater.rb | 26 ++--- .../file_path_fixer.rb | 31 ++++++ .../file_path_fixer_spec.rb | 105 ++++++++++++++++++ 3 files changed, 143 insertions(+), 19 deletions(-) create mode 100644 lib/gitlab/local_and_remote_storage_migration/file_path_fixer.rb create mode 100644 spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb diff --git a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb index afdca82bb38e31..2d55b39c8cbe83 100644 --- a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb @@ -8,31 +8,19 @@ class ArtifactMigrater < Gitlab::LocalAndRemoteStorageMigration::BaseMigrater def migrate(items, store) items.find_each(batch_size: batch_size) do |item| # rubocop:disable CodeReuse/ActiveRecord -- not an active record model item.file.migrate!(store) - fix_file_path_if_necessary(item) if store == ObjectStorage::Store::LOCAL + + if store == ObjectStorage::Store::LOCAL + Gitlab::LocalAndRemoteStorageMigration::FilePathFixer.fix_file_path!(item, logger: ->(msg) { + log_success(item, msg) + }) + end + log_success(item, store) rescue StandardError => e log_error(e, item) end end - def fix_file_path_if_necessary(artifact) - return if artifact.file_final_path.blank? - - desired_file_name = artifact.attributes['file'] - final_file_dir = File.dirname(artifact.file.path) - remote_file_name = File.basename(artifact.file_final_path) - - return if desired_file_name == remote_file_name - return if File.exist?(File.join(final_file_dir, desired_file_name)) - - to_be_renamed_file = File.join(final_file_dir, remote_file_name) - return unless File.exist?(to_be_renamed_file) - - final_file_name = File.join(final_file_dir, desired_file_name) - File.rename(to_be_renamed_file, final_file_name) - log_success(artifact, "Fixed file path for #{final_file_name}") - end - def items_with_files_stored_locally ::Ci::JobArtifact.with_files_stored_locally end diff --git a/lib/gitlab/local_and_remote_storage_migration/file_path_fixer.rb b/lib/gitlab/local_and_remote_storage_migration/file_path_fixer.rb new file mode 100644 index 00000000000000..d0b676bff692b7 --- /dev/null +++ b/lib/gitlab/local_and_remote_storage_migration/file_path_fixer.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Gitlab + module LocalAndRemoteStorageMigration + module FilePathFixer + extend self + + # Fixes the file path if necessary and returns the new path (or nil if nothing changed) + def fix_file_path!(artifact, logger: nil) + return if artifact.file_final_path.blank? + + desired_file_name = artifact.attributes['file'] + final_file_dir = File.dirname(artifact.file.path) + remote_file_name = File.basename(artifact.file_final_path) + + return if desired_file_name == remote_file_name + + return if File.exist?(File.join(final_file_dir, desired_file_name)) + + to_be_renamed_file = File.join(final_file_dir, remote_file_name) + return unless File.exist?(to_be_renamed_file) + + final_file_name = File.join(final_file_dir, desired_file_name) + File.rename(to_be_renamed_file, final_file_name) + + logger&.call("Fixed file path for #{final_file_name}") + final_file_name + end + end + end +end diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb new file mode 100644 index 00000000000000..dffd4c240c0c3d --- /dev/null +++ b/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::LocalAndRemoteStorageMigration::FilePathFixer, feature_category: :job_artifacts do + let(:artifact) do + instance_double( + Ci::JobArtifact, + file_final_path: file_final_path, + file: uploader_double, + attributes: { 'file' => 'desired_name.zip' } + ) + end + + let(:uploader_double) do + instance_double( + JobArtifactUploader, + path: local_path, + migrate!: true + ) + end + + let(:desired_file_name) { 'desired_name.zip' } + let(:remote_file_name) { 'remote_name.zip' } + let(:local_path) { "/local/path/#{remote_file_name}" } + let(:file_final_path) { "/remote/path/#{remote_file_name}" } + + let(:renamed_files) { [] } + let(:logged_messages) { [] } + let(:logger) { ->(msg) { logged_messages << msg } } + + before do + allow(artifact).to receive(:[]).with('file').and_return(desired_file_name) + allow(artifact.file).to receive(:path).and_return("/local/path/#{remote_file_name}") + + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with("/local/path/#{desired_file_name}").and_return(desired_exists) + allow(File).to receive(:exist?).with(local_path).and_return(remote_exists) + + allow(File).to receive(:rename) do |old_path, new_path| + renamed_files << [old_path, new_path] + end + end + + describe '.fix_file_path!' do + subject(:fix_file_path) { described_class.fix_file_path!(artifact, logger: logger) } + + context 'when file_final_path is blank' do + let(:file_final_path) { nil } + let(:desired_exists) { false } + let(:remote_exists) { true } + + it 'does nothing' do + expect(fix_file_path).to be_nil + expect(renamed_files).to be_empty + expect(logged_messages).to be_empty + end + end + + context 'when desired file name matches remote file name' do + let(:desired_file_name) { 'desired_name.zip' } + let(:remote_file_name) { 'desired_name.zip' } + + let(:desired_exists) { false } + let(:remote_exists) { true } + + it 'does nothing' do + expect(fix_file_path).to be_nil + expect(renamed_files).to be_empty + end + end + + context 'when desired file already exists locally' do + let(:desired_exists) { true } + let(:remote_exists) { true } + + it 'does nothing' do + expect(fix_file_path).to be_nil + expect(renamed_files).to be_empty + end + end + + context 'when file needs to be renamed' do + let(:desired_exists) { false } + let(:remote_exists) { true } + + it 'renames the file and logs success' do + is_expected.to eq("/local/path/#{desired_file_name}") + expect(renamed_files).to eq([[local_path, "/local/path/#{desired_file_name}"]]) + expect(logged_messages).to include("Fixed file path for /local/path/#{desired_file_name}") + end + end + + context 'when remote file does not exist' do + let(:desired_exists) { false } + let(:remote_exists) { false } + + it 'does nothing' do + expect(fix_file_path).to be_nil + expect(renamed_files).to be_empty + expect(logged_messages).to be_empty + end + end + end +end -- GitLab From d0fe179438cc6cadbdf2a4ac47a27215cb8aff88 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 16 Oct 2025 16:26:45 +0200 Subject: [PATCH 05/20] Simplify file path fixer call --- .../local_and_remote_storage_migration/artifact_migrater.rb | 6 +----- .../local_and_remote_storage_migration/file_path_fixer.rb | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb index 2d55b39c8cbe83..bdc14b7242ae64 100644 --- a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb @@ -9,11 +9,7 @@ def migrate(items, store) items.find_each(batch_size: batch_size) do |item| # rubocop:disable CodeReuse/ActiveRecord -- not an active record model item.file.migrate!(store) - if store == ObjectStorage::Store::LOCAL - Gitlab::LocalAndRemoteStorageMigration::FilePathFixer.fix_file_path!(item, logger: ->(msg) { - log_success(item, msg) - }) - end + FilePathFixer.fix_file_path!(item) if store == ObjectStorage::Store::LOCAL log_success(item, store) rescue StandardError => e diff --git a/lib/gitlab/local_and_remote_storage_migration/file_path_fixer.rb b/lib/gitlab/local_and_remote_storage_migration/file_path_fixer.rb index d0b676bff692b7..c92de2001ac557 100644 --- a/lib/gitlab/local_and_remote_storage_migration/file_path_fixer.rb +++ b/lib/gitlab/local_and_remote_storage_migration/file_path_fixer.rb @@ -6,7 +6,7 @@ module FilePathFixer extend self # Fixes the file path if necessary and returns the new path (or nil if nothing changed) - def fix_file_path!(artifact, logger: nil) + def fix_file_path!(artifact) return if artifact.file_final_path.blank? desired_file_name = artifact.attributes['file'] @@ -22,8 +22,6 @@ def fix_file_path!(artifact, logger: nil) final_file_name = File.join(final_file_dir, desired_file_name) File.rename(to_be_renamed_file, final_file_name) - - logger&.call("Fixed file path for #{final_file_name}") final_file_name end end -- GitLab From 30738a5ad1107494bdd84f86f3acc81e2ff7992d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 16 Oct 2025 17:31:26 +0200 Subject: [PATCH 06/20] Fix file path fixer spec --- .../file_path_fixer_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb index dffd4c240c0c3d..3920d656538db4 100644 --- a/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb +++ b/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb @@ -43,7 +43,7 @@ end describe '.fix_file_path!' do - subject(:fix_file_path) { described_class.fix_file_path!(artifact, logger: logger) } + subject(:fix_file_path) { described_class.fix_file_path!(artifact) } context 'when file_final_path is blank' do let(:file_final_path) { nil } @@ -84,10 +84,9 @@ let(:desired_exists) { false } let(:remote_exists) { true } - it 'renames the file and logs success' do + it 'renames the file' do is_expected.to eq("/local/path/#{desired_file_name}") expect(renamed_files).to eq([[local_path, "/local/path/#{desired_file_name}"]]) - expect(logged_messages).to include("Fixed file path for /local/path/#{desired_file_name}") end end -- GitLab From 0a147d9f73047e8bf768ffa3e7315f3750b4b53d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 16 Oct 2025 17:43:58 +0200 Subject: [PATCH 07/20] Add rake task to fix local storage filenames --- .../artifact_local_storage_name_fixer.rb | 40 +++++++++++++++++++ .../artifacts/fix_artifact_filepath.rake | 21 ++++++++++ 2 files changed, 61 insertions(+) create mode 100644 lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb create mode 100644 lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake diff --git a/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb b/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb new file mode 100644 index 00000000000000..b3e77040622bc7 --- /dev/null +++ b/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module LocalAndRemoteStorageMigration + class ArtifactLocalStorageNameFixer + def initialize(logger = nil) + @logger = logger + end + + def rename_artifacts + logger.info('Starting renaming process in local storage') + items_with_files_stored_locally.find_each(batch_size: batch_size) do |item| # rubocop:disable CodeReuse/ActiveRecord -- not an active record model + log_success(item) if FilePathFixer.fix_file_path!(item) + rescue StandardError => e + log_error(e, item) + end + end + + private + + attr_reader :logger + + def batch_size + ENV.fetch('MIGRATION_BATCH_SIZE', 10).to_i + end + + def log_success(item) + logger.info("Renamed #{item.class.name} ID #{item.id} with size #{item.size}.") + end + + def log_error(err, item) + logger.warn("Failed to rename #{item.class.name} ID #{item.id} with error: #{err.message}.") + end + + def items_with_files_stored_locally + ::Ci::JobArtifact.with_files_stored_locally + end + end + end +end diff --git a/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake b/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake new file mode 100644 index 00000000000000..f0207bfb9190f0 --- /dev/null +++ b/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +desc 'GitLab | Artifacts | Fix uploaded files for artifacts by renaming them' +namespace :gitlab do + require 'logger' # rubocop:disable Rake/Require -- we need to load the logger + + namespace :artifacts do + task fix_artifact_filepath: :environment do + require 'resolv-replace' + logger = Logger.new($stdout) + + helper = Gitlab::LocalAndRemoteStorageMigration::ArtifactLocalStorageNameFixer.new(logger) + + begin + helper.rename_artifacts + rescue StandardError => e + logger.error(e.message) + end + end + end +end -- GitLab From aad9609f57aad92b6dd2da914f5d2e772b15a6c1 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 16 Oct 2025 17:46:36 +0200 Subject: [PATCH 08/20] Use match_array to avoid possible flakyness --- .../local_and_remote_storage_migration/file_path_fixer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb index 3920d656538db4..b3df6c49c000d5 100644 --- a/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb +++ b/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb @@ -86,7 +86,7 @@ it 'renames the file' do is_expected.to eq("/local/path/#{desired_file_name}") - expect(renamed_files).to eq([[local_path, "/local/path/#{desired_file_name}"]]) + expect(renamed_files).to match_array([[local_path, "/local/path/#{desired_file_name}"]]) end end -- GitLab From c42c6d21b38f9e53c2d82843aab7198c8e3c9442 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 17 Oct 2025 11:30:30 +0200 Subject: [PATCH 09/20] Add and refactor file path fixer specs --- .../file_path_fixer_spec.rb | 7 +- .../artifacts/fix_artifact_filepath_spec.rb | 68 +++++++++++++++++++ 2 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 spec/tasks/gitlab/artifacts/fix_artifact_filepath_spec.rb diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb index b3df6c49c000d5..e1ee111556b6ea 100644 --- a/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb +++ b/spec/lib/gitlab/local_and_remote_storage_migration/file_path_fixer_spec.rb @@ -15,8 +15,7 @@ let(:uploader_double) do instance_double( JobArtifactUploader, - path: local_path, - migrate!: true + path: local_path ) end @@ -26,8 +25,6 @@ let(:file_final_path) { "/remote/path/#{remote_file_name}" } let(:renamed_files) { [] } - let(:logged_messages) { [] } - let(:logger) { ->(msg) { logged_messages << msg } } before do allow(artifact).to receive(:[]).with('file').and_return(desired_file_name) @@ -53,7 +50,6 @@ it 'does nothing' do expect(fix_file_path).to be_nil expect(renamed_files).to be_empty - expect(logged_messages).to be_empty end end @@ -97,7 +93,6 @@ it 'does nothing' do expect(fix_file_path).to be_nil expect(renamed_files).to be_empty - expect(logged_messages).to be_empty end end end diff --git a/spec/tasks/gitlab/artifacts/fix_artifact_filepath_spec.rb b/spec/tasks/gitlab/artifacts/fix_artifact_filepath_spec.rb new file mode 100644 index 00000000000000..17e48a9e50dd95 --- /dev/null +++ b/spec/tasks/gitlab/artifacts/fix_artifact_filepath_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'gitlab:artifacts namespace rake task', :silence_stdout, feature_category: :job_artifacts do + before(:context) do + Rake.application.rake_require 'tasks/gitlab/artifacts/fix_artifact_filepath' + end + + let(:desired_file_name) { 'desired_name.zip' } + let(:remote_file_name) { 'remote_name.zip' } + let(:local_path) { "/local/path/#{remote_file_name}" } + let(:file_final_path) { "/remote/path/#{remote_file_name}" } + + let(:renamed_files) { [] } + let(:relation) { instance_double(ActiveRecord::Relation) } + + let(:artifact) do + instance_double( + Ci::JobArtifact, + file_final_path: file_final_path, + file: uploader_double, + attributes: { 'file' => 'desired_name.zip' }, + id: 1, + size: 0 + ) + end + + let(:uploader_double) do + instance_double( + JobArtifactUploader, + path: local_path + ) + end + + before do + allow(artifact).to receive(:[]).with('file').and_return(desired_file_name) + allow(artifact.file).to receive(:path).and_return("/local/path/#{remote_file_name}") + + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with("/local/path/#{desired_file_name}").and_return(desired_exists) + allow(File).to receive(:exist?).with(local_path).and_return(remote_exists) + + allow(File).to receive(:rename) do |old_path, new_path| + renamed_files << [old_path, new_path] + end + + allow(relation).to receive(:find_each) do |&block| + [artifact].each { |artifact| block.call(artifact) } + end + + allow(Ci::JobArtifact).to receive(:with_files_stored_locally).and_return(relation) + end + + describe 'gitlab:artifacts:migrate' do + subject(:task) { run_rake_task('gitlab:artifacts:fix_artifact_filepath') } + + context 'when file needs to be renamed' do + let(:desired_exists) { false } + let(:remote_exists) { true } + + it 'renames the file' do + task + expect(renamed_files).to match_array([[local_path, "/local/path/#{desired_file_name}"]]) + end + end + end +end -- GitLab From 4243f0c55403b7d8f65aed3263113fb3309bd72e Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 17 Oct 2025 15:13:32 +0200 Subject: [PATCH 10/20] Add missing docs --- doc/administration/cicd/job_artifacts.md | 4 ++++ lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/administration/cicd/job_artifacts.md b/doc/administration/cicd/job_artifacts.md index b2f6aafad0d232..2b54d450258ae3 100644 --- a/doc/administration/cicd/job_artifacts.md +++ b/doc/administration/cicd/job_artifacts.md @@ -324,6 +324,10 @@ To migrate artifacts back to local storage: 1. [Selectively disable the artifacts' storage](../object_storage.md#disable-object-storage-for-specific-features) in `gitlab.rb`. 1. [Reconfigure GitLab](../restart_gitlab.md#reconfigure-a-linux-package-installation). +Until GitLab 18.6, it can happen that a migration from remote to local storage copies artifacts with a wrong file name. +If this has happened to you, please run `gitlab-rake gitlab:artifacts:fix_artifact_filepath`. +This task will check for artifacts that are not downloadable and will rename it on your local storage to the appropriate, expected filename. + ## Expiring artifacts If [`artifacts:expire_in`](../../ci/yaml/_index.md#artifactsexpire_in) is used to set diff --git a/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake b/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake index f0207bfb9190f0..223dac044dc879 100644 --- a/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake +++ b/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake @@ -1,6 +1,6 @@ # frozen_string_literal: true -desc 'GitLab | Artifacts | Fix uploaded files for artifacts by renaming them' +desc 'GitLab | Artifacts | Fix uploaded filenames for artifacts on local storage by renaming them appropriately' namespace :gitlab do require 'logger' # rubocop:disable Rake/Require -- we need to load the logger -- GitLab From c684e591bd018c9b28889e2efaf41e115b1f4e55 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 17 Oct 2025 15:15:08 +0200 Subject: [PATCH 11/20] Fix doc wording --- doc/administration/cicd/job_artifacts.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/administration/cicd/job_artifacts.md b/doc/administration/cicd/job_artifacts.md index 2b54d450258ae3..b8206521421176 100644 --- a/doc/administration/cicd/job_artifacts.md +++ b/doc/administration/cicd/job_artifacts.md @@ -326,7 +326,7 @@ To migrate artifacts back to local storage: Until GitLab 18.6, it can happen that a migration from remote to local storage copies artifacts with a wrong file name. If this has happened to you, please run `gitlab-rake gitlab:artifacts:fix_artifact_filepath`. -This task will check for artifacts that are not downloadable and will rename it on your local storage to the appropriate, expected filename. +This task will check for artifacts that are not downloadable and will rename them on your local storage to the appropriate, expected filename. ## Expiring artifacts -- GitLab From fdd01ff62c4a9c9ee5af9cc0e543f204be58fcb2 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Mon, 20 Oct 2025 16:08:52 +0200 Subject: [PATCH 12/20] Apply 2 suggestion(s) to 1 file(s) Co-authored-by: Hannah Baker --- doc/administration/cicd/job_artifacts.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/administration/cicd/job_artifacts.md b/doc/administration/cicd/job_artifacts.md index b8206521421176..f4c4f0736ebd2a 100644 --- a/doc/administration/cicd/job_artifacts.md +++ b/doc/administration/cicd/job_artifacts.md @@ -324,9 +324,9 @@ To migrate artifacts back to local storage: 1. [Selectively disable the artifacts' storage](../object_storage.md#disable-object-storage-for-specific-features) in `gitlab.rb`. 1. [Reconfigure GitLab](../restart_gitlab.md#reconfigure-a-linux-package-installation). -Until GitLab 18.6, it can happen that a migration from remote to local storage copies artifacts with a wrong file name. +Until GitLab 18.6, a migration from remote to local storage could result in artifacts being copied with incorrect filenames. If this has happened to you, please run `gitlab-rake gitlab:artifacts:fix_artifact_filepath`. -This task will check for artifacts that are not downloadable and will rename them on your local storage to the appropriate, expected filename. +This task will check for artifacts that are not downloadable and will rename them in your local storage to the appropriate, expected file name. ## Expiring artifacts -- GitLab From a758c874604e5912e9f74d92a7321429ceaa44d0 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Tue, 21 Oct 2025 13:44:03 +0200 Subject: [PATCH 13/20] Expect file path fixer to be called --- .../local_and_remote_storage_migration_shared_examples.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb index 4b0e3234750f22..81d014f03100d4 100644 --- a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb @@ -25,6 +25,10 @@ expect(item.file_store).to eq(start_store) + if start_store == ObjectStorage::Store::REMOTE && end_store == ObjectStorage::Store::LOCAL + expect(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer).to receive(:fix_file_path!) + end + migrater.send(method) expect(item.reload.file_store).to eq(end_store) -- GitLab From 47c867c0059bc54f04e6f7ec8a293a9b323913d6 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Tue, 21 Oct 2025 14:06:37 +0200 Subject: [PATCH 14/20] Fix artifact migrater spec --- .../artifact_migrater_spec.rb | 5 +++++ .../local_and_remote_storage_migration_shared_examples.rb | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb index b3f2003c207ea8..0248817e765985 100644 --- a/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb +++ b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb @@ -6,6 +6,11 @@ RSpec.describe Gitlab::LocalAndRemoteStorageMigration::ArtifactMigrater do before do stub_artifacts_object_storage(enabled: true) + + if start_store == ObjectStorage::Store::REMOTE && end_store == ObjectStorage::Store::LOCAL + allow(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer) + .to receive(:fix_file_path!) + end end let!(:item) { create(:ci_job_artifact, :archive, file_store: start_store) } diff --git a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb index 81d014f03100d4..4b0e3234750f22 100644 --- a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb @@ -25,10 +25,6 @@ expect(item.file_store).to eq(start_store) - if start_store == ObjectStorage::Store::REMOTE && end_store == ObjectStorage::Store::LOCAL - expect(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer).to receive(:fix_file_path!) - end - migrater.send(method) expect(item.reload.file_store).to eq(end_store) -- GitLab From fe2ebd036e3143bc372cf2d303a2a2b901a877be Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Tue, 21 Oct 2025 14:20:16 +0200 Subject: [PATCH 15/20] Use condition for rename expectation --- .../artifact_migrater_spec.rb | 7 +------ .../local_and_remote_storage_migration_shared_examples.rb | 6 +++++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb index 0248817e765985..cc92d570418247 100644 --- a/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb +++ b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb @@ -6,14 +6,9 @@ RSpec.describe Gitlab::LocalAndRemoteStorageMigration::ArtifactMigrater do before do stub_artifacts_object_storage(enabled: true) - - if start_store == ObjectStorage::Store::REMOTE && end_store == ObjectStorage::Store::LOCAL - allow(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer) - .to receive(:fix_file_path!) - end end let!(:item) { create(:ci_job_artifact, :archive, file_store: start_store) } - it_behaves_like 'local and remote storage migration' + it_behaves_like 'local and remote storage migration', expect_file_rename: true end diff --git a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb index 4b0e3234750f22..437798c49493a9 100644 --- a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.shared_examples 'local and remote storage migration' do +RSpec.shared_examples 'local and remote storage migration' do |expect_file_rename: false| let(:logger) { Logger.new("/dev/null") } let(:migrater) { described_class.new(logger) } @@ -25,6 +25,10 @@ expect(item.file_store).to eq(start_store) + if expect_file_rename && start_store == ObjectStorage::Store::REMOTE && end_store == ObjectStorage::Store::LOCAL + expect(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer).to receive(:fix_file_path!) + end + migrater.send(method) expect(item.reload.file_store).to eq(end_store) -- GitLab From 94c08e40bd4970bafb0c760a501f5dbc8b5d5e8c Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 22 Oct 2025 10:01:59 +0200 Subject: [PATCH 16/20] Use each_batch instead of find_each --- .../artifact_migrater.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb index bdc14b7242ae64..bf83042c23308c 100644 --- a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb @@ -6,14 +6,16 @@ class ArtifactMigrater < Gitlab::LocalAndRemoteStorageMigration::BaseMigrater private def migrate(items, store) - items.find_each(batch_size: batch_size) do |item| # rubocop:disable CodeReuse/ActiveRecord -- not an active record model - item.file.migrate!(store) + items.each_batch(of: batch_size) do |batch| + batch.each do |item| + item.file.migrate!(store) - FilePathFixer.fix_file_path!(item) if store == ObjectStorage::Store::LOCAL + FilePathFixer.fix_file_path!(item) if store == ObjectStorage::Store::LOCAL - log_success(item, store) - rescue StandardError => e - log_error(e, item) + log_success(item, store) + rescue StandardError => e + log_error(e, item) + end end end -- GitLab From aad5c212a56737256cf0b560a89cd21c3bb91196 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 22 Oct 2025 10:26:18 +0200 Subject: [PATCH 17/20] Add artifact local storage name fixer spec and use EachBatch --- .../artifact_local_storage_name_fixer.rb | 10 ++- .../artifact_local_storage_name_fixer_spec.rb | 84 +++++++++++++++++++ 2 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 spec/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer_spec.rb diff --git a/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb b/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb index b3e77040622bc7..97c37fb2594145 100644 --- a/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb +++ b/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb @@ -9,10 +9,12 @@ def initialize(logger = nil) def rename_artifacts logger.info('Starting renaming process in local storage') - items_with_files_stored_locally.find_each(batch_size: batch_size) do |item| # rubocop:disable CodeReuse/ActiveRecord -- not an active record model - log_success(item) if FilePathFixer.fix_file_path!(item) - rescue StandardError => e - log_error(e, item) + items_with_files_stored_locally.each_batch(of: batch_size) do |batch| + batch.each do |item| + log_success(item) if FilePathFixer.fix_file_path!(item) + rescue StandardError => e + log_error(e, item) + end end end diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer_spec.rb new file mode 100644 index 00000000000000..dada1ff8acba19 --- /dev/null +++ b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::LocalAndRemoteStorageMigration::ArtifactLocalStorageNameFixer, feature_category: :job_artifacts do + let(:logger) { instance_double(Logger, info: nil, warn: nil) } + let(:fixer) { described_class.new(logger) } + let(:artifact_class) { class_double(Ci::JobArtifact) } + + before do + stub_const('Ci::JobArtifact', artifact_class) + end + + describe '#rename_artifacts' do + let(:artifact1) do + instance_double(Ci::JobArtifact, id: 1, size: 123, class: class_double(Ci::JobArtifact, name: 'Ci::JobArtifact')) + end + + let(:artifact2) do + instance_double(Ci::JobArtifact, id: 2, size: 456, class: class_double(Ci::JobArtifact, name: 'Ci::JobArtifact')) + end + + let(:batch_relation_class) do + Class.new do + include EachBatch + end + end + + let(:batch_relation) { class_double(batch_relation_class, each_batch: nil) } + + before do + allow(artifact_class).to receive(:with_files_stored_locally).and_return(batch_relation) + allow(batch_relation).to receive(:each_batch).and_yield([artifact1, artifact2]) + end + + it 'logs start message' do + expect(logger).to receive(:info).with('Starting renaming process in local storage') + allow(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer).to receive(:fix_file_path!).and_return(true) + + fixer.rename_artifacts + end + + it 'calls FilePathFixer for each artifact' do + expect(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer) + .to receive(:fix_file_path!).with(artifact1).and_return(true) + expect(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer) + .to receive(:fix_file_path!).with(artifact2).and_return(true) + + fixer.rename_artifacts + end + + it 'logs successful renames' do + allow(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer) + .to receive(:fix_file_path!).and_return(true) + + expect(logger).to receive(:info).with('Starting renaming process in local storage') + expect(logger).to receive(:info).with('Renamed Ci::JobArtifact ID 1 with size 123.') + expect(logger).to receive(:info).with('Renamed Ci::JobArtifact ID 2 with size 456.') + + fixer.rename_artifacts + end + + it 'logs warnings on errors' do + allow(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer) + .to receive(:fix_file_path!).and_raise(StandardError, 'boom') + + expect(logger).to receive(:warn).with('Failed to rename Ci::JobArtifact ID 1 with error: boom.') + expect(logger).to receive(:warn).with('Failed to rename Ci::JobArtifact ID 2 with error: boom.') + + fixer.rename_artifacts + end + end + + describe '#batch_size' do + it 'returns default value of 10' do + expect(fixer.send(:batch_size)).to eq(10) + end + + it 'returns custom value from ENV' do + stub_env('MIGRATION_BATCH_SIZE', '50') + expect(fixer.send(:batch_size)).to eq(50) + end + end +end -- GitLab From fa34d8ee3bbc6ab19c0f387f1ad458b45a438ab6 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 22 Oct 2025 10:48:05 +0200 Subject: [PATCH 18/20] Fix artifact filepath spec --- .../gitlab/artifacts/fix_artifact_filepath_spec.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/spec/tasks/gitlab/artifacts/fix_artifact_filepath_spec.rb b/spec/tasks/gitlab/artifacts/fix_artifact_filepath_spec.rb index 17e48a9e50dd95..3e5ef78db0c528 100644 --- a/spec/tasks/gitlab/artifacts/fix_artifact_filepath_spec.rb +++ b/spec/tasks/gitlab/artifacts/fix_artifact_filepath_spec.rb @@ -13,7 +13,13 @@ let(:file_final_path) { "/remote/path/#{remote_file_name}" } let(:renamed_files) { [] } - let(:relation) { instance_double(ActiveRecord::Relation) } + let(:relation_class) do + Class.new do + include EachBatch + end + end + + let(:relation) { class_double(relation_class, each_batch: nil) } let(:artifact) do instance_double( @@ -45,9 +51,7 @@ renamed_files << [old_path, new_path] end - allow(relation).to receive(:find_each) do |&block| - [artifact].each { |artifact| block.call(artifact) } - end + allow(relation).to receive(:each_batch).and_yield([artifact]) allow(Ci::JobArtifact).to receive(:with_files_stored_locally).and_return(relation) end -- GitLab From 38a0807aba010543c665c14ec1bca376a66316e9 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 22 Oct 2025 11:22:38 +0200 Subject: [PATCH 19/20] Do not rescue standard error --- .../artifact_local_storage_name_fixer.rb | 2 +- .../local_and_remote_storage_migration/artifact_migrater.rb | 2 +- .../artifact_local_storage_name_fixer_spec.rb | 2 +- .../local_and_remote_storage_migration_shared_examples.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb b/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb index 97c37fb2594145..3feffd4082f510 100644 --- a/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb +++ b/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer.rb @@ -12,7 +12,7 @@ def rename_artifacts items_with_files_stored_locally.each_batch(of: batch_size) do |batch| batch.each do |item| log_success(item) if FilePathFixer.fix_file_path!(item) - rescue StandardError => e + rescue NoMethodError, ArgumentError, Errno::ENOENT, Errno::EACCES, Errno::ENOTDIR, Errno::EEXIST, IOError => e log_error(e, item) end end diff --git a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb index bf83042c23308c..4dd40c3fbf1253 100644 --- a/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/artifact_migrater.rb @@ -13,7 +13,7 @@ def migrate(items, store) FilePathFixer.fix_file_path!(item) if store == ObjectStorage::Store::LOCAL log_success(item, store) - rescue StandardError => e + rescue NoMethodError, ArgumentError, Errno::ENOENT, Errno::EACCES, Errno::ENOTDIR, Errno::EEXIST, IOError => e log_error(e, item) end end diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer_spec.rb index dada1ff8acba19..fbe937292847fa 100644 --- a/spec/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer_spec.rb +++ b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_local_storage_name_fixer_spec.rb @@ -62,7 +62,7 @@ it 'logs warnings on errors' do allow(Gitlab::LocalAndRemoteStorageMigration::FilePathFixer) - .to receive(:fix_file_path!).and_raise(StandardError, 'boom') + .to receive(:fix_file_path!).and_raise(IOError, 'boom') expect(logger).to receive(:warn).with('Failed to rename Ci::JobArtifact ID 1 with error: boom.') expect(logger).to receive(:warn).with('Failed to rename Ci::JobArtifact ID 2 with error: boom.') diff --git a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb index 437798c49493a9..8f8a0822cf3213 100644 --- a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb @@ -40,7 +40,7 @@ it 'prints error' do expect_next_instance_of(item.file.class) do |file| - expect(file).to receive(:migrate!).and_raise("error message") + expect(file).to receive(:migrate!).and_raise(IOError, "error message") end expect(logger).to receive(:info).with("Starting transfer to object storage") -- GitLab From 9751ce03f176ac0fbda44961e8e0aed228416b77 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 23 Oct 2025 14:06:03 +0200 Subject: [PATCH 20/20] Remove superfluous error handling --- lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake b/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake index 223dac044dc879..e1c368d97994e2 100644 --- a/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake +++ b/lib/tasks/gitlab/artifacts/fix_artifact_filepath.rake @@ -11,11 +11,7 @@ namespace :gitlab do helper = Gitlab::LocalAndRemoteStorageMigration::ArtifactLocalStorageNameFixer.new(logger) - begin - helper.rename_artifacts - rescue StandardError => e - logger.error(e.message) - end + helper.rename_artifacts end end end -- GitLab