Restore backup correctly when "BACKUP" environment variable is passedrestore-backup-when-env-variable-is-passed

5 files changed, 136 insertions, 36 deletions

diff --git a/changelogs/unreleased/restore-backup-when-env-variable-is-passed.yml b/changelogs/unreleased/restore-backup-when-env-variable-is-passed.yml
new file mode 100644
index 00000000000..8ec3cfdbb08
--- /dev/null
+++ b/changelogs/unreleased/restore-backup-when-env-variable-is-passed.yml
@@ -0,0 +1,4 @@
+---
+title: Restore backup correctly when "BACKUP" environment variable is passed
+merge_request: 8477
+author:
diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md
index 43ddc419054..51f7454610d 100644
--- a/doc/raketasks/backup_restore.md
+++ b/doc/raketasks/backup_restore.md
@@ -9,6 +9,9 @@ This archive will be saved in `backup_path`, which is specified in the
 The filename will be `[TIMESTAMP]_gitlab_backup.tar`, where `TIMESTAMP`
 identifies the time at which each backup was created.
 
+> In GitLab 8.15 we changed the timestamp format from `EPOCH` (`1393513186`)
+> to `EPOCH_YYYY_MM_DD` (`1393513186_2014_02_27`)
+
 You can only restore a backup to exactly the same version of GitLab on which it
 was created.  The best way to migrate your repositories from one server to
 another is through backup restore.
@@ -223,7 +226,8 @@ For installations from source:
 
 ## Backup archive permissions
 
-The backup archives created by GitLab (123456_gitlab_backup.tar) will have owner/group git:git and 0600 permissions by default.
+The backup archives created by GitLab (`1393513186_2014_02_27_gitlab_backup.tar`)
+will have owner/group git:git and 0600 permissions by default.
 This is meant to avoid other system users reading GitLab's data.
 If you need the backup archives to have different permissions you can use the 'archive_permissions' setting.
 
@@ -335,7 +339,7 @@ First make sure your backup tar file is in the backup directory described in the
 `/var/opt/gitlab/backups`.
 
 ```shell
-sudo cp 1393513186_gitlab_backup.tar /var/opt/gitlab/backups/
+sudo cp 1393513186_2014_02_27_gitlab_backup.tar /var/opt/gitlab/backups/
 ```
 
 Stop the processes that are connected to the database.  Leave the rest of GitLab
diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb
index 7e6537e3d9e..cefbfdce3bb 100644
--- a/lib/backup/manager.rb
+++ b/lib/backup/manager.rb
@@ -2,6 +2,7 @@ module Backup
   class Manager
     ARCHIVES_TO_BACKUP = %w[uploads builds artifacts lfs registry]
     FOLDERS_TO_BACKUP = %w[repositories db]
+    FILE_NAME_SUFFIX = '_gitlab_backup.tar'
 
     def pack
       # Make sure there is a connection
@@ -14,7 +15,7 @@ module Backup
       s[:gitlab_version]     = Gitlab::VERSION
       s[:tar_version]        = tar_version
       s[:skipped]            = ENV["SKIP"]
-      tar_file = s[:backup_created_at].strftime('%s_%Y_%m_%d') + '_gitlab_backup.tar'
+      tar_file = "#{s[:backup_created_at].strftime('%s_%Y_%m_%d')}#{FILE_NAME_SUFFIX}"
 
       Dir.chdir(Gitlab.config.backup.path) do
         File.open("#{Gitlab.config.backup.path}/backup_information.yml",
@@ -82,7 +83,7 @@ module Backup
         removed = 0
 
         Dir.chdir(Gitlab.config.backup.path) do
-          Dir.glob('*_gitlab_backup.tar').each do |file|
+          Dir.glob("*#{FILE_NAME_SUFFIX}").each do |file|
             next unless file =~ /(\d+)(?:_\d{4}_\d{2}_\d{2})?_gitlab_backup\.tar/
 
             timestamp = $1.to_i
@@ -108,41 +109,50 @@ module Backup
       Dir.chdir(Gitlab.config.backup.path)
 
       # check for existing backups in the backup dir
-      file_list = Dir.glob("*_gitlab_backup.tar")
-      puts "no backups found" if file_list.count == 0
+      file_list = Dir.glob("*#{FILE_NAME_SUFFIX}")
+
+      if file_list.count == 0
+        $progress.puts "No backups found in #{Gitlab.config.backup.path}"
+        $progress.puts "Please make sure that file name ends with #{FILE_NAME_SUFFIX}"
+        exit 1
+      end
 
       if file_list.count > 1 && ENV["BACKUP"].nil?
-        puts "Found more than one backup, please specify which one you want to restore:"
-        puts "rake gitlab:backup:restore BACKUP=timestamp_of_backup"
+        $progress.puts 'Found more than one backup, please specify which one you want to restore:'
+        $progress.puts 'rake gitlab:backup:restore BACKUP=timestamp_of_backup'
         exit 1
       end
 
-      tar_file = ENV["BACKUP"].nil? ? file_list.first : file_list.grep(ENV['BACKUP']).first
+      if ENV['BACKUP'].present?
+        tar_file = "#{ENV['BACKUP']}#{FILE_NAME_SUFFIX}"
+      else
+        tar_file = file_list.first
+      end
 
       unless File.exist?(tar_file)
-        puts "The specified backup doesn't exist!"
+        $progress.puts "The backup file #{tar_file} does not exist!"
         exit 1
       end
 
-      $progress.print "Unpacking backup ... "
+      $progress.print 'Unpacking backup ... '
 
       unless Kernel.system(*%W(tar -xf #{tar_file}))
-        puts "unpacking backup failed".color(:red)
+        $progress.puts 'unpacking backup failed'.color(:red)
         exit 1
       else
-        $progress.puts "done".color(:green)
+        $progress.puts 'done'.color(:green)
       end
 
       ENV["VERSION"] = "#{settings[:db_version]}" if settings[:db_version].to_i > 0
 
       # restoring mismatching backups can lead to unexpected problems
       if settings[:gitlab_version] != Gitlab::VERSION
-        puts "GitLab version mismatch:".color(:red)
-        puts "  Your current GitLab version (#{Gitlab::VERSION}) differs from the GitLab version in the backup!".color(:red)
-        puts "  Please switch to the following version and try again:".color(:red)
-        puts "  version: #{settings[:gitlab_version]}".color(:red)
-        puts
-        puts "Hint: git checkout v#{settings[:gitlab_version]}"
+        $progress.puts 'GitLab version mismatch:'.color(:red)
+        $progress.puts "  Your current GitLab version (#{Gitlab::VERSION}) differs from the GitLab version in the backup!".color(:red)
+        $progress.puts '  Please switch to the following version and try again:'.color(:red)
+        $progress.puts "  version: #{settings[:gitlab_version]}".color(:red)
+        $progress.puts
+        $progress.puts "Hint: git checkout v#{settings[:gitlab_version]}"
         exit 1
       end
     end
diff --git a/spec/lib/gitlab/backup/manager_spec.rb b/spec/lib/gitlab/backup/manager_spec.rb
index 1b749d1bd39..f84782ab440 100644
--- a/spec/lib/gitlab/backup/manager_spec.rb
+++ b/spec/lib/gitlab/backup/manager_spec.rb
@@ -1,9 +1,27 @@
 require 'spec_helper'
 
 describe Backup::Manager, lib: true do
-  describe '#remove_old' do
-    let(:progress) { StringIO.new }
+  include StubENV
+
+  let(:progress) { StringIO.new }
+
+  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
+
+    @old_progress = $progress # rubocop:disable Style/GlobalVars
+    $progress = progress # rubocop:disable Style/GlobalVars
+  end
+
+  after do
+    $progress = @old_progress # rubocop:disable Style/GlobalVars
+  end
 
+  describe '#remove_old' do
     let(:files) do
       [
         '1451606400_2016_01_01_gitlab_backup.tar',
@@ -20,20 +38,6 @@ describe Backup::Manager, lib: true do
       allow(Dir).to receive(:glob).and_return(files)
       allow(FileUtils).to receive(:rm)
       allow(Time).to receive(:now).and_return(Time.utc(2016))
-
-      allow(progress).to receive(:puts)
-      allow(progress).to receive(:print)
-
-      allow_any_instance_of(String).to receive(:color) do |string, _color|
-        string
-      end
-
-      @old_progress = $progress # rubocop:disable Style/GlobalVars
-      $progress = progress # rubocop:disable Style/GlobalVars
-    end
-
-    after do
-      $progress = @old_progress # rubocop:disable Style/GlobalVars
     end
 
     context 'when keep_time is zero' do
@@ -124,4 +128,82 @@ describe Backup::Manager, lib: true do
       end
     end
   end
+
+  describe '#unpack' do
+    before do
+      allow(Dir).to receive(:chdir)
+    end
+
+    context 'when there are no backup files in the directory' do
+      before do
+        allow(Dir).to receive(:glob).and_return([])
+      end
+
+      it 'fails the operation and prints an error' do
+        expect { subject.unpack }.to raise_error SystemExit
+        expect(progress).to have_received(:puts)
+          .with(a_string_matching('No backups found'))
+      end
+    end
+
+    context 'when there are two backup files in the directory and BACKUP variable is not set' do
+      before do
+        allow(Dir).to receive(:glob).and_return(
+          [
+            '1451606400_2016_01_01_gitlab_backup.tar',
+            '1451520000_2015_12_31_gitlab_backup.tar',
+          ]
+        )
+      end
+
+      it 'fails the operation and prints an error' do
+        expect { subject.unpack }.to raise_error SystemExit
+        expect(progress).to have_received(:puts)
+          .with(a_string_matching('Found more than one backup'))
+      end
+    end
+
+    context 'when BACKUP variable is set to a non-existing file' do
+      before do
+        allow(Dir).to receive(:glob).and_return(
+          [
+            '1451606400_2016_01_01_gitlab_backup.tar'
+          ]
+        )
+        allow(File).to receive(:exist?).and_return(false)
+
+        stub_env('BACKUP', 'wrong')
+      end
+
+      it 'fails the operation and prints an error' do
+        expect { subject.unpack }.to raise_error SystemExit
+        expect(File).to have_received(:exist?).with('wrong_gitlab_backup.tar')
+        expect(progress).to have_received(:puts)
+          .with(a_string_matching('The backup file wrong_gitlab_backup.tar does not exist'))
+      end
+    end
+
+    context 'when BACKUP variable is set to a correct file' do
+      before do
+        allow(Dir).to receive(:glob).and_return(
+          [
+            '1451606400_2016_01_01_gitlab_backup.tar'
+          ]
+        )
+        allow(File).to receive(:exist?).and_return(true)
+        allow(Kernel).to receive(:system).and_return(true)
+        allow(YAML).to receive(:load_file).and_return(gitlab_version: Gitlab::VERSION)
+
+        stub_env('BACKUP', '1451606400_2016_01_01')
+      end
+
+      it 'unpacks the file' do
+        subject.unpack
+
+        expect(Kernel).to have_received(:system)
+          .with("tar", "-xf", "1451606400_2016_01_01_gitlab_backup.tar")
+        expect(progress).to have_received(:puts).with(a_string_matching('done'))
+      end
+    end
+  end
 end
diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb
index a9fea5f1e81..bc751d20ce1 100644
--- a/spec/tasks/gitlab/backup_rake_spec.rb
+++ b/spec/tasks/gitlab/backup_rake_spec.rb
@@ -41,7 +41,7 @@ describe 'gitlab:app namespace rake task' do
 
     context 'gitlab version' do
       before do
-        allow(Dir).to receive(:glob).and_return([])
+        allow(Dir).to receive(:glob).and_return(['1_gitlab_backup.tar'])
         allow(Dir).to receive(:chdir)
         allow(File).to receive(:exist?).and_return(true)
         allow(Kernel).to receive(:system).and_return(true)