Skip to content

Commit

Permalink
Type cleanup, no use of Any type in disk builder
Browse files Browse the repository at this point in the history
Use proper Union declaration for system variable and add
consistency layer into Filesystem/VolumeManager classes to
meet the type declaration as well as to simplify further
refactoring on these classes
  • Loading branch information
schaefi committed Jan 15, 2024
1 parent 075a58d commit 4aca8a2
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 13 deletions.
29 changes: 19 additions & 10 deletions kiwi/builder/disk.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import os
import logging
from typing import (
Dict, List, Optional, Tuple, Any
Dict, List, Optional, Tuple, Union
)

# project
Expand Down Expand Up @@ -49,6 +49,7 @@
from kiwi.filesystem import FileSystem
from kiwi.filesystem.squashfs import FileSystemSquashFs
from kiwi.volume_manager import VolumeManager
from kiwi.volume_manager.base import VolumeManagerBase
from kiwi.command import Command
from kiwi.system.setup import SystemSetup
from kiwi.builder.install import InstallImageBuilder
Expand Down Expand Up @@ -234,7 +235,8 @@ def create_disk(self) -> Result:
# an instance of a class with the sync_data capability
# representing the entire image system except for the boot/ area
# which could live on another part of the disk
system: Any = None
system: Union[FileSystemBase, VolumeManagerBase] = \
FileSystemBase(DeviceProvider())

# an instance of a class with the sync_data capability
# representing the boot/ area of the disk if not part of
Expand Down Expand Up @@ -644,7 +646,7 @@ def _build_main_system(
self,
device_map: Dict,
disk: Disk,
system: Any,
system: Union[FileSystemBase, VolumeManagerBase],
system_boot: Optional[FileSystemBase],
system_efi: Optional[FileSystemBase],
system_spare: Optional[FileSystemBase],
Expand Down Expand Up @@ -1136,13 +1138,15 @@ def _write_crypttab_to_system_image(
)

def _write_generic_fstab_to_system_image(
self, device_map: Dict, system: Any
self, device_map: Dict,
system: Union[FileSystemBase, VolumeManagerBase]
) -> None:
log.info('Creating generic system etc/fstab')
self._write_generic_fstab(device_map, self.system_setup, system)

def _write_generic_fstab_to_boot_image(
self, device_map: Dict, system: Any
self, device_map: Dict,
system: Union[FileSystemBase, VolumeManagerBase]
) -> None:
if self.initrd_system == 'kiwi':
log.info('Creating generic boot image etc/fstab')
Expand All @@ -1152,7 +1156,7 @@ def _write_generic_fstab_to_boot_image(

def _write_generic_fstab(
self, device_map: Dict, setup: SystemSetup,
system: Any
system: Union[FileSystemBase, VolumeManagerBase]
) -> None:
root_is_snapshot = \
self.xml_state.build_type.get_btrfs_root_is_snapshot()
Expand Down Expand Up @@ -1291,7 +1295,8 @@ def _write_recovery_metadata_to_boot_image(self) -> None:
)

def _write_bootloader_meta_data_to_system_image(
self, device_map: Dict, disk: Disk, system: Any
self, device_map: Dict, disk: Disk,
system: Union[FileSystemBase, VolumeManagerBase]
) -> None:
if self.bootloader != 'custom':
log.info('Creating %s bootloader configuration', self.bootloader)
Expand Down Expand Up @@ -1353,7 +1358,8 @@ def _write_bootloader_meta_data_to_system_image(
)

def _sync_system_to_image(
self, device_map: Dict, system: Any,
self, device_map: Dict,
system: Union[FileSystemBase, VolumeManagerBase],
system_boot: Optional[FileSystemBase],
system_efi: Optional[FileSystemBase],
system_spare: Optional[FileSystemBase],
Expand Down Expand Up @@ -1551,7 +1557,8 @@ def _sync_system_to_image(
self.integrity_root.write_integrity_metadata()

def _install_bootloader(
self, device_map: Dict, disk, system: Any
self, device_map: Dict, disk,
system: Union[FileSystemBase, VolumeManagerBase]
) -> None:
root_device = device_map['root']
boot_device = root_device
Expand Down Expand Up @@ -1632,7 +1639,9 @@ def _install_bootloader(
self.diskname, boot_device.get_device()
)

def _setup_property_root_is_readonly_snapshot(self, system: Any) -> None:
def _setup_property_root_is_readonly_snapshot(
self, system: Union[FileSystemBase, VolumeManagerBase]
) -> None:
if self.volume_manager_name:
root_is_snapshot = \
self.xml_state.build_type.get_btrfs_root_is_snapshot()
Expand Down
50 changes: 50 additions & 0 deletions kiwi/filesystem/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,56 @@ def umount(self) -> None:
log.info('umount %s instance', type(self).__name__)
self.filesystem_mount.umount()

def umount_volumes(self) -> None:
"""
Consistency layer with regards to VolumeManager classes
Invokes umount
"""
self.umount()

def mount_volumes(self) -> None:
"""
Consistency layer with regards to VolumeManager classes
Invokes mount
"""
self.mount()

def get_volumes(self) -> Dict:
"""
Consistency layer with regards to VolumeManager classes
Raises
"""
raise NotImplementedError

def get_root_volume_name(self) -> None:
"""
Consistency layer with regards to VolumeManager classes
Raises
"""
raise NotImplementedError

def get_fstab(
self, persistency_type: str='by-label', filesystem_name: str=''
) -> List[str]:
"""
Consistency layer with regards to VolumeManager classes
Raises
"""
raise NotImplementedError

def set_property_readonly_root(self) -> None:
"""
Consistency layer with regards to VolumeManager classes
Raises
"""
raise NotImplementedError

def _map_size(self, size: float, from_unit: str, to_unit: str) -> float:
"""
Return byte size value for given size and unit
Expand Down
16 changes: 16 additions & 0 deletions kiwi/volume_manager/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,22 @@ def umount_volumes(self):
"""
raise NotImplementedError

def umount(self):
"""
Consistency layer with regards to FileSystem classes
Invokes umount_volumes
"""
self.umount_volumes()

def mount(self):
"""
Consistency layer with regards to FileSystem classes
Invokes mount_volumes
"""
self.mount_volumes()

def is_loop(self):
"""
Check if storage provider is loop based
Expand Down
4 changes: 3 additions & 1 deletion kiwi/volume_manager/btrfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,9 @@ def create_volumes(self, filesystem_name):
volume_mount
)

def get_fstab(self, persistency_type='by-label', filesystem_name=None):
def get_fstab(
self, persistency_type: str='by-label', filesystem_name: str=''
) -> List[str]:
"""
Implements creation of the fstab entries. The method
returns a list of fstab compatible entries
Expand Down
9 changes: 7 additions & 2 deletions kiwi/volume_manager/lvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
#
import os
import logging
from typing import (
Dict, List
)

# project
import kiwi.defaults as defaults
Expand Down Expand Up @@ -228,7 +231,9 @@ def create_volumes(self, filesystem_name):
for mount_path in Path.sort_by_hierarchy(sorted(volume_paths.keys())):
self.mount_list.append(volume_paths[mount_path])

def get_fstab(self, persistency_type, filesystem_name):
def get_fstab(
self, persistency_type: str='by-label', filesystem_name: str=''
) -> List[str]:
"""
Implements creation of the fstab entries. The method
returns a list of fstab compatible entries
Expand Down Expand Up @@ -266,7 +271,7 @@ def get_fstab(self, persistency_type, filesystem_name):

return fstab_entries

def get_volumes(self):
def get_volumes(self) -> Dict:
"""
Return dict of volumes
Expand Down
28 changes: 28 additions & 0 deletions test/unit/filesystem/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,40 @@ def test_umount(self):
self.fsbase.umount()
mount.umount.assert_called_once_with()

def test_umount_volumes(self):
mount = Mock()
self.fsbase.filesystem_mount = mount
self.fsbase.umount_volumes()
mount.umount.assert_called_once_with()

def test_mount(self):
mount = Mock()
self.fsbase.filesystem_mount = mount
self.fsbase.mount()
mount.mount.assert_called_once_with([])

def test_mount_volumes(self):
mount = Mock()
self.fsbase.filesystem_mount = mount
self.fsbase.mount_volumes()
mount.mount.assert_called_once_with([])

def test_get_volumes(self):
with raises(NotImplementedError):
self.fsbase.get_volumes()

def test_get_root_volume_name(self):
with raises(NotImplementedError):
self.fsbase.get_root_volume_name()

def test_get_fstab(self):
with raises(NotImplementedError):
self.fsbase.get_fstab()

def test_set_property_readonly_root(self):
with raises(NotImplementedError):
self.fsbase.set_property_readonly_root()

def test_fs_size(self):
# size of 100k must be 100k (default unit)
assert self.fsbase._fs_size(100) == '100'
Expand Down
8 changes: 8 additions & 0 deletions test/unit/volume_manager/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,18 @@ def test_mount_volumes(self):
with raises(NotImplementedError):
self.volume_manager.mount_volumes()

def test_mount(self):
with raises(NotImplementedError):
self.volume_manager.mount()

def test_umount_volumes(self):
with raises(NotImplementedError):
self.volume_manager.umount_volumes()

def test_umount(self):
with raises(NotImplementedError):
self.volume_manager.umount()

def test_get_volumes(self):
with raises(NotImplementedError):
self.volume_manager.get_volumes()
Expand Down

0 comments on commit 4aca8a2

Please sign in to comment.