Merge branch 'skip-tar' into 'master'

Allow user to skip archive creation on backup

See merge request gitlab-org/gitlab!25003

doc/raketasks/backup_restore.md

@@ -271,6 +271,31 @@ For installations from source:
 sudo -u git -H bundle exec rake gitlab:backup:create SKIP=db,uploads RAILS_ENV=production
 ```
 
+### Skipping tar creation
+
+The last part of creating a backup is generation of a `.tar` file containing
+all the parts. In some cases (for example, if the backup is picked up by other
+backup software) creating a `.tar` file might be wasted effort or even directly
+harmful, so you can skip this step by adding `tar` to the `SKIP` environment
+variable.
+
+Adding `tar` to the `SKIP` variable leaves the files and directories containing the
+backup in the directory used for the intermediate files. These files will be
+overwritten when a new backup is created, so you should make sure they are copied
+elsewhere, because you can only have one backup on the system.
+
+For Omnibus GitLab packages:
+
+```shell
+sudo gitlab-backup create SKIP=tar
+```
+
+For installations from source:
+
+```shell
+sudo -u git -H bundle exec rake gitlab:backup:create SKIP=tar RAILS_ENV=production
+```
+
 ### Uploading backups to a remote (cloud) storage
 
 Starting with GitLab 7.4 you can let the backup script upload the '.tar' file it creates.
@@ -658,6 +683,10 @@ lose access to your GitLab server.
 
 You may also want to restore any TLS keys, certificates, or [SSH host keys](https://superuser.com/questions/532040/copy-ssh-keys-from-one-server-to-another-server/532079#532079).
 
+Starting with GitLab 12.9 if an untarred backup (like the ones made with
+`SKIP=tar`) is found, and no backup is chosen with `BACKUP=<timestamp>`, the
+untarred backup is used.
+
 Depending on your case, you might want to run the restore command with one or
 more of the following options:
 

lib/backup/manager.rb

@@ -12,7 +12,7 @@ module Backup
       @progress = progress
     end
 
-    def pack
+    def write_info
       # Make sure there is a connection
       ActiveRecord::Base.connection.reconnect!
 
@@ -20,7 +20,11 @@ module Backup
         File.open("#{backup_path}/backup_information.yml", "w+") do |file|
           file << backup_information.to_yaml.gsub(/^---\n/, '')
         end
+      end
+    end
 
+    def pack
+      Dir.chdir(backup_path) do
         # create archive
         progress.print "Creating backup archive: #{tar_file} ... "
         # Set file permissions on open to prevent chmod races.
@@ -31,8 +35,6 @@ module Backup
           puts "creating archive #{tar_file} failed".color(:red)
           raise Backup::Error, 'Backup failed'
         end
-
-        upload
       end
     end
 
@@ -105,8 +107,30 @@ module Backup
       end
     end
 
-    # rubocop: disable Metrics/AbcSize
+    def verify_backup_version
+      Dir.chdir(backup_path) do
+        # restoring mismatching backups can lead to unexpected problems
+        if settings[:gitlab_version] != Gitlab::VERSION
+          progress.puts(<<~HEREDOC.color(:red))
+            GitLab version mismatch:
+              Your current GitLab version (#{Gitlab::VERSION}) differs from the GitLab version in the backup!
+              Please switch to the following version and try again:
+              version: #{settings[:gitlab_version]}
+          HEREDOC
+          progress.puts
+          progress.puts "Hint: git checkout v#{settings[:gitlab_version]}"
+          exit 1
+        end
+      end
+    end
+
     def unpack
+      if ENV['BACKUP'].blank? && non_tarred_backup?
+        progress.puts "Non tarred backup found in #{backup_path}, using that"
+
+        return false
+      end
+
       Dir.chdir(backup_path) do
         # check for existing backups in the backup dir
         if backup_file_list.empty?
@@ -141,21 +165,6 @@ module Backup
           progress.puts 'unpacking backup failed'.color(:red)
           exit 1
         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
-          progress.puts(<<~HEREDOC.color(:red))
-            GitLab version mismatch:
-              Your current GitLab version (#{Gitlab::VERSION}) differs from the GitLab version in the backup!
-              Please switch to the following version and try again:
-              version: #{settings[:gitlab_version]}
-          HEREDOC
-          progress.puts
-          progress.puts "Hint: git checkout v#{settings[:gitlab_version]}"
-          exit 1
-        end
       end
     end
 
@@ -170,6 +179,10 @@ module Backup
 
     private
 
+    def non_tarred_backup?
+      File.exist?(File.join(backup_path, 'backup_information.yml'))
+    end
+
     def backup_path
       Gitlab.config.backup.path
     end
@@ -252,7 +265,7 @@ module Backup
     def create_attributes
       attrs = {
         key: remote_target,
-        body: File.open(tar_file),
+        body: File.open(File.join(backup_path, tar_file)),
         multipart_chunk_size: Gitlab.config.backup.upload.multipart_chunk_size,
         encryption: Gitlab.config.backup.upload.encryption,
         encryption_key: Gitlab.config.backup.upload.encryption_key,

lib/tasks/gitlab/backup.rake

@@ -17,9 +17,16 @@ namespace :gitlab do
       Rake::Task['gitlab:backup:registry:create'].invoke
 
       backup = Backup::Manager.new(progress)
+      backup.write_info
+
+      if ENV['SKIP'] && ENV['SKIP'].include?('tar')
+        backup.upload
+      else
         backup.pack
+        backup.upload
         backup.cleanup
         backup.remove_old
+      end
 
       progress.puts "Warning: Your gitlab.rb and gitlab-secrets.json files contain sensitive data \n" \
            "and are not included in this backup. You will need these files to restore a backup.\n" \
@@ -33,7 +40,8 @@ namespace :gitlab do
       warn_user_is_not_gitlab
 
       backup = Backup::Manager.new(progress)
-      backup.unpack
+      cleanup_required = backup.unpack
+      backup.verify_backup_version
 
       unless backup.skipped?('db')
         begin
@@ -72,7 +80,10 @@ namespace :gitlab do
       Rake::Task['gitlab:shell:setup'].invoke
       Rake::Task['cache:clear'].invoke
 
+      if cleanup_required
         backup.cleanup
+      end
+
       puts "Warning: Your gitlab.rb and gitlab-secrets.json files contain sensitive data \n" \
            "and are not included in this backup. You will need to restore these files manually.".color(:red)
       puts "Restore task is done."

spec/lib/backup/manager_spec.rb

@@ -214,6 +214,30 @@ describe Backup::Manager do
     end
   end
 
+  describe 'verify_backup_version' do
+    context 'on version mismatch' do
+      let(:gitlab_version) { Gitlab::VERSION }
+
+      it 'stops the process' do
+        allow(YAML).to receive(:load_file)
+          .and_return({ gitlab_version: "not #{gitlab_version}" })
+
+        expect { subject.verify_backup_version }.to raise_error SystemExit
+      end
+    end
+
+    context 'on version match' do
+      let(:gitlab_version) { Gitlab::VERSION }
+
+      it 'does nothing' do
+        allow(YAML).to receive(:load_file)
+          .and_return({ gitlab_version: "#{gitlab_version}" })
+
+        expect { subject.verify_backup_version }.not_to raise_error
+      end
+    end
+  end
+
   describe '#unpack' do
     context 'when there are no backup files in the directory' do
       before do
@@ -292,6 +316,23 @@ describe Backup::Manager do
         expect(progress).to have_received(:puts).with(a_string_matching('done'))
       end
     end
+
+    context 'when there is a non-tarred backup in the directory' do
+      before do
+        allow(Dir).to receieve(:glob).and_return(
+          [
+            'backup_information.yml'
+          ]
+        )
+
+        it 'selects the non-tarred backup to restore from' do
+          expect { subject.unpack }.to output.to_stdout
+          expect(progress).to have_received(:puts)
+            .with(a_string_matching('Non tarred backup found '))
+          expect(Kernel).not_to receive(:system)
+        end
+      end
+    end
   end
 
   describe '#upload' do
@@ -329,10 +370,8 @@ describe Backup::Manager do
           .with(hash_including(key: backup_filename, public: false))
           .and_return(true)
 
-        Dir.chdir(Gitlab.config.backup.path) do
         subject.upload
       end
-      end
 
       it 'adds the DIRECTORY environment variable if present' do
         stub_env('DIRECTORY', 'daily')
@@ -341,11 +380,9 @@ describe Backup::Manager do
           .with(hash_including(key: "daily/#{backup_filename}", public: false))
           .and_return(true)
 
-        Dir.chdir(Gitlab.config.backup.path) do
         subject.upload
       end
     end
-    end
 
     context 'with Google provider' do
       before do
@@ -373,10 +410,8 @@ describe Backup::Manager do
           .with(hash_excluding(public: false))
           .and_return(true)
 
-        Dir.chdir(Gitlab.config.backup.path) do
         subject.upload
       end
     end
   end
-  end
 end

spec/tasks/gitlab/backup_rake_spec.rb

@@ -14,6 +14,14 @@ describe 'gitlab:app namespace rake task' do
     tars_glob.first
   end
 
+  def backup_files
+    %w(backup_information.yml artifacts.tar.gz builds.tar.gz lfs.tar.gz pages.tar.gz)
+  end
+
+  def backup_directories
+    %w(db repositories)
+  end
+
   before(:all) do
     Rake.application.rake_require 'tasks/gitlab/helpers'
     Rake.application.rake_require 'tasks/gitlab/backup'
@@ -28,12 +36,16 @@ describe 'gitlab:app namespace rake task' do
   before do
     stub_env('force', 'yes')
     FileUtils.rm(tars_glob, force: true)
+    FileUtils.rm(backup_files, force: true)
+    FileUtils.rm_rf(backup_directories, secure: true)
     reenable_backup_sub_tasks
     stub_container_registry_config(enabled: enable_registry)
   end
 
   after do
     FileUtils.rm(tars_glob, force: true)
+    FileUtils.rm(backup_files, force: true)
+    FileUtils.rm_rf(backup_directories, secure: true)
   end
 
   def run_rake_task(task_name)
@@ -62,15 +74,6 @@ describe 'gitlab:app namespace rake task' do
 
       let(:gitlab_version) { Gitlab::VERSION }
 
-      it 'fails on mismatch' do
-        allow(YAML).to receive(:load_file)
-          .and_return({ gitlab_version: "not #{gitlab_version}" })
-
-        expect do
-          expect { run_rake_task('gitlab:backup:restore') }.to output.to_stdout
-        end.to raise_error(SystemExit)
-      end
-
       context 'restore with matching gitlab version' do
         before do
           allow(YAML).to receive(:load_file)
@@ -241,7 +244,7 @@ describe 'gitlab:app namespace rake task' do
         )
 
         expect(exit_status).to eq(0)
-        expect(tar_contents).to match('db/')
+        expect(tar_contents).to match('db')
         expect(tar_contents).to match('uploads.tar.gz')
         expect(tar_contents).to match('repositories/')
         expect(tar_contents).to match('builds.tar.gz')
@@ -379,6 +382,50 @@ describe 'gitlab:app namespace rake task' do
     end
   end
 
+  describe 'skipping tar archive creation' do
+    before do
+      stub_env('SKIP', 'tar')
+    end
+
+    it 'created files with backup content and no tar archive' do
+      expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout
+
+      dir_contents = Dir.children(Gitlab.config.backup.path)
+
+      expect(dir_contents).to contain_exactly(
+        'backup_information.yml',
+        'db',
+        'uploads.tar.gz',
+        'builds.tar.gz',
+        'artifacts.tar.gz',
+        'lfs.tar.gz',
+        'pages.tar.gz',
+        'registry.tar.gz',
+        'repositories',
+        'tmp'
+      )
+    end
+
+    it 'those component files can be restored from' do
+      expect { run_rake_task("gitlab:backup:create") }.to output.to_stdout
+
+      allow(Rake::Task['gitlab:shell:setup'])
+        .to receive(:invoke).and_return(true)
+
+      expect(Rake::Task['gitlab:db:drop_tables']).to receive :invoke
+      expect(Rake::Task['gitlab:backup:db:restore']).to receive :invoke
+      expect(Rake::Task['gitlab:backup:repo:restore']).to receive :invoke
+      expect(Rake::Task['gitlab:backup:uploads:restore']).to receive :invoke
+      expect(Rake::Task['gitlab:backup:builds:restore']).to receive :invoke
+      expect(Rake::Task['gitlab:backup:artifacts:restore']).to receive :invoke
+      expect(Rake::Task['gitlab:backup:pages:restore']).to receive :invoke
+      expect(Rake::Task['gitlab:backup:lfs:restore']).to receive :invoke
+      expect(Rake::Task['gitlab:backup:registry:restore']).to receive :invoke
+      expect(Rake::Task['gitlab:shell:setup']).to receive :invoke
+      expect { run_rake_task("gitlab:backup:restore") }.to output.to_stdout
+    end
+  end
+
   describe "Human Readable Backup Name" do
     it 'name has human readable time' do
       expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout