diff --git a/definitions/checks/foreman/check_tuning_requirements.rb b/definitions/checks/foreman/check_tuning_requirements.rb index 00c639c5e..5ea777d64 100644 --- a/definitions/checks/foreman/check_tuning_requirements.rb +++ b/definitions/checks/foreman/check_tuning_requirements.rb @@ -8,7 +8,6 @@ class TuningRequirements < ForemanMaintain::Check confine do feature(:katello) end - do_not_whitelist end def run diff --git a/definitions/checks/repositories/check_upstream_repository.rb b/definitions/checks/repositories/check_upstream_repository.rb index 59ad3f5b1..edea3f515 100644 --- a/definitions/checks/repositories/check_upstream_repository.rb +++ b/definitions/checks/repositories/check_upstream_repository.rb @@ -5,6 +5,7 @@ class Checks::CheckUpstreamRepository < ForemanMaintain::Check label :check_upstream_repository description 'Check if any upstream repositories are enabled on system' tags :pre_upgrade + skippable preparation_steps do [Checks::Repositories::CheckNonRhRepository.new, Procedures::Packages::Install.new(:packages => %w[yum-utils])] diff --git a/definitions/checks/repositories/validate.rb b/definitions/checks/repositories/validate.rb index 5c195053f..9a7145aa7 100644 --- a/definitions/checks/repositories/validate.rb +++ b/definitions/checks/repositories/validate.rb @@ -2,6 +2,8 @@ module Checks::Repositories class Validate < ForemanMaintain::Check metadata do description 'Validate availability of repositories' + skippable + preparation_steps do Checks::Repositories::CheckNonRhRepository.new end diff --git a/definitions/procedures/content/prepare.rb b/definitions/procedures/content/prepare.rb index 10bb07207..fb6312bfa 100644 --- a/definitions/procedures/content/prepare.rb +++ b/definitions/procedures/content/prepare.rb @@ -4,7 +4,6 @@ class Prepare < ForemanMaintain::Procedure description 'Prepare content for Pulp 3' for_feature :pulpcore param :quiet, 'Keep the output on a single line', :flag => true, :default => false - do_not_whitelist end def run diff --git a/definitions/procedures/content/switchover.rb b/definitions/procedures/content/switchover.rb index 47b3cf5d9..5b42e43c5 100644 --- a/definitions/procedures/content/switchover.rb +++ b/definitions/procedures/content/switchover.rb @@ -9,7 +9,6 @@ class Switchover < ForemanMaintain::Procedure end param :skip_deb, 'Do not run debian options in installer.' - do_not_whitelist end def run diff --git a/definitions/procedures/repositories/setup.rb b/definitions/procedures/repositories/setup.rb index 890a120d7..4360a4b3f 100644 --- a/definitions/procedures/repositories/setup.rb +++ b/definitions/procedures/repositories/setup.rb @@ -2,6 +2,7 @@ module Procedures::Repositories class Setup < ForemanMaintain::Procedure metadata do description 'Setup repositories' + skippable confine do feature(:instance).downstream || feature(:upstream) end diff --git a/lib/foreman_maintain/concerns/metadata.rb b/lib/foreman_maintain/concerns/metadata.rb index bd3a78846..bf5d2a926 100644 --- a/lib/foreman_maintain/concerns/metadata.rb +++ b/lib/foreman_maintain/concerns/metadata.rb @@ -100,8 +100,8 @@ def advanced_run(advanced_run) @data[:advanced_run] = advanced_run end - def do_not_whitelist - @data[:do_not_whitelist] = true + def skippable + @data[:skippable] = true end def self.eval_dsl(metadata, &block) diff --git a/lib/foreman_maintain/reporter/cli_reporter.rb b/lib/foreman_maintain/reporter/cli_reporter.rb index 71e95563f..6ca35c416 100644 --- a/lib/foreman_maintain/reporter/cli_reporter.rb +++ b/lib/foreman_maintain/reporter/cli_reporter.rb @@ -315,19 +315,17 @@ def scenario_failure_message(scenario) end steps_with_error = scenario.steps_with_error(:whitelisted => false) - steps_with_skipped = scenario.steps_with_skipped(:whitelisted => true) - not_skippable_steps = scenario.steps_with_error.select do |step| - step.metadata[:do_not_whitelist] == true + skippable_steps = scenario.steps_with_error.select do |step| + step.metadata[:skippable] == true end - steps_to_whitelist = steps_with_error + steps_with_skipped - not_skippable_steps unless steps_with_error.empty? message << format(<<-MESSAGE.strip_heredoc, format_steps(steps_with_error, "\n", 2)) The following steps ended up in failing state: %s MESSAGE - whitelist_labels = steps_to_whitelist.map(&:label_dashed).join(',') + whitelist_labels = skippable_steps.map(&:label_dashed).join(',') unless whitelist_labels.empty? recommend << if scenario.detector.feature(:instance).downstream format(<<-MESSAGE.strip_heredoc, whitelist_labels) diff --git a/lib/foreman_maintain/runner.rb b/lib/foreman_maintain/runner.rb index ab73d236a..019719766 100644 --- a/lib/foreman_maintain/runner.rb +++ b/lib/foreman_maintain/runner.rb @@ -52,6 +52,8 @@ def run_scenario(scenario) return if scenario.steps.empty? raise 'The runner is already in quit state' if quit? && !rescue? + validate_steps(scenario.steps) + confirm_scenario(scenario) return if quit? && !rescue? @@ -177,5 +179,13 @@ def ask_about_offered_steps(step, scenario) def rerun_check?(step) @last_decision_step == step end + + def validate_steps(steps) + steps.each do |step| + if whitelisted_step?(step) && !step.metadata[:skippable] + raise "#{step} is not skippable. Please remove from whitelist." + end + end + end end end diff --git a/test/definitions/checks/foreman/check_tuning_profile_test.rb b/test/definitions/checks/foreman/check_tuning_profile_test.rb index 9db915f04..4134f4185 100644 --- a/test/definitions/checks/foreman/check_tuning_profile_test.rb +++ b/test/definitions/checks/foreman/check_tuning_profile_test.rb @@ -42,8 +42,4 @@ assert_includes result.output, 'The number of CPU cores for the system is 6 but the currently configured tuning profile requires 8.' # rubocop:disable Metrics/LineLength assert result.fail? end - - it 'does not allow being whitelisted' do - assert subject.metadata[:do_not_whitelist] - end end diff --git a/test/lib/cli/upgrade_command_test.rb b/test/lib/cli/upgrade_command_test.rb index 561342764..8c5ead434 100644 --- a/test/lib/cli/upgrade_command_test.rb +++ b/test/lib/cli/upgrade_command_test.rb @@ -230,6 +230,7 @@ def foreman_maintain_update_unavailable end it 'with --phase it runs only a specific phase of the upgrade' do + foreman_maintain_update_unavailable UpgradeRunner.any_instance.expects(:run_phase).with(:pre_migrations) assert_cmd(<<-OUTPUT.strip_heredoc, ['--phase=pre_migrations', '--target-version=1.15']) Checking for new version of #{ForemanMaintain.main_package_name}... diff --git a/test/lib/reporter_test.rb b/test/lib/reporter_test.rb index 1a965aaaf..87b374506 100644 --- a/test/lib/reporter_test.rb +++ b/test/lib/reporter_test.rb @@ -132,7 +132,7 @@ def decision_question(description) contact Red Hat Technical Support. In case the failures are false positives, use - --whitelist="dummy-check-fail,dummy-check-fail2" + --whitelist="dummy-check-fail" MESSAGE end @@ -160,7 +160,7 @@ def decision_question(description) Resolve the failed steps and rerun the command. In case the failures are false positives, use - --whitelist="dummy-check-fail,dummy-check-fail2" + --whitelist="dummy-check-fail" MESSAGE end diff --git a/test/lib/support/definitions/checks/dummy.rb b/test/lib/support/definitions/checks/dummy.rb index 83e2d8968..4fdddeea7 100644 --- a/test/lib/support/definitions/checks/dummy.rb +++ b/test/lib/support/definitions/checks/dummy.rb @@ -13,6 +13,7 @@ class Fail < ForemanMaintain::Check metadata do label :dummy_check_fail description 'Check that ends up with fail' + skippable end def run @@ -35,7 +36,6 @@ class FailSkipWhitelist < ForemanMaintain::Check metadata do label :dummy_check_fail_skipwhitelist description 'Check that ends up with fail' - do_not_whitelist end def run diff --git a/test/lib/support/definitions/checks/service_is_stopped.rb b/test/lib/support/definitions/checks/service_is_stopped.rb index 79ca45f55..bc3101355 100644 --- a/test/lib/support/definitions/checks/service_is_stopped.rb +++ b/test/lib/support/definitions/checks/service_is_stopped.rb @@ -9,7 +9,7 @@ class Checks::ServiceIsStopped < ForemanMaintain::Check end def run - assert(false, 'service is running', + assert(TestHelper.service_is_stopped, 'service is running', :next_steps => Procedures::StopService.new) end end diff --git a/test/lib/support/definitions/features/present_service.rb b/test/lib/support/definitions/features/present_service.rb index 340dcf812..2a1a96086 100644 --- a/test/lib/support/definitions/features/present_service.rb +++ b/test/lib/support/definitions/features/present_service.rb @@ -20,6 +20,6 @@ def restart end def running? - false + TestHelper.present_service_is_running end end diff --git a/test/lib/test_helper.rb b/test/lib/test_helper.rb index 67e2d47f7..bb1000afe 100644 --- a/test/lib/test_helper.rb +++ b/test/lib/test_helper.rb @@ -2,11 +2,13 @@ class TestHelper class << self - attr_accessor :use_present_service_2, :present_service_is_running, :migrations_fail_at + attr_accessor :use_present_service_2, :present_service_is_running, :migrations_fail_at, + :service_is_stopped def reset self.use_present_service_2 = false self.present_service_is_running = false + self.service_is_stopped = false self.migrations_fail_at = nil end end diff --git a/test/lib/upgrade_runner_test.rb b/test/lib/upgrade_runner_test.rb index 90c7e8929..b79acaaf3 100644 --- a/test/lib/upgrade_runner_test.rb +++ b/test/lib/upgrade_runner_test.rb @@ -15,11 +15,6 @@ module ForemanMaintain UpgradeRunner.new('1.15', reporter) end - let(:upgrade_runner_with_whitelist) do - UpgradeRunner.new('1.15', reporter, - :whitelist => %w[present-service-is-running service-is-stopped]) - end - it 'lists versions available for upgrading, based on available scenarios' do _(UpgradeRunner.available_targets).must_equal ['1.15'] end @@ -36,55 +31,43 @@ module ForemanMaintain end it 'asks for confirmation before getting into pre_migrations from pre upgrade checks' do - upgrade_runner_with_whitelist.run + TestHelper.present_service_is_running = true + TestHelper.service_is_stopped = true + upgrade_runner.run + _(reporter.log.last).must_equal ['ask', <<-MESSAGE.strip_heredoc.strip] The pre-upgrade checks indicate that the system is ready for upgrade. It's recommended to perform a backup at this stage. Confirm to continue with the modification part of the upgrade, [y(yes), n(no), q(quit)] MESSAGE - assert_equal(:pre_upgrade_checks, upgrade_runner_with_whitelist.phase, + assert_equal(:pre_upgrade_checks, upgrade_runner.phase, 'The phase should not be switched until confirmed') end - it 'remembers the state of the previous run of the upgrade' do - TestHelper.migrations_fail_at = :migrations - reporter.input << 'y' - upgrade_runner_with_whitelist.storage.data.clear - upgrade_runner_with_whitelist.run - upgrade_runner_with_whitelist.save - original_scenario = upgrade_runner_with_whitelist.scenario(:pre_upgrade_checks) - - ForemanMaintain.detector.refresh - new_upgrade_runner = UpgradeRunner.new('1.15', reporter) - new_upgrade_runner.load - _(new_upgrade_runner.phase).must_equal :migrations - restored_scenario = new_upgrade_runner.scenario(:pre_upgrade_checks) - _(restored_scenario.steps.map { |s| s.execution.status }). - must_equal(original_scenario.steps.map { |step| step.execution.status }) - end - it 'remembers the current target version' do TestHelper.migrations_fail_at = :migrations + TestHelper.present_service_is_running = true + TestHelper.service_is_stopped = true reporter.input << 'y' - upgrade_runner_with_whitelist.storage.data.clear - upgrade_runner_with_whitelist.run - upgrade_runner_with_whitelist.save + upgrade_runner.storage.data.clear + upgrade_runner.run + upgrade_runner.save _(UpgradeRunner.current_target_version).must_equal '1.15' _(UpgradeRunner.available_targets).must_equal(['1.15']) end it 'does not remember the current target version when failed on pre_upgrade_checks ===' do TestHelper.migrations_fail_at = :pre_upgrade_checks - upgrade_runner_with_whitelist.run - upgrade_runner_with_whitelist.save + upgrade_runner.run + upgrade_runner.save _(UpgradeRunner.current_target_version).must_be_nil end it 'cleans the state when the upgrade finished successfully' do reporter.input << 'y' - upgrade_runner_with_whitelist.storage.data.clear - upgrade_runner_with_whitelist.run - upgrade_runner_with_whitelist.save + upgrade_runner.storage.data.clear + upgrade_runner.run + upgrade_runner.save new_upgrade_runner = UpgradeRunner.new('1.15', reporter) new_upgrade_runner.load @@ -100,48 +83,47 @@ module ForemanMaintain end it 'runs migrations if pre_migrations succeed' do + TestHelper.present_service_is_running = true + TestHelper.service_is_stopped = true reporter.input << 'y' - upgrade_runner_with_whitelist.run + upgrade_runner.run _(reporter.log).must_include ['before_execution_starts', :upgrade_migration] end - it 'runs post_migrations and post_upgrade checks if pre_migrations fail' do - reporter.input << 'y' - TestHelper.migrations_fail_at = :pre_migrations - upgrade_runner_with_whitelist.run - _(upgrade_runner_with_whitelist.phase).must_equal :pre_upgrade_checks - _(reporter.log).wont_include ['before_execution_starts', :upgrade_migration] - _(reporter.log).must_include ['before_execution_starts', :upgrade_post_migration] - _(reporter.log).must_include ['before_execution_starts', :upgrade_post_upgrade_check] - _(upgrade_runner_with_whitelist.exit_code).must_equal 1 - end - it 'runs post_migrations if migrations succeed' do reporter.input << 'y' - upgrade_runner_with_whitelist.run + TestHelper.present_service_is_running = true + TestHelper.service_is_stopped = true + upgrade_runner.run _(reporter.log).must_include ['before_execution_starts', :upgrade_post_migration] end it 'fails if migrations fail' do reporter.input << 'y' TestHelper.migrations_fail_at = :migrations - upgrade_runner_with_whitelist.run - _(upgrade_runner_with_whitelist.phase).must_equal :migrations - _(upgrade_runner_with_whitelist.exit_code).must_equal 1 + TestHelper.present_service_is_running = true + TestHelper.service_is_stopped = true + upgrade_runner.run + _(upgrade_runner.phase).must_equal :migrations + _(upgrade_runner.exit_code).must_equal 1 end it 'runs post_upgrade_checks if post_migrations succeed' do reporter.input << 'y' - upgrade_runner_with_whitelist.run + TestHelper.present_service_is_running = true + TestHelper.service_is_stopped = true + upgrade_runner.run _(reporter.log).must_include ['before_execution_starts', :upgrade_post_upgrade_check] end it 'fails if post_migrations fail' do reporter.input << 'y' TestHelper.migrations_fail_at = :post_migrations - upgrade_runner_with_whitelist.run - _(upgrade_runner_with_whitelist.phase).must_equal :post_migrations - _(upgrade_runner_with_whitelist.exit_code).must_equal 1 + TestHelper.present_service_is_running = true + TestHelper.service_is_stopped = true + upgrade_runner.run + _(upgrade_runner.phase).must_equal :post_migrations + _(upgrade_runner.exit_code).must_equal 1 end end end