Merge branch 'fix-registry-backups' into 'master'

Fix backups if registry is disabled

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/17749

See merge request !4263

3 files changed, 44 insertions, 9 deletions

diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb
index 7d0608f09da..3e07096e6cc 100644
--- a/lib/backup/manager.rb
+++ b/lib/backup/manager.rb
@@ -1,5 +1,8 @@
 module Backup
   class Manager
+    ARCHIVES_TO_BACKUP = %w[uploads builds artifacts lfs registry]
+    FOLDERS_TO_BACKUP = %w[repositories db]
+
     def pack
       # Make sure there is a connection
       ActiveRecord::Base.connection.reconnect!
@@ -147,7 +150,7 @@ module Backup
     end
 
     def skipped?(item)
-      settings[:skipped] && settings[:skipped].include?(item)
+      settings[:skipped] && settings[:skipped].include?(item) || disabled_features.include?(item)
     end
 
     private
@@ -157,11 +160,17 @@ module Backup
     end
 
     def archives_to_backup
-      %w{uploads builds artifacts lfs registry}.map{ |name| (name + ".tar.gz") unless skipped?(name) }.compact
+      ARCHIVES_TO_BACKUP.map{ |name| (name + ".tar.gz") unless skipped?(name) }.compact
     end
 
     def folders_to_backup
-      %w{repositories db}.reject{ |name| skipped?(name) }
+      FOLDERS_TO_BACKUP.reject{ |name| skipped?(name) }
+    end
+
+    def disabled_features
+      features = []
+      features << 'registry' unless Gitlab.config.registry.enabled
+      features
     end
 
     def settings
diff --git a/lib/tasks/gitlab/backup.rake b/lib/tasks/gitlab/backup.rake
index d97d974ec20..596eaca6d0d 100644
--- a/lib/tasks/gitlab/backup.rake
+++ b/lib/tasks/gitlab/backup.rake
@@ -179,18 +179,26 @@ namespace :gitlab do
       task create: :environment do
         $progress.puts "Dumping container registry images ... ".blue
 
-        if ENV["SKIP"] && ENV["SKIP"].include?("registry")
-          $progress.puts "[SKIPPED]".cyan
+        if Gitlab.config.registry.enabled
+          if ENV["SKIP"] && ENV["SKIP"].include?("registry")
+            $progress.puts "[SKIPPED]".cyan
+          else
+            Backup::Registry.new.dump
+            $progress.puts "done".green
+          end
         else
-          Backup::Registry.new.dump
-          $progress.puts "done".green
+          $progress.puts "[DISABLED]".cyan
         end
       end
 
       task restore: :environment do
         $progress.puts "Restoring container registry images ... ".blue
-        Backup::Registry.new.restore
-        $progress.puts "done".green
+        if Gitlab.config.registry.enabled
+          Backup::Registry.new.restore
+          $progress.puts "done".green
+        else
+          $progress.puts "[DISABLED]".cyan
+        end
       end
     end
 
diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb
index 8aeb013eec6..25da0917134 100644
--- a/spec/tasks/gitlab/backup_rake_spec.rb
+++ b/spec/tasks/gitlab/backup_rake_spec.rb
@@ -2,6 +2,8 @@ require 'spec_helper'
 require 'rake'
 
 describe 'gitlab:app namespace rake task' do
+  let(:enable_registry) { true }
+
   before :all do
     Rake.application.rake_require 'tasks/gitlab/task_helpers'
     Rake.application.rake_require 'tasks/gitlab/backup'
@@ -15,6 +17,10 @@ describe 'gitlab:app namespace rake task' do
     FileUtils.mkdir_p('public/uploads')
   end
 
+  before do
+    stub_container_registry_config(enabled: enable_registry)
+  end
+
   def run_rake_task(task_name)
     Rake::Task[task_name].reenable
     Rake.application.invoke_task task_name
@@ -143,6 +149,18 @@ describe 'gitlab:app namespace rake task' do
 
       expect(temp_dirs).to be_empty
     end
+
+    context 'registry disabled' do
+      let(:enable_registry) { false }
+
+      it 'should not create registry.tar.gz' do
+        tar_contents, exit_status = Gitlab::Popen.popen(
+          %W{tar -tvf #{@backup_tar}}
+        )
+        expect(exit_status).to eq(0)
+        expect(tar_contents).not_to match('registry.tar.gz')
+      end
+    end
   end # backup_create task
 
   describe "Skipping items" do