Skip to content

Fix btrfs snapshot services #473

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.

Sign up for GitHub

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

Merged
schaefi merged 1 commit into from
Mar 30, 2026
Merged

Fix btrfs snapshot services #473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
60 changes: 32 additions & 28 deletions suse_migration_services/units/btrfs_snapshot_post_migration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
100 changes: 52 additions & 48 deletions suse_migration_services/units/btrfs_snapshot_pre_migration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
12 changes: 5 additions & 7 deletions test/unit/pre_checks_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
75 changes: 55 additions & 20 deletions test/unit/units/btrfs_snapshot_post_migration_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,62 @@
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


@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):
Expand All @@ -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)
Expand Down
49 changes: 38 additions & 11 deletions test/unit/units/btrfs_snapshot_pre_migration_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down
Loading