Use --file and --format with pg_dump

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]}'")

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

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

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)

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,

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
 

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
 

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]

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" \

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