Merge branch 'fix/backup-restore-resume' into 'master'

Fix backup task to continue on corrupt repositories

Closes #31767

See merge request !11962

3 files changed, 104 insertions, 38 deletions

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