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:
- Create specialized classes for each "check"
- Create specialized class to control the flow and most of the common output (single place to improve output when/if we want)
- 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
)