diff --git a/definitions/procedures/backup/prepare_directory.rb b/definitions/procedures/backup/prepare_directory.rb index 548bc3eb1..9b304a15f 100644 --- a/definitions/procedures/backup/prepare_directory.rb +++ b/definitions/procedures/backup/prepare_directory.rb @@ -6,18 +6,29 @@ class PrepareDirectory < ForemanMaintain::Procedure param :backup_dir, 'Directory where to backup to', :required => true param :preserve_dir, 'Directory where to backup to', :flag => true param :incremental_dir, 'Changes since specified backup only' + param :online_backup, 'Select for online backup', :flag => true, :default => false end + # rubocop:disable Metrics/MethodLength def run - puts "Creating backup folder #{@backup_dir}" - unless @preserve_dir + puts "Creating backup folder #{@backup_dir}" + FileUtils.mkdir_p @backup_dir FileUtils.chmod_R 0o770, @backup_dir - end - if feature(:instance).postgresql_local? && !@preserve_dir - FileUtils.chown_R(nil, 'postgres', @backup_dir) + if feature(:instance).postgresql_local? && @online_backup + begin + FileUtils.chown_R(nil, 'postgres', @backup_dir) + rescue Errno::EPERM + warn_msg = <<~MSG + #{@backup_dir} could not be made readable by the 'postgres' user. + This won't affect the backup procedure, but you have to ensure that + the 'postgres' user can read the data during restore. + MSG + set_status(:warning, warn_msg) + end + end end FileUtils.rm(Dir.glob(File.join(@backup_dir, '.*.snar'))) if @preserve_dir @@ -30,5 +41,6 @@ def run end end end + # rubocop:enable Metrics/MethodLength end end diff --git a/definitions/scenarios/backup.rb b/definitions/scenarios/backup.rb index d8f96a75e..1ad0008df 100644 --- a/definitions/scenarios/backup.rb +++ b/definitions/scenarios/backup.rb @@ -20,7 +20,8 @@ def compose check_valid_strategy safety_confirmation add_step_with_context(Procedures::Backup::AccessibilityConfirmation) if strategy == :offline - add_step_with_context(Procedures::Backup::PrepareDirectory) + add_step_with_context(Procedures::Backup::PrepareDirectory, + :online_backup => strategy == :online) add_step_with_context(Procedures::Backup::Metadata, :online_backup => strategy == :online) case strategy diff --git a/test/definitions/procedures/backup/prepare_directory_test.rb b/test/definitions/procedures/backup/prepare_directory_test.rb new file mode 100644 index 000000000..bd693dc2a --- /dev/null +++ b/test/definitions/procedures/backup/prepare_directory_test.rb @@ -0,0 +1,109 @@ +require 'test_helper' + +describe Procedures::Backup::PrepareDirectory do + include DefinitionsTestHelper + + let(:backup_dir) { '/mnt/backup' } + + context 'with default params' do + subject do + Procedures::Backup::PrepareDirectory.new(:backup_dir => backup_dir) + end + + it 'creates backup directory for local DB' do + assume_feature_present(:instance, :postgresql_local? => true) + + FileUtils.expects(:mkdir_p).with(backup_dir).once + FileUtils.expects(:chmod_R).with(0o770, backup_dir).once + FileUtils.expects(:chown_R).with(nil, 'postgres', backup_dir).never + + result = run_procedure(subject) + assert result.success?, 'the procedure was expected to succeed' + end + + it 'creates backup directory for remote DB' do + assume_feature_present(:instance, :postgresql_local? => false) + + FileUtils.expects(:mkdir_p).with(backup_dir).once + FileUtils.expects(:chmod_R).with(0o770, backup_dir).once + FileUtils.expects(:chown_R).with(nil, 'postgres', backup_dir).never + + result = run_procedure(subject) + assert result.success?, 'the procedure was expected to succeed' + end + end + + context 'with online_backup=>true' do + subject do + Procedures::Backup::PrepareDirectory.new(:backup_dir => backup_dir, :online_backup => true) + end + + it 'creates backup directory for local DB' do + assume_feature_present(:instance, :postgresql_local? => true) + + FileUtils.expects(:mkdir_p).with(backup_dir).once + FileUtils.expects(:chmod_R).with(0o770, backup_dir).once + FileUtils.expects(:chown_R).with(nil, 'postgres', backup_dir).once + + result = run_procedure(subject) + assert result.success?, 'the procedure was expected to succeed' + end + + it 'does not fail when chown to postgres fails' do + assume_feature_present(:instance, :postgresql_local? => true) + + FileUtils.expects(:mkdir_p).with(backup_dir).once + FileUtils.expects(:chmod_R).with(0o770, backup_dir).once + FileUtils.expects(:chown_R).with(nil, 'postgres', backup_dir).once.raises(Errno::EPERM) + + result = run_procedure(subject) + assert result.warning?, 'the procedure was expected to warn' + + warn_msg = <<~MSG + /mnt/backup could not be made readable by the 'postgres' user. + This won't affect the backup procedure, but you have to ensure that + the 'postgres' user can read the data during restore. + MSG + assert_equal warn_msg, result.output + end + + it 'creates backup directory for remote DB' do + assume_feature_present(:instance, :postgresql_local? => false) + + FileUtils.expects(:mkdir_p).with(backup_dir).once + FileUtils.expects(:chmod_R).with(0o770, backup_dir).once + FileUtils.expects(:chown_R).with(nil, 'postgres', backup_dir).never + + result = run_procedure(subject) + assert result.success?, 'the procedure was expected to succeed' + end + end + + context 'with preserve_dir=>true' do + subject do + Procedures::Backup::PrepareDirectory.new(:backup_dir => backup_dir, :preserve_dir => true) + end + + it 'does not create backup directory for local DB' do + assume_feature_present(:instance, :postgresql_local? => true) + + FileUtils.expects(:mkdir_p).with(backup_dir).never + FileUtils.expects(:chmod_R).with(0o770, backup_dir).never + FileUtils.expects(:chown_R).with(nil, 'postgres', backup_dir).never + + result = run_procedure(subject) + assert result.success?, 'the procedure was expected to succeed' + end + + it 'does not create backup directory for remote DB' do + assume_feature_present(:instance, :postgresql_local? => false) + + FileUtils.expects(:mkdir_p).with(backup_dir).never + FileUtils.expects(:chmod_R).with(0o770, backup_dir).never + FileUtils.expects(:chown_R).with(nil, 'postgres', backup_dir).never + + result = run_procedure(subject) + assert result.success?, 'the procedure was expected to succeed' + end + end +end