Merge branch 'fix_cron_backup' into 'master'

Fix CRON setting when using gitaly-backup

See merge request gitlab-org/gitlab!65695

lib/backup/gitaly_backup.rb

@@ -25,18 +25,21 @@ module Backup
       args += ['-parallel', @parallel.to_s] if type == :create && @parallel
       args += ['-parallel-storage', @parallel_storage.to_s] if type == :create && @parallel_storage
 
-      @read_io, @write_io = IO.pipe
-      @pid = Process.spawn(bin_path, command, '-path', backup_repos_path, *args, in: @read_io, out: @progress)
+      @stdin, stdout, @thread = Open3.popen2(ENV, bin_path, command, '-path', backup_repos_path, *args)
+
+      @out_reader = Thread.new do
+        IO.copy_stream(stdout, @progress)
+      end
     end
 
     def wait
       return unless started?
 
-      @write_io.close
-      Process.wait(@pid)
-      status = $?
+      @stdin.close
+      [@thread, @out_reader].each(&:join)
+      status =  @thread.value
 
-      @pid = nil
+      @thread = nil
 
       raise Error, "gitaly-backup exit status #{status.exitstatus}" if status.exitstatus != 0
     end
@@ -46,7 +49,7 @@ module Backup
 
       repository = repo_type.repository_for(container)
 
-      @write_io.puts({
+      @stdin.puts({
         storage_name: repository.storage,
         relative_path: repository.relative_path,
         gl_project_path: repository.gl_project_path,
@@ -61,7 +64,7 @@ module Backup
     private
 
     def started?
-      @pid.present?
+      @thread.present?
     end
 
     def backup_repos_path

spec/lib/backup/gitaly_backup_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe Backup::GitalyBackup do
         project_snippet = create(:project_snippet, :repository, project: project)
         personal_snippet = create(:personal_snippet, :repository, author: project.owner)
 
-        expect(Process).to receive(:spawn).with(anything, 'create', '-path', anything, { in: anything, out: progress }).and_call_original
+        expect(Open3).to receive(:popen2).with(ENV, anything, 'create', '-path', anything).and_call_original
 
         subject.start(:create)
         subject.enqueue(project, Gitlab::GlRepository::PROJECT)
@@ -53,7 +53,7 @@ RSpec.describe Backup::GitalyBackup do
         let(:parallel) { 3 }
 
         it 'passes parallel option through' do
-          expect(Process).to receive(:spawn).with(anything, 'create', '-path', anything, '-parallel', '3', { in: anything, out: progress }).and_call_original
+          expect(Open3).to receive(:popen2).with(ENV, anything, 'create', '-path', anything, '-parallel', '3').and_call_original
 
           subject.start(:create)
           subject.wait
@@ -64,7 +64,7 @@ RSpec.describe Backup::GitalyBackup do
         let(:parallel_storage) { 3 }
 
         it 'passes parallel option through' do
-          expect(Process).to receive(:spawn).with(anything, 'create', '-path', anything, '-parallel-storage', '3', { in: anything, out: progress }).and_call_original
+          expect(Open3).to receive(:popen2).with(ENV, anything, 'create', '-path', anything, '-parallel-storage', '3').and_call_original
 
           subject.start(:create)
           subject.wait
@@ -109,7 +109,7 @@ RSpec.describe Backup::GitalyBackup do
       copy_bundle_to_backup_path('personal_snippet_repo.bundle', personal_snippet.disk_path + '.bundle')
       copy_bundle_to_backup_path('project_snippet_repo.bundle', project_snippet.disk_path + '.bundle')
 
-      expect(Process).to receive(:spawn).with(anything, 'restore', '-path', anything, { in: anything, out: progress }).and_call_original
+      expect(Open3).to receive(:popen2).with(ENV, anything, 'restore', '-path', anything).and_call_original
 
       subject.start(:restore)
       subject.enqueue(project, Gitlab::GlRepository::PROJECT)
@@ -132,7 +132,7 @@ RSpec.describe Backup::GitalyBackup do
       let(:parallel) { 3 }
 
       it 'does not pass parallel option through' do
-        expect(Process).to receive(:spawn).with(anything, 'restore', '-path', anything, { in: anything, out: progress }).and_call_original
+        expect(Open3).to receive(:popen2).with(ENV, anything, 'restore', '-path', anything).and_call_original
 
         subject.start(:restore)
         subject.wait

spec/tasks/gitlab/backup_rake_spec.rb

@@ -412,6 +412,16 @@ RSpec.describe 'gitlab:app namespace rake task', :delete do
         expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout_from_any_process
       end
     end
+
+    context 'CRON env is set' do
+      before do
+        stub_env('CRON', '1')
+      end
+
+      it 'does not output to stdout' do
+        expect { run_rake_task('gitlab:backup:create') }.not_to output.to_stdout_from_any_process
+      end
+    end
   end
   # backup_create task