diff --git a/ee/spec/lib/ee/backup/repositories_spec.rb b/ee/spec/lib/ee/backup/repositories_spec.rb index 6bf71f05385ae0a12dbbbc3d16973eabd15e8d64..096e543b43bb8822c70bd678b82c6a70329bf5e3 100644 --- a/ee/spec/lib/ee/backup/repositories_spec.rb +++ b/ee/spec/lib/ee/backup/repositories_spec.rb @@ -85,9 +85,14 @@ let_it_be(:group) { create(:group, :wiki_repo) } it 'calls enqueue for each repository type', :aggregate_failures do - subject.restore(destination) - - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default]) + subject.restore(destination, backup_id) + + expect(strategy).to have_received(:start).with( + :restore, + destination, + remove_all_repositories: %w[default], + backup_id: backup_id + ) expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::PROJECT) expect(strategy).to have_received(:enqueue).with(group, Gitlab::GlRepository::WIKI) expect(strategy).to have_received(:finish!) @@ -107,9 +112,14 @@ excluded_group = create(:group, :wiki_repo) excluded_group.group_wiki_repository.update!(shard_name: 'test_second_storage') - subject.restore(destination) + subject.restore(destination, backup_id) - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default]) + expect(strategy).to have_received(:start).with( + :restore, + destination, + remove_all_repositories: %w[default], + backup_id: backup_id + ) expect(strategy).not_to have_received(:enqueue).with(excluded_group, Gitlab::GlRepository::WIKI) expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::PROJECT) expect(strategy).to have_received(:enqueue).with(group, Gitlab::GlRepository::WIKI) @@ -121,9 +131,14 @@ let(:paths) { [group.full_path] } it 'calls enqueue for all descendant repositories on the specified group', :aggregate_failures do - subject.restore(destination) - - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: nil) + subject.restore(destination, backup_id) + + expect(strategy).to have_received(:start).with( + :restore, + destination, + remove_all_repositories: nil, + backup_id: backup_id + ) expect(strategy).not_to have_received(:enqueue).with(project, Gitlab::GlRepository::PROJECT) expect(strategy).to have_received(:enqueue).with(group, Gitlab::GlRepository::WIKI) expect(strategy).to have_received(:finish!) diff --git a/lib/backup/database.rb b/lib/backup/database.rb index f70a7e4186267742b0cccf12210d503e36092243..58a8c19c1ce522f96afc1f9b871f501532083427 100644 --- a/lib/backup/database.rb +++ b/lib/backup/database.rb @@ -75,7 +75,7 @@ def dump(destination_dir, backup_id) end override :restore - def restore(destination_dir) + def restore(destination_dir, backup_id) base_models_for_backup.each do |database_name, _base_model| backup_model = Backup::DatabaseModel.new(database_name) diff --git a/lib/backup/files.rb b/lib/backup/files.rb index 9b019f16ddd8af0119c092f9ab02ccda942d0dfa..b8ff7fff591b0cc86a7c06b6b531ba4334002e80 100644 --- a/lib/backup/files.rb +++ b/lib/backup/files.rb @@ -53,7 +53,7 @@ def dump(backup_tarball, backup_id) end override :restore - def restore(backup_tarball) + def restore(backup_tarball, backup_id) backup_existing_files_dir(backup_tarball) cmd_list = [%w[gzip -cd], %W[#{tar} --unlink-first --recursive-unlink -C #{app_files_realpath} -xf -]] diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index 004617c412ac9d237bade78dedf17adf1bbb1749..1c53e675b2abaefda1ac2827f3316923533681de 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -102,7 +102,7 @@ def run_restore_task(task_name) Gitlab::TaskHelpers.ask_to_continue end - definition.task.restore(File.join(Gitlab.config.backup.path, definition.destination_path)) + definition.task.restore(File.join(Gitlab.config.backup.path, definition.destination_path), backup_id) puts_time "Restoring #{definition.human_name} ... ".color(:blue) + "done".color(:green) diff --git a/lib/backup/repositories.rb b/lib/backup/repositories.rb index 3b1547148d8ac78eff9daa8fed15606ed4fc0753..538edcebd3ee40e5b955fc576cc389e4d372536b 100644 --- a/lib/backup/repositories.rb +++ b/lib/backup/repositories.rb @@ -31,8 +31,8 @@ def dump(destination_path, backup_id) end override :restore - def restore(destination_path) - strategy.start(:restore, destination_path, remove_all_repositories: remove_all_repositories) + def restore(destination_path, backup_id) + strategy.start(:restore, destination_path, remove_all_repositories: remove_all_repositories, backup_id: backup_id) enqueue_consecutive ensure diff --git a/lib/backup/task.rb b/lib/backup/task.rb index 776c19130a734675116a30d5a5d1f04883f44936..65059f3a3cba71b7929e5dc0f282a357ac3aae16 100644 --- a/lib/backup/task.rb +++ b/lib/backup/task.rb @@ -15,7 +15,7 @@ def dump(path, backup_id) end # restore task backup from `path` - def restore(path) + def restore(path, backup_id) raise NotImplementedError end diff --git a/spec/lib/backup/database_spec.rb b/spec/lib/backup/database_spec.rb index 2f14b4035761a542c3e0e19658e43bf2acf0e0f1..7a5a9e5fa37fd5e97c5fc48116948d6f74b20bac 100644 --- a/spec/lib/backup/database_spec.rb +++ b/spec/lib/backup/database_spec.rb @@ -5,6 +5,7 @@ RSpec.describe Backup::Database, :reestablished_active_record_base, feature_category: :backup_restore do let(:progress) { StringIO.new } let(:output) { progress.string } + let(:backup_id) { 'some_id' } let(:one_database_configured?) { base_models_for_backup.one? } let(:timeout_service) do instance_double(Gitlab::Database::TransactionTimeoutSettings, restore_timeouts: nil, disable_timeouts: nil) @@ -26,7 +27,6 @@ end describe '#dump', :delete do - let(:backup_id) { 'some_id' } let(:force) { true } subject { described_class.new(progress, force: force) } @@ -222,7 +222,7 @@ expect(Rake::Task['gitlab:db:drop_tables:main']).to receive(:invoke) end - subject.restore(backup_dir) + subject.restore(backup_dir, backup_id) expect(output).to include('Removing all tables. Press `Ctrl-C` within 5 seconds to abort') end @@ -240,7 +240,7 @@ expect(Rake::Task['gitlab:db:drop_tables:main']).to receive(:invoke) end - subject.restore(backup_dir) + subject.restore(backup_dir, backup_id) expect(output).to include("Restoring PostgreSQL database") expect(output).to include("[DONE]") @@ -260,7 +260,7 @@ expect(Rake::Task['gitlab:db:drop_tables:main']).to receive(:invoke) end - expect { subject.restore(backup_dir) }.to raise_error(Backup::Error) + expect { subject.restore(backup_dir, backup_id) }.to raise_error(Backup::Error) end end @@ -276,7 +276,7 @@ expect(Rake::Task['gitlab:db:drop_tables:main']).to receive(:invoke) end - subject.restore(backup_dir) + subject.restore(backup_dir, backup_id) expect(output).to include("ERRORS") expect(output).not_to include(noise) @@ -305,7 +305,7 @@ expect(ENV).to receive(:merge!).with(hash_including { 'PGHOST' => 'test.example.com' }) expect(ENV).not_to receive(:[]=).with('PGPASSWORD', anything) - subject.restore(backup_dir) + subject.restore(backup_dir, backup_id) expect(ENV['PGPORT']).to eq(config['port']) if config['port'] expect(ENV['PGUSER']).to eq(config['username']) if config['username'] @@ -328,14 +328,14 @@ end expect do - subject.restore('db') + subject.restore('db', backup_id) end.to raise_error(Backup::Error, /Source database file does not exist/) end end context 'for ci database' do it 'ci database tolerates missing source file' do - expect { subject.restore(backup_dir) }.not_to raise_error + expect { subject.restore(backup_dir, backup_id) }.not_to raise_error end end end diff --git a/spec/lib/backup/files_spec.rb b/spec/lib/backup/files_spec.rb index f98b5e1414f5eafd6677c5a9cc84ff71c2b95544..9cb57baba25c9a935058ba690780b298c9df647a 100644 --- a/spec/lib/backup/files_spec.rb +++ b/spec/lib/backup/files_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Backup::Files do +RSpec.describe Backup::Files, feature_category: :backup_restore do let(:progress) { StringIO.new } let!(:project) { create(:project) } @@ -58,11 +58,11 @@ it 'moves all necessary files' do allow(subject).to receive(:backup_existing_files).and_call_original expect(FileUtils).to receive(:mv).with(["/var/gitlab-registry/sample1"], File.join(Gitlab.config.backup.path, "tmp", "registry.#{Time.now.to_i}")) - subject.restore('registry.tar.gz') + subject.restore('registry.tar.gz', 'backup_id') end it 'raises no errors' do - expect { subject.restore('registry.tar.gz') }.not_to raise_error + expect { subject.restore('registry.tar.gz', 'backup_id') }.not_to raise_error end it 'calls tar command with unlink' do @@ -70,13 +70,13 @@ expect(subject).to receive(:run_pipeline!).with([%w(gzip -cd), %w(blabla-tar --unlink-first --recursive-unlink -C /var/gitlab-registry -xf -)], any_args) expect(subject).to receive(:pipeline_succeeded?).and_return(true) - subject.restore('registry.tar.gz') + subject.restore('registry.tar.gz', 'backup_id') end it 'raises an error on failure' do expect(subject).to receive(:pipeline_succeeded?).and_return(false) - expect { subject.restore('registry.tar.gz') }.to raise_error(/Restore operation failed:/) + expect { subject.restore('registry.tar.gz', 'backup_id') }.to raise_error(/Restore operation failed:/) end end @@ -89,7 +89,7 @@ it 'shows error message' do expect(subject).to receive(:access_denied_error).with("/var/gitlab-registry") - subject.restore('registry.tar.gz') + subject.restore('registry.tar.gz', 'backup_id') end end @@ -104,7 +104,7 @@ expect(subject).to receive(:resource_busy_error).with("/var/gitlab-registry") .and_call_original - expect { subject.restore('registry.tar.gz') }.to raise_error(/is a mountpoint/) + expect { subject.restore('registry.tar.gz', 'backup_id') }.to raise_error(/is a mountpoint/) end end end diff --git a/spec/lib/backup/manager_spec.rb b/spec/lib/backup/manager_spec.rb index dbd82ec05ebcdb8564cab5ea4aaf8dfdf707c08f..6061441d5f51949e1dccf3234f904b36ebfb6d71 100644 --- a/spec/lib/backup/manager_spec.rb +++ b/spec/lib/backup/manager_spec.rb @@ -69,7 +69,7 @@ let(:pre_restore_warning) { nil } let(:post_restore_warning) { nil } let(:definitions) { { 'my_task' => Backup::Manager::TaskDefinition.new(task: task, enabled: enabled, human_name: 'my task', destination_path: 'my_task.tar.gz') } } - let(:backup_information) { {} } + let(:backup_information) { { backup_created_at: Time.zone.parse('2019-01-01'), gitlab_version: '12.3' } } let(:task) do instance_double(Backup::Task, pre_restore_warning: pre_restore_warning, @@ -936,6 +936,8 @@ end let(:gitlab_version) { Gitlab::VERSION } + let(:backup_id) { "1546300800_2019_01_01_#{gitlab_version}" } + let(:backup_information) do { backup_created_at: Time.zone.parse('2019-01-01'), @@ -948,8 +950,8 @@ Rake.application.rake_require 'tasks/cache' allow(Gitlab::BackupLogger).to receive(:info) - allow(task1).to receive(:restore).with(File.join(Gitlab.config.backup.path, 'task1.tar.gz')) - allow(task2).to receive(:restore).with(File.join(Gitlab.config.backup.path, 'task2.tar.gz')) + allow(task1).to receive(:restore).with(File.join(Gitlab.config.backup.path, 'task1.tar.gz'), backup_id) + allow(task2).to receive(:restore).with(File.join(Gitlab.config.backup.path, 'task2.tar.gz'), backup_id) allow(YAML).to receive(:safe_load_file).with(File.join(Gitlab.config.backup.path, 'backup_information.yml'), permitted_classes: described_class::YAML_PERMITTED_CLASSES) .and_return(backup_information) @@ -1014,6 +1016,7 @@ context 'when BACKUP variable is set to a correct file' do let(:tar_cmdline) { %w{tar -xf 1451606400_2016_01_01_1.2.3_gitlab_backup.tar} } + let(:backup_id) { "1451606400_2016_01_01_1.2.3" } before do allow(Gitlab::BackupLogger).to receive(:info) diff --git a/spec/lib/backup/repositories_spec.rb b/spec/lib/backup/repositories_spec.rb index 1f3818de4a0c04a37e197ddbbfdf782a70a9cf88..b549f2d355d724cb71c0306b448b7739c1f24a89 100644 --- a/spec/lib/backup/repositories_spec.rb +++ b/spec/lib/backup/repositories_spec.rb @@ -215,9 +215,9 @@ let_it_be(:project_snippet) { create(:project_snippet, :repository, project: project, author: project.first_owner) } it 'calls enqueue for each repository type', :aggregate_failures do - subject.restore(destination) + subject.restore(destination, backup_id) - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default]) + expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default], backup_id: backup_id) expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::PROJECT) expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::WIKI) expect(strategy).to have_received(:enqueue).with(project.design_management_repository, Gitlab::GlRepository::DESIGN) @@ -231,7 +231,7 @@ pool_repository = create(:pool_repository, :failed) pool_repository.delete_object_pool - subject.restore(destination) + subject.restore(destination, backup_id) pool_repository.reload expect(pool_repository).not_to be_failed @@ -242,7 +242,7 @@ pool_repository = create(:pool_repository, state: :obsolete) pool_repository.update_column(:source_project_id, nil) - subject.restore(destination) + subject.restore(destination, backup_id) pool_repository.reload expect(pool_repository).to be_obsolete @@ -256,14 +256,14 @@ end it 'shows the appropriate error' do - subject.restore(destination) + subject.restore(destination, backup_id) expect(progress).to have_received(:puts).with("Snippet #{personal_snippet.full_path} can't be restored: Repository has more than one branch") expect(progress).to have_received(:puts).with("Snippet #{project_snippet.full_path} can't be restored: Repository has more than one branch") end it 'removes the snippets from the DB' do - expect { subject.restore(destination) }.to change(PersonalSnippet, :count).by(-1) + expect { subject.restore(destination, backup_id) }.to change(PersonalSnippet, :count).by(-1) .and change(ProjectSnippet, :count).by(-1) .and change(SnippetRepository, :count).by(-2) end @@ -273,7 +273,7 @@ shard_name = personal_snippet.repository.shard path = personal_snippet.disk_path + '.git' - subject.restore(destination) + subject.restore(destination, backup_id) expect(gitlab_shell.repository_exists?(shard_name, path)).to eq false end @@ -296,9 +296,9 @@ excluded_personal_snippet = create(:personal_snippet, :repository, author: excluded_project.first_owner) excluded_personal_snippet.track_snippet_repository('test_second_storage') - subject.restore(destination) + subject.restore(destination, backup_id) - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default]) + expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default], backup_id: backup_id) expect(strategy).not_to have_received(:enqueue).with(excluded_project, Gitlab::GlRepository::PROJECT) expect(strategy).not_to have_received(:enqueue).with(excluded_project_snippet, Gitlab::GlRepository::SNIPPET) expect(strategy).not_to have_received(:enqueue).with(excluded_personal_snippet, Gitlab::GlRepository::SNIPPET) @@ -318,9 +318,9 @@ excluded_project_snippet = create(:project_snippet, :repository, project: excluded_project) excluded_personal_snippet = create(:personal_snippet, :repository, author: excluded_project.first_owner) - subject.restore(destination) + subject.restore(destination, backup_id) - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: nil) + expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: nil, backup_id: backup_id) expect(strategy).not_to have_received(:enqueue).with(excluded_project, Gitlab::GlRepository::PROJECT) expect(strategy).not_to have_received(:enqueue).with(excluded_project_snippet, Gitlab::GlRepository::SNIPPET) expect(strategy).not_to have_received(:enqueue).with(excluded_personal_snippet, Gitlab::GlRepository::SNIPPET) @@ -339,9 +339,9 @@ excluded_project_snippet = create(:project_snippet, :repository, project: excluded_project) excluded_personal_snippet = create(:personal_snippet, :repository, author: excluded_project.first_owner) - subject.restore(destination) + subject.restore(destination, backup_id) - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: nil) + expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: nil, backup_id: backup_id) expect(strategy).not_to have_received(:enqueue).with(excluded_project, Gitlab::GlRepository::PROJECT) expect(strategy).not_to have_received(:enqueue).with(excluded_project_snippet, Gitlab::GlRepository::SNIPPET) expect(strategy).not_to have_received(:enqueue).with(excluded_personal_snippet, Gitlab::GlRepository::SNIPPET) @@ -363,9 +363,9 @@ excluded_project_snippet = create(:project_snippet, :repository, project: excluded_project) included_personal_snippet = create(:personal_snippet, :repository, author: excluded_project.first_owner) - subject.restore(destination) + subject.restore(destination, backup_id) - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default]) + expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default], backup_id: backup_id) expect(strategy).not_to have_received(:enqueue).with(excluded_project, Gitlab::GlRepository::PROJECT) expect(strategy).not_to have_received(:enqueue).with(excluded_project_snippet, Gitlab::GlRepository::SNIPPET) expect(strategy).to have_received(:enqueue).with(included_personal_snippet, Gitlab::GlRepository::SNIPPET) @@ -383,9 +383,9 @@ excluded_project_snippet = create(:project_snippet, :repository, project: excluded_project) included_personal_snippet = create(:personal_snippet, :repository, author: excluded_project.first_owner) - subject.restore(destination) + subject.restore(destination, backup_id) - expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default]) + expect(strategy).to have_received(:start).with(:restore, destination, remove_all_repositories: %w[default], backup_id: backup_id) expect(strategy).not_to have_received(:enqueue).with(excluded_project, Gitlab::GlRepository::PROJECT) expect(strategy).not_to have_received(:enqueue).with(excluded_project_snippet, Gitlab::GlRepository::SNIPPET) expect(strategy).to have_received(:enqueue).with(included_personal_snippet, Gitlab::GlRepository::SNIPPET) diff --git a/spec/lib/backup/task_spec.rb b/spec/lib/backup/task_spec.rb index 1de997295128e5d9a1625acd41a9ba9ff266b5b6..370d9e4a64f491cb9093ec5a44b568f078f7ab6e 100644 --- a/spec/lib/backup/task_spec.rb +++ b/spec/lib/backup/task_spec.rb @@ -15,7 +15,7 @@ describe '#restore' do it 'must be implemented by the subclass' do - expect { subject.restore('some/path') }.to raise_error(NotImplementedError) + expect { subject.restore('some/path', 'backup_id') }.to raise_error(NotImplementedError) end end end