Refactor `gitlab:*:check` rake tasks

Description

Current implementation of our rake tasks has 1K lines in a single check.rake file. Most of the logic is inside the rake own DSL, which makes it really hard to test and to re-use or improve output.

While there are gems that we can use to test rake tasks, it will be a blackbox testing, and will be highly coupled with either parsing output strings, or stubbing and spying changes in other classes, which makes any refactor unreliable as tests will also need change no matter what.

Proposal

Move away from having the login inside the rake files and use some Object Orientation to:

  1. Create specialized classes for each "check"
  2. Create specialized class to control the flow and most of the common output (single place to improve output when/if we want)
  3. Create single entrypoint that will instantiate and run specified "checks"

This will make possible to have unit tests for each "check", and have a more behavioral spec that will inspect this structure to validate "intention" without having to actually execute checks again, like:

it { is_expected.to execute_check(MyCheck) } 

Migrating our current codebase to this new way, will take some time and will be part of other merge requests (we can try one MR for each group of checks)

This will also help with CE/EE specific checks, reducing conflicts in a single file.

Example codes

This code is called by gitlab:app:check:

    def check_gitlab_config_exists
      print "GitLab config exists? ... "

      gitlab_config_file = Rails.root.join("config", "gitlab.yml")

      if File.exist?(gitlab_config_file)
        puts "yes".color(:green)
      else
        puts "no".color(:red)
        try_fixing_it(
          "Copy config/gitlab.yml.example to config/gitlab.yml",
          "Update config/gitlab.yml to match your setup"
        )
        for_more_information(
          see_installation_guide_section "GitLab"
        )
        fix_and_rerun
      end
    end

Becomes:

module SystemCheck
  module App
    class GitlabConfigExistsCheck < SystemCheck::BaseCheck
      name 'GitLab config exists?'

      def check?
        File.exist?(Rails.root.join("config", "gitlab.yml"))
      end

      def show_error
        try_fixing_it(
          "Copy config/gitlab.yml.example to config/gitlab.yml",
          "Update config/gitlab.yml to match your setup"
        )
        for_more_information(
          see_installation_guide_section "GitLab"
        )
        fix_and_rerun
      end
    end
end

And in the rake task instead of initializing it as:

desc "GitLab | Check the configuration of the GitLab Rails app"
task check: :environment  do
  warn_user_is_not_gitlab
  start_checking "GitLab"

  check_git_config
  check_database_config_exists
  check_migrations_are_up
  check_orphaned_group_members
  check_gitlab_config_exists
  check_gitlab_config_not_outdated
  check_log_writable
  check_tmp_writable
  check_uploads
  check_init_script_exists
  check_init_script_up_to_date
  check_projects_have_namespace
  check_redis_version
  check_ruby_version
  check_git_version
  check_active_users

  finished_checking "GitLab"
end

We can do:

desc "GitLab | Check the configuration of the GitLab Rails app"
task check: :environment  do
  checks = [
    #...
    SystemCheck::App::GitlabConfigExistsCheck,
    SystemCheck::App::GitlabConfigNotOutdatedCheck,
    #...
  ]
  SystemCheck.run('GitLab', checks)
end

cc @DouweM @rspeicher @smcgivern


Checklist

  • Refactor App Checks (rake gitlab:app:check)
  • Refactor GitLab Shell Checks (rake gitlab:gitlab_shell:check)
  • Refactor Sidekiq Checks (rake gitlab:sidekiq:check)
  • Refactor Incoming Email Checks (rake gitlab:incoming_email:check)
  • Refactor LDAP Checks (rake gitlab:ldap:check)
  • Refactor Geo Checks (rake gitlab:geo:check)
Edited by Gabriel Mazetto