diff --git a/definitions/checks/restore/validate_postgresql_dump_permissions.rb b/definitions/checks/restore/validate_postgresql_dump_permissions.rb index 486c2ec93..5ade52612 100644 --- a/definitions/checks/restore/validate_postgresql_dump_permissions.rb +++ b/definitions/checks/restore/validate_postgresql_dump_permissions.rb @@ -13,7 +13,7 @@ def run backup = ForemanMaintain::Utils::Backup.new(@backup_dir) if feature(:instance).postgresql_local? errors = [] - [:candlepin_dump, :foreman_dump, :pulpcore_dump].each do |dump| + [:candlepin_dump, :foreman_dump, :pulpcore_dump, :container_gateway_dump].each do |dump| next unless backup.file_map[dump][:present] unless system("runuser - postgres -c 'test -r #{backup.file_map[dump][:path]}'") diff --git a/definitions/procedures/backup/online/container_gateway_db.rb b/definitions/procedures/backup/online/container_gateway_db.rb index 8721d0ed2..d2c8a3ab2 100644 --- a/definitions/procedures/backup/online/container_gateway_db.rb +++ b/definitions/procedures/backup/online/container_gateway_db.rb @@ -12,7 +12,8 @@ class ContainerGatewayDB < ForemanMaintain::Procedure def run with_spinner('Getting Container Gateway DB dump') do - feature(:container_gateway_database).dump_db(File.join(@backup_dir, 'container_gateway.dump')) + feature(:container_gateway_database). + dump_db(File.join(@backup_dir, 'container_gateway.dump')) end end end diff --git a/definitions/procedures/restore/container_gateway_dump.rb b/definitions/procedures/restore/container_gateway_dump.rb index 38bea5145..63fc5700a 100644 --- a/definitions/procedures/restore/container_gateway_dump.rb +++ b/definitions/procedures/restore/container_gateway_dump.rb @@ -23,7 +23,8 @@ def restore_container_gateway_dump(backup, spinner) if backup.file_map[:container_gateway_dump][:present] spinner.update('Restoring container gateway dump') local = feature(:container_gateway_database).local? - feature(:container_gateway_database).restore_dump(backup.file_map[:container_gateway_dump][:path], local) + feature(:container_gateway_database). + restore_dump(backup.file_map[:container_gateway_dump][:path], local) end end end diff --git a/definitions/procedures/restore/extract_files.rb b/definitions/procedures/restore/extract_files.rb index 4698a1c01..ce736b708 100644 --- a/definitions/procedures/restore/extract_files.rb +++ b/definitions/procedures/restore/extract_files.rb @@ -49,7 +49,8 @@ def extract_pulp_data(backup) end def any_database - feature(:foreman_database) || feature(:candlepin_database) || feature(:pulpcore_database) || feature(:container_gateway_database) + feature(:foreman_database) || feature(:candlepin_database) || + feature(:pulpcore_database) || feature(:container_gateway_database) end def extract_pgsql_data(backup) diff --git a/definitions/scenarios/restore.rb b/definitions/scenarios/restore.rb index bb0acb077..842976259 100644 --- a/definitions/scenarios/restore.rb +++ b/definitions/scenarios/restore.rb @@ -50,7 +50,7 @@ def compose add_step_with_context(Procedures::Installer::UpgradeRakeTask) add_step_with_context(Procedures::Crond::Start) if feature(:cron) end - # rubocop:enable Metrics/MethodLength,Metrics/AbcSize + # rubocop:enable Metrics/MethodLength def restore_sql_dumps(backup) if feature(:instance).postgresql_local? @@ -72,6 +72,7 @@ def restore_sql_dumps(backup) add_step(Procedures::Service::Stop.new(:only => ['postgresql'])) end end + # rubocop:enable Metrics/AbcSize def set_context_mapping context.map(:backup_dir, diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb index f6aff6979..5e08a0180 100644 --- a/lib/foreman_maintain/concerns/base_database.rb +++ b/lib/foreman_maintain/concerns/base_database.rb @@ -66,14 +66,17 @@ def psql(query, config = configuration) end def ping(config = configuration) - execute?(psql_command(config), - :stdin => 'SELECT 1 as ping', + execute?(psql_command(config) + ' -c "SELECT 1 as ping"', :hidden_patterns => [config['password']], :user => config['user']) end def dump_db(file, config = configuration) - execute!(dump_command(config) + " > #{file}", :hidden_patterns => [config['password']], :user => config['user']) + if config['user'] + execute!("chown -R #{config['user']}:#{config['user']} #{File.dirname(file)}") + end + execute!(dump_command(config) + " -f #{file}", + :hidden_patterns => [config['password']], :user => config['user']) end def restore_dump(file, localdb, config = configuration) @@ -83,13 +86,16 @@ def restore_dump(file, localdb, config = configuration) else # TODO: figure out how to completely ignore errors. Currently this # sometimes exits with 1 even though errors are ignored by pg_restore - dump_cmd = '' - if config['connection_string'] - dump_cmd = "pg_restore --no-privileges --clean --disable-triggers -n public -d #{config['database']} #{file}" - else - dump_cmd = base_command(config, 'pg_restore') + - ' --no-privileges --clean --disable-triggers -n public ' \ - "-d #{config['database']} #{file}" + base_cmd = if config['connection_string'] + 'pg_restore' + else + base_command(config, 'pg_restore') + end + dump_cmd = base_cmd + + ' --no-privileges --clean --disable-triggers -n public ' \ + "-d #{config['database']} #{file}" + if config['user'] + execute!("chown -R #{config['user']}:#{config['user']} #{File.dirname(file)}") end execute!(dump_cmd, :hidden_patterns => [config['password']], :valid_exit_statuses => [0, 1], :user => config['user']) @@ -169,9 +175,9 @@ def psql_command(config) def dump_command(config) if config['connection_string'] - "pg_dump -Fc #{config['connection_string']}" + "pg_dump --format=c #{config['connection_string']}" else - base_command(config, 'pg_dump') + " -Fc #{config['database']}" + base_command(config, 'pg_dump') + " --format=c #{config['database']}" end end diff --git a/lib/foreman_maintain/utils/backup.rb b/lib/foreman_maintain/utils/backup.rb index fa150b6ad..029c668a9 100644 --- a/lib/foreman_maintain/utils/backup.rb +++ b/lib/foreman_maintain/utils/backup.rb @@ -235,7 +235,7 @@ def sql_tar_files_exist? def sql_dump_files_exist? file_map[:foreman_dump][:present] || file_map[:candlepin_dump][:present] || - file_map[:pulpcore_dump][:present] + file_map[:pulpcore_dump][:present] || file_map[:container_gateway_dump][:present] end diff --git a/lib/foreman_maintain/utils/command_runner.rb b/lib/foreman_maintain/utils/command_runner.rb index ca5e0f454..39af18493 100644 --- a/lib/foreman_maintain/utils/command_runner.rb +++ b/lib/foreman_maintain/utils/command_runner.rb @@ -8,15 +8,16 @@ class CommandRunner attr_reader :logger, :command def initialize(logger, command, options) - options.validate_options!(:stdin, :hidden_patterns, :interactive, :valid_exit_statuses, :user) + options.validate_options!(:stdin, :hidden_patterns, + :interactive, :valid_exit_statuses, :user) options[:valid_exit_statuses] ||= [0] @logger = logger @user = options[:user] - if @user && !@user.empty? - @command = "sudo -u #{@user} -- " + command - else - @command = command - end + @command = if @user && !@user.empty? + "runuser -u #{@user} -- " + command + else + command + end @stdin = options[:stdin] @hidden_patterns = Array(options[:hidden_patterns]).compact @interactive = options[:interactive] diff --git a/test/definitions/checks/restore/validate_postgresql_dump_permissions_test.rb b/test/definitions/checks/restore/validate_postgresql_dump_permissions_test.rb index ad0f70609..1edfbb13b 100644 --- a/test/definitions/checks/restore/validate_postgresql_dump_permissions_test.rb +++ b/test/definitions/checks/restore/validate_postgresql_dump_permissions_test.rb @@ -12,6 +12,9 @@ :foreman_dump => { :present => true, :path => '/nonexistant/foreman.dump' }, :candlepin_dump => { :present => true, :path => '/nonexistant/candlepin.dump' }, :pulpcore_dump => { :present => true, :path => '/nonexistant/pulpcore.dump' }, + :container_gateway_dump => { + :present => true, :path => '/nonexistant/container_gateway.dump' + }, } ForemanMaintain::Utils::Backup.any_instance.stubs(:file_map).returns(file_map) end @@ -53,7 +56,8 @@ result = run_check(subject) refute result.success?, 'Check expected to fail' expected = "The 'postgres' user needs read access to the following files:\n" \ - "/nonexistant/candlepin.dump\n/nonexistant/foreman.dump\n/nonexistant/pulpcore.dump" + "/nonexistant/candlepin.dump\n/nonexistant/foreman.dump\n/nonexistant/" \ + "pulpcore.dump\n/nonexistant/container_gateway.dump" assert_equal result.output, expected end @@ -66,6 +70,9 @@ returns(true) subject.stubs(:system).with("runuser - postgres -c 'test -r /nonexistant/foreman.dump'"). returns(true) + subject.stubs(:system). + with("runuser - postgres -c 'test -r /nonexistant/container_gateway.dump'"). + returns(true) result = run_check(subject) refute result.success?, 'Check expected to fail' expected = "The 'postgres' user needs read access to the following files:\n" \ diff --git a/test/definitions/scenarios/restore_test.rb b/test/definitions/scenarios/restore_test.rb index 10f74a80f..912b53371 100644 --- a/test/definitions/scenarios/restore_test.rb +++ b/test/definitions/scenarios/restore_test.rb @@ -56,12 +56,14 @@ module Scenarios assume_feature_present(:foreman_database, :configuration => {}) assume_feature_present(:candlepin_database, :configuration => {}) assume_feature_present(:pulpcore_database, :configuration => {}) + assume_feature_present(:container_gateway_database, :configuration => {}) ForemanMaintain::Utils::Backup.any_instance.stubs(:file_map).returns( { :foreman_dump => { :present => true }, :candlepin_dump => { :present => true }, :pulpcore_dump => { :present => true }, + :container_gateway_dump => { :present => true }, :pulp_data => { :present => true }, :pgsql_data => { :present => false }, :metadata => { :present => false }, @@ -73,6 +75,7 @@ module Scenarios assert_scenario_has_step(scenario, Procedures::Restore::ForemanDump) assert_scenario_has_step(scenario, Procedures::Restore::CandlepinDump) assert_scenario_has_step(scenario, Procedures::Restore::PulpcoreDump) + assert_scenario_has_step(scenario, Procedures::Restore::ContainerGatewayDump) end it 'does not try to drop/restore DB dumps if present and DB data present' do @@ -96,6 +99,7 @@ module Scenarios refute_scenario_has_step(scenario, Procedures::Restore::ForemanDump) refute_scenario_has_step(scenario, Procedures::Restore::CandlepinDump) refute_scenario_has_step(scenario, Procedures::Restore::PulpcoreDump) + refute_scenario_has_step(scenario, Procedures::Restore::ContainerGatewayDump) end it 'does not try to drop/restore DB dumps when these are absent' do @@ -109,6 +113,7 @@ module Scenarios refute_scenario_has_step(scenario, Procedures::Restore::ForemanDump) refute_scenario_has_step(scenario, Procedures::Restore::CandlepinDump) refute_scenario_has_step(scenario, Procedures::Restore::PulpcoreDump) + refute_scenario_has_step(scenario, Procedures::Restore::ContainerGatewayDump) end end