Merge branch '40228-verify-integrity-of-repositories' into 'master'

Resolve "Verify integrity of repositories"

Closes #40228

See merge request gitlab-org/gitlab-ce!15931

6 files changed, 92 insertions, 42 deletions

diff --git a/changelogs/unreleased/40228-verify-integrity-of-repositories.yml b/changelogs/unreleased/40228-verify-integrity-of-repositories.yml
new file mode 100644
index 00000000000..261d48652db
--- /dev/null
+++ b/changelogs/unreleased/40228-verify-integrity-of-repositories.yml
@@ -0,0 +1,5 @@
+---
+title: Fix gitlab-rake gitlab:import:repos import schedule
+merge_request: 15931
+author:
+type: fixed
diff --git a/doc/administration/raketasks/check.md b/doc/administration/raketasks/check.md
index c8b5434c068..c39cb49b1c6 100644
--- a/doc/administration/raketasks/check.md
+++ b/doc/administration/raketasks/check.md
@@ -28,19 +28,25 @@ exactly which repositories are causing the trouble.
 
 ### Check all GitLab repositories
 
+>**Note:**
+>
+>  - `gitlab:repo:check` has been deprecated in favor of `gitlab:git:fsck`
+>  - [Deprecated][ce-15931] in GitLab 10.4.
+>  - `gitlab:repo:check` will be removed in the future. [Removal issue][ce-41699]
+
 This task loops through all repositories on the GitLab server and runs the
 3 integrity checks described previously.
 
 **Omnibus Installation**
 
 ```
-sudo gitlab-rake gitlab:repo:check
+sudo gitlab-rake gitlab:git:fsck
 ```
 
 **Source Installation**
 
 ```bash
-sudo -u git -H bundle exec rake gitlab:repo:check RAILS_ENV=production
+sudo -u git -H bundle exec rake gitlab:git:fsck RAILS_ENV=production
 ```
 
 ### Check repositories for a specific user
@@ -76,3 +82,6 @@ The LDAP check Rake task will test the bind_dn and password credentials
 (if configured) and will list a sample of LDAP users. This task is also
 executed as part of the `gitlab:check` task, but can run independently.
 See [LDAP Rake Tasks - LDAP Check](ldap.md#check) for details.
+
+[ce-15931]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/15931
+[ce-41699]: https://gitlab.com/gitlab-org/gitlab-ce/issues/41699
diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake
index dfade1f3885..903e84359cd 100644
--- a/lib/tasks/gitlab/check.rake
+++ b/lib/tasks/gitlab/check.rake
@@ -387,14 +387,8 @@ namespace :gitlab do
   namespace :repo do
     desc "GitLab | Check the integrity of the repositories managed by GitLab"
     task check: :environment do
-      Gitlab.config.repositories.storages.each do |name, repository_storage|
-        namespace_dirs = Dir.glob(File.join(repository_storage['path'], '*'))
-
-        namespace_dirs.each do |namespace_dir|
-          repo_dirs = Dir.glob(File.join(namespace_dir, '*'))
-          repo_dirs.each { |repo_dir| check_repo_integrity(repo_dir) }
-        end
-      end
+      puts "This task is deprecated. Please use gitlab:git:fsck instead".color(:red)
+      Rake::Task["gitlab:git:fsck"].execute
     end
   end
 
@@ -461,35 +455,4 @@ namespace :gitlab do
       puts "FAIL. Please update gitlab-shell to #{required_version} from #{current_version}".color(:red)
     end
   end
-
-  def check_repo_integrity(repo_dir)
-    puts "\nChecking repo at #{repo_dir.color(:yellow)}"
-
-    git_fsck(repo_dir)
-    check_config_lock(repo_dir)
-    check_ref_locks(repo_dir)
-  end
-
-  def git_fsck(repo_dir)
-    puts "Running `git fsck`".color(:yellow)
-    system(*%W(#{Gitlab.config.git.bin_path} fsck), chdir: repo_dir)
-  end
-
-  def check_config_lock(repo_dir)
-    config_exists = File.exist?(File.join(repo_dir, 'config.lock'))
-    config_output = config_exists ? 'yes'.color(:red) : 'no'.color(:green)
-    puts "'config.lock' file exists?".color(:yellow) + " ... #{config_output}"
-  end
-
-  def check_ref_locks(repo_dir)
-    lock_files = Dir.glob(File.join(repo_dir, 'refs/heads/*.lock'))
-    if lock_files.present?
-      puts "Ref lock files exist:".color(:red)
-      lock_files.each do |lock_file|
-        puts "  #{lock_file}"
-      end
-    else
-      puts "No ref lock files exist".color(:green)
-    end
-  end
 end
diff --git a/lib/tasks/gitlab/git.rake b/lib/tasks/gitlab/git.rake
index cf82134d97e..3f5dd2ae3b3 100644
--- a/lib/tasks/gitlab/git.rake
+++ b/lib/tasks/gitlab/git.rake
@@ -30,6 +30,20 @@ namespace :gitlab do
       end
     end
 
+    desc 'GitLab | Git | Check all repos integrity'
+    task fsck: :environment do
+      failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} fsck --name-objects --no-progress), "Checking integrity") do |repo|
+        check_config_lock(repo)
+        check_ref_locks(repo)
+      end
+
+      if failures.empty?
+        puts "Done".color(:green)
+      else
+        output_failures(failures)
+      end
+    end
+
     def perform_git_cmd(cmd, message)
       puts "Starting #{message} on all repositories"
 
@@ -40,6 +54,8 @@ namespace :gitlab do
         else
           failures << repo
         end
+
+        yield(repo) if block_given?
       end
 
       failures
@@ -49,5 +65,24 @@ namespace :gitlab do
       puts "The following repositories reported errors:".color(:red)
       failures.each { |f| puts "- #{f}" }
     end
+
+    def check_config_lock(repo_dir)
+      config_exists = File.exist?(File.join(repo_dir, 'config.lock'))
+      config_output = config_exists ? 'yes'.color(:red) : 'no'.color(:green)
+
+      puts "'config.lock' file exists?".color(:yellow) + " ... #{config_output}"
+    end
+
+    def check_ref_locks(repo_dir)
+      lock_files = Dir.glob(File.join(repo_dir, 'refs/heads/*.lock'))
+
+      if lock_files.present?
+        puts "Ref lock files exist:".color(:red)
+
+        lock_files.each { |lock_file| puts "  #{lock_file}" }
+      else
+        puts "No ref lock files exist".color(:green)
+      end
+    end
   end
 end
diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb
index 6723662703c..c1182af1014 100644
--- a/lib/tasks/gitlab/task_helpers.rb
+++ b/lib/tasks/gitlab/task_helpers.rb
@@ -130,7 +130,7 @@ module Gitlab
 
     def all_repos
       Gitlab.config.repositories.storages.each_value do |repository_storage|
-        IO.popen(%W(find #{repository_storage['path']} -mindepth 2 -maxdepth 2 -type d -name *.git)) do |find|
+        IO.popen(%W(find #{repository_storage['path']} -mindepth 2 -type d -name *.git)) do |find|
           find.each_line do |path|
             yield path.chomp
           end
diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb
new file mode 100644
index 00000000000..dacc5dc5ae7
--- /dev/null
+++ b/spec/tasks/gitlab/git_rake_spec.rb
@@ -0,0 +1,38 @@
+require 'rake_helper'
+
+describe 'gitlab:git rake tasks' do
+  before do
+    Rake.application.rake_require 'tasks/gitlab/git'
+
+    storages = { 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage') } }
+
+    FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git'))
+    allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+    allow_any_instance_of(String).to receive(:color) { |string, _color| string }
+
+    stub_warn_user_is_not_gitlab
+  end
+
+  after do
+    FileUtils.rm_rf(Settings.absolute('tmp/tests/default_storage'))
+  end
+
+  describe 'fsck' do
+    it 'outputs the integrity check for a repo' do
+      expect { run_rake_task('gitlab:git:fsck') }.to output(/Performed Checking integrity at .*@hashed\/1\/2\/test.git/).to_stdout
+    end
+
+    it 'errors out about config.lock issues' do
+      FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/config.lock'))
+
+      expect { run_rake_task('gitlab:git:fsck') }.to output(/file exists\? ... yes/).to_stdout
+    end
+
+    it 'errors out about ref lock issues' do
+      FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/refs/heads'))
+      FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/refs/heads/blah.lock'))
+
+      expect { run_rake_task('gitlab:git:fsck') }.to output(/Ref lock files exist:/).to_stdout
+    end
+  end
+end