diff --git a/suse_migration_services/units/btrfs_snapshot_post_migration.py b/suse_migration_services/units/btrfs_snapshot_post_migration.py index ea88d218..f42bff30 100644 --- a/suse_migration_services/units/btrfs_snapshot_post_migration.py +++ b/suse_migration_services/units/btrfs_snapshot_post_migration.py @@ -39,35 +39,39 @@ def __init__(self): def perform(self): try: self.log.info('Running Post-migration btrfs snapshot creation') - with open( - '/run/suse_migration_snapper_btrfs_pre_snapshot_number' - ) as pre_snapshot_number_file: - pre_snapshot_number = pre_snapshot_number_file.read().strip() - if not pre_snapshot_number.isdigit(): - message = f'Invalid snapshot number: {pre_snapshot_number}' - self.log.error(message) - raise ValueError(message) + stat = Command.run(['stat', '-f', '-c', '%T', self.root_path]) + if stat and stat.output.strip() == 'btrfs': + with open( + '/run/suse_migration_snapper_btrfs_pre_snapshot_number' + ) as pre_snapshot_number_file: + pre_snapshot_number = pre_snapshot_number_file.read().strip() + if not pre_snapshot_number.isdigit(): + message = f'Invalid snapshot number: {pre_snapshot_number}' + self.log.error(message) + raise ValueError(message) - Command.run( - [ - 'chroot', - self.root_path, - 'snapper', - '--no-dbus', - 'create', - '--type', - 'single', - '--read-only', - '--cleanup-algorithm', - 'number', - '--print-number', - '--userdata', - 'important=yes', - '--description', - 'after offline migration', - ] - ) - self.log.info('BTRFS post-migration snapshot creation completed successfully.') + Command.run( + [ + 'chroot', + self.root_path, + 'snapper', + '--no-dbus', + 'create', + '--type', + 'single', + '--read-only', + '--cleanup-algorithm', + 'number', + '--print-number', + '--userdata', + 'important=yes', + '--description', + 'after offline migration', + ] + ) + self.log.info('BTRFS post-migration snapshot creation completed successfully.') + else: + self.log.info('The root filesystem is not btrfs, skipping snapshot creation') except Exception as issue: message = 'BTRFS post-migration snapshot creation failed with: {}' self.log.error(message.format(issue)) diff --git a/suse_migration_services/units/btrfs_snapshot_pre_migration.py b/suse_migration_services/units/btrfs_snapshot_pre_migration.py index 7bcdec1d..b0dab3ba 100644 --- a/suse_migration_services/units/btrfs_snapshot_pre_migration.py +++ b/suse_migration_services/units/btrfs_snapshot_pre_migration.py @@ -43,56 +43,60 @@ def __init__(self): def perform(self): try: self.log.info('Running Pre-migration btrfs snapshot creation') - # we want pre snapshot to be created before we installed - # migration-activation package, if it was installed - pre_migration_activation_package_snapshot_number = 0 - snapshot_number_base_file = 'suse_migration_snapper_btrfs_pre_snapshot_number' - snapshot_number_file = os.path.normpath( - os.sep.join([self.root_path, '/var/cache', snapshot_number_base_file]) - ) - if os.path.isfile(snapshot_number_file): - with open(snapshot_number_file) as file: - pre_migration_activation_package_snapshot_number = file.read().strip() + stat = Command.run(['stat', '-f', '-c', '%T', self.root_path]) + if stat and stat.output.strip() == 'btrfs': + # we want pre snapshot to be created before we installed + # migration-activation package, if it was installed + pre_migration_activation_package_snapshot_number = 0 + snapshot_number_base_file = 'suse_migration_snapper_btrfs_pre_snapshot_number' + snapshot_number_file = os.path.normpath( + os.sep.join([self.root_path, '/var/cache', snapshot_number_base_file]) + ) + if os.path.isfile(snapshot_number_file): + with open(snapshot_number_file) as file: + pre_migration_activation_package_snapshot_number = file.read().strip() - # Note: The btrfs snapshot will be created before the - # mount system setup in order to avoid any potential issues - # with the new filesystem layout while - # the migration process proceeds. - snapper_enabled = Command.run( - ['chroot', self.root_path, 'snapper', '--no-dbus', 'get-config'], - raise_on_error=False, - ) - if snapper_enabled.returncode != 0: - return + # Note: The btrfs snapshot will be created before the + # mount system setup in order to avoid any potential issues + # with the new filesystem layout while + # the migration process proceeds. + snapper_enabled = Command.run( + ['chroot', self.root_path, 'snapper', '--no-dbus', 'get-config'], + raise_on_error=False, + ) + if snapper_enabled.returncode != 0: + return - # Create a new backup before any changes - snapper_call = Command.run( - [ - 'chroot', - self.root_path, - 'snapper', - '--no-dbus', - 'create', - '--from', - format(pre_migration_activation_package_snapshot_number), - '--read-only', - '--type', - 'single', - '--cleanup-algorithm', - 'number', - '--print-number', - '--userdata', - 'important=yes', - '--description', - 'before offline migration', - ] - ) - if snapper_call.returncode == 0: - pre_snapshot_number = '{}'.format(snapper_call.output) - with open( - '/run/{}'.format(snapshot_number_base_file), 'w' - ) as pre_snapshot_number_file: - pre_snapshot_number_file.write(pre_snapshot_number) + # Create a new backup before any changes + snapper_call = Command.run( + [ + 'chroot', + self.root_path, + 'snapper', + '--no-dbus', + 'create', + '--from', + format(pre_migration_activation_package_snapshot_number), + '--read-only', + '--type', + 'single', + '--cleanup-algorithm', + 'number', + '--print-number', + '--userdata', + 'important=yes', + '--description', + 'before offline migration', + ] + ) + if snapper_call.returncode == 0: + pre_snapshot_number = '{}'.format(snapper_call.output) + with open( + '/run/{}'.format(snapshot_number_base_file), 'w' + ) as pre_snapshot_number_file: + pre_snapshot_number_file.write(pre_snapshot_number) + else: + self.log.info('The root filesystem is not btrfs, skipping snapshot creation') except Exception as issue: message = 'BTRFS pre-migration snapshot creation failed with {}' self.log.error(message.format(issue)) diff --git a/test/unit/pre_checks_test.py b/test/unit/pre_checks_test.py index 09e9a151..597d06a8 100644 --- a/test/unit/pre_checks_test.py +++ b/test/unit/pre_checks_test.py @@ -604,7 +604,8 @@ def test_check_lsm_migration_failed( mock_shutil_which, mock_apparmor_enabled, mock_Command_run, - mock_os_geteuid, mock_log + mock_os_geteuid, + mock_log, ): mock_get_migration_target.return_value = {'version': '16.0'} mock_shutil_which.return_value = True @@ -625,7 +626,8 @@ def test_check_lsm_migration_simple( mock_shutil_which, mock_Command_run, mock_os_path_exists, - mock_os_geteuid, mock_log + mock_os_geteuid, + mock_log, ): mock_get_migration_target.return_value = {'version': '16.0'} mock_shutil_which.return_value = False @@ -644,11 +646,7 @@ def test_check_lsm_migration_simple( @patch('suse_migration_services.prechecks.lsm._apparmor_enabled') @patch.object(MigrationTarget, 'get_migration_target') def test_check_lsm_migration_apparmor_disabled( - self, - mock_get_migration_target, - mock_apparmor_enabled, - mock_os_geteuid, - mock_log + self, mock_get_migration_target, mock_apparmor_enabled, mock_os_geteuid, mock_log ): mock_get_migration_target.return_value = {'version': '16.0'} mock_apparmor_enabled.return_value = False diff --git a/test/unit/units/btrfs_snapshot_post_migration_test.py b/test/unit/units/btrfs_snapshot_post_migration_test.py index 255d73e4..33efb8c7 100644 --- a/test/unit/units/btrfs_snapshot_post_migration_test.py +++ b/test/unit/units/btrfs_snapshot_post_migration_test.py @@ -1,6 +1,6 @@ import io from pytest import raises -from unittest.mock import patch, MagicMock +from unittest.mock import patch, MagicMock, call from suse_migration_services.units.btrfs_snapshot_post_migration import main from suse_migration_services.exceptions import DistMigrationBtrfsSnapshotPostMigrationException @@ -8,32 +8,55 @@ @patch('suse_migration_services.logger.Logger.setup') class TestMigrationBtrfsSnapshotPost(object): + @patch('suse_migration_services.command.Command.run') + def test_main_not_btrfs(self, mock_Command_run, mock_logger_setup): + mock_Command_run.return_value.returncode = 0 + mock_Command_run.return_value.output = 'ext4' + main() + mock_Command_run.assert_called_once_with(['stat', '-f', '-c', '%T', '/system-root']) + @patch('suse_migration_services.command.Command.run') def test_main(self, mock_Command_run, mock_logger_setup): + def command_run_callback(*args, **kwargs): + snapper_call = MagicMock() + if args[0][0] == 'stat': + snapper_call = MagicMock() + snapper_call.returncode = 0 + snapper_call.output = 'btrfs' + else: + snapper_call.returncode = 0 + snapper_call.output = '42' + return snapper_call + + mock_Command_run.side_effect = command_run_callback + with patch('builtins.open', create=True) as mock_open: mock_open.return_value = MagicMock(spec=io.IOBase) file_handle = mock_open.return_value.__enter__.return_value file_handle.read.return_value = b'42' main() - mock_Command_run.assert_called_once_with( - [ - 'chroot', - '/system-root', - 'snapper', - '--no-dbus', - 'create', - '--type', - 'single', - '--read-only', - '--cleanup-algorithm', - 'number', - '--print-number', - '--userdata', - 'important=yes', - '--description', - 'after offline migration', - ] - ) + assert mock_Command_run.call_args_list == [ + call(['stat', '-f', '-c', '%T', '/system-root']), + call( + [ + 'chroot', + '/system-root', + 'snapper', + '--no-dbus', + 'create', + '--type', + 'single', + '--read-only', + '--cleanup-algorithm', + 'number', + '--print-number', + '--userdata', + 'important=yes', + '--description', + 'after offline migration', + ] + ), + ] @patch('suse_migration_services.command.Command.run') def test_main_raises_on_snapper(self, mock_Command_run, mock_logger_setup): @@ -44,6 +67,18 @@ def test_main_raises_on_snapper(self, mock_Command_run, mock_logger_setup): @patch('suse_migration_services.command.Command.run') def test_main_raises_on_invalid_snapshot_number(self, mock_Command_run, mock_logger_setup): + def command_run_callback(*args, **kwargs): + snapper_call = MagicMock() + if args[0][0] == 'stat': + snapper_call = MagicMock() + snapper_call.returncode = 0 + snapper_call.output = 'btrfs' + else: + snapper_call.returncode = 0 + snapper_call.output = '42' + return snapper_call + + mock_Command_run.side_effect = command_run_callback with raises(DistMigrationBtrfsSnapshotPostMigrationException): with patch('builtins.open', create=True) as mock_open: mock_open.return_value = MagicMock(spec=io.IOBase) diff --git a/test/unit/units/btrfs_snapshot_pre_migration_test.py b/test/unit/units/btrfs_snapshot_pre_migration_test.py index 8d751e9c..41b91c6a 100644 --- a/test/unit/units/btrfs_snapshot_pre_migration_test.py +++ b/test/unit/units/btrfs_snapshot_pre_migration_test.py @@ -8,20 +8,36 @@ @patch('suse_migration_services.logger.Logger.setup') class TestMigrationBtrfsSnapshotPre(object): + @patch('suse_migration_services.command.Command.run') + def test_main_not_btrfs(self, mock_Command_run, mock_logger_setup): + mock_Command_run.return_value.returncode = 0 + mock_Command_run.return_value.output = 'ext4' + main() + mock_Command_run.assert_called_once_with(['stat', '-f', '-c', '%T', '/system-root']) + @patch('suse_migration_services.command.Command.run') @patch('os.path.isfile') def test_main(self, mock_os_path_isfile, mock_Command_run, mock_logger_setup): + def command_run_callback(*args, **kwargs): + snapper_call = MagicMock() + if args[0][0] == 'stat': + snapper_call = MagicMock() + snapper_call.returncode = 0 + snapper_call.output = 'btrfs' + else: + snapper_call.returncode = 0 + snapper_call.output = '42' + return snapper_call + mock_os_path_isfile.return_value = True - snapper_call = MagicMock() - snapper_call.returncode = 0 - snapper_call.output = '42' - mock_Command_run.return_value = snapper_call + mock_Command_run.side_effect = command_run_callback with patch('builtins.open', create=True) as mock_open: mock_open.return_value = MagicMock(spec=io.IOBase) file_handle = mock_open.return_value.__enter__.return_value file_handle.read.return_value = '42' main() assert mock_Command_run.call_args_list == [ + call(['stat', '-f', '-c', '%T', '/system-root']), call( ['chroot', '/system-root', 'snapper', '--no-dbus', 'get-config'], raise_on_error=False, @@ -55,16 +71,27 @@ def test_main(self, mock_os_path_isfile, mock_Command_run, mock_logger_setup): def test_main_return_early_no_snapper_config( self, mock_os_path_isfile, mock_Command_run, mock_logger_setup ): + def command_run_callback(*args, **kwargs): + snapper_call = MagicMock() + if args[0][0] == 'stat': + snapper_call = MagicMock() + snapper_call.returncode = 0 + snapper_call.output = 'btrfs' + else: + snapper_call.returncode = 1 + return snapper_call + mock_os_path_isfile.return_value = False - snapper_call = MagicMock() - snapper_call.returncode = 1 - mock_Command_run.return_value = snapper_call + mock_Command_run.side_effect = command_run_callback with patch('builtins.open', create=True): main() - mock_Command_run.assert_called_once_with( - ['chroot', '/system-root', 'snapper', '--no-dbus', 'get-config'], - raise_on_error=False, - ) + assert mock_Command_run.call_args_list == [ + call(['stat', '-f', '-c', '%T', '/system-root']), + call( + ['chroot', '/system-root', 'snapper', '--no-dbus', 'get-config'], + raise_on_error=False, + ), + ] @patch('os.path.isfile') def test_main_raises(self, mock_os_path_isfile, mock_logger_setup):