Pass backup ID to gitaly-backup

This allows whichever persistent storage that is used for gitaly-backup
to be tied back to a specific gitlab backup.

ee/spec/lib/ee/backup/repositories_spec.rb

@@ -17,7 +17,7 @@ RSpec.describe Backup::Repositories do
     it 'calls enqueue for each repository type', :aggregate_failures do
       subject.dump(destination, backup_id)
 
-      expect(strategy).to have_received(:start).with(:create, destination)
+      expect(strategy).to have_received(:start).with(:create, destination, backup_id: backup_id)
       expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::PROJECT)
       groups.each do |group|
         expect(strategy).to have_received(:enqueue).with(group, Gitlab::GlRepository::WIKI)

lib/backup/gitaly_backup.rb

@@ -9,16 +9,14 @@ module Backup
     # @param [StringIO] progress IO interface to output progress
     # @param [Integer] max_parallelism max parallelism when running backups
     # @param [Integer] storage_parallelism max parallelism per storage (is affected by max_parallelism)
-    # @param [String] backup_id unique identifier for the backup
     def initialize(progress, max_parallelism: nil, storage_parallelism: nil, incremental: false, backup_id: nil)
       @progress = progress
       @max_parallelism = max_parallelism
       @storage_parallelism = storage_parallelism
       @incremental = incremental
-      @backup_id = backup_id
     end
 
-    def start(type, backup_repos_path)
+    def start(type, backup_repos_path, backup_id: nil)
       raise Error, 'already started' if started?
 
       command = case type
@@ -37,7 +35,7 @@ module Backup
         args += ['-layout', 'pointer']
         if type == :create
           args += ['-incremental'] if @incremental
-          args += ['-id', @backup_id] if @backup_id
+          args += ['-id', backup_id] if backup_id
         end
       end
 

lib/backup/repositories.rb

@@ -14,7 +14,7 @@ module Backup
 
     override :dump
     def dump(path, backup_id)
-      strategy.start(:create, path)
+      strategy.start(:create, path, backup_id: backup_id)
       enqueue_consecutive
 
     ensure

spec/lib/backup/gitaly_backup_spec.rb

@@ -25,11 +25,11 @@ RSpec.describe Backup::GitalyBackup do
     progress.close
   end
 
-  subject { described_class.new(progress, max_parallelism: max_parallelism, storage_parallelism: storage_parallelism, backup_id: backup_id) }
+  subject { described_class.new(progress, max_parallelism: max_parallelism, storage_parallelism: storage_parallelism) }
 
   context 'unknown' do
     it 'fails to start unknown' do
-      expect { subject.start(:unknown, destination) }.to raise_error(::Backup::Error, 'unknown backup type: unknown')
+      expect { subject.start(:unknown, destination, backup_id: backup_id) }.to raise_error(::Backup::Error, 'unknown backup type: unknown')
     end
   end
 
@@ -44,7 +44,7 @@ RSpec.describe Backup::GitalyBackup do
 
         expect(Open3).to receive(:popen2).with(expected_env, anything, 'create', '-path', anything, '-layout', 'pointer', '-id', backup_id).and_call_original
 
-        subject.start(:create, destination)
+        subject.start(:create, destination, backup_id: backup_id)
         subject.enqueue(project, Gitlab::GlRepository::PROJECT)
         subject.enqueue(project, Gitlab::GlRepository::WIKI)
         subject.enqueue(project, Gitlab::GlRepository::DESIGN)
@@ -65,7 +65,7 @@ RSpec.describe Backup::GitalyBackup do
         it 'passes parallel option through' do
           expect(Open3).to receive(:popen2).with(expected_env, anything, 'create', '-path', anything, '-parallel', '3', '-layout', 'pointer', '-id', backup_id).and_call_original
 
-          subject.start(:create, destination)
+          subject.start(:create, destination, backup_id: backup_id)
           subject.finish!
         end
       end
@@ -76,7 +76,7 @@ RSpec.describe Backup::GitalyBackup do
         it 'passes parallel option through' do
           expect(Open3).to receive(:popen2).with(expected_env, anything, 'create', '-path', anything, '-parallel-storage', '3', '-layout', 'pointer', '-id', backup_id).and_call_original
 
-          subject.start(:create, destination)
+          subject.start(:create, destination, backup_id: backup_id)
           subject.finish!
         end
       end
@@ -84,14 +84,14 @@ RSpec.describe Backup::GitalyBackup do
       it 'raises when the exit code not zero' do
         expect(subject).to receive(:bin_path).and_return(Gitlab::Utils.which('false'))
 
-        subject.start(:create, destination)
+        subject.start(:create, destination, backup_id: backup_id)
         expect { subject.finish! }.to raise_error(::Backup::Error, 'gitaly-backup exit status 1')
       end
 
       it 'raises when gitaly_backup_path is not set' do
         stub_backup_setting(gitaly_backup_path: nil)
 
-        expect { subject.start(:create, destination) }.to raise_error(::Backup::Error, 'gitaly-backup binary not found and gitaly_backup_path is not configured')
+        expect { subject.start(:create, destination, backup_id: backup_id) }.to raise_error(::Backup::Error, 'gitaly-backup binary not found and gitaly_backup_path is not configured')
       end
 
       context 'feature flag incremental_repository_backup disabled' do
@@ -108,7 +108,7 @@ RSpec.describe Backup::GitalyBackup do
 
           expect(Open3).to receive(:popen2).with(expected_env, anything, 'create', '-path', anything).and_call_original
 
-          subject.start(:create, destination)
+          subject.start(:create, destination, backup_id: backup_id)
           subject.enqueue(project, Gitlab::GlRepository::PROJECT)
           subject.enqueue(project, Gitlab::GlRepository::WIKI)
           subject.enqueue(project, Gitlab::GlRepository::DESIGN)
@@ -152,7 +152,7 @@ RSpec.describe Backup::GitalyBackup do
       it 'passes through SSL envs' do
         expect(Open3).to receive(:popen2).with(ssl_env, anything, 'create', '-path', anything, '-layout', 'pointer', '-id', backup_id).and_call_original
 
-        subject.start(:create, destination)
+        subject.start(:create, destination, backup_id: backup_id)
         subject.finish!
       end
     end
@@ -177,7 +177,7 @@ RSpec.describe Backup::GitalyBackup do
 
       expect(Open3).to receive(:popen2).with(expected_env, anything, 'restore', '-path', anything, '-layout', 'pointer').and_call_original
 
-      subject.start(:restore, destination)
+      subject.start(:restore, destination, backup_id: backup_id)
       subject.enqueue(project, Gitlab::GlRepository::PROJECT)
       subject.enqueue(project, Gitlab::GlRepository::WIKI)
       subject.enqueue(project, Gitlab::GlRepository::DESIGN)
@@ -200,7 +200,7 @@ RSpec.describe Backup::GitalyBackup do
       it 'passes parallel option through' do
         expect(Open3).to receive(:popen2).with(expected_env, anything, 'restore', '-path', anything, '-parallel', '3', '-layout', 'pointer').and_call_original
 
-        subject.start(:restore, destination)
+        subject.start(:restore, destination, backup_id: backup_id)
         subject.finish!
       end
     end
@@ -211,7 +211,7 @@ RSpec.describe Backup::GitalyBackup do
       it 'passes parallel option through' do
         expect(Open3).to receive(:popen2).with(expected_env, anything, 'restore', '-path', anything, '-parallel-storage', '3', '-layout', 'pointer').and_call_original
 
-        subject.start(:restore, destination)
+        subject.start(:restore, destination, backup_id: backup_id)
         subject.finish!
       end
     end
@@ -230,7 +230,7 @@ RSpec.describe Backup::GitalyBackup do
 
         expect(Open3).to receive(:popen2).with(expected_env, anything, 'restore', '-path', anything).and_call_original
 
-        subject.start(:restore, destination)
+        subject.start(:restore, destination, backup_id: backup_id)
         subject.enqueue(project, Gitlab::GlRepository::PROJECT)
         subject.enqueue(project, Gitlab::GlRepository::WIKI)
         subject.enqueue(project, Gitlab::GlRepository::DESIGN)
@@ -251,14 +251,14 @@ RSpec.describe Backup::GitalyBackup do
     it 'raises when the exit code not zero' do
       expect(subject).to receive(:bin_path).and_return(Gitlab::Utils.which('false'))
 
-      subject.start(:restore, destination)
+      subject.start(:restore, destination, backup_id: backup_id)
       expect { subject.finish! }.to raise_error(::Backup::Error, 'gitaly-backup exit status 1')
     end
 
     it 'raises when gitaly_backup_path is not set' do
       stub_backup_setting(gitaly_backup_path: nil)
 
-      expect { subject.start(:restore, destination) }.to raise_error(::Backup::Error, 'gitaly-backup binary not found and gitaly_backup_path is not configured')
+      expect { subject.start(:restore, destination, backup_id: backup_id) }.to raise_error(::Backup::Error, 'gitaly-backup binary not found and gitaly_backup_path is not configured')
     end
   end
 end

spec/lib/backup/repositories_spec.rb

@@ -25,7 +25,7 @@ RSpec.describe Backup::Repositories do
 
         subject.dump(destination, backup_id)
 
-        expect(strategy).to have_received(:start).with(:create, destination)
+        expect(strategy).to have_received(:start).with(:create, destination, backup_id: backup_id)
         expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::PROJECT)
         expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::WIKI)
         expect(strategy).to have_received(:enqueue).with(project, Gitlab::GlRepository::DESIGN)