diff --git a/kiwi/builder/disk.py b/kiwi/builder/disk.py index cd5fb58b574..55a6771b4f4 100644 --- a/kiwi/builder/disk.py +++ b/kiwi/builder/disk.py @@ -374,8 +374,24 @@ def create_disk(self) -> Result: device_map, luks_root ) + # create system layout for root system + device_map = self._create_system_instance( + device_map, stack + ) # build bootable disk - self._process_build(device_map, disk) + self._build_main_system( + stack, + device_map, + disk, + self.storage_map['system'], + self.storage_map['system_boot'], + self.storage_map['system_efi'], + self.storage_map['system_spare'], + self.storage_map['system_custom_parts'] or {}, + self.storage_map['luks_root'], + self.storage_map['raid_root'], + self.storage_map['integrity_root'] + ) # store image bundle_format in result if self.bundle_format: @@ -562,7 +578,9 @@ def _integrity_instance(self, device_map: Dict) -> IntegrityDevice: ) ) - def _create_system_instance(self, device_map: Dict) -> None: + def _create_system_instance( + self, device_map: Dict, stack: ExitStack + ) -> Dict: # create spare filesystem on spare partition if present self.storage_map[ 'system_spare' @@ -586,16 +604,20 @@ def _create_system_instance(self, device_map: Dict) -> None: self.volumes, self.volume_manager_custom_parameters ) + stack.push(volume_manager) device_map = self._map_root_volume_manager( device_map, volume_manager ) elif self.need_root_filesystem: - with FileSystem.new( + filesystem = FileSystem.new( self.requested_filesystem, device_map['root'], self.root_dir + '/', self.filesystem_custom_parameters - ) as filesystem: - self._map_root_filesystem(device_map, filesystem) + ) + stack.push(filesystem) + self._map_root_filesystem(device_map, filesystem) + + return device_map def _map_raid( self, device_map: Dict, disk: Disk, raid_root: RaidDevice @@ -709,59 +731,6 @@ def _map_root_filesystem( ) self.storage_map['system'] = filesystem - def _process_build(self, device_map: Dict, disk: Disk) -> None: - # create system layout for root system - self._create_system_instance(device_map) - - # representing the entire image system except for the boot/ area - # which could live on another part of the disk - system: Optional[Union[FileSystemBase, VolumeManagerBase]] = \ - self.storage_map['system'] - - # an instance of a class with the sync_data capability - # representing the boot/ area of the disk if not part of - # system - system_boot: Optional[FileSystemBase] = \ - self.storage_map['system_boot'] - - # an instance of a class with the sync_data capability - # representing the boot/efi area of the disk - system_efi: Optional[FileSystemBase] = \ - self.storage_map['system_efi'] - - # an instance of a class with the sync_data capability - # representing the spare_part_mountpoint area of the disk - system_spare: Optional[FileSystemBase] = \ - self.storage_map['system_spare'] - - # a dict of instances with the sync_data capability - # representing the custom partitions area of the disk - system_custom_parts: Dict[str, FileSystemBase] = \ - self.storage_map['system_custom_parts'] or {} - - luks_root = self.storage_map['luks_root'] - raid_root = self.storage_map['raid_root'] - integrity_root = self.storage_map['integrity_root'] - try: - with ExitStack() as stack: - self._build_main_system( - stack, - device_map, - disk, - system, - system_boot, - system_efi, - system_spare, - system_custom_parts, - luks_root, - raid_root, - integrity_root - ) - finally: - if system: - if self.volume_manager_name: - system.umount_volumes() - def _build_main_system( self, stack: ExitStack, diff --git a/kiwi/filesystem/base.py b/kiwi/filesystem/base.py index 724ab9eedfe..2a26b8a8b99 100644 --- a/kiwi/filesystem/base.py +++ b/kiwi/filesystem/base.py @@ -60,9 +60,7 @@ def __init__( # as a file there self.filesystem_mount: Optional[MountManager] = None - # bind the block device providing class instance to this object. - # This is done to guarantee the correct destructor order when - # the device should be released. This is only required if the + # the underlaying device provider. This is only required if the # filesystem required a block device to become created self.device_provider = device_provider diff --git a/kiwi/storage/disk.py b/kiwi/storage/disk.py index 4fee3de1f90..9cf8d524b9c 100644 --- a/kiwi/storage/disk.py +++ b/kiwi/storage/disk.py @@ -68,15 +68,13 @@ def __init__( of that extended partition """ self.partition_mapper = RuntimeConfig().get_mapper_tool() - # bind the underlaying block device providing class instance - # to this object (e.g loop) if present. This is done to guarantee - # the correct destructor order when the device should be released. + #: the underlaying device provider self.storage_provider = storage_provider - # list of protected map ids. If used in a custom partitions - # setup this will lead to a raise conditition in order to - # avoid conflicts with the existing partition layout and its - # customizaton capabilities + #: list of protected map ids. If used in a custom partitions + #: setup this will lead to a raise conditition in order to + #: avoid conflicts with the existing partition layout and its + #: customizaton capabilities self.protected_map_ids = [ 'root', 'readonly', diff --git a/kiwi/storage/integrity_device.py b/kiwi/storage/integrity_device.py index a487f6cfcc7..700325e7bf7 100644 --- a/kiwi/storage/integrity_device.py +++ b/kiwi/storage/integrity_device.py @@ -61,9 +61,7 @@ def __init__( self, storage_provider: DeviceProvider, integrity_algorithm: str, credentials: integrity_credentials_type = None ) -> None: - # bind the underlaying block device providing class instance - # to this object (e.g loop) if present. This is done to guarantee - # the correct destructor order when the device should be released. + #: the underlaying device provider self.storage_provider = storage_provider self.integrity_device: Optional[str] = None diff --git a/kiwi/storage/luks_device.py b/kiwi/storage/luks_device.py index 624cdca9fcf..ba90ad9ce28 100644 --- a/kiwi/storage/luks_device.py +++ b/kiwi/storage/luks_device.py @@ -40,9 +40,7 @@ class LuksDevice(DeviceProvider): :param object storage_provider: Instance of class based on DeviceProvider """ def __init__(self, storage_provider: DeviceProvider) -> None: - # bind the underlaying block device providing class instance - # to this object (e.g loop) if present. This is done to guarantee - # the correct destructor order when the device should be released. + #: the underlaying device provider self.storage_provider = storage_provider self.luks_device: Optional[str] = None diff --git a/kiwi/storage/raid_device.py b/kiwi/storage/raid_device.py index 21f35aa1a10..2b4cf444c5d 100644 --- a/kiwi/storage/raid_device.py +++ b/kiwi/storage/raid_device.py @@ -38,9 +38,7 @@ class RaidDevice(DeviceProvider): :param object storage_provider: Instance of class based on DeviceProvider """ def __init__(self, storage_provider: DeviceProvider): - # bind the underlaying block device providing class instance - # to this object (e.g loop) if present. This is done to guarantee - # the correct destructor order when the device should be released. + #: the underlaying device provider self.storage_provider = storage_provider self.raid_level_map = { diff --git a/kiwi/system/prepare.py b/kiwi/system/prepare.py index 83c686332b7..22bc81a044a 100644 --- a/kiwi/system/prepare.py +++ b/kiwi/system/prepare.py @@ -105,10 +105,7 @@ def __init__( self.profiles = xml_state.profiles self.root_bind = root_bind - # A list of Uri references is stored inside of the System instance - # in order to delay the Uri destructors until the System instance - # dies. This is needed to keep bind mounted Uri locations alive - # for System operations + #: A list of Uri references self.uri_list: List[Uri] = [] def setup_repositories( diff --git a/kiwi/volume_manager/base.py b/kiwi/volume_manager/base.py index 94fa038f001..ebc4675b4bd 100644 --- a/kiwi/volume_manager/base.py +++ b/kiwi/volume_manager/base.py @@ -61,18 +61,15 @@ def __init__( ) -> None: self.temp_directories: List[Temporary] = [] - # all volumes are combined into one mountpoint. This is - # needed at sync_data time. How to mount the volumes is - # special to the volume management class - #: root mountpoint for volumes + #: all volumes are combined into one mountpoint. This is + #: needed at sync_data time. How to mount the volumes is + #: special to the volume management class self.mountpoint: Optional[str] = None #: dictionary of mapped DeviceProviders self.device_map = device_map - # bind the device providing class instance to this object. - # This is done to guarantee the correct destructor order when - # the device should be released. + #: the underlaying device provider self.device_provider_root = device_map['root'] #: An indicator for the mount of the filesystem and its volumes @@ -98,10 +95,12 @@ def __init__( 'given root directory %s does not exist' % root_dir ) + #: custom arguments passed to setup the volumes self.custom_args: Dict[str, Any] = {} #: custom filesystem creation and mount arguments, subset of the - #: custom_args information suitable to be passed to a FileSystem instance + #: custom_args information suitable to be passed to a + #: FileSystem instance self.custom_filesystem_args: Dict[str, Any] = { 'create_options': [], 'mount_options': [] @@ -121,6 +120,9 @@ def __init__( self.post_init(custom_args) + def __enter__(self): + return self + def post_init(self, custom_args): """ Post initialization method @@ -164,7 +166,9 @@ def apply_attributes_on_volume(self, toplevel, volume): ] ) - def get_fstab(self, persistency_type: str, filesystem_name: str) -> List[str]: + def get_fstab( + self, persistency_type: str, filesystem_name: str + ) -> List[str]: """ Implements setup of the fstab entries. The method should return a list of fstab compatible entries @@ -370,7 +374,9 @@ def get_root_volume_name(self) -> str: """ return '/' - def sync_data(self, exclude: Optional[List[str]] = None) -> Optional[MountManager]: + def sync_data( + self, exclude: Optional[List[str]] = None + ) -> Optional[MountManager]: """ Implements sync of root directory to mounted volumes @@ -421,3 +427,6 @@ def setup_mountpoint(self): self.mountpoint_tempdir = Temporary(prefix='kiwi_volumes.').new_dir() self.mountpoint = self.mountpoint_tempdir.name self.temp_directories.append(self.mountpoint_tempdir) + + def __exit__(self, exc_type, exc_value, traceback): + pass diff --git a/kiwi/volume_manager/btrfs.py b/kiwi/volume_manager/btrfs.py index c1342d6feb9..54932179b80 100644 --- a/kiwi/volume_manager/btrfs.py +++ b/kiwi/volume_manager/btrfs.py @@ -376,26 +376,16 @@ def mount_volumes(self): self.volumes_mounted_initially = True - def umount_volumes(self): + def umount_volumes(self) -> None: """ Umount btrfs subvolumes - - :return: True if all subvolumes are successfully unmounted - - :rtype: bool """ - all_volumes_umounted = True for volume_mount in reversed(self.subvol_mount_list): if volume_mount.is_mounted(): - if not volume_mount.umount(): - all_volumes_umounted = False - - if all_volumes_umounted: - if self.toplevel_mount.is_mounted(): - if not self.toplevel_mount.umount(): - all_volumes_umounted = False + volume_mount.umount() - return all_volumes_umounted + if self.toplevel_mount.is_mounted(): + self.toplevel_mount.umount() def get_mountpoint(self) -> str: """ @@ -576,8 +566,6 @@ def _create_snapshot_info(self, filename): with open(filename, 'w') as snapshot_info_file: snapshot_info_file.write(self._xml_pretty(snapshot)) - def __del__(self): + def __exit__(self, exc_type, exc_value, traceback): if self.toplevel_mount: - log.info('Cleaning up %s instance', type(self).__name__) - if not self.umount_volumes(): - log.warning('Subvolumes still busy') + self.umount_volumes() diff --git a/kiwi/volume_manager/lvm.py b/kiwi/volume_manager/lvm.py index aa97b6a0b55..6e3e4124bc2 100644 --- a/kiwi/volume_manager/lvm.py +++ b/kiwi/volume_manager/lvm.py @@ -32,7 +32,8 @@ from kiwi.path import Path from kiwi.exceptions import ( - KiwiVolumeGroupConflict + KiwiVolumeGroupConflict, + KiwiCommandError ) log = logging.getLogger('kiwi') @@ -306,17 +307,10 @@ def mount_volumes(self): def umount_volumes(self): """ Umount lvm volumes - - :return: True if all subvolumes are successfully unmounted - - :rtype: bool """ - all_volumes_umounted = True for volume_mount in reversed(self.mount_list): if volume_mount.is_mounted(): - if not volume_mount.umount(): - all_volumes_umounted = False - return all_volumes_umounted + volume_mount.umount() def _is_volume_enabled_for_fs_check(self, volume_name): for volume in self.volumes: @@ -397,18 +391,18 @@ def _is_swap_volume(self, volume_name): if volume_name == volume.name and volume.label == 'SWAP': return True - def __del__(self): + def __exit__(self, exc_type, exc_value, traceback): if self.volume_group: - log.info('Cleaning up %s instance', type(self).__name__) - if self.umount_volumes(): - try: - Command.run( - ['vgchange'] + self.lvm_tool_options + [ - '-an', self.volume_group - ] - ) - except Exception: - log.warning( - 'volume group %s still busy', self.volume_group + try: + self.umount_volumes() + Command.run( + ['vgchange'] + self.lvm_tool_options + [ + '-an', self.volume_group + ] + ) + except KiwiCommandError as issue: + log.error( + 'volume group {0} detach failed with {1}'.format( + self.volume_group, issue ) - return + ) diff --git a/test/unit/builder/disk_test.py b/test/unit/builder/disk_test.py index 773d0e6ee42..53975aceabc 100644 --- a/test/unit/builder/disk_test.py +++ b/test/unit/builder/disk_test.py @@ -289,8 +289,10 @@ def test_create_disk_standard_root_with_kiwi_initrd( mock_Disk.return_value.__enter__.return_value = disk mock_path.return_value = True mock_rand.return_value = 15 - filesystem = Mock() - mock_fs.return_value.__enter__.return_value = filesystem + + filesystem = MagicMock() + mock_fs.return_value = filesystem + self.disk_builder.volume_manager_name = None self.disk_builder.initrd_system = 'kiwi' @@ -360,24 +362,33 @@ def test_create_disk_standard_root_with_kiwi_initrd( self.boot_image_task.prepare.assert_called_once_with() - call = filesystem.create_on_device.call_args_list[0] - assert filesystem.create_on_device.call_args_list[0] == \ + call = filesystem.__enter__.return_value \ + .create_on_device.call_args_list[0] + assert filesystem.__enter__.return_value \ + .create_on_device.call_args_list[0] == \ call(label='EFI') - call = filesystem.create_on_device.call_args_list[1] - assert filesystem.create_on_device.call_args_list[1] == \ + call = filesystem.__enter__.return_value \ + .create_on_device.call_args_list[1] + assert filesystem.__enter__.return_value \ + .create_on_device.call_args_list[1] == \ call(label='BOOT') - call = filesystem.create_on_device.call_args_list[2] - assert filesystem.create_on_device.call_args_list[2] == \ + + call = filesystem.create_on_device.call_args_list[0] + assert filesystem.create_on_device.call_args_list[0] == \ call(label='ROOT') - call = filesystem.sync_data.call_args_list[0] - assert filesystem.sync_data.call_args_list[0] == \ + call = filesystem.__enter__.return_value \ + .sync_data.call_args_list[0] + assert filesystem.__enter__.return_value \ + .sync_data.call_args_list[0] == \ call() - call = filesystem.sync_data.call_args_list[1] - assert filesystem.sync_data.call_args_list[1] == \ + call = filesystem.__enter__.return_value \ + .sync_data.call_args_list[1] + assert filesystem.__enter__.return_value \ + .sync_data.call_args_list[1] == \ call(['efi/*']) - call = filesystem.sync_data.call_args_list[2] - assert filesystem.sync_data.call_args_list[2] == \ + call = filesystem.sync_data.call_args_list[0] + assert filesystem.sync_data.call_args_list[0] == \ call([ 'image', '.profile', '.kconfig', 'run/*', 'tmp/*', '.buildenv', 'var/cache/kiwi', 'boot/*', 'boot/.*', @@ -629,8 +640,8 @@ def test_create_disk_standard_root_with_dm_integrity( initrd_name='initramfs-1.2.3.img' ) mock_path.return_value = True - filesystem = Mock() - mock_fs.return_value.__enter__.return_value = filesystem + filesystem = MagicMock() + mock_fs.return_value = filesystem self.disk_builder.integrity = True self.disk_builder.integrity_legacy_hmac = True self.disk_builder.root_filesystem_embed_integrity_metadata = True @@ -650,13 +661,15 @@ def test_create_disk_standard_root_with_dm_integrity( self.integrity_root.create_integrity_metadata.assert_called_once_with() self.integrity_root.sign_integrity_metadata.assert_called_once_with() self.integrity_root.write_integrity_metadata.assert_called_once_with() - assert filesystem.create_on_device.call_args_list == [ + assert filesystem.__enter__.return_value.create_on_device.call_args_list == [ call(label='EFI'), call(label='BOOT'), - # root must be created DM_METADATA_OFFSET smaller - call(label='ROOT', size=-4096, unit='b'), call(label='SWAP') ] + assert filesystem.create_on_device.call_args_list == [ + # root must be created DM_METADATA_OFFSET smaller + call(label='ROOT', size=-4096, unit='b') + ] @patch('kiwi.builder.disk.Disk') @patch('kiwi.builder.disk.create_boot_loader_config') @@ -698,8 +711,8 @@ def test_create_disk_standard_root_with_dracut_initrd( ) mock_path.return_value = True mock_rand.return_value = 15 - filesystem = Mock() - mock_fs.return_value.__enter__.return_value = filesystem + filesystem = MagicMock() + mock_fs.return_value = filesystem self.disk_builder.root_filesystem_verity_blocks = 10 self.disk_builder.root_filesystem_embed_verity_metadata = True self.disk_builder.root_filesystem_is_overlay = False @@ -773,19 +786,23 @@ def test_create_disk_standard_root_with_dracut_initrd( '/dev/boot-device' ) self.boot_image_task.prepare.assert_called_once_with() - assert filesystem.create_on_device.call_args_list[0] == \ - call(label='EFI') - assert filesystem.create_on_device.call_args_list[1] == \ - call(label='BOOT') - assert filesystem.create_on_device.call_args_list[2] == \ - call(label='ROOT') - assert filesystem.sync_data.call_args_list[0] == \ - call() - assert filesystem.sync_data.call_args_list[1] == \ - call(['efi/*']) - assert filesystem.sync_data.call_args_list[2] == \ - call([ + filesystem.create_on_device.assert_called_once_with( + label='ROOT' + ) + + assert filesystem.__enter__.return_value \ + .create_on_device.call_args_list[0] == call(label='EFI') + assert filesystem.__enter__.return_value \ + .create_on_device.call_args_list[1] == call(label='BOOT') + + assert filesystem.__enter__.return_value \ + .sync_data.call_args_list[0] == call() + assert filesystem.__enter__.return_value \ + .sync_data.call_args_list[1] == call(['efi/*']) + + assert filesystem.__enter__.return_value \ + .sync_data.call_args_list[2] == call([ 'image', '.profile', '.kconfig', 'run/*', 'tmp/*', '.buildenv', 'var/cache/kiwi', 'boot/*', 'boot/.*', 'boot/efi/*', 'boot/efi/.*' @@ -828,10 +845,10 @@ def test_create_disk_standard_root_with_dracut_initrd( self.boot_image_task.omit_module.assert_called_once_with('multipath') assert self.boot_image_task.write_system_config_file.call_args_list == \ [] - filesystem.create_verity_layer.assert_called_once_with(10, 'tempfile') - filesystem.create_verification_metadata.assert_called_once_with( - '/dev/root-device' - ) + filesystem.__enter__.return_value \ + .create_verity_layer.assert_called_once_with(10, 'tempfile') + filesystem.__enter__.return_value \ + .create_verification_metadata.assert_called_once_with('/dev/root-device') @patch('kiwi.builder.disk.Disk') @patch('kiwi.builder.disk.create_boot_loader_config') @@ -1573,8 +1590,8 @@ def test_create_disk_spare_part_requested( ) mock_create_boot_loader_config.return_value.__enter__.return_value = \ bootloader_config - filesystem = Mock() - mock_fs.return_value.__enter__.return_value = filesystem + filesystem = MagicMock() + mock_fs.return_value = filesystem self.disk_builder.volume_manager_name = None self.disk_builder.install_media = False self.disk_builder.spare_part_mbsize = 42 diff --git a/test/unit/cli_test.py b/test/unit/cli_test.py index b915d6ed46d..4d2cd150d09 100644 --- a/test/unit/cli_test.py +++ b/test/unit/cli_test.py @@ -134,13 +134,13 @@ def test_warning_on_use_of_legacy_disk_type(self): def test_set_target_arch(self): sys.argv = [ sys.argv[0], - '--target-arch', 'artificial', 'system', 'build', + '--target-arch', 'x86_64', 'system', 'build', '--description', 'description', '--target-dir', 'directory' ] cli = Cli() cli.get_global_args() - assert Defaults.get_platform_name() == 'artificial' + assert Defaults.get_platform_name() == 'x86_64' def test_get_servicename_image(self): sys.argv = [ diff --git a/test/unit/volume_manager/base_test.py b/test/unit/volume_manager/base_test.py index 8eb82c79fd7..d91a7d57caa 100644 --- a/test/unit/volume_manager/base_test.py +++ b/test/unit/volume_manager/base_test.py @@ -291,3 +291,10 @@ def test_apply_attributes_on_volume(self, mock_command): mock_command.assert_called_once_with( ['chattr', '+C', 'toplevel/etc'] ) + + @patch('os.path.exists') + def test_context_manager_exit(self, mock_os_path_exists): + mock_os_path_exists.return_value = True + with VolumeManagerBase(self.device_map, 'root_dir', Mock()): + pass + # just pass diff --git a/test/unit/volume_manager/btrfs_test.py b/test/unit/volume_manager/btrfs_test.py index 699a770905a..5ec0fc3c155 100644 --- a/test/unit/volume_manager/btrfs_test.py +++ b/test/unit/volume_manager/btrfs_test.py @@ -1,5 +1,4 @@ import datetime -import logging from unittest.mock import ( patch, call, Mock, mock_open ) @@ -455,32 +454,6 @@ def test_umount_volumes(self): self.volume_manager.toplevel_mount.is_mounted.assert_called_once_with() self.volume_manager.toplevel_mount.umount.assert_called_once_with() - def test_umount_sub_volumes_busy(self): - self.volume_manager.toplevel_mount = Mock() - volume_mount = Mock() - volume_mount.is_mounted = Mock( - return_value=True - ) - volume_mount.umount = Mock( - return_value=False - ) - self.volume_manager.subvol_mount_list = [volume_mount] - assert self.volume_manager.umount_volumes() is False - - def test_umount_toplevel_busy(self): - self.volume_manager.toplevel_mount = Mock() - volume_mount = Mock() - volume_mount.is_mounted = Mock( - return_value=True - ) - self.volume_manager.toplevel_mount.is_mounted = Mock( - return_value=True - ) - self.volume_manager.toplevel_mount.umount = Mock( - return_value=False - ) - assert self.volume_manager.umount_volumes() is False - @patch('kiwi.volume_manager.btrfs.SysConfig') @patch('kiwi.volume_manager.btrfs.DataSync') @patch('kiwi.volume_manager.btrfs.Command.run') @@ -656,14 +629,14 @@ def test_set_property_readonly_root(self, mock_command): ) @patch('kiwi.volume_manager.btrfs.VolumeManagerBtrfs.umount_volumes') - def test_destructor(self, mock_umount_volumes): - mock_umount_volumes.return_value = True - self.volume_manager.toplevel_mount = Mock() - with self._caplog.at_level(logging.INFO): - self.volume_manager.__del__() - mock_umount_volumes.assert_called_once_with() - mock_umount_volumes.reset_mock() - mock_umount_volumes.return_value = False - with self._caplog.at_level(logging.WARNING): - self.volume_manager.__del__() - mock_umount_volumes.assert_called_once_with() + @patch('os.path.exists') + def test_context_manager_exit( + self, mock_os_path_exists, mock_VolumeManagerBtrfs_umount_volumes + ): + mock_os_path_exists.return_value = True + with VolumeManagerBtrfs( + self.device_map, 'root_dir', self.volumes + ) as volume_manager: + volume_manager.toplevel_mount = Mock() + + mock_VolumeManagerBtrfs_umount_volumes.assert_called_once_with() diff --git a/test/unit/volume_manager/lvm_test.py b/test/unit/volume_manager/lvm_test.py index b58e255f447..2f744b54259 100644 --- a/test/unit/volume_manager/lvm_test.py +++ b/test/unit/volume_manager/lvm_test.py @@ -9,7 +9,10 @@ from kiwi.volume_manager.lvm import VolumeManagerLVM from kiwi.defaults import Defaults -from kiwi.exceptions import KiwiVolumeGroupConflict +from kiwi.exceptions import ( + KiwiVolumeGroupConflict, + KiwiCommandError +) from kiwi.xml_state import volume_type @@ -313,7 +316,7 @@ def test_umount_volumes(self): volume_mount = Mock() volume_mount.mountpoint = 'volume_mount_point' self.volume_manager.mount_list = [volume_mount] - assert self.volume_manager.umount_volumes() is True + self.volume_manager.umount_volumes() volume_mount.umount.assert_called_once_with() def test_get_volumes(self): @@ -357,37 +360,23 @@ def test_get_fstab(self): ] @patch('kiwi.volume_manager.lvm.Command.run') - def test_destructor_busy_volumes(self, mock_command): - self.volume_manager.mountpoint = 'tmpdir' - self.volume_manager.volume_group = 'volume_group' - volume_mount = Mock() - volume_mount.is_mounted.return_value = True - volume_mount.umount.return_value = False - volume_mount.mountpoint = 'volume_mount_point' - volume_mount.device = '/dev/volume_group/LVRoot' - self.volume_manager.mount_list = [volume_mount] - - self.volume_manager.__del__() - - volume_mount.umount.assert_called_once_with() - self.volume_manager.volume_group = None - @patch('kiwi.volume_manager.lvm.VolumeManagerLVM.umount_volumes') - @patch('kiwi.volume_manager.lvm.Command.run') - def test_destructor( - self, mock_command, mock_umount_volumes + @patch('os.path.exists') + def test_context_manager_exit( + self, mock_os_path_exists, mock_VolumeManagerLVM_umount_volumes, + mock_Command_run ): - mock_umount_volumes.return_value = True - mock_command.side_effect = Exception - self.volume_manager.mountpoint = 'tmpdir' - self.volume_manager.volume_group = 'volume_group' + mock_os_path_exists.return_value = True + mock_Command_run.side_effect = KiwiCommandError('error') + with self._caplog.at_level(logging.ERROR): + with VolumeManagerLVM( + self.device_map, 'root_dir', self.volumes + ) as volume_manager: + volume_manager.volume_group = 'volume_group' - with self._caplog.at_level(logging.WARNING): - self.volume_manager.__del__() - mock_umount_volumes.assert_called_once_with() - mock_command.assert_called_once_with( - ['vgchange'] + self.volume_manager.lvm_tool_options + [ - '-an', 'volume_group' - ] - ) - self.volume_manager.volume_group = None + mock_VolumeManagerLVM_umount_volumes.assert_called_once_with() + mock_Command_run.assert_called_once_with( + ['vgchange'] + self.volume_manager.lvm_tool_options + [ + '-an', 'volume_group' + ] + )