From 1d1a55d03380538fc3aa34afb7c2535af0416136 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 7 Jun 2017 17:55:49 +0000 Subject: Merge branch 'fix/backup-restore-resume' into 'master' Fix backup task to continue on corrupt repositories Closes #31767 See merge request !11962 --- .../unreleased/fix-backup-restore-resume.yml | 4 ++ lib/backup/repository.rb | 75 +++++++++++----------- spec/lib/gitlab/backup/repository_spec.rb | 63 ++++++++++++++++++ 3 files changed, 104 insertions(+), 38 deletions(-) create mode 100644 changelogs/unreleased/fix-backup-restore-resume.yml create mode 100644 spec/lib/gitlab/backup/repository_spec.rb diff --git a/changelogs/unreleased/fix-backup-restore-resume.yml b/changelogs/unreleased/fix-backup-restore-resume.yml new file mode 100644 index 00000000000..b7dfd451f5d --- /dev/null +++ b/changelogs/unreleased/fix-backup-restore-resume.yml @@ -0,0 +1,4 @@ +--- +title: Make backup task to continue on corrupt repositories +merge_request: 11962 +author: diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 6b29600a751..a1685c77916 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -7,15 +7,15 @@ module Backup prepare Project.find_each(batch_size: 1000) do |project| - $progress.print " * #{project.path_with_namespace} ... " + progress.print " * #{project.path_with_namespace} ... " path_to_project_repo = path_to_repo(project) path_to_project_bundle = path_to_bundle(project) # Create namespace dir if missing FileUtils.mkdir_p(File.join(backup_repos_path, project.namespace.full_path)) if project.namespace - if project.empty_repo? - $progress.puts "[SKIPPED]".color(:cyan) + if empty_repo?(project) + progress.puts "[SKIPPED]".color(:cyan) else in_path(path_to_project_repo) do |dir| FileUtils.mkdir_p(path_to_tars(project)) @@ -23,10 +23,7 @@ module Backup output, status = Gitlab::Popen.popen(cmd) unless status.zero? - puts "[FAILED]".color(:red) - puts "failed: #{cmd.join(' ')}" - puts output - abort 'Backup failed' + progress_warn(project, cmd.join(' '), output) end end @@ -34,12 +31,9 @@ module Backup output, status = Gitlab::Popen.popen(cmd) if status.zero? - $progress.puts "[DONE]".color(:green) + progress.puts "[DONE]".color(:green) else - puts "[FAILED]".color(:red) - puts "failed: #{cmd.join(' ')}" - puts output - abort 'Backup failed' + progress_warn(project, cmd.join(' '), output) end end @@ -48,19 +42,16 @@ module Backup path_to_wiki_bundle = path_to_bundle(wiki) if File.exist?(path_to_wiki_repo) - $progress.print " * #{wiki.path_with_namespace} ... " - if wiki.repository.empty? - $progress.puts " [SKIPPED]".color(:cyan) + progress.print " * #{wiki.path_with_namespace} ... " + if empty_repo?(wiki) + progress.puts " [SKIPPED]".color(:cyan) else cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path_to_wiki_repo} bundle create #{path_to_wiki_bundle} --all) output, status = Gitlab::Popen.popen(cmd) if status.zero? - $progress.puts " [DONE]".color(:green) + progress.puts " [DONE]".color(:green) else - puts " [FAILED]".color(:red) - puts "failed: #{cmd.join(' ')}" - puts output - abort 'Backup failed' + progress_warn(wiki, cmd.join(' '), output) end end end @@ -80,7 +71,7 @@ module Backup end Project.find_each(batch_size: 1000) do |project| - $progress.print " * #{project.path_with_namespace} ... " + progress.print " * #{project.path_with_namespace} ... " path_to_project_repo = path_to_repo(project) path_to_project_bundle = path_to_bundle(project) @@ -94,12 +85,9 @@ module Backup output, status = Gitlab::Popen.popen(cmd) if status.zero? - $progress.puts "[DONE]".color(:green) + progress.puts "[DONE]".color(:green) else - puts "[FAILED]".color(:red) - puts "failed: #{cmd.join(' ')}" - puts output - abort 'Restore failed' + progress_warn(project, cmd.join(' '), output) end in_path(path_to_tars(project)) do |dir| @@ -107,10 +95,7 @@ module Backup output, status = Gitlab::Popen.popen(cmd) unless status.zero? - puts "[FAILED]".color(:red) - puts "failed: #{cmd.join(' ')}" - puts output - abort 'Restore failed' + progress_warn(project, cmd.join(' '), output) end end @@ -119,7 +104,7 @@ module Backup path_to_wiki_bundle = path_to_bundle(wiki) if File.exist?(path_to_wiki_bundle) - $progress.print " * #{wiki.path_with_namespace} ... " + progress.print " * #{wiki.path_with_namespace} ... " # If a wiki bundle exists, first remove the empty repo # that was initialized with ProjectWiki.new() and then @@ -129,22 +114,19 @@ module Backup output, status = Gitlab::Popen.popen(cmd) if status.zero? - $progress.puts " [DONE]".color(:green) + progress.puts " [DONE]".color(:green) else - puts " [FAILED]".color(:red) - puts "failed: #{cmd.join(' ')}" - puts output - abort 'Restore failed' + progress_warn(project, cmd.join(' '), output) end end end - $progress.print 'Put GitLab hooks in repositories dirs'.color(:yellow) + progress.print 'Put GitLab hooks in repositories dirs'.color(:yellow) cmd = %W(#{Gitlab.config.gitlab_shell.path}/bin/create-hooks) + repository_storage_paths_args output, status = Gitlab::Popen.popen(cmd) if status.zero? - $progress.puts " [DONE]".color(:green) + progress.puts " [DONE]".color(:green) else puts " [FAILED]".color(:red) puts "failed: #{cmd}" @@ -201,8 +183,25 @@ module Backup private + def progress_warn(project, cmd, output) + progress.puts "[WARNING] Executing #{cmd}".color(:orange) + progress.puts "Ignoring error on #{project.path_with_namespace} - #{output}".color(:orange) + end + + def empty_repo?(project_or_wiki) + project_or_wiki.repository.empty_repo? + rescue => e + progress.puts "Ignoring repository error and continuing backing up project: #{project_or_wiki.path_with_namespace} - #{e.message}".color(:orange) + + false + end + def repository_storage_paths_args Gitlab.config.repositories.storages.values.map { |rs| rs['path'] } end + + def progress + $progress + end end end diff --git a/spec/lib/gitlab/backup/repository_spec.rb b/spec/lib/gitlab/backup/repository_spec.rb new file mode 100644 index 00000000000..51c1e9d657b --- /dev/null +++ b/spec/lib/gitlab/backup/repository_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe Backup::Repository, lib: true do + let(:progress) { StringIO.new } + let!(:project) { create(:empty_project) } + + before do + allow(progress).to receive(:puts) + allow(progress).to receive(:print) + + allow_any_instance_of(String).to receive(:color) do |string, _color| + string + end + + allow_any_instance_of(described_class).to receive(:progress).and_return(progress) + end + + describe '#dump' do + describe 'repo failure' do + before do + allow_any_instance_of(Repository).to receive(:empty_repo?).and_raise(Rugged::OdbError) + allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0]) + end + + it 'does not raise error' do + expect { described_class.new.dump }.not_to raise_error + end + + it 'shows the appropriate error' do + described_class.new.dump + + expect(progress).to have_received(:puts).with("Ignoring repository error and continuing backing up project: #{project.full_path} - Rugged::OdbError") + end + end + + describe 'command failure' do + before do + allow_any_instance_of(Repository).to receive(:empty_repo?).and_return(false) + allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) + end + + it 'shows the appropriate error' do + described_class.new.dump + + expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error") + end + end + end + + describe '#restore' do + describe 'command failure' do + before do + allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) + end + + it 'shows the appropriate error' do + described_class.new.restore + + expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error") + end + end + end +end -- cgit v1.2.1