summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-06-09 05:56:11 -0700
committerStan Hu <stanhu@gmail.com>2019-06-09 05:56:11 -0700
commitb7e3a1e0409d88c86bf67797749b507a3d480c59 (patch)
tree3b8774ba169415b9fe58449635697dbddb165051
parent2a01cb79560a861a7e32e9ee11e823f22e181aa1 (diff)
downloadgitlab-ce-b7e3a1e0409d88c86bf67797749b507a3d480c59.tar.gz
Revert "Merge branch '50070-legacy-attachments' into 'master'"sh-revert-mr-24679
This reverts commit fd19f887dfeeeedb483c4a4fb32f9f768e89389c, reversing changes made to abb2d4c601d796339c8d7cb0c00946696730f198.
-rw-r--r--changelogs/unreleased/50070-legacy-attachments.yml5
-rw-r--r--db/migrate/20190114184258_migrate_legacy_attachments.rb32
-rw-r--r--doc/administration/troubleshooting/migration.md82
-rw-r--r--lib/gitlab/background_migration/migrate_legacy_uploads.rb128
-rw-r--r--spec/factories/uploads.rb5
-rw-r--r--spec/lib/gitlab/background_migration/migrate_legacy_uploads_spec.rb253
6 files changed, 4 insertions, 501 deletions
diff --git a/changelogs/unreleased/50070-legacy-attachments.yml b/changelogs/unreleased/50070-legacy-attachments.yml
deleted file mode 100644
index 95917f2b5b5..00000000000
--- a/changelogs/unreleased/50070-legacy-attachments.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Migrate legacy uploads out of deprecated paths
-merge_request: 24679
-author:
-type: other
diff --git a/db/migrate/20190114184258_migrate_legacy_attachments.rb b/db/migrate/20190114184258_migrate_legacy_attachments.rb
deleted file mode 100644
index e9fb7952dc9..00000000000
--- a/db/migrate/20190114184258_migrate_legacy_attachments.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-# frozen_string_literal: true
-
-class MigrateLegacyAttachments < ActiveRecord::Migration[5.0]
- include Gitlab::Database::MigrationHelpers
-
- disable_ddl_transaction!
-
- DOWNTIME = false
-
- MIGRATION = 'MigrateLegacyUploads'.freeze
- BATCH_SIZE = 5000
- DELAY_INTERVAL = 5.minutes.to_i
-
- class Upload < ActiveRecord::Base
- self.table_name = 'uploads'
-
- include ::EachBatch
- end
-
- def up
- Upload.where(uploader: 'AttachmentUploader').each_batch(of: BATCH_SIZE) do |relation, index|
- start_id, end_id = relation.pluck('MIN(id), MAX(id)').first
- delay = index * DELAY_INTERVAL
-
- BackgroundMigrationWorker.perform_in(delay, MIGRATION, [start_id, end_id])
- end
- end
-
- # not needed
- def down
- end
-end
diff --git a/doc/administration/troubleshooting/migration.md b/doc/administration/troubleshooting/migration.md
deleted file mode 100644
index 4d2d268b9df..00000000000
--- a/doc/administration/troubleshooting/migration.md
+++ /dev/null
@@ -1,82 +0,0 @@
-# Migrations problems
-
-## Legacy upload migration
-
-> Introduced in GitLab 12.0.
-
- The migration takes all attachments uploaded by legacy `AttachmentUploader` and
- migrate them to the path that current uploaders expect.
-
-Although it should not usually happen there could possibly be some attachments belonging to
-LegacyDiffNotes. These attachments can't be seen before running the migration by users and
-they should not be present in your instance.
-
-However, if you have some of them, you will need to handle them manually.
-You can find the ids of failed notes in logs as "MigrateLegacyUploads: LegacyDiffNote"
-
-1. Run a Rails console:
-
- ```sh
- sudo gitlab-rails console production
- ```
-
- or for source installs:
-
- ```sh
- bundle exec rails console production
- ```
-
- 1. Check the failed upload and find the note (you can see their ids in the logs)
-
- ```ruby
- upload = Upload.find(upload_id)
- note = Note.find(note_id)
- ```
-
-
- 1. Check the path - it should contain `system/note/attachment`
-
- ```ruby
- upload.absolut_path
- ```
-
- 1. Check the path in the uploader - it should differ from the upload path and should contain `system/legacy_diff_note`
-
- ```ruby
- uploader = upload.build_uploader
- uploader.file
- ```
-
- 1. First, you need to move the file to the path that is expected from the uploader
-
- ```ruby
- old_path = upload.absolute_path
- new_path = upload.absolute_path.sub('-/system/note/attachment', '-/system/legacy_diff_note')
- new_dir = File.dirname(new_path)
- FileUtils.mkdir_p(new_dir)
-
- FileUtils.mv(old_path, new_path)
- ```
-
- 1. You then need to move the file to the `FileUploader` and create a new `Upload` object
-
- ```ruby
- file_uploader = UploadService.new(note.project, File.read(new_path)).execute
- ```
-
- 1. And update the legacy note to contain the file.
-
- ```ruby
- new_text = "#{note.note} \n #{file_uploader.markdown_link}"
- note.update!(
- note: new_text
- )
- ```
-
- 1. And finally, you can remove the old upload
-
- ```ruby
- upload.destroy
- ```
-
-If you have any problems feel free to contact [GitLab Support](https://about.gitlab.com/support/).
diff --git a/lib/gitlab/background_migration/migrate_legacy_uploads.rb b/lib/gitlab/background_migration/migrate_legacy_uploads.rb
deleted file mode 100644
index af1ad930aed..00000000000
--- a/lib/gitlab/background_migration/migrate_legacy_uploads.rb
+++ /dev/null
@@ -1,128 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
- module BackgroundMigration
- # This migration takes all legacy uploads (that were uploaded using AttachmentUploader)
- # and migrate them to the new (FileUploader) location (=under projects).
- #
- # We have dependencies (uploaders) in this migration because extracting code would add a lot of complexity
- # and possible errors could appear as the logic in the uploaders is not trivial.
- #
- # This migration will be removed in 12.4 in order to get rid of a migration that depends on
- # the application code.
- class MigrateLegacyUploads
- include Database::MigrationHelpers
- include ::Gitlab::Utils::StrongMemoize
-
- # This class takes a legacy upload and migrates it to the correct location
- class UploadMover
- include Gitlab::Utils::StrongMemoize
-
- attr_reader :upload, :project, :note
-
- def initialize(upload)
- @upload = upload
- @note = Note.find_by(id: upload.model_id)
- @project = note&.project
- end
-
- def execute
- return unless upload
-
- if !project
- # if we don't have models associated with the upload we can not move it
- say "MigrateLegacyUploads: Deleting upload due to model not found: #{upload.inspect}"
- destroy_legacy_upload
- elsif note.is_a?(LegacyDiffNote)
- handle_legacy_note_upload
- elsif !legacy_file_exists?
- # if we can not find the file we just remove the upload record
- say "MigrateLegacyUploads: Deleting upload due to file not found: #{upload.inspect}"
- destroy_legacy_upload
- else
- migrate_upload
- end
- end
-
- private
-
- def migrate_upload
- return unless copy_upload_to_project
-
- add_upload_link_to_note_text
- destroy_legacy_file
- destroy_legacy_upload
- end
-
- # we should proceed and log whenever one upload copy fails, no matter the reasons
- # rubocop: disable Lint/RescueException
- def copy_upload_to_project
- @uploader = FileUploader.copy_to(legacy_file_uploader, project)
-
- say "MigrateLegacyUploads: Copied file #{legacy_file_uploader.file.path} -> #{@uploader.file.path}"
- true
- rescue Exception => e
- say "MigrateLegacyUploads: File #{legacy_file_uploader.file.path} couldn't be copied to project uploads. Error: #{e.message}"
- false
- end
- # rubocop: enable Lint/RescueException
-
- def destroy_legacy_upload
- note.remove_attachment = true
- note.save
-
- if upload.destroy
- say "MigrateLegacyUploads: Upload #{upload.inspect} was destroyed."
- else
- say "MigrateLegacyUploads: Upload #{upload.inspect} destroy failed."
- end
- end
-
- def destroy_legacy_file
- legacy_file_uploader.file.delete
- end
-
- def add_upload_link_to_note_text
- new_text = "#{note.note} \n #{@uploader.markdown_link}"
- note.update!(
- note: new_text
- )
- end
-
- def legacy_file_uploader
- strong_memoize(:legacy_file_uploader) do
- uploader = upload.build_uploader
- uploader.retrieve_from_store!(File.basename(upload.path))
- uploader
- end
- end
-
- def legacy_file_exists?
- legacy_file_uploader.file.exists?
- end
-
- def handle_legacy_note_upload
- note.note += "\n \n Attachment ##{upload.id} with URL \"#{note.attachment.url}\" failed to migrate \
- for model class #{note.class}. See #{help_doc_link}."
- note.save
-
- say "MigrateLegacyUploads: LegacyDiffNote ##{note.id} found, can't move the file: #{upload.inspect} for upload ##{upload.id}. See #{help_doc_link}."
- end
-
- def say(message)
- Rails.logger.info(message)
- end
-
- def help_doc_link
- 'https://docs.gitlab.com/ee/administration/troubleshooting/migrations.html#legacy-upload-migration'
- end
- end
-
- def perform(start_id, end_id)
- Upload.where(id: start_id..end_id, uploader: 'AttachmentUploader').find_each do |upload|
- UploadMover.new(upload).execute
- end
- end
- end
- end
-end
diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb
index 52f6962f16b..426abdc2a6c 100644
--- a/spec/factories/uploads.rb
+++ b/spec/factories/uploads.rb
@@ -54,7 +54,10 @@ FactoryBot.define do
end
trait :attachment_upload do
- mount_point :attachment
+ transient do
+ mount_point :attachment
+ end
+
model { build(:note) }
uploader "AttachmentUploader"
end
diff --git a/spec/lib/gitlab/background_migration/migrate_legacy_uploads_spec.rb b/spec/lib/gitlab/background_migration/migrate_legacy_uploads_spec.rb
deleted file mode 100644
index 16b9de6a84e..00000000000
--- a/spec/lib/gitlab/background_migration/migrate_legacy_uploads_spec.rb
+++ /dev/null
@@ -1,253 +0,0 @@
-# frozen_string_literal: true
-require 'spec_helper'
-
-describe Gitlab::BackgroundMigration::MigrateLegacyUploads, :migration, schema: 20190103140724 do
- let(:test_dir) { FileUploader.options['storage_path'] }
-
- # rubocop: disable RSpec/FactoriesInMigrationSpecs
- let(:namespace) { create(:namespace) }
- let(:project) { create(:project, :legacy_storage, namespace: namespace) }
- let(:issue) { create(:issue, project: project) }
-
- let(:note1) { create(:note, note: 'some note text awesome', project: project, noteable: issue) }
- let(:note2) { create(:note, note: 'some note', project: project, noteable: issue) }
-
- let(:hashed_project) { create(:project, namespace: namespace) }
- let(:issue_hashed_project) { create(:issue, project: hashed_project) }
- let(:note_hashed_project) { create(:note, note: 'some note', project: hashed_project, attachment: 'text.pdf', noteable: issue_hashed_project) }
-
- let(:standard_upload) do
- create(:upload,
- path: "secretabcde/image.png",
- model_id: create(:project).id, model_type: 'Project', uploader: 'FileUploader')
- end
-
- before do
- # This migration was created before we introduced ProjectCiCdSetting#default_git_depth
- allow_any_instance_of(ProjectCiCdSetting).to receive(:default_git_depth).and_return(nil)
- allow_any_instance_of(ProjectCiCdSetting).to receive(:default_git_depth=).and_return(0)
-
- namespace
- project
- issue
- note1
- note2
- hashed_project
- issue_hashed_project
- note_hashed_project
- standard_upload
- end
-
- def create_remote_upload(model, filename)
- create(:upload, :attachment_upload,
- path: "note/attachment/#{model.id}/#{filename}", secret: nil,
- store: ObjectStorage::Store::REMOTE, model: model)
- end
-
- def create_upload(model, filename, with_file = true)
- params = {
- path: "uploads/-/system/note/attachment/#{model.id}/#{filename}",
- model: model,
- store: ObjectStorage::Store::LOCAL
- }
-
- upload = if with_file
- create(:upload, :with_file, :attachment_upload, params)
- else
- create(:upload, :attachment_upload, params)
- end
-
- model.update(attachment: upload.build_uploader)
- model.attachment.upload
- end
-
- let(:start_id) { 1 }
- let(:end_id) { 10000 }
-
- def new_upload_legacy
- Upload.find_by(model_id: project.id, model_type: 'Project')
- end
-
- def new_upload_hashed
- Upload.find_by(model_id: hashed_project.id, model_type: 'Project')
- end
-
- shared_examples 'migrates files correctly' do
- before do
- described_class.new.perform(start_id, end_id)
- end
-
- it 'removes all the legacy upload records' do
- expect(Upload.where(uploader: 'AttachmentUploader')).to be_empty
-
- expect(standard_upload.reload).to eq(standard_upload)
- end
-
- it 'creates new upload records correctly' do
- expect(new_upload_legacy.secret).not_to be_nil
- expect(new_upload_legacy.path).to end_with("#{new_upload_legacy.secret}/image.png")
- expect(new_upload_legacy.model_id).to eq(project.id)
- expect(new_upload_legacy.model_type).to eq('Project')
- expect(new_upload_legacy.uploader).to eq('FileUploader')
-
- expect(new_upload_hashed.secret).not_to be_nil
- expect(new_upload_hashed.path).to end_with("#{new_upload_hashed.secret}/text.pdf")
- expect(new_upload_hashed.model_id).to eq(hashed_project.id)
- expect(new_upload_hashed.model_type).to eq('Project')
- expect(new_upload_hashed.uploader).to eq('FileUploader')
- end
-
- it 'updates the legacy upload notes so that they include the file references in the markdown' do
- expected_path = File.join('/uploads', new_upload_legacy.secret, 'image.png')
- expected_markdown = "some note text awesome \n ![image](#{expected_path})"
- expect(note1.reload.note).to eq(expected_markdown)
-
- expected_path = File.join('/uploads', new_upload_hashed.secret, 'text.pdf')
- expected_markdown = "some note \n [text.pdf](#{expected_path})"
- expect(note_hashed_project.reload.note).to eq(expected_markdown)
- end
-
- it 'removed the attachments from the note model' do
- expect(note1.reload.attachment.file).to be_nil
- expect(note2.reload.attachment.file).to be_nil
- expect(note_hashed_project.reload.attachment.file).to be_nil
- end
- end
-
- context 'when legacy uploads are stored in local storage' do
- let!(:legacy_upload1) { create_upload(note1, 'image.png') }
- let!(:legacy_upload_not_found) { create_upload(note2, 'image.png', false) }
- let!(:legacy_upload_hashed) { create_upload(note_hashed_project, 'text.pdf', with_file: true) }
-
- shared_examples 'removes legacy local files' do
- it 'removes all the legacy upload records' do
- expect(File.exist?(legacy_upload1.absolute_path)).to be_truthy
- expect(File.exist?(legacy_upload_hashed.absolute_path)).to be_truthy
-
- described_class.new.perform(start_id, end_id)
-
- expect(File.exist?(legacy_upload1.absolute_path)).to be_falsey
- expect(File.exist?(legacy_upload_hashed.absolute_path)).to be_falsey
- end
- end
-
- context 'when object storage is disabled for FileUploader' do
- it_behaves_like 'migrates files correctly'
- it_behaves_like 'removes legacy local files'
-
- it 'moves legacy uploads to the correct location' do
- described_class.new.perform(start_id, end_id)
-
- expected_path1 = File.join(test_dir, 'uploads', namespace.path, project.path, new_upload_legacy.secret, 'image.png')
- expected_path2 = File.join(test_dir, 'uploads', hashed_project.disk_path, new_upload_hashed.secret, 'text.pdf')
-
- expect(File.exist?(expected_path1)).to be_truthy
- expect(File.exist?(expected_path2)).to be_truthy
- end
-
- context 'when the upload move fails' do
- it 'does not remove old uploads' do
- expect(FileUploader).to receive(:copy_to).twice.and_raise('failed')
-
- described_class.new.perform(start_id, end_id)
-
- expect(legacy_upload1.reload).to eq(legacy_upload1)
- expect(legacy_upload_hashed.reload).to eq(legacy_upload_hashed)
- expect(standard_upload.reload).to eq(standard_upload)
- end
- end
- end
-
- context 'when object storage is enabled for FileUploader' do
- before do
- stub_uploads_object_storage(FileUploader)
- end
-
- it_behaves_like 'migrates files correctly'
- it_behaves_like 'removes legacy local files'
-
- # The process of migrating to object storage is a manual one,
- # so it would go against expectations to automatically migrate these files
- # to object storage during this migration.
- # After this migration, these files should be able to successfully migrate to object storage.
- it 'stores files locally' do
- described_class.new.perform(start_id, end_id)
-
- expected_path1 = File.join(test_dir, 'uploads', namespace.path, project.path, new_upload_legacy.secret, 'image.png')
- expected_path2 = File.join(test_dir, 'uploads', hashed_project.disk_path, new_upload_hashed.secret, 'text.pdf')
-
- expect(File.exist?(expected_path1)).to be_truthy
- expect(File.exist?(expected_path2)).to be_truthy
- end
- end
-
- context 'with legacy_diff_note upload' do
- let!(:merge_request) { create(:merge_request, source_project: project) }
- let!(:legacy_diff_note) { create(:legacy_diff_note_on_merge_request, note: 'some note', project: project, noteable: merge_request) }
- let!(:legacy_upload_diff_note) do
- create(:upload, :with_file, :attachment_upload,
- path: "uploads/-/system/note/attachment/#{legacy_diff_note.id}/some_legacy.pdf", model: legacy_diff_note)
- end
-
- before do
- described_class.new.perform(start_id, end_id)
- end
-
- it 'does not remove legacy diff note file' do
- expect(File.exist?(legacy_upload_diff_note.absolute_path)).to be_truthy
- end
-
- it 'removes all the legacy upload records except for the one with legacy_diff_note' do
- expect(Upload.where(uploader: 'AttachmentUploader')).to eq([legacy_upload_diff_note])
- end
-
- it 'adds link to the troubleshooting documentation to the note' do
- help_doc_link = 'https://docs.gitlab.com/ee/administration/troubleshooting/migrations.html#legacy-upload-migration'
-
- expect(legacy_diff_note.reload.note).to include(help_doc_link)
- end
- end
- end
-
- context 'when legacy uploads are stored in object storage' do
- let!(:legacy_upload1) { create_remote_upload(note1, 'image.png') }
- let!(:legacy_upload_not_found) { create_remote_upload(note2, 'non-existing.pdf') }
- let!(:legacy_upload_hashed) { create_remote_upload(note_hashed_project, 'text.pdf') }
- let(:remote_files) do
- [
- { key: "#{legacy_upload1.path}" },
- { key: "#{legacy_upload_hashed.path}" }
- ]
- end
- let(:connection) { ::Fog::Storage.new(FileUploader.object_store_credentials) }
- let(:bucket) { connection.directories.create(key: 'uploads') }
-
- def create_remote_files
- remote_files.each { |file| bucket.files.create(file) }
- end
-
- before do
- stub_uploads_object_storage(FileUploader)
- create_remote_files
- end
-
- it_behaves_like 'migrates files correctly'
-
- it 'moves legacy uploads to the correct remote location' do
- described_class.new.perform(start_id, end_id)
-
- connection = ::Fog::Storage.new(FileUploader.object_store_credentials)
- expect(connection.get_object('uploads', new_upload_legacy.path)[:status]).to eq(200)
- expect(connection.get_object('uploads', new_upload_hashed.path)[:status]).to eq(200)
- end
-
- it 'removes all the legacy upload records' do
- described_class.new.perform(start_id, end_id)
-
- remote_files.each do |remote_file|
- expect(bucket.files.get(remote_file[:key])).to be_nil
- end
- end
- end
- # rubocop: enable RSpec/FactoriesInMigrationSpecs
-end