Restore backup correctly when "BACKUP" environment variable is passed

changelogs/unreleased/restore-backup-when-env-variable-is-passed.yml

+---
+title: Restore backup correctly when "BACKUP" environment variable is passed
+merge_request: 8477
+author:

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

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

spec/lib/gitlab/backup/manager_spec.rb

 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

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)