From 2897a6ca58710b77f43821172a0d7c843f8beabf Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Mon, 17 Jun 2024 11:16:24 +0200 Subject: [PATCH] stop workers during online backup this ensures more consistent on-disk data --- definitions/features/dynflow_sidekiq.rb | 4 +++ definitions/scenarios/backup.rb | 36 +++++++++++++--------- test/definitions/scenarios/backup_test.rb | 37 +++++++++++++++++++++-- 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/definitions/features/dynflow_sidekiq.rb b/definitions/features/dynflow_sidekiq.rb index ba4e3e302..68290640d 100644 --- a/definitions/features/dynflow_sidekiq.rb +++ b/definitions/features/dynflow_sidekiq.rb @@ -20,6 +20,10 @@ def services end end + def workers + services.reject { |service| service.name.end_with?('@orchestrator') } + end + private def instance_priority(instance) diff --git a/definitions/scenarios/backup.rb b/definitions/scenarios/backup.rb index bd1377146..e1757a210 100644 --- a/definitions/scenarios/backup.rb +++ b/definitions/scenarios/backup.rb @@ -1,5 +1,20 @@ module ForemanMaintain::Scenarios - class Backup < ForemanMaintain::Scenario + class BackupBase < ForemanMaintain::Scenario + private + + def strategy + context.get(:strategy) + end + + def online_worker_services + services = [] + services += feature(:dynflow_sidekiq).workers if feature(:dynflow_sidekiq) + services += feature(:pulpcore).configured_workers if feature(:pulpcore) + services + end + end + + class Backup < BackupBase metadata do description 'Backup' manual_detection @@ -114,6 +129,9 @@ def include_dumps end def add_online_backup_steps + services = online_worker_services + add_step(Procedures::Service::Stop.new(:only => services)) unless services.empty? + add_step_with_context(Procedures::Backup::ConfigFiles, :ignore_changed_files => true, :online_backup => true) add_step_with_context(Procedures::Backup::Pulp, :ensure_unchanged => true) @@ -122,10 +140,8 @@ def add_online_backup_steps Procedures::Backup::Online::ForemanDB, Procedures::Backup::Online::PulpcoreDB ) - end - def strategy - context.get(:strategy) + add_step(Procedures::Service::Start.new(:only => services)) unless services.empty? end def include_db_dumps? @@ -137,7 +153,7 @@ def wait_for_tasks? end end - class BackupRescueCleanup < ForemanMaintain::Scenario + class BackupRescueCleanup < BackupBase metadata do description 'Failed backup cleanup' manual_detection @@ -148,9 +164,7 @@ class BackupRescueCleanup < ForemanMaintain::Scenario end def compose - if strategy == :offline - add_step_with_context(Procedures::Service::Start) - end + add_step_with_context(Procedures::Service::Start) add_step_with_context(Procedures::Backup::Clean) end @@ -160,11 +174,5 @@ def set_context_mapping context.map(:preserve_dir, Procedures::Backup::Clean => :preserve_dir) end - - private - - def strategy - context.get(:strategy) - end end end diff --git a/test/definitions/scenarios/backup_test.rb b/test/definitions/scenarios/backup_test.rb index 67052c93d..0a7af7701 100644 --- a/test/definitions/scenarios/backup_test.rb +++ b/test/definitions/scenarios/backup_test.rb @@ -10,8 +10,18 @@ module Scenarios db.any_instance.stubs(:local?).returns(true) end end + + assume_feature_present(:pulpcore) do |feature| + feature.any_instance.stubs(:configured_workers).returns([existing_pulpcore_worker]) + end + + assume_feature_present(:dynflow_sidekiq) do |feature| + feature.any_instance.stubs(:workers).returns([existing_dynflow_worker]) + end end + let(:existing_pulpcore_worker) { existing_system_service('pulpcore-worker@1', 10) } + let(:existing_dynflow_worker) { existing_system_service('dynflow-sidekiq@worker-1', 10) } let(:task_checks) do [Checks::ForemanTasks::NotRunning, @@ -87,13 +97,21 @@ module Scenarios refute_scenario_has_step(scenario, Procedures::Backup::AccessibilityConfirmation) assert_scenario_has_step(scenario, Procedures::Backup::PrepareDirectory) assert_scenario_has_step(scenario, Procedures::Backup::Metadata) - refute_scenario_has_step(scenario, Procedures::Service::Stop) + assert_scenario_has_step(scenario, Procedures::Service::Stop) do |step| + assert_includes(step.common_options[:only], existing_pulpcore_worker) + assert_includes(step.common_options[:only], existing_dynflow_worker) + assert_equal(step.common_options[:only].length, 2) + end assert_scenario_has_step(scenario, Procedures::Backup::ConfigFiles) assert_scenario_has_step(scenario, Procedures::Backup::Pulp) assert_scenario_has_step(scenario, Procedures::Backup::Online::CandlepinDB) assert_scenario_has_step(scenario, Procedures::Backup::Online::ForemanDB) assert_scenario_has_step(scenario, Procedures::Backup::Online::PulpcoreDB) - refute_scenario_has_step(scenario, Procedures::Service::Start) + assert_scenario_has_step(scenario, Procedures::Service::Start) do |step| + assert_includes(step.common_options[:only], existing_pulpcore_worker) + assert_includes(step.common_options[:only], existing_dynflow_worker) + assert_equal(step.common_options[:only].length, 2) + end assert_scenario_has_step(scenario, Procedures::Backup::CompressData) end end @@ -117,6 +135,19 @@ module Scenarios describe ForemanMaintain::Scenarios::BackupRescueCleanup do include DefinitionsTestHelper + before do + assume_feature_present(:pulpcore) do |feature| + feature.any_instance.stubs(:configured_workers).returns([existing_pulpcore_worker]) + end + + assume_feature_present(:dynflow_sidekiq) do |feature| + feature.any_instance.stubs(:workers).returns([existing_dynflow_worker]) + end + end + + let(:existing_pulpcore_worker) { existing_system_service('pulpcore-worker@1', 10) } + let(:existing_dynflow_worker) { existing_system_service('dynflow-sidekiq@worker-1', 10) } + describe 'offline' do let(:scenario) do ForemanMaintain::Scenarios::BackupRescueCleanup.new(:backup_dir => '.', @@ -136,7 +167,7 @@ module Scenarios end it 'composes all steps' do - refute_scenario_has_step(scenario, Procedures::Service::Start) + assert_scenario_has_step(scenario, Procedures::Service::Start) assert_scenario_has_step(scenario, Procedures::Backup::Clean) end end