From 500e5227a08bd603a2943c3c7d2efcaf4a40cc15 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Mon, 13 Feb 2017 00:33:31 +0100 Subject: [PATCH 01/14] WIP SystemCheck library for executing checks from a rake task --- lib/system_check.rb | 12 +++++++++ lib/system_check/base_check.rb | 42 +++++++++++++++++++++++++++++ lib/system_check/base_executor.rb | 18 +++++++++++++ lib/system_check/simple_executor.rb | 34 +++++++++++++++++++++++ lib/tasks/gitlab/check.rake | 5 ++++ 5 files changed, 111 insertions(+) create mode 100644 lib/system_check.rb create mode 100644 lib/system_check/base_check.rb create mode 100644 lib/system_check/base_executor.rb create mode 100644 lib/system_check/simple_executor.rb diff --git a/lib/system_check.rb b/lib/system_check.rb new file mode 100644 index 00000000000..1d09b57911a --- /dev/null +++ b/lib/system_check.rb @@ -0,0 +1,12 @@ +module SystemCheck + def self.run(component, checks = {}, executor_klass = SimpleExecutor) + unless executor_klass.is_a? BaseExecutor + raise ArgumentError, 'Invalid executor' + end + + executor = executor_klass.new(component) + executor.checks = checks.map do |check| + raise ArgumentError unless check.is_a? BaseCheck + end + end +end diff --git a/lib/system_check/base_check.rb b/lib/system_check/base_check.rb new file mode 100644 index 00000000000..6e9f7e509a9 --- /dev/null +++ b/lib/system_check/base_check.rb @@ -0,0 +1,42 @@ +module SystemCheck + class BaseCheck + def check? + raise NotImplementedError + end + + def show_error + raise NotImplementedError + end + + def skip? + false + end + + def skip_message + end + + protected + + def try_fixing_it(*steps) + steps = steps.shift if steps.first.is_a?(Array) + + puts ' Try fixing it:'.color(:blue) + steps.each do |step| + puts " #{step}" + end + end + + def fix_and_rerun + puts ' Please fix the error above and rerun the checks.'.color(:red) + end + + def for_more_information(*sources) + sources = sources.shift if sources.first.is_a?(Array) + + puts ' For more information see:'.color(:blue) + sources.each do |source| + puts ' #{source}' + end + end + end +end diff --git a/lib/system_check/base_executor.rb b/lib/system_check/base_executor.rb new file mode 100644 index 00000000000..18319843dea --- /dev/null +++ b/lib/system_check/base_executor.rb @@ -0,0 +1,18 @@ +module SystemCheck + class BaseExecutor + attr_reader :checks + attr_reader :component + + def initialize(component) + raise ArgumentError unless component.is_a? String + + @component = component + @checks = Set.new + end + + def <<(check) + raise ArgumentError unless check.is_a? BaseCheck + @checks << check + end + end +end diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb new file mode 100644 index 00000000000..2ffe837e326 --- /dev/null +++ b/lib/system_check/simple_executor.rb @@ -0,0 +1,34 @@ +module SystemCheck + class SimpleExecutor < BaseExecutor + def execute + start_checking(component) + + @checks.each do |check| + print "#{check.name}" + if check.skip? + puts "skipped #{'('+skip_message+')' if skip_message}".color(:magenta) + elsif check.check? + puts 'yes'.color(:green) + else + puts 'no'.color(:red) + check.show_error + end + end + + finished_checking(component) + end + + private + + def start_checking(component) + puts "Checking #{component.color(:yellow)} ..." + puts '' + end + + def finished_checking(component) + puts '' + puts "Checking #{component.color(:yellow)} ... #{"Finished".color(:green)}" + puts '' + end + end +end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index f41c73154f5..88ec9cbc8cc 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -848,10 +848,12 @@ namespace :gitlab do # Helper methods ########################## + # @deprecated Please use SystemChecks def fix_and_rerun puts " Please fix the error above and rerun the checks.".color(:red) end + # @deprecated Please use SystemChecks def for_more_information(*sources) sources = sources.shift if sources.first.is_a?(Array) @@ -861,6 +863,7 @@ namespace :gitlab do end end + # @deprecated Please use SystemChecks def finished_checking(component) puts "" puts "Checking #{component.color(:yellow)} ... #{"Finished".color(:green)}" @@ -883,11 +886,13 @@ namespace :gitlab do Gitlab.config.gitlab.user end + # @deprecated Please use SystemChecks def start_checking(component) puts "Checking #{component.color(:yellow)} ..." puts "" end + # @deprecated Please use SystemChecks def try_fixing_it(*steps) steps = steps.shift if steps.first.is_a?(Array) From f182ea4ea5fe76349a584da12e5d4a9f681a8401 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Mon, 13 Feb 2017 09:45:39 +0100 Subject: [PATCH 02/14] Some code-style fixes and documentation --- lib/system_check.rb | 17 +++++++++--- lib/system_check/base_check.rb | 40 +++++++++++++++++++++++++---- lib/system_check/base_executor.rb | 6 +++++ lib/system_check/simple_executor.rb | 30 +++++++++++++++------- 4 files changed, 76 insertions(+), 17 deletions(-) diff --git a/lib/system_check.rb b/lib/system_check.rb index 1d09b57911a..79603dfc7cb 100644 --- a/lib/system_check.rb +++ b/lib/system_check.rb @@ -1,12 +1,23 @@ +# Library to perform System Checks +# +# Every Check is implemented as its own class inherited from SystemCheck::BaseCheck +# Execution coordination and boilerplate output is done by the SystemCheck::SimpleExecutor +# +# This structure decouples checks from Rake tasks and facilitates unit-testing module SystemCheck - def self.run(component, checks = {}, executor_klass = SimpleExecutor) + # Executes a bunch of checks for specified component + # + # @param [String] component name of the component relative to the checks being executed + # @param [Array] checks classes of corresponding checks to be executed in the same order + # @param [BaseExecutor] executor_klass optionally specifiy a different executor class + def self.run(component, checks = [], executor_klass = SimpleExecutor) unless executor_klass.is_a? BaseExecutor raise ArgumentError, 'Invalid executor' end executor = executor_klass.new(component) - executor.checks = checks.map do |check| - raise ArgumentError unless check.is_a? BaseCheck + checks.each do |check| + executor << check end end end diff --git a/lib/system_check/base_check.rb b/lib/system_check/base_check.rb index 6e9f7e509a9..d465711d719 100644 --- a/lib/system_check/base_check.rb +++ b/lib/system_check/base_check.rb @@ -1,41 +1,71 @@ module SystemCheck + # Base class for Checks. You must inherit from here + # and implement the methods below when necessary class BaseCheck + # This is where you should implement the main logic that will return + # a boolean at the end + # + # You should not print any output to STDOUT here, use the specific methods instead + # + # @return [Boolean] whether the check passed or not def check? raise NotImplementedError end + # This is where you should print detailed information for any error found during #check? + # + # You may use helper methods to help format the output: + # + # @see #try_fixing_it + # @see #fix_and_rerun + # @see #for_more_infromation def show_error raise NotImplementedError end + # If skip returns true, than no other method on this check will be executed + # + # @return [Boolean] whether or not this check should be skipped def skip? false end + # If you enabled #skip? here is where you define a custom message explaining why + # + # Do not print anything to STDOUT, return a string. + # + # @return [String] message why this check was skipped def skip_message end protected + # Display a formatted list of instructions on how to fix the issue identified by the #check? + # + # @param [Array] steps one or short sentences with help how to fix the issue def try_fixing_it(*steps) steps = steps.shift if steps.first.is_a?(Array) - puts ' Try fixing it:'.color(:blue) + $stdout.puts ' Try fixing it:'.color(:blue) steps.each do |step| - puts " #{step}" + $stdout.puts " #{step}" end end + # Display a message telling to fix and rerun the checks def fix_and_rerun - puts ' Please fix the error above and rerun the checks.'.color(:red) + $stdout.puts ' Please fix the error above and rerun the checks.'.color(:red) end + # Display a formatted list of references (documentation or links) where to find more information + # + # @param [Array] sources one or more references (documentation or links) def for_more_information(*sources) sources = sources.shift if sources.first.is_a?(Array) - puts ' For more information see:'.color(:blue) + $stdout.puts ' For more information see:'.color(:blue) sources.each do |source| - puts ' #{source}' + $stdout.puts ' #{source}' end end end diff --git a/lib/system_check/base_executor.rb b/lib/system_check/base_executor.rb index 18319843dea..2c2b33461d9 100644 --- a/lib/system_check/base_executor.rb +++ b/lib/system_check/base_executor.rb @@ -1,8 +1,11 @@ module SystemCheck + # @attr_reader [Array] checks classes of corresponding checks to be executed in the same order + # @attr_reader [String] component name of the component relative to the checks being executed class BaseExecutor attr_reader :checks attr_reader :component + # @param [String] component name of the component relative to the checks being executed def initialize(component) raise ArgumentError unless component.is_a? String @@ -10,6 +13,9 @@ module SystemCheck @checks = Set.new end + # Add a check to be executed + # + # @param [BaseCheck] check class def <<(check) raise ArgumentError unless check.is_a? BaseCheck @checks << check diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb index 2ffe837e326..fc07f09dcb2 100644 --- a/lib/system_check/simple_executor.rb +++ b/lib/system_check/simple_executor.rb @@ -1,16 +1,22 @@ module SystemCheck + # Simple Executor is current default executor for GitLab + # It is a simple port from display logic in the old check.rake + # + # There is no concurrency level and the output is progressively + # printed into the STDOUT class SimpleExecutor < BaseExecutor + # Executes defined checks in the specified order and outputs confirmation or error information def execute start_checking(component) @checks.each do |check| - print "#{check.name}" + $stdout.print "#{check.name}" if check.skip? - puts "skipped #{'('+skip_message+')' if skip_message}".color(:magenta) + $stdout.puts "skipped #{'(' + skip_message + ')' if skip_message}".color(:magenta) elsif check.check? - puts 'yes'.color(:green) + $stdout.puts 'yes'.color(:green) else - puts 'no'.color(:red) + $stdout.puts 'no'.color(:red) check.show_error end end @@ -20,15 +26,21 @@ module SystemCheck private + # Prints header content for the series of checks to be executed for this component + # + # @param [String] component name of the component relative to the checks being executed def start_checking(component) - puts "Checking #{component.color(:yellow)} ..." - puts '' + $stdout.puts "Checking #{component.color(:yellow)} ..." + $stdout.puts '' end + # Prints footer content for the series of checks executed for this component + # + # @param [String] component name of the component relative to the checks being executed def finished_checking(component) - puts '' - puts "Checking #{component.color(:yellow)} ... #{"Finished".color(:green)}" - puts '' + $stdout.puts '' + $stdout.puts "Checking #{component.color(:yellow)} ... #{'Finished'.color(:green)}" + $stdout.puts '' end end end From 442735978ca8f3e074a282edfc65c110bd06e079 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Mon, 13 Feb 2017 12:19:28 +0100 Subject: [PATCH 03/14] Move rainbow monkey patch to String to spec_helper --- spec/db/production/settings.rb | 1 - spec/spec_helper.rb | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/db/production/settings.rb b/spec/db/production/settings.rb index 007b35bbb77..3cbb173c4cc 100644 --- a/spec/db/production/settings.rb +++ b/spec/db/production/settings.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'rainbow/ext/string' describe 'seed production settings', lib: true do include StubENV diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4c2eba8fa46..994c7dcbb46 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,6 +26,9 @@ if ENV['CI'] && !ENV['NO_KNAPSACK'] Knapsack::Adapters::RSpecAdapter.bind end +# require rainbow gem String monkeypatch, so we can test SystemChecks +require 'rainbow/ext/string' + # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } From a4460f420bbbac30fbcec3395261c89749b52bbd Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Mon, 13 Feb 2017 12:20:56 +0100 Subject: [PATCH 04/14] Fix a few method signature checks --- lib/system_check.rb | 2 +- lib/system_check/base_executor.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/system_check.rb b/lib/system_check.rb index 79603dfc7cb..ba22a40f5af 100644 --- a/lib/system_check.rb +++ b/lib/system_check.rb @@ -11,7 +11,7 @@ module SystemCheck # @param [Array] checks classes of corresponding checks to be executed in the same order # @param [BaseExecutor] executor_klass optionally specifiy a different executor class def self.run(component, checks = [], executor_klass = SimpleExecutor) - unless executor_klass.is_a? BaseExecutor + unless executor_klass < BaseExecutor raise ArgumentError, 'Invalid executor' end diff --git a/lib/system_check/base_executor.rb b/lib/system_check/base_executor.rb index 2c2b33461d9..f76f44f3a28 100644 --- a/lib/system_check/base_executor.rb +++ b/lib/system_check/base_executor.rb @@ -17,7 +17,7 @@ module SystemCheck # # @param [BaseCheck] check class def <<(check) - raise ArgumentError unless check.is_a? BaseCheck + raise ArgumentError unless check < BaseCheck @checks << check end end From bc6d131b74ba4cdf7acadea5a5b7d23f083f47ed Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Mon, 13 Feb 2017 12:21:12 +0100 Subject: [PATCH 05/14] Added specs for SystemCheck and custom matcher --- lib/system_check.rb | 7 +++++ spec/lib/system_check_spec.rb | 39 ++++++++++++++++++++++++++ spec/support/matchers/execute_check.rb | 19 +++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 spec/lib/system_check_spec.rb create mode 100644 spec/support/matchers/execute_check.rb diff --git a/lib/system_check.rb b/lib/system_check.rb index ba22a40f5af..e9cbf6b8258 100644 --- a/lib/system_check.rb +++ b/lib/system_check.rb @@ -15,9 +15,16 @@ module SystemCheck raise ArgumentError, 'Invalid executor' end + prepare(component, checks, executor_klass).execute + end + + def self.prepare(component, checks = [], executor_klass = SimpleExecutor) executor = executor_klass.new(component) checks.each do |check| executor << check end + + executor end + private_class_method :prepare end diff --git a/spec/lib/system_check_spec.rb b/spec/lib/system_check_spec.rb new file mode 100644 index 00000000000..399a492ea2e --- /dev/null +++ b/spec/lib/system_check_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe SystemCheck, lib: true do + subject { SystemCheck } + + describe '.run' do + it 'requires custom executor to be a BasicExecutor' do + expect { subject.run('Component', [], SystemCheck::SimpleExecutor) }.not_to raise_error + end + + context 'custom matcher' do + class SimpleCheck < SystemCheck::BaseCheck + def check? + true + end + end + + class OtherCheck < SystemCheck::BaseCheck + def check? + false + end + end + + subject { SystemCheck } + + it 'detects execution of SimpleCheck' do + is_expected.to execute_check(SimpleCheck) + + SystemCheck.run('Test', [SimpleCheck]) + end + + it 'detects exclusion of OtherCheck in execution' do + is_expected.not_to execute_check(OtherCheck) + + SystemCheck.run('Test', [SimpleCheck]) + end + end + end +end diff --git a/spec/support/matchers/execute_check.rb b/spec/support/matchers/execute_check.rb new file mode 100644 index 00000000000..9664eb3879d --- /dev/null +++ b/spec/support/matchers/execute_check.rb @@ -0,0 +1,19 @@ +RSpec::Matchers.define :execute_check do |expected| + match do |actual| + expect(actual).to eq(SystemCheck) + expect(actual).to receive(:run) do |*args| + expect(args[1]).to include(expected) + end + end + + match_when_negated do |actual| + expect(actual).to eq(SystemCheck) + expect(actual).to receive(:run) do |*args| + expect(args[1]).not_to include(expected) + end + end + + failure_message do |actual| + return 'This matcher must be used with SystemCheck' unless actual == SystemCheck + end +end From 45378bdd3aae75cb155677d91be61e3d39f5f515 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Mon, 20 Feb 2017 14:01:37 +0100 Subject: [PATCH 06/14] Added specs for BaseExecutor --- spec/lib/system_check/base_executor_spec.rb | 51 +++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 spec/lib/system_check/base_executor_spec.rb diff --git a/spec/lib/system_check/base_executor_spec.rb b/spec/lib/system_check/base_executor_spec.rb new file mode 100644 index 00000000000..139f7f3248f --- /dev/null +++ b/spec/lib/system_check/base_executor_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe SystemCheck::BaseExecutor, lib: true do + class SimpleCheck < SystemCheck::BaseCheck + def check? + true + end + end + + class OtherCheck < SystemCheck::BaseCheck + def check? + false + end + end + + subject { described_class.new('Test') } + + describe '#component' do + it 'returns stored component name' do + expect(subject.component).to eq('Test') + end + end + + describe '#checks' do + before do + subject << SimpleCheck + end + + it 'returns an array of classes' do + expect(subject.checks).to include(SimpleCheck) + end + end + + describe '#<<' do + before do + subject << SimpleCheck + end + + it 'appends a new check to the Set' do + subject << OtherCheck + stored_checks = subject.checks.to_a + expect(stored_checks.first).to eq(SimpleCheck) + expect(stored_checks.last).to eq(OtherCheck) + end + + it 'inserts unique itens only' do + subject << SimpleCheck + expect(subject.checks.size).to eq(1) + end + end +end From 13e88c93956b5b350515b919ef7217a3dccf28ff Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 25 May 2017 16:16:25 +0200 Subject: [PATCH 07/14] Refactor gitlab:app:checks to use SystemCheck --- .../app/database_config_exists_check.rb | 31 ++ lib/system_check/app/git_config_check.rb | 50 ++ lib/system_check/app/git_version_check.rb | 29 ++ .../app/gitlab_config_exists_check.rb | 24 + .../app/gitlab_config_not_outdated_check.rb | 32 ++ .../app/init_script_exists_check.rb | 27 + .../app/init_script_up_to_date_check.rb | 44 ++ lib/system_check/app/log_writable_check.rb | 28 + .../app/migrations_are_up_check.rb | 20 + .../app/orphaned_group_members_check.rb | 20 + .../app/projects_have_namespace_check.rb | 37 ++ lib/system_check/app/redis_version_check.rb | 25 + lib/system_check/app/ruby_version_check.rb | 27 + lib/system_check/app/tmp_writable_check.rb | 28 + .../app/uploads_directory_exists_check.rb | 21 + .../app/uploads_path_permission_check.rb | 36 ++ .../app/uploads_path_tmp_permission_check.rb | 40 ++ lib/system_check/base_check.rb | 138 +++-- lib/system_check/helpers.rb | 80 +++ lib/system_check/simple_executor.rb | 49 +- lib/tasks/gitlab/check.rake | 493 ++---------------- lib/tasks/gitlab/task_helpers.rb | 1 + 22 files changed, 777 insertions(+), 503 deletions(-) create mode 100644 lib/system_check/app/database_config_exists_check.rb create mode 100644 lib/system_check/app/git_config_check.rb create mode 100644 lib/system_check/app/git_version_check.rb create mode 100644 lib/system_check/app/gitlab_config_exists_check.rb create mode 100644 lib/system_check/app/gitlab_config_not_outdated_check.rb create mode 100644 lib/system_check/app/init_script_exists_check.rb create mode 100644 lib/system_check/app/init_script_up_to_date_check.rb create mode 100644 lib/system_check/app/log_writable_check.rb create mode 100644 lib/system_check/app/migrations_are_up_check.rb create mode 100644 lib/system_check/app/orphaned_group_members_check.rb create mode 100644 lib/system_check/app/projects_have_namespace_check.rb create mode 100644 lib/system_check/app/redis_version_check.rb create mode 100644 lib/system_check/app/ruby_version_check.rb create mode 100644 lib/system_check/app/tmp_writable_check.rb create mode 100644 lib/system_check/app/uploads_directory_exists_check.rb create mode 100644 lib/system_check/app/uploads_path_permission_check.rb create mode 100644 lib/system_check/app/uploads_path_tmp_permission_check.rb create mode 100644 lib/system_check/helpers.rb diff --git a/lib/system_check/app/database_config_exists_check.rb b/lib/system_check/app/database_config_exists_check.rb new file mode 100644 index 00000000000..d557cee47b4 --- /dev/null +++ b/lib/system_check/app/database_config_exists_check.rb @@ -0,0 +1,31 @@ +module SystemCheck + module App + class DatabaseConfigExistsCheck < SystemCheck::BaseCheck + set_name 'Database config exists?' + + def check? + database_config_file = Rails.root.join('config', 'database.yml') + + File.exist?(database_config_file) + end + + def show_error + try_fixing_it( + 'Copy config/database.yml. to config/database.yml', + 'Check that the information in config/database.yml is correct' + ) + for_more_information( + see_database_guide, + 'http://guides.rubyonrails.org/getting_started.html#configuring-a-database' + ) + fix_and_rerun + end + + private + + def see_database_guide + 'doc/install/databases.md' + end + end + end +end diff --git a/lib/system_check/app/git_config_check.rb b/lib/system_check/app/git_config_check.rb new file mode 100644 index 00000000000..7f0c792eb35 --- /dev/null +++ b/lib/system_check/app/git_config_check.rb @@ -0,0 +1,50 @@ +module SystemCheck + module App + class GitConfigCheck < SystemCheck::BaseCheck + OPTIONS = { + 'core.autocrlf' => 'input' + }.freeze + + set_name 'Git configured with autocrlf=input?' + + def check? + correct_options = OPTIONS.map do |name, value| + run_command(%W(#{Gitlab.config.git.bin_path} config --global --get #{name})).try(:squish) == value + end + + correct_options.all? + end + + def repair! + auto_fix_git_config(OPTIONS) + end + + def show_error + try_fixing_it( + sudo_gitlab("\"#{Gitlab.config.git.bin_path}\" config --global core.autocrlf \"#{OPTIONS['core.autocrlf']}\"") + ) + for_more_information( + see_installation_guide_section 'GitLab' + ) + end + + private + + # Tries to configure git itself + # + # Returns true if all subcommands were successfull (according to their exit code) + # Returns false if any or all subcommands failed. + def auto_fix_git_config(options) + if !@warned_user_not_gitlab + command_success = options.map do |name, value| + system(*%W(#{Gitlab.config.git.bin_path} config --global #{name} #{value})) + end + + command_success.all? + else + false + end + end + end + end +end diff --git a/lib/system_check/app/git_version_check.rb b/lib/system_check/app/git_version_check.rb new file mode 100644 index 00000000000..3a128f00911 --- /dev/null +++ b/lib/system_check/app/git_version_check.rb @@ -0,0 +1,29 @@ +module SystemCheck + module App + class GitVersionCheck < SystemCheck::BaseCheck + set_name -> { "Git version >= #{self.required_version} ?" } + set_check_pass -> { "yes (#{self.current_version})" } + + def self.required_version + @required_version ||= Gitlab::VersionInfo.new(2, 7, 3) + end + + def self.current_version + @current_version ||= Gitlab::VersionInfo.parse(run_command(%W(#{Gitlab.config.git.bin_path} --version))) + end + + def check? + self.class.current_version.valid? && self.class.required_version <= self.class.current_version + end + + def show_error + puts "Your git bin path is \"#{Gitlab.config.git.bin_path}\"" + + try_fixing_it( + "Update your git to a version >= #{self.class.required_version} from #{self.class.current_version}" + ) + fix_and_rerun + end + end + end +end diff --git a/lib/system_check/app/gitlab_config_exists_check.rb b/lib/system_check/app/gitlab_config_exists_check.rb new file mode 100644 index 00000000000..247aa0994e4 --- /dev/null +++ b/lib/system_check/app/gitlab_config_exists_check.rb @@ -0,0 +1,24 @@ +module SystemCheck + module App + class GitlabConfigExistsCheck < SystemCheck::BaseCheck + set_name 'GitLab config exists?' + + def check? + gitlab_config_file = Rails.root.join('config', 'gitlab.yml') + + File.exist?(gitlab_config_file) + 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 +end diff --git a/lib/system_check/app/gitlab_config_not_outdated_check.rb b/lib/system_check/app/gitlab_config_not_outdated_check.rb new file mode 100644 index 00000000000..8a4d7b29977 --- /dev/null +++ b/lib/system_check/app/gitlab_config_not_outdated_check.rb @@ -0,0 +1,32 @@ +module SystemCheck + module App + class GitlabConfigNotOutdatedCheck < SystemCheck::BaseCheck + set_name 'GitLab config outdated?' + set_check_pass 'no' + set_check_fail 'yes' + set_skip_reason "can't check because of previous errors" + + def skip? + gitlab_config_file = Rails.root.join('config', 'gitlab.yml') + !File.exist?(gitlab_config_file) + end + + def check? + # omniauth or ldap could have been deleted from the file + !Gitlab.config['git_host'] + end + + def show_error + try_fixing_it( + 'Backup your config/gitlab.yml', + '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 +end diff --git a/lib/system_check/app/init_script_exists_check.rb b/lib/system_check/app/init_script_exists_check.rb new file mode 100644 index 00000000000..d246e058e86 --- /dev/null +++ b/lib/system_check/app/init_script_exists_check.rb @@ -0,0 +1,27 @@ +module SystemCheck + module App + class InitScriptExistsCheck < SystemCheck::BaseCheck + set_name 'Init script exists?' + set_skip_reason 'skipped (omnibus-gitlab has no init script)' + + def skip? + omnibus_gitlab? + end + + def check? + script_path = '/etc/init.d/gitlab' + File.exist?(script_path) + end + + def show_error + try_fixing_it( + 'Install the init script' + ) + for_more_information( + see_installation_guide_section 'Install Init Script' + ) + fix_and_rerun + end + end + end +end diff --git a/lib/system_check/app/init_script_up_to_date_check.rb b/lib/system_check/app/init_script_up_to_date_check.rb new file mode 100644 index 00000000000..28c1451b8d2 --- /dev/null +++ b/lib/system_check/app/init_script_up_to_date_check.rb @@ -0,0 +1,44 @@ +module SystemCheck + module App + class InitScriptUpToDateCheck < SystemCheck::BaseCheck + SCRIPT_PATH = '/etc/init.d/gitlab'.freeze + + set_name 'Init script up-to-date?' + set_skip_reason 'skipped (omnibus-gitlab has no init script)' + + def skip? + omnibus_gitlab? + end + + def multi_check + recipe_path = Rails.root.join('lib/support/init.d/', 'gitlab') + + unless File.exist?(SCRIPT_PATH) + puts "can't check because of previous errors".color(:magenta) + return + end + + recipe_content = File.read(recipe_path) + script_content = File.read(SCRIPT_PATH) + + if recipe_content == script_content + puts 'yes'.color(:green) + else + puts 'no'.color(:red) + show_error + end + end + + + def show_error + try_fixing_it( + 'Re-download the init script' + ) + for_more_information( + see_installation_guide_section 'Install Init Script' + ) + fix_and_rerun + end + end + end +end diff --git a/lib/system_check/app/log_writable_check.rb b/lib/system_check/app/log_writable_check.rb new file mode 100644 index 00000000000..3e0c436d6ee --- /dev/null +++ b/lib/system_check/app/log_writable_check.rb @@ -0,0 +1,28 @@ +module SystemCheck + module App + class LogWritableCheck < SystemCheck::BaseCheck + set_name 'Log directory writable?' + + def check? + File.writable?(log_path) + end + + def show_error + try_fixing_it( + "sudo chown -R gitlab #{log_path}", + "sudo chmod -R u+rwX #{log_path}" + ) + for_more_information( + see_installation_guide_section 'GitLab' + ) + fix_and_rerun + end + + private + + def log_path + Rails.root.join('log') + end + end + end +end diff --git a/lib/system_check/app/migrations_are_up_check.rb b/lib/system_check/app/migrations_are_up_check.rb new file mode 100644 index 00000000000..5eedbacce77 --- /dev/null +++ b/lib/system_check/app/migrations_are_up_check.rb @@ -0,0 +1,20 @@ +module SystemCheck + module App + class MigrationsAreUpCheck < SystemCheck::BaseCheck + set_name 'All migrations up?' + + def check? + migration_status, _ = Gitlab::Popen.popen(%w(bundle exec rake db:migrate:status)) + + migration_status !~ /down\s+\d{14}/ + end + + def show_error + try_fixing_it( + sudo_gitlab('bundle exec rake db:migrate RAILS_ENV=production') + ) + fix_and_rerun + end + end + end +end diff --git a/lib/system_check/app/orphaned_group_members_check.rb b/lib/system_check/app/orphaned_group_members_check.rb new file mode 100644 index 00000000000..2b46d36fe51 --- /dev/null +++ b/lib/system_check/app/orphaned_group_members_check.rb @@ -0,0 +1,20 @@ +module SystemCheck + module App + class OrphanedGroupMembersCheck < SystemCheck::BaseCheck + set_name 'Database contains orphaned GroupMembers?' + set_check_pass 'no' + set_check_fail 'yes' + + def check? + !GroupMember.where('user_id not in (select id from users)').exists? + end + + def show_error + try_fixing_it( + 'You can delete the orphaned records using something along the lines of:', + sudo_gitlab("bundle exec rails runner -e production 'GroupMember.where(\"user_id NOT IN (SELECT id FROM users)\").delete_all'") + ) + end + end + end +end diff --git a/lib/system_check/app/projects_have_namespace_check.rb b/lib/system_check/app/projects_have_namespace_check.rb new file mode 100644 index 00000000000..d21ae225b0b --- /dev/null +++ b/lib/system_check/app/projects_have_namespace_check.rb @@ -0,0 +1,37 @@ +module SystemCheck + module App + class ProjectsHaveNamespaceCheck < SystemCheck::BaseCheck + set_name 'projects have namespace: ' + set_skip_reason "can't check, you have no projects" + + def skip? + !Project.exists? + end + + def multi_check + puts '' + + Project.find_each(batch_size: 100) do |project| + print sanitized_message(project) + + if project.namespace + puts 'yes'.color(:green) + else + puts 'no'.color(:red) + show_error + end + end + end + + def show_error + try_fixing_it( + "Migrate global projects" + ) + for_more_information( + "doc/update/5.4-to-6.0.md in section \"#global-projects\"" + ) + fix_and_rerun + end + end + end +end diff --git a/lib/system_check/app/redis_version_check.rb b/lib/system_check/app/redis_version_check.rb new file mode 100644 index 00000000000..a0610e73576 --- /dev/null +++ b/lib/system_check/app/redis_version_check.rb @@ -0,0 +1,25 @@ +module SystemCheck + module App + class RedisVersionCheck < SystemCheck::BaseCheck + MIN_REDIS_VERSION = '2.8.0'.freeze + set_name "Redis version >= #{MIN_REDIS_VERSION}?" + + def check? + redis_version = run_command(%w(redis-cli --version)) + redis_version = redis_version.try(:match, /redis-cli (\d+\.\d+\.\d+)/) + + redis_version && (Gem::Version.new(redis_version[1]) > Gem::Version.new(MIN_REDIS_VERSION)) + end + + def show_error + try_fixing_it( + "Update your redis server to a version >= #{MIN_REDIS_VERSION}" + ) + for_more_information( + 'gitlab-public-wiki/wiki/Trouble-Shooting-Guide in section sidekiq' + ) + fix_and_rerun + end + end + end +end diff --git a/lib/system_check/app/ruby_version_check.rb b/lib/system_check/app/ruby_version_check.rb new file mode 100644 index 00000000000..37b4d24aa55 --- /dev/null +++ b/lib/system_check/app/ruby_version_check.rb @@ -0,0 +1,27 @@ +module SystemCheck + module App + class RubyVersionCheck < SystemCheck::BaseCheck + set_name -> { "Ruby version >= #{self.required_version} ?" } + set_check_pass -> { "yes (#{self.current_version})" } + + def self.required_version + @required_version ||= Gitlab::VersionInfo.new(2, 1, 0) + end + + def self.current_version + @current_version ||= Gitlab::VersionInfo.parse(run_command(%w(ruby --version))) + end + + def check? + self.class.current_version.valid? && self.class.required_version <= self.class.current_version + end + + def show_error + try_fixing_it( + "Update your ruby to a version >= #{self.class.required_version} from #{self.class.current_version}" + ) + fix_and_rerun + end + end + end +end diff --git a/lib/system_check/app/tmp_writable_check.rb b/lib/system_check/app/tmp_writable_check.rb new file mode 100644 index 00000000000..99a75e57abf --- /dev/null +++ b/lib/system_check/app/tmp_writable_check.rb @@ -0,0 +1,28 @@ +module SystemCheck + module App + class TmpWritableCheck < SystemCheck::BaseCheck + set_name 'Tmp directory writable?' + + def check? + File.writable?(tmp_path) + end + + def show_error + try_fixing_it( + "sudo chown -R gitlab #{tmp_path}", + "sudo chmod -R u+rwX #{tmp_path}" + ) + for_more_information( + see_installation_guide_section 'GitLab' + ) + fix_and_rerun + end + + private + + def tmp_path + Rails.root.join('tmp') + end + end + end +end diff --git a/lib/system_check/app/uploads_directory_exists_check.rb b/lib/system_check/app/uploads_directory_exists_check.rb new file mode 100644 index 00000000000..7026d0ba075 --- /dev/null +++ b/lib/system_check/app/uploads_directory_exists_check.rb @@ -0,0 +1,21 @@ +module SystemCheck + module App + class UploadsDirectoryExistsCheck < SystemCheck::BaseCheck + set_name 'Uploads directory exists?' + + def check? + File.directory?(Rails.root.join('public/uploads')) + end + + def show_error + try_fixing_it( + "sudo -u #{gitlab_user} mkdir #{Rails.root}/public/uploads" + ) + for_more_information( + see_installation_guide_section 'GitLab' + ) + fix_and_rerun + end + end + end +end diff --git a/lib/system_check/app/uploads_path_permission_check.rb b/lib/system_check/app/uploads_path_permission_check.rb new file mode 100644 index 00000000000..7df6c060254 --- /dev/null +++ b/lib/system_check/app/uploads_path_permission_check.rb @@ -0,0 +1,36 @@ +module SystemCheck + module App + class UploadsPathPermissionCheck < SystemCheck::BaseCheck + set_name 'Uploads directory has correct permissions?' + set_skip_reason 'skipped (no uploads folder found)' + + def skip? + !File.directory?(rails_uploads_path) + end + + def check? + File.stat(uploads_fullpath).mode == 040700 + end + + def show_error + try_fixing_it( + "sudo chmod 700 #{uploads_fullpath}" + ) + for_more_information( + see_installation_guide_section 'GitLab' + ) + fix_and_rerun + end + + private + + def rails_uploads_path + Rails.root.join('public/uploads') + end + + def uploads_fullpath + File.realpath(rails_uploads_path) + end + end + end +end diff --git a/lib/system_check/app/uploads_path_tmp_permission_check.rb b/lib/system_check/app/uploads_path_tmp_permission_check.rb new file mode 100644 index 00000000000..b276a81eac1 --- /dev/null +++ b/lib/system_check/app/uploads_path_tmp_permission_check.rb @@ -0,0 +1,40 @@ +module SystemCheck + module App + class UploadsPathTmpPermissionCheck < SystemCheck::BaseCheck + set_name 'Uploads directory tmp has correct permissions?' + set_skip_reason 'skipped (no tmp uploads folder yet)' + + def skip? + !File.directory?(uploads_fullpath) || !Dir.exist?(upload_path_tmp) + end + + def check? + # If tmp upload dir has incorrect permissions, assume others do as well + # Verify drwx------ permissions + File.stat(upload_path_tmp).mode == 040700 && File.owned?(upload_path_tmp) + end + + def show_error + try_fixing_it( + "sudo chown -R #{gitlab_user} #{uploads_fullpath}", + "sudo find #{uploads_fullpath} -type f -exec chmod 0644 {} \\;", + "sudo find #{uploads_fullpath} -type d -not -path #{uploads_fullpath} -exec chmod 0700 {} \\;" + ) + for_more_information( + see_installation_guide_section 'GitLab' + ) + fix_and_rerun + end + + private + + def upload_path_tmp + File.join(uploads_fullpath, 'tmp') + end + + def uploads_fullpath + File.realpath(Rails.root.join('public/uploads')) + end + end + end +end diff --git a/lib/system_check/base_check.rb b/lib/system_check/base_check.rb index d465711d719..aff830f54d7 100644 --- a/lib/system_check/base_check.rb +++ b/lib/system_check/base_check.rb @@ -2,16 +2,103 @@ module SystemCheck # Base class for Checks. You must inherit from here # and implement the methods below when necessary class BaseCheck + include ::Gitlab::TaskHelpers + include Helpers + + # Define a custom term for when check passed + # + # @param [String] term used when check passed (default: 'yes') + def self.set_check_pass(term) + @check_pass = term + end + + # Define a custom term for when check failed + # + # @param [String] term used when check failed (default: 'no') + def self.set_check_fail(term) + @check_fail = term + end + + # Define the name of the SystemCheck that will be displayed during execution + # + # @param [String] name of the check + def self.set_name(name) + @name = name + end + + # Define the reason why we skipped the SystemCheck + # + # This is only used if subclass implements `#skip?` + # + # @param [String] reason to be displayed + def self.set_skip_reason(reason) + @skip_reason = reason + end + + # Term to be displayed when check passed + # + # @return [String] term when check passed ('yes' if not re-defined in a subclass) + def self.check_pass + call_or_return(@check_pass) || 'yes' + end + + ## Term to be displayed when check failed + # + # @return [String] term when check failed ('no' if not re-defined in a subclass) + def self.check_fail + call_or_return(@check_fail) || 'no' + end + + # Name of the SystemCheck defined by the subclass + # + # @return [String] the name + def self.display_name + call_or_return(@name) || self.name + end + + # Skip reason defined by the subclass + # + # @return [String] the reason + def self.skip_reason + call_or_return(@skip_reason) || 'skipped' + end + + # Does the check support automatically repair routine? + # + # @return [Boolean] whether check implemented `#repair!` method or not + def can_repair? + self.class.instance_methods(false).include?(:repair!) + end + + def can_skip? + self.class.instance_methods(false).include?(:skip?) + end + + def is_multi_check? + self.class.instance_methods(false).include?(:multi_check) + end + + # Execute the check routine + # # This is where you should implement the main logic that will return # a boolean at the end # # You should not print any output to STDOUT here, use the specific methods instead # - # @return [Boolean] whether the check passed or not + # @return [Boolean] whether check passed or failed def check? raise NotImplementedError end + # Execute a custom check that cover multiple unities + # + # When using multi_check you have to provide the output yourself + def multi_check + raise NotImplementedError + end + + # Prints troubleshooting instructions + # # This is where you should print detailed information for any error found during #check? # # You may use helper methods to help format the output: @@ -23,50 +110,21 @@ module SystemCheck raise NotImplementedError end - # If skip returns true, than no other method on this check will be executed + # When implemented by a subclass, will attempt to fix the issue automatically + def repair! + raise NotImplementedError + end + + # When implemented by a subclass, will evaluate whether check should be skipped or not # # @return [Boolean] whether or not this check should be skipped def skip? - false + raise NotImplementedError end - # If you enabled #skip? here is where you define a custom message explaining why - # - # Do not print anything to STDOUT, return a string. - # - # @return [String] message why this check was skipped - def skip_message - end - - protected - - # Display a formatted list of instructions on how to fix the issue identified by the #check? - # - # @param [Array] steps one or short sentences with help how to fix the issue - def try_fixing_it(*steps) - steps = steps.shift if steps.first.is_a?(Array) - - $stdout.puts ' Try fixing it:'.color(:blue) - steps.each do |step| - $stdout.puts " #{step}" - end - end - - # Display a message telling to fix and rerun the checks - def fix_and_rerun - $stdout.puts ' Please fix the error above and rerun the checks.'.color(:red) - end - - # Display a formatted list of references (documentation or links) where to find more information - # - # @param [Array] sources one or more references (documentation or links) - def for_more_information(*sources) - sources = sources.shift if sources.first.is_a?(Array) - - $stdout.puts ' For more information see:'.color(:blue) - sources.each do |source| - $stdout.puts ' #{source}' - end + def self.call_or_return(input) + input.respond_to?(:call) ? input.call : input end + private_class_method :call_or_return end end diff --git a/lib/system_check/helpers.rb b/lib/system_check/helpers.rb new file mode 100644 index 00000000000..91454c99838 --- /dev/null +++ b/lib/system_check/helpers.rb @@ -0,0 +1,80 @@ +module SystemCheck + module Helpers + # Display a message telling to fix and rerun the checks + def fix_and_rerun + $stdout.puts ' Please fix the error above and rerun the checks.'.color(:red) + end + + # Display a formatted list of references (documentation or links) where to find more information + # + # @param [Array] sources one or more references (documentation or links) + def for_more_information(*sources) + sources = sources.shift if sources.first.is_a?(Array) + + $stdout.puts ' For more information see:'.color(:blue) + sources.each do |source| + $stdout.puts " #{source}" + end + end + + def see_installation_guide_section(section) + "doc/install/installation.md in section \"#{section}\"" + end + + # @deprecated This will no longer be used when all checks were executed using SystemCheck + def finished_checking(component) + $stdout.puts '' + $stdout.puts "Checking #{component.color(:yellow)} ... #{'Finished'.color(:green)}" + $stdout.puts '' + end + + # @deprecated This will no longer be used when all checks were executed using SystemCheck + def start_checking(component) + $stdout.puts "Checking #{component.color(:yellow)} ..." + $stdout.puts '' + end + + # Display a formatted list of instructions on how to fix the issue identified by the #check? + # + # @param [Array] steps one or short sentences with help how to fix the issue + def try_fixing_it(*steps) + steps = steps.shift if steps.first.is_a?(Array) + + $stdout.puts ' Try fixing it:'.color(:blue) + steps.each do |step| + $stdout.puts " #{step}" + end + end + + def sanitized_message(project) + if should_sanitize? + "#{project.namespace_id.to_s.color(:yellow)}/#{project.id.to_s.color(:yellow)} ... " + else + "#{project.name_with_namespace.color(:yellow)} ... " + end + end + + def should_sanitize? + if ENV['SANITIZE'] == 'true' + true + else + false + end + end + + def omnibus_gitlab? + Dir.pwd == '/opt/gitlab/embedded/service/gitlab-rails' + end + + def gitlab_user + Gitlab.config.gitlab.user + end + + def sudo_gitlab(command) + "sudo -u #{gitlab_user} -H #{command}" + end + end +end + + + diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb index fc07f09dcb2..7a5042047ab 100644 --- a/lib/system_check/simple_executor.rb +++ b/lib/system_check/simple_executor.rb @@ -10,20 +10,51 @@ module SystemCheck start_checking(component) @checks.each do |check| - $stdout.print "#{check.name}" - if check.skip? - $stdout.puts "skipped #{'(' + skip_message + ')' if skip_message}".color(:magenta) - elsif check.check? - $stdout.puts 'yes'.color(:green) - else - $stdout.puts 'no'.color(:red) - check.show_error - end + run_check(check) end finished_checking(component) end + # Executes a single check + # + # @param [SystemCheck::BaseCheck] check + def run_check(check) + $stdout.print "#{check.display_name} ... " + + c = check.new + + # When implements a multi check, we don't control the output + if c.is_multi_check? + c.multi_check + return + end + + # When implements skip method, we run it first, and if true, skip the check + if c.can_skip? && c.skip? + $stdout.puts check.skip_reason.color(:magenta) + return + end + + if c.check? + $stdout.puts check.check_pass.color(:green) + else + $stdout.puts check.check_fail.color(:red) + + if c.can_repair? + $stdout.print 'Trying to fix error automatically. ...' + if c.repair! + $stdout.puts 'Success'.color(:green) + return + else + $stdout.puts 'Failed'.color(:red) + end + end + + c.show_error + end + end + private # Prints header content for the series of checks to be executed for this component diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 88ec9cbc8cc..0d177663066 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -1,5 +1,9 @@ +# Temporary hack, until we migrate all checks to SystemCheck format +require 'system_check' +require 'system_check/helpers' + namespace :gitlab do - desc "GitLab | Check the configuration of GitLab and its environment" + desc 'GitLab | Check the configuration of GitLab and its environment' task check: %w{gitlab:gitlab_shell:check gitlab:sidekiq:check gitlab:incoming_email:check @@ -7,331 +11,38 @@ namespace :gitlab do gitlab:app:check} namespace :app do - desc "GitLab | Check the configuration of the GitLab Rails app" + 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 + checks = [ + SystemCheck::App::GitConfigCheck, + SystemCheck::App::DatabaseConfigExistsCheck, + SystemCheck::App::MigrationsAreUpCheck, + SystemCheck::App::OrphanedGroupMembersCheck, + SystemCheck::App::GitlabConfigExistsCheck, + SystemCheck::App::GitlabConfigNotOutdatedCheck, + SystemCheck::App::LogWritableCheck, + SystemCheck::App::TmpWritableCheck, + SystemCheck::App::UploadsDirectoryExistsCheck, + SystemCheck::App::UploadsPathPermissionCheck, + SystemCheck::App::UploadsPathTmpPermissionCheck, + SystemCheck::App::InitScriptExistsCheck, + SystemCheck::App::InitScriptUpToDateCheck, + SystemCheck::App::ProjectsHaveNamespaceCheck, + SystemCheck::App::RedisVersionCheck, + SystemCheck::App::RubyVersionCheck, + SystemCheck::App::GitVersionCheck + ] + + SystemCheck.run('GitLab', checks) check_active_users - - finished_checking "GitLab" - end - - # Checks - ######################## - - def check_git_config - print "Git configured with autocrlf=input? ... " - - options = { - "core.autocrlf" => "input" - } - - correct_options = options.map do |name, value| - run_command(%W(#{Gitlab.config.git.bin_path} config --global --get #{name})).try(:squish) == value - end - - if correct_options.all? - puts "yes".color(:green) - else - print "Trying to fix Git error automatically. ..." - - if auto_fix_git_config(options) - puts "Success".color(:green) - else - puts "Failed".color(:red) - try_fixing_it( - sudo_gitlab("\"#{Gitlab.config.git.bin_path}\" config --global core.autocrlf \"#{options["core.autocrlf"]}\"") - ) - for_more_information( - see_installation_guide_section "GitLab" - ) - end - end - end - - def check_database_config_exists - print "Database config exists? ... " - - database_config_file = Rails.root.join("config", "database.yml") - - if File.exist?(database_config_file) - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "Copy config/database.yml. to config/database.yml", - "Check that the information in config/database.yml is correct" - ) - for_more_information( - see_database_guide, - "http://guides.rubyonrails.org/getting_started.html#configuring-a-database" - ) - fix_and_rerun - end - end - - 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 - - def check_gitlab_config_not_outdated - print "GitLab config outdated? ... " - - gitlab_config_file = Rails.root.join("config", "gitlab.yml") - unless File.exist?(gitlab_config_file) - puts "can't check because of previous errors".color(:magenta) - end - - # omniauth or ldap could have been deleted from the file - unless Gitlab.config['git_host'] - puts "no".color(:green) - else - puts "yes".color(:red) - try_fixing_it( - "Backup your config/gitlab.yml", - "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 - - def check_init_script_exists - print "Init script exists? ... " - - if omnibus_gitlab? - puts 'skipped (omnibus-gitlab has no init script)'.color(:magenta) - return - end - - script_path = "/etc/init.d/gitlab" - - if File.exist?(script_path) - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "Install the init script" - ) - for_more_information( - see_installation_guide_section "Install Init Script" - ) - fix_and_rerun - end - end - - def check_init_script_up_to_date - print "Init script up-to-date? ... " - - if omnibus_gitlab? - puts 'skipped (omnibus-gitlab has no init script)'.color(:magenta) - return - end - - recipe_path = Rails.root.join("lib/support/init.d/", "gitlab") - script_path = "/etc/init.d/gitlab" - - unless File.exist?(script_path) - puts "can't check because of previous errors".color(:magenta) - return - end - - recipe_content = File.read(recipe_path) - script_content = File.read(script_path) - - if recipe_content == script_content - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "Redownload the init script" - ) - for_more_information( - see_installation_guide_section "Install Init Script" - ) - fix_and_rerun - end - end - - def check_migrations_are_up - print "All migrations up? ... " - - migration_status, _ = Gitlab::Popen.popen(%w(bundle exec rake db:migrate:status)) - - unless migration_status =~ /down\s+\d{14}/ - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - sudo_gitlab("bundle exec rake db:migrate RAILS_ENV=production") - ) - fix_and_rerun - end - end - - def check_orphaned_group_members - print "Database contains orphaned GroupMembers? ... " - if GroupMember.where("user_id not in (select id from users)").count > 0 - puts "yes".color(:red) - try_fixing_it( - "You can delete the orphaned records using something along the lines of:", - sudo_gitlab("bundle exec rails runner -e production 'GroupMember.where(\"user_id NOT IN (SELECT id FROM users)\").delete_all'") - ) - else - puts "no".color(:green) - end - end - - def check_log_writable - print "Log directory writable? ... " - - log_path = Rails.root.join("log") - - if File.writable?(log_path) - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "sudo chown -R gitlab #{log_path}", - "sudo chmod -R u+rwX #{log_path}" - ) - for_more_information( - see_installation_guide_section "GitLab" - ) - fix_and_rerun - end - end - - def check_tmp_writable - print "Tmp directory writable? ... " - - tmp_path = Rails.root.join("tmp") - - if File.writable?(tmp_path) - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "sudo chown -R gitlab #{tmp_path}", - "sudo chmod -R u+rwX #{tmp_path}" - ) - for_more_information( - see_installation_guide_section "GitLab" - ) - fix_and_rerun - end - end - - def check_uploads - print "Uploads directory setup correctly? ... " - - unless File.directory?(Rails.root.join('public/uploads')) - puts "no".color(:red) - try_fixing_it( - "sudo -u #{gitlab_user} mkdir #{Rails.root}/public/uploads" - ) - for_more_information( - see_installation_guide_section "GitLab" - ) - fix_and_rerun - return - end - - upload_path = File.realpath(Rails.root.join('public/uploads')) - upload_path_tmp = File.join(upload_path, 'tmp') - - if File.stat(upload_path).mode == 040700 - unless Dir.exist?(upload_path_tmp) - puts 'skipped (no tmp uploads folder yet)'.color(:magenta) - return - end - - # If tmp upload dir has incorrect permissions, assume others do as well - # Verify drwx------ permissions - if File.stat(upload_path_tmp).mode == 040700 && File.owned?(upload_path_tmp) - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "sudo chown -R #{gitlab_user} #{upload_path}", - "sudo find #{upload_path} -type f -exec chmod 0644 {} \\;", - "sudo find #{upload_path} -type d -not -path #{upload_path} -exec chmod 0700 {} \\;" - ) - for_more_information( - see_installation_guide_section "GitLab" - ) - fix_and_rerun - end - else - puts "no".color(:red) - try_fixing_it( - "sudo chmod 700 #{upload_path}" - ) - for_more_information( - see_installation_guide_section "GitLab" - ) - fix_and_rerun - end - end - - def check_redis_version - min_redis_version = "2.8.0" - print "Redis version >= #{min_redis_version}? ... " - - redis_version = run_command(%w(redis-cli --version)) - redis_version = redis_version.try(:match, /redis-cli (\d+\.\d+\.\d+)/) - if redis_version && - (Gem::Version.new(redis_version[1]) > Gem::Version.new(min_redis_version)) - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "Update your redis server to a version >= #{min_redis_version}" - ) - for_more_information( - "gitlab-public-wiki/wiki/Trouble-Shooting-Guide in section sidekiq" - ) - fix_and_rerun - end end end namespace :gitlab_shell do + include SystemCheck::Helpers + desc "GitLab | Check the configuration of GitLab Shell" task check: :environment do warn_user_is_not_gitlab @@ -513,33 +224,6 @@ namespace :gitlab do end end - def check_projects_have_namespace - print "projects have namespace: ... " - - unless Project.count > 0 - puts "can't check, you have no projects".color(:magenta) - return - end - puts "" - - Project.find_each(batch_size: 100) do |project| - print sanitized_message(project) - - if project.namespace - puts "yes".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "Migrate global projects" - ) - for_more_information( - "doc/update/5.4-to-6.0.md in section \"#global-projects\"" - ) - fix_and_rerun - end - end - end - # Helper methods ######################## @@ -565,6 +249,8 @@ namespace :gitlab do end namespace :sidekiq do + include SystemCheck::Helpers + desc "GitLab | Check the configuration of Sidekiq" task check: :environment do warn_user_is_not_gitlab @@ -623,6 +309,8 @@ namespace :gitlab do end namespace :incoming_email do + include SystemCheck::Helpers + desc "GitLab | Check the configuration of Reply by email" task check: :environment do warn_user_is_not_gitlab @@ -757,6 +445,8 @@ namespace :gitlab do end namespace :ldap do + include SystemCheck::Helpers + task :check, [:limit] => :environment do |_, args| # Only show up to 100 results because LDAP directories can be very big. # This setting only affects the `rake gitlab:check` script. @@ -812,6 +502,8 @@ namespace :gitlab do end namespace :repo do + include SystemCheck::Helpers + desc "GitLab | Check the integrity of the repositories managed by GitLab" task check: :environment do Gitlab.config.repositories.storages.each do |name, repository_storage| @@ -826,6 +518,8 @@ namespace :gitlab do end namespace :user do + include SystemCheck::Helpers + desc "GitLab | Check the integrity of a specific user's repositories" task :check_repos, [:username] => :environment do |t, args| username = args[:username] || prompt("Check repository integrity for fsername? ".color(:blue)) @@ -848,60 +542,6 @@ namespace :gitlab do # Helper methods ########################## - # @deprecated Please use SystemChecks - def fix_and_rerun - puts " Please fix the error above and rerun the checks.".color(:red) - end - - # @deprecated Please use SystemChecks - def for_more_information(*sources) - sources = sources.shift if sources.first.is_a?(Array) - - puts " For more information see:".color(:blue) - sources.each do |source| - puts " #{source}" - end - end - - # @deprecated Please use SystemChecks - def finished_checking(component) - puts "" - puts "Checking #{component.color(:yellow)} ... #{"Finished".color(:green)}" - puts "" - end - - def see_database_guide - "doc/install/databases.md" - end - - def see_installation_guide_section(section) - "doc/install/installation.md in section \"#{section}\"" - end - - def sudo_gitlab(command) - "sudo -u #{gitlab_user} -H #{command}" - end - - def gitlab_user - Gitlab.config.gitlab.user - end - - # @deprecated Please use SystemChecks - def start_checking(component) - puts "Checking #{component.color(:yellow)} ..." - puts "" - end - - # @deprecated Please use SystemChecks - def try_fixing_it(*steps) - steps = steps.shift if steps.first.is_a?(Array) - - puts " Try fixing it:".color(:blue) - steps.each do |step| - puts " #{step}" - end - end - def check_gitlab_shell required_version = Gitlab::VersionInfo.new(gitlab_shell_major_version, gitlab_shell_minor_version, gitlab_shell_patch_version) current_version = Gitlab::VersionInfo.parse(gitlab_shell_version) @@ -914,65 +554,10 @@ namespace :gitlab do end end - def check_ruby_version - required_version = Gitlab::VersionInfo.new(2, 1, 0) - current_version = Gitlab::VersionInfo.parse(run_command(%w(ruby --version))) - - print "Ruby version >= #{required_version} ? ... " - - if current_version.valid? && required_version <= current_version - puts "yes (#{current_version})".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "Update your ruby to a version >= #{required_version} from #{current_version}" - ) - fix_and_rerun - end - end - - def check_git_version - required_version = Gitlab::VersionInfo.new(2, 7, 3) - current_version = Gitlab::VersionInfo.parse(run_command(%W(#{Gitlab.config.git.bin_path} --version))) - - puts "Your git bin path is \"#{Gitlab.config.git.bin_path}\"" - print "Git version >= #{required_version} ? ... " - - if current_version.valid? && required_version <= current_version - puts "yes (#{current_version})".color(:green) - else - puts "no".color(:red) - try_fixing_it( - "Update your git to a version >= #{required_version} from #{current_version}" - ) - fix_and_rerun - end - end - def check_active_users puts "Active users: #{User.active.count}" end - def omnibus_gitlab? - Dir.pwd == '/opt/gitlab/embedded/service/gitlab-rails' - end - - def sanitized_message(project) - if should_sanitize? - "#{project.namespace_id.to_s.color(:yellow)}/#{project.id.to_s.color(:yellow)} ... " - else - "#{project.name_with_namespace.color(:yellow)} ... " - end - end - - def should_sanitize? - if ENV['SANITIZE'] == "true" - true - else - false - end - end - def check_repo_integrity(repo_dir) puts "\nChecking repo at #{repo_dir.color(:yellow)}" diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index e3c9d3b491c..e38e21b149f 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -113,6 +113,7 @@ module Gitlab end end + # TODO: MIGRATED # Tries to configure git itself # # Returns true if all subcommands were successfull (according to their exit code) From 27e632758feed94d8b3ff065b7c8928a854cecc5 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 26 May 2017 01:46:30 +0200 Subject: [PATCH 08/14] Add ActiveUsers Check --- lib/system_check/app/active_users_check.rb | 17 +++++++++++++++++ lib/tasks/gitlab/check.rake | 8 ++------ 2 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 lib/system_check/app/active_users_check.rb diff --git a/lib/system_check/app/active_users_check.rb b/lib/system_check/app/active_users_check.rb new file mode 100644 index 00000000000..1d72c8d6903 --- /dev/null +++ b/lib/system_check/app/active_users_check.rb @@ -0,0 +1,17 @@ +module SystemCheck + module App + class ActiveUsersCheck < SystemCheck::BaseCheck + set_name 'Active users:' + + def multi_check + active_users = User.active.count + + if active_users > 0 + $stdout.puts active_users.to_s.color(:green) + else + $stdout.puts active_users.to_s.color(:red) + end + end + end + end +end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 0d177663066..973517c99dd 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -32,11 +32,11 @@ namespace :gitlab do SystemCheck::App::ProjectsHaveNamespaceCheck, SystemCheck::App::RedisVersionCheck, SystemCheck::App::RubyVersionCheck, - SystemCheck::App::GitVersionCheck + SystemCheck::App::GitVersionCheck, + SystemCheck::App::ActiveUsersCheck ] SystemCheck.run('GitLab', checks) - check_active_users end end @@ -554,10 +554,6 @@ namespace :gitlab do end end - def check_active_users - puts "Active users: #{User.active.count}" - end - def check_repo_integrity(repo_dir) puts "\nChecking repo at #{repo_dir.color(:yellow)}" From ecdbde3d95b7abf11bae47d3a3b05693d84c27cc Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 26 May 2017 01:46:54 +0200 Subject: [PATCH 09/14] Improve Specs and some fixes --- lib/system_check/base_check.rb | 4 +- lib/system_check/simple_executor.rb | 12 +- lib/tasks/gitlab/task_helpers.rb | 1 - spec/lib/system_check/simple_executor_spec.rb | 179 ++++++++++++++++++ spec/lib/system_check_spec.rb | 5 + spec/support/rake_helpers.rb | 5 + 6 files changed, 198 insertions(+), 8 deletions(-) create mode 100644 spec/lib/system_check/simple_executor_spec.rb diff --git a/lib/system_check/base_check.rb b/lib/system_check/base_check.rb index aff830f54d7..63b7eea5add 100644 --- a/lib/system_check/base_check.rb +++ b/lib/system_check/base_check.rb @@ -1,9 +1,11 @@ +require 'tasks/gitlab/task_helpers' + module SystemCheck # Base class for Checks. You must inherit from here # and implement the methods below when necessary class BaseCheck include ::Gitlab::TaskHelpers - include Helpers + include ::SystemCheck::Helpers # Define a custom term for when check passed # diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb index 7a5042047ab..ad2f549b3fb 100644 --- a/lib/system_check/simple_executor.rb +++ b/lib/system_check/simple_executor.rb @@ -24,17 +24,17 @@ module SystemCheck c = check.new - # When implements a multi check, we don't control the output - if c.is_multi_check? - c.multi_check - return - end - # When implements skip method, we run it first, and if true, skip the check if c.can_skip? && c.skip? $stdout.puts check.skip_reason.color(:magenta) return end + + # When implements a multi check, we don't control the output + if c.is_multi_check? + c.multi_check + return + end if c.check? $stdout.puts check.check_pass.color(:green) diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index e38e21b149f..e3c9d3b491c 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -113,7 +113,6 @@ module Gitlab end end - # TODO: MIGRATED # Tries to configure git itself # # Returns true if all subcommands were successfull (according to their exit code) diff --git a/spec/lib/system_check/simple_executor_spec.rb b/spec/lib/system_check/simple_executor_spec.rb new file mode 100644 index 00000000000..f6ba437b71a --- /dev/null +++ b/spec/lib/system_check/simple_executor_spec.rb @@ -0,0 +1,179 @@ +require 'spec_helper' +require 'rake_helper' + +describe SystemCheck::SimpleExecutor, lib: true do + class SimpleCheck < SystemCheck::BaseCheck + set_name 'my simple check' + + def check? + true + end + end + + class OtherCheck < SystemCheck::BaseCheck + set_name 'other check' + + def check? + false + end + + def show_error + $stdout.puts 'this is an error text' + end + end + + class SkipCheck < SystemCheck::BaseCheck + set_name 'skip check' + set_skip_reason 'this is a skip reason' + + def skip? + true + end + + def check? + raise 'should not execute this' + end + end + + class MultiCheck < SystemCheck::BaseCheck + set_name 'multi check' + + def multi_check + $stdout.puts 'this is a multi output check' + end + + def check? + raise 'should not execute this' + end + end + + class SkipMultiCheck < SystemCheck::BaseCheck + set_name 'skip multi check' + + def skip? + true + end + + def multi_check + raise 'should not execute this' + end + end + + class RepairCheck < SystemCheck::BaseCheck + set_name 'repair check' + + def check? + false + end + + def repair! + true + end + + def show_error + $stdout.puts 'this is an error message' + end + end + + subject { described_class.new('Test') } + + describe '#execute' do + before do + silence_output + + subject << SimpleCheck + subject << OtherCheck + end + + it 'runs included checks' do + expect(subject).to receive(:run_check).with(SimpleCheck) + expect(subject).to receive(:run_check).with(OtherCheck) + + subject.execute + end + end + + describe '#run_check' do + it 'prints check name' do + expect(SimpleCheck).to receive(:display_name).and_call_original + expect { subject.run_check(SimpleCheck) }.to output(/my simple check/).to_stdout + end + + context 'when check pass' do + it 'prints yes' do + expect_any_instance_of(SimpleCheck).to receive(:check?).and_call_original + expect { subject.run_check(SimpleCheck) }.to output(/ \.\.\. yes/).to_stdout + end + end + + context 'when check fails' do + it 'prints no' do + expect_any_instance_of(OtherCheck).to receive(:check?).and_call_original + expect { subject.run_check(OtherCheck) }.to output(/ \.\.\. no/).to_stdout + end + + it 'displays error message from #show_error' do + expect_any_instance_of(OtherCheck).to receive(:show_error).and_call_original + expect { subject.run_check(OtherCheck) }.to output(/this is an error text/).to_stdout + end + + context 'when check implements #repair!' do + it 'executes #repair!' do + expect_any_instance_of(RepairCheck).to receive(:repair!) + subject.run_check(RepairCheck) + end + + context 'when repair succeeds' do + it 'does not execute #show_error' do + expect_any_instance_of(RepairCheck).to receive(:repair!).and_call_original + expect_any_instance_of(RepairCheck).not_to receive(:show_error) + subject.run_check(RepairCheck) + end + end + + context 'when repair fails' do + it 'does not execute #show_error' do + expect_any_instance_of(RepairCheck).to receive(:repair!) { false } + expect_any_instance_of(RepairCheck).to receive(:show_error) + subject.run_check(RepairCheck) + end + end + end + end + + context 'when check implements skip?' do + it 'executes #skip? method' do + expect_any_instance_of(SkipCheck).to receive(:skip?).and_call_original + subject.run_check(SkipCheck) + end + + it 'displays #skip_reason' do + expect { subject.run_check(SkipCheck) }.to output(/this is a skip reason/).to_stdout + end + + it 'does not execute #check when #skip? is true' do + expect_any_instance_of(SkipCheck).not_to receive(:check?) + subject.run_check(SkipCheck) + end + end + + context 'when implements a #multi_check' do + it 'executes #multi_check method' do + expect_any_instance_of(MultiCheck).to receive(:multi_check) + subject.run_check(MultiCheck) + end + + it 'does not execute #check method' do + expect_any_instance_of(MultiCheck).not_to receive(:check) + subject.run_check(MultiCheck) + end + + context 'when check implements #skip?' do + it 'executes #skip? method' do + expect_any_instance_of(SkipMultiCheck).to receive(:skip?).and_call_original + subject.run_check(SkipMultiCheck) + end + end + end + end +end diff --git a/spec/lib/system_check_spec.rb b/spec/lib/system_check_spec.rb index 399a492ea2e..d2087ad2d83 100644 --- a/spec/lib/system_check_spec.rb +++ b/spec/lib/system_check_spec.rb @@ -1,8 +1,13 @@ require 'spec_helper' +require 'rake_helper' describe SystemCheck, lib: true do subject { SystemCheck } + before do + silence_output + end + describe '.run' do it 'requires custom executor to be a BasicExecutor' do expect { subject.run('Component', [], SystemCheck::SimpleExecutor) }.not_to raise_error diff --git a/spec/support/rake_helpers.rb b/spec/support/rake_helpers.rb index 4a8158ed79b..5cb415111d2 100644 --- a/spec/support/rake_helpers.rb +++ b/spec/support/rake_helpers.rb @@ -7,4 +7,9 @@ module RakeHelpers def stub_warn_user_is_not_gitlab allow_any_instance_of(Object).to receive(:warn_user_is_not_gitlab) end + + def silence_output + allow($stdout).to receive(:puts) + allow($stdout).to receive(:print) + end end From bca5603740f77667dda6355c457ad1791b4fa42e Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 26 May 2017 02:34:25 +0200 Subject: [PATCH 10/14] Fix codestyle --- lib/system_check/app/git_version_check.rb | 2 +- lib/system_check/app/init_script_up_to_date_check.rb | 7 +++---- lib/system_check/app/projects_have_namespace_check.rb | 8 ++++---- lib/system_check/helpers.rb | 3 --- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/lib/system_check/app/git_version_check.rb b/lib/system_check/app/git_version_check.rb index 3a128f00911..c388682dfb4 100644 --- a/lib/system_check/app/git_version_check.rb +++ b/lib/system_check/app/git_version_check.rb @@ -17,7 +17,7 @@ module SystemCheck end def show_error - puts "Your git bin path is \"#{Gitlab.config.git.bin_path}\"" + $stdout.puts "Your git bin path is \"#{Gitlab.config.git.bin_path}\"" try_fixing_it( "Update your git to a version >= #{self.class.required_version} from #{self.class.current_version}" diff --git a/lib/system_check/app/init_script_up_to_date_check.rb b/lib/system_check/app/init_script_up_to_date_check.rb index 28c1451b8d2..015c7ed1731 100644 --- a/lib/system_check/app/init_script_up_to_date_check.rb +++ b/lib/system_check/app/init_script_up_to_date_check.rb @@ -14,7 +14,7 @@ module SystemCheck recipe_path = Rails.root.join('lib/support/init.d/', 'gitlab') unless File.exist?(SCRIPT_PATH) - puts "can't check because of previous errors".color(:magenta) + $stdout.puts "can't check because of previous errors".color(:magenta) return end @@ -22,14 +22,13 @@ module SystemCheck script_content = File.read(SCRIPT_PATH) if recipe_content == script_content - puts 'yes'.color(:green) + $stdout.puts 'yes'.color(:green) else - puts 'no'.color(:red) + $stdout.puts 'no'.color(:red) show_error end end - def show_error try_fixing_it( 'Re-download the init script' diff --git a/lib/system_check/app/projects_have_namespace_check.rb b/lib/system_check/app/projects_have_namespace_check.rb index d21ae225b0b..c70633a6d4f 100644 --- a/lib/system_check/app/projects_have_namespace_check.rb +++ b/lib/system_check/app/projects_have_namespace_check.rb @@ -9,15 +9,15 @@ module SystemCheck end def multi_check - puts '' + $stdout.puts '' Project.find_each(batch_size: 100) do |project| - print sanitized_message(project) + $stdout.print sanitized_message(project) if project.namespace - puts 'yes'.color(:green) + $stdout.puts 'yes'.color(:green) else - puts 'no'.color(:red) + $stdout.puts 'no'.color(:red) show_error end end diff --git a/lib/system_check/helpers.rb b/lib/system_check/helpers.rb index 91454c99838..0a6be42a7d1 100644 --- a/lib/system_check/helpers.rb +++ b/lib/system_check/helpers.rb @@ -75,6 +75,3 @@ module SystemCheck end end end - - - From 3f187751d40a687ab9b76857c04849bab0f84357 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 30 May 2017 19:06:58 +0200 Subject: [PATCH 11/14] Fixed and improved some existing checks and SystemCheck library --- lib/system_check.rb | 15 ++------- .../app/database_config_exists_check.rb | 8 +---- lib/system_check/app/git_config_check.rb | 32 +++++++------------ ...k.rb => gitlab_config_up_to_date_check.rb} | 8 ++--- .../app/projects_have_namespace_check.rb | 2 +- lib/system_check/app/ruby_version_check.rb | 2 +- lib/system_check/base_check.rb | 3 -- lib/system_check/helpers.rb | 13 ++++++-- lib/system_check/simple_executor.rb | 30 ++++++++--------- lib/tasks/gitlab/check.rake | 2 +- lib/tasks/gitlab/task_helpers.rb | 19 +++++------ spec/lib/system_check/base_executor_spec.rb | 4 ++- 12 files changed, 61 insertions(+), 77 deletions(-) rename lib/system_check/app/{gitlab_config_not_outdated_check.rb => gitlab_config_up_to_date_check.rb} (77%) diff --git a/lib/system_check.rb b/lib/system_check.rb index e9cbf6b8258..466c39904fa 100644 --- a/lib/system_check.rb +++ b/lib/system_check.rb @@ -9,22 +9,13 @@ module SystemCheck # # @param [String] component name of the component relative to the checks being executed # @param [Array] checks classes of corresponding checks to be executed in the same order - # @param [BaseExecutor] executor_klass optionally specifiy a different executor class - def self.run(component, checks = [], executor_klass = SimpleExecutor) - unless executor_klass < BaseExecutor - raise ArgumentError, 'Invalid executor' - end + def self.run(component, checks = []) + executor = SimpleExecutor.new(component) - prepare(component, checks, executor_klass).execute - end - - def self.prepare(component, checks = [], executor_klass = SimpleExecutor) - executor = executor_klass.new(component) checks.each do |check| executor << check end - executor + executor.execute end - private_class_method :prepare end diff --git a/lib/system_check/app/database_config_exists_check.rb b/lib/system_check/app/database_config_exists_check.rb index d557cee47b4..d1fae192350 100644 --- a/lib/system_check/app/database_config_exists_check.rb +++ b/lib/system_check/app/database_config_exists_check.rb @@ -15,17 +15,11 @@ module SystemCheck 'Check that the information in config/database.yml is correct' ) for_more_information( - see_database_guide, + 'doc/install/databases.md', 'http://guides.rubyonrails.org/getting_started.html#configuring-a-database' ) fix_and_rerun end - - private - - def see_database_guide - 'doc/install/databases.md' - end end end end diff --git a/lib/system_check/app/git_config_check.rb b/lib/system_check/app/git_config_check.rb index 7f0c792eb35..198867f7ac6 100644 --- a/lib/system_check/app/git_config_check.rb +++ b/lib/system_check/app/git_config_check.rb @@ -5,7 +5,7 @@ module SystemCheck 'core.autocrlf' => 'input' }.freeze - set_name 'Git configured with autocrlf=input?' + set_name 'Git configured correctly?' def check? correct_options = OPTIONS.map do |name, value| @@ -15,8 +15,18 @@ module SystemCheck correct_options.all? end + # Tries to configure git itself + # + # Returns true if all subcommands were successful (according to their exit code) + # Returns false if any or all subcommands failed. def repair! - auto_fix_git_config(OPTIONS) + return false unless is_gitlab_user? + + command_success = OPTIONS.map do |name, value| + system(*%W(#{Gitlab.config.git.bin_path} config --global #{name} #{value})) + end + + command_success.all? end def show_error @@ -27,24 +37,6 @@ module SystemCheck see_installation_guide_section 'GitLab' ) end - - private - - # Tries to configure git itself - # - # Returns true if all subcommands were successfull (according to their exit code) - # Returns false if any or all subcommands failed. - def auto_fix_git_config(options) - if !@warned_user_not_gitlab - command_success = options.map do |name, value| - system(*%W(#{Gitlab.config.git.bin_path} config --global #{name} #{value})) - end - - command_success.all? - else - false - end - end end end end diff --git a/lib/system_check/app/gitlab_config_not_outdated_check.rb b/lib/system_check/app/gitlab_config_up_to_date_check.rb similarity index 77% rename from lib/system_check/app/gitlab_config_not_outdated_check.rb rename to lib/system_check/app/gitlab_config_up_to_date_check.rb index 8a4d7b29977..c609e48e133 100644 --- a/lib/system_check/app/gitlab_config_not_outdated_check.rb +++ b/lib/system_check/app/gitlab_config_up_to_date_check.rb @@ -1,9 +1,7 @@ module SystemCheck module App - class GitlabConfigNotOutdatedCheck < SystemCheck::BaseCheck - set_name 'GitLab config outdated?' - set_check_pass 'no' - set_check_fail 'yes' + class GitlabConfigUpToDateCheck < SystemCheck::BaseCheck + set_name 'GitLab config up to date?' set_skip_reason "can't check because of previous errors" def skip? @@ -18,7 +16,7 @@ module SystemCheck def show_error try_fixing_it( - 'Backup your config/gitlab.yml', + 'Back-up your config/gitlab.yml', 'Copy config/gitlab.yml.example to config/gitlab.yml', 'Update config/gitlab.yml to match your setup' ) diff --git a/lib/system_check/app/projects_have_namespace_check.rb b/lib/system_check/app/projects_have_namespace_check.rb index c70633a6d4f..a6ec9f7665c 100644 --- a/lib/system_check/app/projects_have_namespace_check.rb +++ b/lib/system_check/app/projects_have_namespace_check.rb @@ -1,7 +1,7 @@ module SystemCheck module App class ProjectsHaveNamespaceCheck < SystemCheck::BaseCheck - set_name 'projects have namespace: ' + set_name 'Projects have namespace:' set_skip_reason "can't check, you have no projects" def skip? diff --git a/lib/system_check/app/ruby_version_check.rb b/lib/system_check/app/ruby_version_check.rb index 37b4d24aa55..fd82f5f8a4a 100644 --- a/lib/system_check/app/ruby_version_check.rb +++ b/lib/system_check/app/ruby_version_check.rb @@ -5,7 +5,7 @@ module SystemCheck set_check_pass -> { "yes (#{self.current_version})" } def self.required_version - @required_version ||= Gitlab::VersionInfo.new(2, 1, 0) + @required_version ||= Gitlab::VersionInfo.new(2, 3, 3) end def self.current_version diff --git a/lib/system_check/base_check.rb b/lib/system_check/base_check.rb index 63b7eea5add..5dcb3f0886b 100644 --- a/lib/system_check/base_check.rb +++ b/lib/system_check/base_check.rb @@ -1,10 +1,7 @@ -require 'tasks/gitlab/task_helpers' - module SystemCheck # Base class for Checks. You must inherit from here # and implement the methods below when necessary class BaseCheck - include ::Gitlab::TaskHelpers include ::SystemCheck::Helpers # Define a custom term for when check passed diff --git a/lib/system_check/helpers.rb b/lib/system_check/helpers.rb index 0a6be42a7d1..cd54baa494f 100644 --- a/lib/system_check/helpers.rb +++ b/lib/system_check/helpers.rb @@ -1,5 +1,9 @@ +require 'tasks/gitlab/task_helpers' + module SystemCheck module Helpers + include ::Gitlab::TaskHelpers + # Display a message telling to fix and rerun the checks def fix_and_rerun $stdout.puts ' Please fix the error above and rerun the checks.'.color(:red) @@ -9,8 +13,6 @@ module SystemCheck # # @param [Array] sources one or more references (documentation or links) def for_more_information(*sources) - sources = sources.shift if sources.first.is_a?(Array) - $stdout.puts ' For more information see:'.color(:blue) sources.each do |source| $stdout.puts " #{source}" @@ -73,5 +75,12 @@ module SystemCheck def sudo_gitlab(command) "sudo -u #{gitlab_user} -H #{command}" end + + def is_gitlab_user? + return @is_gitlab_user unless @is_gitlab_user.nil? + + current_user = run_command(%w(whoami)).chomp + @is_gitlab_user = current_user == gitlab_user + end end end diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb index ad2f549b3fb..c5403693f7a 100644 --- a/lib/system_check/simple_executor.rb +++ b/lib/system_check/simple_executor.rb @@ -18,32 +18,32 @@ module SystemCheck # Executes a single check # - # @param [SystemCheck::BaseCheck] check - def run_check(check) - $stdout.print "#{check.display_name} ... " + # @param [SystemCheck::BaseCheck] check_klass + def run_check(check_klass) + $stdout.print "#{check_klass.display_name} ... " - c = check.new + check = check_klass.new # When implements skip method, we run it first, and if true, skip the check - if c.can_skip? && c.skip? - $stdout.puts check.skip_reason.color(:magenta) + if check.can_skip? && check.skip? + $stdout.puts check_klass.skip_reason.color(:magenta) return end - + # When implements a multi check, we don't control the output - if c.is_multi_check? - c.multi_check + if check.is_multi_check? + check.multi_check return end - if c.check? - $stdout.puts check.check_pass.color(:green) + if check.check? + $stdout.puts check_klass.check_pass.color(:green) else - $stdout.puts check.check_fail.color(:red) + $stdout.puts check_klass.check_fail.color(:red) - if c.can_repair? + if check.can_repair? $stdout.print 'Trying to fix error automatically. ...' - if c.repair! + if check.repair! $stdout.puts 'Success'.color(:green) return else @@ -51,7 +51,7 @@ module SystemCheck end end - c.show_error + check.show_error end end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 973517c99dd..63c5e9b9c83 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -21,7 +21,7 @@ namespace :gitlab do SystemCheck::App::MigrationsAreUpCheck, SystemCheck::App::OrphanedGroupMembersCheck, SystemCheck::App::GitlabConfigExistsCheck, - SystemCheck::App::GitlabConfigNotOutdatedCheck, + SystemCheck::App::GitlabConfigUpToDateCheck, SystemCheck::App::LogWritableCheck, SystemCheck::App::TmpWritableCheck, SystemCheck::App::UploadsDirectoryExistsCheck, diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index e3c9d3b491c..e2307e41be1 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -99,16 +99,17 @@ module Gitlab end def warn_user_is_not_gitlab - unless @warned_user_not_gitlab - gitlab_user = Gitlab.config.gitlab.user + return if @warned_user_not_gitlab + + unless is_gitlab_user? current_user = run_command(%w(whoami)).chomp - unless current_user == gitlab_user - puts " Warning ".color(:black).background(:yellow) - puts " You are running as user #{current_user.color(:magenta)}, we hope you know what you are doing." - puts " Things may work\/fail for the wrong reasons." - puts " For correct results you should run this as user #{gitlab_user.color(:magenta)}." - puts "" - end + + puts " Warning ".color(:black).background(:yellow) + puts " You are running as user #{current_user.color(:magenta)}, we hope you know what you are doing." + puts " Things may work\/fail for the wrong reasons." + puts " For correct results you should run this as user #{gitlab_user.color(:magenta)}." + puts "" + @warned_user_not_gitlab = true end end diff --git a/spec/lib/system_check/base_executor_spec.rb b/spec/lib/system_check/base_executor_spec.rb index 139f7f3248f..4b379392da0 100644 --- a/spec/lib/system_check/base_executor_spec.rb +++ b/spec/lib/system_check/base_executor_spec.rb @@ -26,7 +26,7 @@ describe SystemCheck::BaseExecutor, lib: true do subject << SimpleCheck end - it 'returns an array of classes' do + it 'returns a set of classes' do expect(subject.checks).to include(SimpleCheck) end end @@ -39,12 +39,14 @@ describe SystemCheck::BaseExecutor, lib: true do it 'appends a new check to the Set' do subject << OtherCheck stored_checks = subject.checks.to_a + expect(stored_checks.first).to eq(SimpleCheck) expect(stored_checks.last).to eq(OtherCheck) end it 'inserts unique itens only' do subject << SimpleCheck + expect(subject.checks.size).to eq(1) end end From 23f9e95e8a7dfa3371f0c60a90a4cffc300c0f72 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 30 May 2017 19:07:06 +0200 Subject: [PATCH 12/14] Changelog --- changelogs/unreleased/28080-system-checks.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/28080-system-checks.yml diff --git a/changelogs/unreleased/28080-system-checks.yml b/changelogs/unreleased/28080-system-checks.yml new file mode 100644 index 00000000000..7d83014279a --- /dev/null +++ b/changelogs/unreleased/28080-system-checks.yml @@ -0,0 +1,4 @@ +--- +title: Refactored gitlab:app:check into SystemCheck liberary and improve some checks +merge_request: 9173 +author: From d219f9a691973708f70d9517765a2ec926d6d903 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 31 May 2017 11:32:11 +0200 Subject: [PATCH 13/14] Fix BasicExecutor specs --- lib/system_check/helpers.rb | 11 ----------- lib/tasks/gitlab/task_helpers.rb | 27 +++++++++++---------------- spec/lib/system_check_spec.rb | 4 ---- 3 files changed, 11 insertions(+), 31 deletions(-) diff --git a/lib/system_check/helpers.rb b/lib/system_check/helpers.rb index cd54baa494f..c42ae4fe4c4 100644 --- a/lib/system_check/helpers.rb +++ b/lib/system_check/helpers.rb @@ -68,19 +68,8 @@ module SystemCheck Dir.pwd == '/opt/gitlab/embedded/service/gitlab-rails' end - def gitlab_user - Gitlab.config.gitlab.user - end - def sudo_gitlab(command) "sudo -u #{gitlab_user} -H #{command}" end - - def is_gitlab_user? - return @is_gitlab_user unless @is_gitlab_user.nil? - - current_user = run_command(%w(whoami)).chomp - @is_gitlab_user = current_user == gitlab_user - end end end diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index e2307e41be1..964aa0fe1bc 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -98,6 +98,17 @@ module Gitlab end end + def gitlab_user + Gitlab.config.gitlab.user + end + + def is_gitlab_user? + return @is_gitlab_user unless @is_gitlab_user.nil? + + current_user = run_command(%w(whoami)).chomp + @is_gitlab_user = current_user == gitlab_user + end + def warn_user_is_not_gitlab return if @warned_user_not_gitlab @@ -114,22 +125,6 @@ module Gitlab end end - # Tries to configure git itself - # - # Returns true if all subcommands were successfull (according to their exit code) - # Returns false if any or all subcommands failed. - def auto_fix_git_config(options) - if !@warned_user_not_gitlab - command_success = options.map do |name, value| - system(*%W(#{Gitlab.config.git.bin_path} config --global #{name} #{value})) - end - - command_success.all? - else - false - end - end - 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| diff --git a/spec/lib/system_check_spec.rb b/spec/lib/system_check_spec.rb index d2087ad2d83..01679282f87 100644 --- a/spec/lib/system_check_spec.rb +++ b/spec/lib/system_check_spec.rb @@ -9,10 +9,6 @@ describe SystemCheck, lib: true do end describe '.run' do - it 'requires custom executor to be a BasicExecutor' do - expect { subject.run('Component', [], SystemCheck::SimpleExecutor) }.not_to raise_error - end - context 'custom matcher' do class SimpleCheck < SystemCheck::BaseCheck def check? From 0d4d9a6a63b2ac60ffeeeaef389295919c3119f4 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 31 May 2017 20:45:00 +0200 Subject: [PATCH 14/14] Refactor and move things around to improve in YAGNI perspective --- lib/system_check/base_executor.rb | 24 --------- lib/system_check/simple_executor.rb | 24 ++++++++- spec/lib/system_check/base_executor_spec.rb | 53 ------------------- spec/lib/system_check/simple_executor_spec.rb | 44 +++++++++++++++ spec/lib/system_check_spec.rb | 42 +++++++-------- spec/support/matchers/execute_check.rb | 6 ++- 6 files changed, 91 insertions(+), 102 deletions(-) delete mode 100644 lib/system_check/base_executor.rb delete mode 100644 spec/lib/system_check/base_executor_spec.rb diff --git a/lib/system_check/base_executor.rb b/lib/system_check/base_executor.rb deleted file mode 100644 index f76f44f3a28..00000000000 --- a/lib/system_check/base_executor.rb +++ /dev/null @@ -1,24 +0,0 @@ -module SystemCheck - # @attr_reader [Array] checks classes of corresponding checks to be executed in the same order - # @attr_reader [String] component name of the component relative to the checks being executed - class BaseExecutor - attr_reader :checks - attr_reader :component - - # @param [String] component name of the component relative to the checks being executed - def initialize(component) - raise ArgumentError unless component.is_a? String - - @component = component - @checks = Set.new - end - - # Add a check to be executed - # - # @param [BaseCheck] check class - def <<(check) - raise ArgumentError unless check < BaseCheck - @checks << check - end - end -end diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb index c5403693f7a..dc2d4643a01 100644 --- a/lib/system_check/simple_executor.rb +++ b/lib/system_check/simple_executor.rb @@ -4,7 +4,29 @@ module SystemCheck # # There is no concurrency level and the output is progressively # printed into the STDOUT - class SimpleExecutor < BaseExecutor + # + # @attr_reader [Array] checks classes of corresponding checks to be executed in the same order + # @attr_reader [String] component name of the component relative to the checks being executed + class SimpleExecutor + attr_reader :checks + attr_reader :component + + # @param [String] component name of the component relative to the checks being executed + def initialize(component) + raise ArgumentError unless component.is_a? String + + @component = component + @checks = Set.new + end + + # Add a check to be executed + # + # @param [BaseCheck] check class + def <<(check) + raise ArgumentError unless check < BaseCheck + @checks << check + end + # Executes defined checks in the specified order and outputs confirmation or error information def execute start_checking(component) diff --git a/spec/lib/system_check/base_executor_spec.rb b/spec/lib/system_check/base_executor_spec.rb deleted file mode 100644 index 4b379392da0..00000000000 --- a/spec/lib/system_check/base_executor_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'spec_helper' - -describe SystemCheck::BaseExecutor, lib: true do - class SimpleCheck < SystemCheck::BaseCheck - def check? - true - end - end - - class OtherCheck < SystemCheck::BaseCheck - def check? - false - end - end - - subject { described_class.new('Test') } - - describe '#component' do - it 'returns stored component name' do - expect(subject.component).to eq('Test') - end - end - - describe '#checks' do - before do - subject << SimpleCheck - end - - it 'returns a set of classes' do - expect(subject.checks).to include(SimpleCheck) - end - end - - describe '#<<' do - before do - subject << SimpleCheck - end - - it 'appends a new check to the Set' do - subject << OtherCheck - stored_checks = subject.checks.to_a - - expect(stored_checks.first).to eq(SimpleCheck) - expect(stored_checks.last).to eq(OtherCheck) - end - - it 'inserts unique itens only' do - subject << SimpleCheck - - expect(subject.checks.size).to eq(1) - end - end -end diff --git a/spec/lib/system_check/simple_executor_spec.rb b/spec/lib/system_check/simple_executor_spec.rb index f6ba437b71a..a5c6170cd7d 100644 --- a/spec/lib/system_check/simple_executor_spec.rb +++ b/spec/lib/system_check/simple_executor_spec.rb @@ -75,6 +75,42 @@ describe SystemCheck::SimpleExecutor, lib: true do end end + describe '#component' do + it 'returns stored component name' do + expect(subject.component).to eq('Test') + end + end + + describe '#checks' do + before do + subject << SimpleCheck + end + + it 'returns a set of classes' do + expect(subject.checks).to include(SimpleCheck) + end + end + + describe '#<<' do + before do + subject << SimpleCheck + end + + it 'appends a new check to the Set' do + subject << OtherCheck + stored_checks = subject.checks.to_a + + expect(stored_checks.first).to eq(SimpleCheck) + expect(stored_checks.last).to eq(OtherCheck) + end + + it 'inserts unique itens only' do + subject << SimpleCheck + + expect(subject.checks.size).to eq(1) + end + end + subject { described_class.new('Test') } describe '#execute' do @@ -120,6 +156,7 @@ describe SystemCheck::SimpleExecutor, lib: true do context 'when check implements #repair!' do it 'executes #repair!' do expect_any_instance_of(RepairCheck).to receive(:repair!) + subject.run_check(RepairCheck) end @@ -127,6 +164,7 @@ describe SystemCheck::SimpleExecutor, lib: true do it 'does not execute #show_error' do expect_any_instance_of(RepairCheck).to receive(:repair!).and_call_original expect_any_instance_of(RepairCheck).not_to receive(:show_error) + subject.run_check(RepairCheck) end end @@ -135,6 +173,7 @@ describe SystemCheck::SimpleExecutor, lib: true do it 'does not execute #show_error' do expect_any_instance_of(RepairCheck).to receive(:repair!) { false } expect_any_instance_of(RepairCheck).to receive(:show_error) + subject.run_check(RepairCheck) end end @@ -144,6 +183,7 @@ describe SystemCheck::SimpleExecutor, lib: true do context 'when check implements skip?' do it 'executes #skip? method' do expect_any_instance_of(SkipCheck).to receive(:skip?).and_call_original + subject.run_check(SkipCheck) end @@ -153,6 +193,7 @@ describe SystemCheck::SimpleExecutor, lib: true do it 'does not execute #check when #skip? is true' do expect_any_instance_of(SkipCheck).not_to receive(:check?) + subject.run_check(SkipCheck) end end @@ -160,17 +201,20 @@ describe SystemCheck::SimpleExecutor, lib: true do context 'when implements a #multi_check' do it 'executes #multi_check method' do expect_any_instance_of(MultiCheck).to receive(:multi_check) + subject.run_check(MultiCheck) end it 'does not execute #check method' do expect_any_instance_of(MultiCheck).not_to receive(:check) + subject.run_check(MultiCheck) end context 'when check implements #skip?' do it 'executes #skip? method' do expect_any_instance_of(SkipMultiCheck).to receive(:skip?).and_call_original + subject.run_check(SkipMultiCheck) end end diff --git a/spec/lib/system_check_spec.rb b/spec/lib/system_check_spec.rb index 01679282f87..23d9beddb08 100644 --- a/spec/lib/system_check_spec.rb +++ b/spec/lib/system_check_spec.rb @@ -2,39 +2,35 @@ require 'spec_helper' require 'rake_helper' describe SystemCheck, lib: true do - subject { SystemCheck } + class SimpleCheck < SystemCheck::BaseCheck + def check? + true + end + end + + class OtherCheck < SystemCheck::BaseCheck + def check? + false + end + end before do silence_output end describe '.run' do - context 'custom matcher' do - class SimpleCheck < SystemCheck::BaseCheck - def check? - true - end - end + subject { SystemCheck } - class OtherCheck < SystemCheck::BaseCheck - def check? - false - end - end + it 'detects execution of SimpleCheck' do + is_expected.to execute_check(SimpleCheck) - subject { SystemCheck } + subject.run('Test', [SimpleCheck]) + end - it 'detects execution of SimpleCheck' do - is_expected.to execute_check(SimpleCheck) + it 'detects exclusion of OtherCheck in execution' do + is_expected.not_to execute_check(OtherCheck) - SystemCheck.run('Test', [SimpleCheck]) - end - - it 'detects exclusion of OtherCheck in execution' do - is_expected.not_to execute_check(OtherCheck) - - SystemCheck.run('Test', [SimpleCheck]) - end + subject.run('Test', [SimpleCheck]) end end end diff --git a/spec/support/matchers/execute_check.rb b/spec/support/matchers/execute_check.rb index 9664eb3879d..7232fad52fb 100644 --- a/spec/support/matchers/execute_check.rb +++ b/spec/support/matchers/execute_check.rb @@ -14,6 +14,10 @@ RSpec::Matchers.define :execute_check do |expected| end failure_message do |actual| - return 'This matcher must be used with SystemCheck' unless actual == SystemCheck + 'This matcher must be used with SystemCheck' unless actual == SystemCheck + end + + failure_message_when_negated do |actual| + 'This matcher must be used with SystemCheck' unless actual == SystemCheck end end