Merge branch '5613-backups-fail' into 'master'

Backups fail occasionally with "tar: ./objects: file changed as we read it" error

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

Closes #5613

See merge request !5814

CHANGELOG.md

 Please view this file on the master branch, on stable branches it's out of date.
 
 ## 8.14.0 (2016-11-22)
+  - Backups do not fail anymore when using tar on annex and custom_hooks only. !5814
   - Adds user project membership expired event to clarify why user was removed (Callum Dryden)
   - Trim leading and trailing whitespace on project_path (Linus Thiel)
   - Prevent award emoji via notes for issues/MRs authored by user (barthc)

lib/backup/repository.rb

@@ -2,11 +2,14 @@ require 'yaml'
 
 module Backup
   class Repository
+
     def dump
       prepare
 
       Project.find_each(batch_size: 1000) do |project|
         $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.path)) if project.namespace
@@ -14,8 +17,22 @@ module Backup
         if project.empty_repo?
           $progress.puts "[SKIPPED]".color(:cyan)
         else
-          cmd = %W(tar -cf #{path_to_bundle(project)} -C #{path_to_repo(project)} .)
+          in_path(path_to_project_repo) do |dir|
+            FileUtils.mkdir_p(path_to_tars(project))
+            cmd = %W(tar -cf #{path_to_tars(project, dir)} -C #{path_to_project_repo} #{dir})
+            output, status = Gitlab::Popen.popen(cmd)
+
+            unless status.zero?
+              puts "[FAILED]".color(:red)
+              puts "failed: #{cmd.join(' ')}"
+              puts output
+              abort 'Backup failed'
+            end
+          end
+
+          cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path_to_project_repo} bundle create #{path_to_project_bundle} --all)
           output, status = Gitlab::Popen.popen(cmd)
+
           if status.zero?
             $progress.puts "[DONE]".color(:green)
           else
@@ -27,19 +44,22 @@ module Backup
         end
 
         wiki = ProjectWiki.new(project)
+        path_to_wiki_repo = path_to_repo(wiki)
+        path_to_wiki_bundle = path_to_bundle(wiki)
 
-        if File.exist?(path_to_repo(wiki))
+        if File.exist?(path_to_wiki_repo)
           $progress.print " * #{wiki.path_with_namespace} ... "
           if wiki.repository.empty?
             $progress.puts " [SKIPPED]".color(:cyan)
           else
-            cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path_to_repo(wiki)} bundle create #{path_to_bundle(wiki)} --all)
+            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)
             else
               puts " [FAILED]".color(:red)
               puts "failed: #{cmd.join(' ')}"
+              puts output
               abort 'Backup failed'
             end
           end
@@ -60,40 +80,59 @@ module Backup
 
       Project.find_each(batch_size: 1000) do |project|
         $progress.print " * #{project.path_with_namespace} ... "
+        path_to_project_repo = path_to_repo(project)
+        path_to_project_bundle = path_to_bundle(project)
 
         project.ensure_dir_exist
 
-        if File.exist?(path_to_bundle(project))
-          FileUtils.mkdir_p(path_to_repo(project))
-          cmd = %W(tar -xf #{path_to_bundle(project)} -C #{path_to_repo(project)})
+        if File.exists?(path_to_project_bundle)
+          cmd = %W(#{Gitlab.config.git.bin_path} clone --bare #{path_to_project_bundle} #{path_to_project_repo})
         else
-          cmd = %W(#{Gitlab.config.git.bin_path} init --bare #{path_to_repo(project)})
+          cmd = %W(#{Gitlab.config.git.bin_path} init --bare #{path_to_project_repo})
         end
 
-        if system(*cmd, silent)
+        output, status = Gitlab::Popen.popen(cmd)
+        if status.zero?
           $progress.puts "[DONE]".color(:green)
         else
           puts "[FAILED]".color(:red)
           puts "failed: #{cmd.join(' ')}"
+          puts output
           abort 'Restore failed'
         end
 
+        in_path(path_to_tars(project)) do |dir|
+          cmd = %W(tar -xf #{path_to_tars(project, dir)} -C #{path_to_project_repo} #{dir})
+
+          output, status = Gitlab::Popen.popen(cmd)
+          unless status.zero?
+            puts "[FAILED]".color(:red)
+            puts "failed: #{cmd.join(' ')}"
+            puts output
+            abort 'Restore failed'
+          end
+        end
+
         wiki = ProjectWiki.new(project)
+        path_to_wiki_repo = path_to_repo(wiki)
+        path_to_wiki_bundle = path_to_bundle(wiki)
 
-        if File.exist?(path_to_bundle(wiki))
+        if File.exist?(path_to_wiki_bundle)
           $progress.print " * #{wiki.path_with_namespace} ... "
 
           # If a wiki bundle exists, first remove the empty repo
           # that was initialized with ProjectWiki.new() and then
           # try to restore with 'git clone --bare'.
-          FileUtils.rm_rf(path_to_repo(wiki))
-          cmd = %W(#{Gitlab.config.git.bin_path} clone --bare #{path_to_bundle(wiki)} #{path_to_repo(wiki)})
+          FileUtils.rm_rf(path_to_wiki_repo)
+          cmd = %W(#{Gitlab.config.git.bin_path} clone --bare #{path_to_wiki_bundle} #{path_to_wiki_repo})
 
-          if system(*cmd, silent)
+          output, status = Gitlab::Popen.popen(cmd)
+          if status.zero?
             $progress.puts " [DONE]".color(:green)
           else
             puts " [FAILED]".color(:red)
             puts "failed: #{cmd.join(' ')}"
+            puts output
             abort 'Restore failed'
           end
         end
@@ -101,13 +140,15 @@ module Backup
 
       $progress.print 'Put GitLab hooks in repositories dirs'.color(:yellow)
       cmd = %W(#{Gitlab.config.gitlab_shell.path}/bin/create-hooks) + repository_storage_paths_args
-      if system(*cmd)
+
+      output, status = Gitlab::Popen.popen(cmd)
+      if status.zero?
         $progress.puts " [DONE]".color(:green)
       else
         puts " [FAILED]".color(:red)
         puts "failed: #{cmd}"
+        puts output
       end
-
     end
 
     protected
@@ -117,11 +158,30 @@ module Backup
     end
 
     def path_to_bundle(project)
-      File.join(backup_repos_path, project.path_with_namespace + ".bundle")
+      File.join(backup_repos_path, project.path_with_namespace + '.bundle')
+    end
+
+    def path_to_tars(project, dir = nil)
+      path = File.join(backup_repos_path, project.path_with_namespace)
+
+      if dir
+        File.join(path, "#{dir}.tar")
+      else
+        path
+      end
     end
 
     def backup_repos_path
-      File.join(Gitlab.config.backup.path, "repositories")
+      File.join(Gitlab.config.backup.path, 'repositories')
+    end
+
+    def in_path(path)
+      return unless Dir.exist?(path)
+
+      dir_entries = Dir.entries(path)
+      %w[annex custom_hooks].each do |entry|
+        yield(entry) if dir_entries.include?(entry)
+      end
     end
 
     def prepare

spec/tasks/gitlab/backup_rake_spec.rb

@@ -79,7 +79,7 @@ describe 'gitlab:app namespace rake task' do
     end
   end # backup_restore task
 
-  describe 'backup_create' do
+  describe 'backup' do
     def tars_glob
       Dir.glob(File.join(Gitlab.config.backup.path, '*_gitlab_backup.tar'))
     end
@@ -98,6 +98,78 @@ describe 'gitlab:app namespace rake task' do
       @backup_tar = tars_glob.first
     end
 
+    def restore_backup
+      orig_stdout = $stdout
+      $stdout = StringIO.new
+      reenable_backup_sub_tasks
+      run_rake_task('gitlab:backup:restore')
+      reenable_backup_sub_tasks
+      $stdout = orig_stdout
+    end
+
+    describe 'backup creation and deletion using annex and custom_hooks' do
+      let(:project) { create(:project) }
+      let(:user_backup_path) { "repositories/#{project.path_with_namespace}" }
+
+      before(:each) do
+        @origin_cd = Dir.pwd
+
+        path = File.join(project.repository.path_to_repo, filename)
+        FileUtils.mkdir_p(path)
+        FileUtils.touch(File.join(path, "dummy.txt"))
+
+        # We need to use the full path instead of the relative one
+        allow(Gitlab.config.gitlab_shell).to receive(:path).and_return(File.expand_path(Gitlab.config.gitlab_shell.path, Rails.root.to_s))
+
+        ENV["SKIP"] = "db"
+        create_backup
+      end
+
+      after(:each) do
+        ENV["SKIP"] = ""
+        FileUtils.rm(@backup_tar)
+        Dir.chdir(@origin_cd)
+      end
+
+      context 'project uses git-annex and successfully creates backup' do
+        let(:filename) { "annex" }
+
+        it 'creates annex.tar and project bundle' do
+          tar_contents, exit_status = Gitlab::Popen.popen(%W{tar -tvf #{@backup_tar}})
+
+          expect(exit_status).to eq(0)
+          expect(tar_contents).to match(user_backup_path)
+          expect(tar_contents).to match("#{user_backup_path}/annex.tar")
+          expect(tar_contents).to match("#{user_backup_path}.bundle")
+        end
+
+        it 'restores files correctly' do
+          restore_backup
+
+          expect(Dir.entries(File.join(project.repository.path, "annex"))).to include("dummy.txt")
+        end
+      end
+
+      context 'project uses custom_hooks and successfully creates backup' do
+        let(:filename) { "custom_hooks" }
+
+        it 'creates custom_hooks.tar and project bundle' do
+          tar_contents, exit_status = Gitlab::Popen.popen(%W{tar -tvf #{@backup_tar}})
+
+          expect(exit_status).to eq(0)
+          expect(tar_contents).to match(user_backup_path)
+          expect(tar_contents).to match("#{user_backup_path}/custom_hooks.tar")
+          expect(tar_contents).to match("#{user_backup_path}.bundle")
+        end
+
+        it 'restores files correctly' do
+          restore_backup
+
+          expect(Dir.entries(File.join(project.repository.path, "custom_hooks"))).to include("dummy.txt")
+        end
+      end
+    end
+
     context 'tar creation' do
       before do
         create_backup