Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure that the backup types match when doing incremental backups #891

Merged
merged 1 commit into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions definitions/checks/backup/incremental_parent_type.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
module Checks::Backup
class IncrementalParentType < ForemanMaintain::Check
metadata do
description 'Check if the incremental backup has the right type'
tags :backup
param :incremental_dir, 'Path to existing backup directory'
param :online_backup, 'Select for online backup', :flag => true, :default => false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the supposition that if this is not set, it defaults to offline backup?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

param :sql_tar, 'Will backup include PostgreSQL tarball', :flag => true, :default => false
manual_detection
end

def run
return unless @incremental_dir
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter is required right? So can this ever be true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter can be nil (as in: set to nil by the caller) when we're creating a non-incremental (=base) backup


backup = ForemanMaintain::Utils::Backup.new(@incremental_dir)

existing_type = backup.backup_type
new_type = if @online_backup
ForemanMaintain::Utils::Backup::ONLINE_BACKUP
else
ForemanMaintain::Utils::Backup::OFFLINE_BACKUP
end
msg = "The existing backup is an #{existing_type} backup, "\
"but an #{new_type} backup was requested."
assert(existing_type == new_type, msg)

unless @online_backup
existing_sql = backup.sql_tar_files_exist? ? 'tarball' : 'dump'
new_sql = @sql_tar ? 'tarball' : 'dump'
msg = "The existing backup has PostgreSQL as a #{existing_sql}, "\
"but the new one will have a #{new_sql}."
assert(existing_sql == new_sql, msg)
end
end
end
end
4 changes: 4 additions & 0 deletions definitions/scenarios/backup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ class Backup < ForemanMaintain::Scenario

def compose
check_valid_strategy
add_step_with_context(Checks::Backup::IncrementalParentType,
:online_backup => strategy == :online,
:sql_tar => feature(:instance).postgresql_local?)
safety_confirmation
add_step_with_context(Procedures::Backup::AccessibilityConfirmation) if strategy == :offline
add_step_with_context(Procedures::Backup::PrepareDirectory,
Expand Down Expand Up @@ -51,6 +54,7 @@ def set_context_mapping
context.map(:preserve_dir,
Procedures::Backup::PrepareDirectory => :preserve_dir)
context.map(:incremental_dir,
Checks::Backup::IncrementalParentType => :incremental_dir,
Procedures::Backup::PrepareDirectory => :incremental_dir,
Procedures::Backup::Metadata => :incremental_dir)
context.map(:proxy_features,
Expand Down
7 changes: 7 additions & 0 deletions lib/foreman_maintain/utils/backup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ class Backup
:foreman_online_files, :foreman_offline_files, :fpc_offline_files,
:fpc_online_files

ONLINE_BACKUP = 'online'.freeze
OFFLINE_BACKUP = 'offline'.freeze

def initialize(backup_dir)
# fpc stands for foreman proxy w/ content
@backup_dir = backup_dir
Expand Down Expand Up @@ -260,6 +263,10 @@ def source_os_version
def different_source_os?
source_os_version != "#{os_name} #{os_version}"
end

def backup_type
online_backup? ? ONLINE_BACKUP : OFFLINE_BACKUP
end
end
end
end
101 changes: 101 additions & 0 deletions test/definitions/checks/backup/incremental_parent_type_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
require 'test_helper'

describe Checks::Backup::IncrementalParentType do
include DefinitionsTestHelper

context 'without incremental_dir' do
subject do
Checks::Backup::IncrementalParentType.new
end

it 'passes without performing any checks' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).never
result = run_check(subject)
assert result.success?, 'Check expected to succeed'
end
end

context 'for offline backup with local PostreSQL tarballs' do
subject do
Checks::Backup::IncrementalParentType.new(:incremental_dir => '.', :online_backup => false,
:sql_tar => true)
end

it 'passes when existing backup is offline with tarballs' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).returns(false)
ForemanMaintain::Utils::Backup.any_instance.expects(:sql_tar_files_exist?).returns(true)
result = run_check(subject)
assert result.success?, 'Check expected to succeed'
end

it 'fails when existing backup is online' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).returns(true)
result = run_check(subject)
refute result.success?, 'Check expected to fail'
expected = 'The existing backup is an online backup, but an offline backup was requested.'
assert_equal expected, result.output
end

it 'fails when existing backup uses dumps' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).returns(false)
ForemanMaintain::Utils::Backup.any_instance.expects(:sql_tar_files_exist?).returns(false)
result = run_check(subject)
refute result.success?, 'Check expected to fail'
expected = 'The existing backup has PostgreSQL as a dump, '\
'but the new one will have a tarball.'
assert_equal expected, result.output
end
end

context 'for offline backup with remote PostgreSQL dumps' do
subject do
Checks::Backup::IncrementalParentType.new(:incremental_dir => '.', :online_backup => false,
:sql_tar => false)
end

it 'passes when existing backup is offline' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).returns(false)
ForemanMaintain::Utils::Backup.any_instance.expects(:sql_tar_files_exist?).returns(false)
result = run_check(subject)
assert result.success?, 'Check expected to succeed'
end

it 'fails when existing backup is online' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).returns(true)
result = run_check(subject)
refute result.success?, 'Check expected to fail'
expected = 'The existing backup is an online backup, but an offline backup was requested.'
assert_equal expected, result.output
end

it 'fails when existing backup uses psql_data.tar' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).returns(false)
ForemanMaintain::Utils::Backup.any_instance.expects(:sql_tar_files_exist?).returns(true)
result = run_check(subject)
refute result.success?, 'Check expected to fail'
expected = 'The existing backup has PostgreSQL as a tarball, '\
'but the new one will have a dump.'
assert_equal expected, result.output
end
end

context 'for online backup' do
subject do
Checks::Backup::IncrementalParentType.new(:incremental_dir => '.', :online_backup => true)
end

it 'passes when existing backup is online' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).returns(true)
result = run_check(subject)
assert result.success?, 'Check expected to succeed'
end

it 'fails when existing backup is offline' do
ForemanMaintain::Utils::Backup.any_instance.expects(:online_backup?).returns(false)
result = run_check(subject)
refute result.success?, 'Check expected to fail'
expected = 'The existing backup is an offline backup, but an online backup was requested.'
assert_equal expected, result.output
end
end
end
2 changes: 2 additions & 0 deletions test/definitions/scenarios/backup_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module Scenarios
end

it 'composes all steps' do
assert_scenario_has_step(scenario, Checks::Backup::IncrementalParentType)
assert_scenario_has_step(scenario, Procedures::Backup::AccessibilityConfirmation)
assert_scenario_has_step(scenario, Procedures::Backup::PrepareDirectory)
assert_scenario_has_step(scenario, Procedures::Backup::Metadata)
Expand All @@ -42,6 +43,7 @@ module Scenarios
end

it 'composes all steps' do
assert_scenario_has_step(scenario, Checks::Backup::IncrementalParentType)
assert_scenario_has_step(scenario, Procedures::Backup::Online::SafetyConfirmation)
refute_scenario_has_step(scenario, Procedures::Backup::AccessibilityConfirmation)
assert_scenario_has_step(scenario, Procedures::Backup::PrepareDirectory)
Expand Down
Loading