Refactor FileSystem Factory

With regards to Issue #1486 a discussion came up that the
way factories are implemented are questionable when thinking
about strong typing for the public kiwi interface. This
commit refactors the FileSystem factory to be a real factory
and to add type hints such that its use from an api perspective
is clear and enforced.
This commit is contained in:
Marcus Schäfer 2020-06-26 18:59:13 +02:00
parent e782e62db5
commit 0fee3bbb60
No known key found for this signature in database
GPG Key ID: AD11DD02B44996EF
13 changed files with 117 additions and 131 deletions

View File

@ -348,7 +348,7 @@ class DiskBuilder:
'mount_options': self.custom_root_mount_args,
'create_options': self.custom_root_creation_args
}
filesystem = FileSystem(
filesystem = FileSystem.new(
self.requested_filesystem, device_map['root'],
self.root_dir + '/',
filesystem_custom_parameters
@ -360,7 +360,7 @@ class DiskBuilder:
# create swap on current root device if requested
if self.swap_mbytes:
swap = FileSystem(
swap = FileSystem.new(
'swap', device_map['swap']
)
swap.create_on_device(
@ -669,7 +669,7 @@ class DiskBuilder:
spare_part_data_path = self.root_dir + '{0}/'.format(
self.spare_part_mountpoint
)
filesystem = FileSystem(
filesystem = FileSystem.new(
self.spare_part_fs,
device_map['spare'],
spare_part_data_path,
@ -686,7 +686,7 @@ class DiskBuilder:
'Creating EFI(fat16) filesystem on %s',
device_map['efi'].get_device()
)
filesystem = FileSystem(
filesystem = FileSystem.new(
'fat16', device_map['efi'], self.root_dir + '/boot/efi/'
)
filesystem.create_on_device(
@ -706,7 +706,7 @@ class DiskBuilder:
'Creating boot(%s) filesystem on %s',
boot_filesystem, device_map['boot'].get_device()
)
filesystem = FileSystem(
filesystem = FileSystem.new(
boot_filesystem, device_map['boot'], boot_directory
)
filesystem.create_on_device(

View File

@ -167,7 +167,7 @@ class FileSystemBuilder:
self.blocksize
)
loop_provider.create()
filesystem = FileSystem(
filesystem = FileSystem.new(
self.requested_filesystem, loop_provider,
self.root_dir + os.sep, self.filesystem_custom_parameters
)
@ -182,7 +182,7 @@ class FileSystemBuilder:
def _operate_on_file(self):
default_provider = DeviceProvider()
filesystem = FileSystem(
filesystem = FileSystem.new(
self.requested_filesystem, default_provider,
self.root_dir, self.filesystem_custom_parameters
)

View File

@ -228,7 +228,7 @@ class LiveImageBuilder:
self.xml_state.build_type.get_target_blocksize()
)
loop_provider.create()
live_filesystem = FileSystem(
live_filesystem = FileSystem.new(
name=root_filesystem,
device_provider=loop_provider,
root_dir=self.root_dir + os.sep,
@ -251,7 +251,7 @@ class LiveImageBuilder:
shutil.copy(
root_image.name, self.live_container_dir + '/LiveOS/rootfs.img'
)
live_container_image = FileSystem(
live_container_image = FileSystem.new(
name='squashfs',
device_provider=None,
root_dir=self.live_container_dir,

View File

@ -15,24 +15,18 @@
# You should have received a copy of the GNU General Public License
# along with kiwi. If not, see <http://www.gnu.org/licenses/>
#
# project
from kiwi.filesystem.ext2 import FileSystemExt2
from kiwi.filesystem.ext3 import FileSystemExt3
from kiwi.filesystem.ext4 import FileSystemExt4
from kiwi.filesystem.btrfs import FileSystemBtrfs
from kiwi.filesystem.xfs import FileSystemXfs
from kiwi.filesystem.fat16 import FileSystemFat16
from kiwi.filesystem.fat32 import FileSystemFat32
from kiwi.filesystem.squashfs import FileSystemSquashFs
from kiwi.filesystem.clicfs import FileSystemClicFs
from kiwi.filesystem.swap import FileSystemSwap
from kiwi.exceptions import (
KiwiFileSystemSetupError
import importlib
from typing import Dict
from abc import (
ABCMeta,
abstractmethod
)
# project
from kiwi.exceptions import KiwiFileSystemSetupError
class FileSystem:
class FileSystem(metaclass=ABCMeta):
"""
**FileSystem factory**
@ -41,48 +35,37 @@ class FileSystem:
:param string root_dir: root directory path name
:param dict custom_args: dict of custom filesystem arguments
"""
def __new__(self, name, device_provider, root_dir=None, custom_args=None):
if name == 'ext2':
return FileSystemExt2(
@abstractmethod
def __init__(self) -> None:
return None
@staticmethod
def new(
name: str, device_provider: object,
root_dir: str=None, custom_args: Dict=None # noqa: E252
):
name_map = {
'ext2': 'Ext2',
'ext3': 'Ext3',
'ext4': 'Ext4',
'btrfs': 'Btrfs',
'xfs': 'Xfs',
'fat16': 'Fat16',
'fat32': 'Fat32',
'squashfs': 'SquashFs',
'clicfs': 'ClicFs',
'swap': 'Swap'
}
try:
filesystem = importlib.import_module(
'kiwi.filesystem.{0}'.format(name)
)
return filesystem.__dict__['FileSystem{0}'.format(name_map[name])](
device_provider, root_dir, custom_args
)
elif name == 'ext3':
return FileSystemExt3(
device_provider, root_dir, custom_args
)
elif name == 'ext4':
return FileSystemExt4(
device_provider, root_dir, custom_args
)
elif name == 'btrfs':
return FileSystemBtrfs(
device_provider, root_dir, custom_args
)
elif name == 'xfs':
return FileSystemXfs(
device_provider, root_dir, custom_args
)
elif name == 'fat16':
return FileSystemFat16(
device_provider, root_dir, custom_args
)
elif name == 'fat32':
return FileSystemFat32(
device_provider, root_dir, custom_args
)
elif name == 'squashfs':
return FileSystemSquashFs(
device_provider, root_dir, custom_args
)
elif name == 'clicfs':
return FileSystemClicFs(
device_provider, root_dir, custom_args
)
elif name == 'swap':
return FileSystemSwap(
device_provider, root_dir, custom_args
)
else:
except Exception as issue:
raise KiwiFileSystemSetupError(
'Support for %s filesystem not implemented' % name
'Support for {0} filesystem not implemented: {1}'.format(
name, issue
)
)

View File

@ -90,7 +90,7 @@ class VolumeManagerBtrfs(VolumeManagerBase):
"""
self.setup_mountpoint()
filesystem = FileSystem(
filesystem = FileSystem.new(
name='btrfs',
device_provider=MappedDevice(
device=self.device, device_provider=self.device_provider_root

View File

@ -293,7 +293,7 @@ class VolumeManagerLVM(VolumeManagerBase):
# perform a second lookup of a label specified via the
# rootfs_label from the type setup
volume_label = self.custom_args['root_label']
filesystem = FileSystem(
filesystem = FileSystem.new(
name=filesystem_name,
device_provider=MappedDevice(
device=device_node, device_provider=self.device_provider_root

View File

@ -6,6 +6,7 @@ omit =
[report]
omit =
*/filesystem/__init__.py
*/app.py
*/kiwi.py
*/xml_parse.py

View File

@ -214,7 +214,7 @@ class TestDiskBuilder:
)
assert disk_builder.arch == 'ix86'
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
def test_create_invalid_type_for_install_media(
self, mock_cmd, mock_fs
@ -264,7 +264,7 @@ class TestDiskBuilder:
with raises(KiwiInstallMediaError):
self.disk_builder.create_install_media(result_instance)
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('random.randrange')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
@ -390,7 +390,7 @@ class TestDiskBuilder:
'target_dir'
)
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('random.randrange')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
@ -528,7 +528,7 @@ class TestDiskBuilder:
assert self.boot_image_task.write_system_config_file.call_args_list == \
[]
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.FileSystemSquashFs')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
@ -591,7 +591,7 @@ class TestDiskBuilder:
config={'modules': ['kiwi-overlay']}
)
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
def test_create_disk_standard_root_no_hypervisor_found(
self, mock_command, mock_fs
@ -604,7 +604,7 @@ class TestDiskBuilder:
with raises(KiwiDiskBootImageError):
self.disk_builder.create_disk()
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
def test_create_disk_standard_root_xen_server_boot(
self, mock_command, mock_fs
@ -624,7 +624,7 @@ class TestDiskBuilder:
'root_dir', '/boot/xen.gz'
)
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
def test_create_disk_standard_root_s390_boot(
@ -648,7 +648,7 @@ class TestDiskBuilder:
'ext2', self.device_map['boot'], 'root_dir/boot/zipl/'
)
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
def test_create_disk_standard_root_secure_boot(
@ -666,7 +666,7 @@ class TestDiskBuilder:
bootloader = self.bootloader_config
bootloader.setup_disk_boot_images.assert_called_once_with('0815')
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
def test_create_disk_mdraid_root(
@ -695,7 +695,7 @@ class TestDiskBuilder:
'kiwi_RaidDev': '/dev/md0'
}
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
def test_create_disk_luks_root(
@ -727,7 +727,7 @@ class TestDiskBuilder:
config_file='root_dir/etc/dracut.conf.d/99-luks-boot.conf'
)
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.VolumeManager')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
@ -778,7 +778,7 @@ class TestDiskBuilder:
self.disk_builder.fstab
)
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
def test_create_disk_hybrid_gpt_requested(
@ -795,7 +795,7 @@ class TestDiskBuilder:
self.disk.create_hybrid_mbr.assert_called_once_with()
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
def test_create_disk_force_mbr_requested(
@ -829,7 +829,7 @@ class TestDiskBuilder:
builder.append_unpartitioned_space.assert_called_once_with()
builder.create_disk_format.assert_called_once_with(result)
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
@patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name')
def test_create_disk_spare_part_requested(
@ -900,7 +900,7 @@ class TestDiskBuilder:
loopdevice.create.assert_called_once_with(overwrite=False)
assert partitioner.resize_table.called
@patch('kiwi.builder.disk.FileSystem')
@patch('kiwi.builder.disk.FileSystem.new')
@patch('kiwi.builder.disk.Command.run')
def test_create_disk_format(self, mock_command, mock_fs):
result_instance = mock.Mock()

View File

@ -88,7 +88,7 @@ class TestFileSystemBuilder:
)
@patch('kiwi.builder.filesystem.LoopDevice')
@patch('kiwi.builder.filesystem.FileSystem')
@patch('kiwi.builder.filesystem.FileSystem.new')
@patch('kiwi.builder.filesystem.FileSystemSetup')
@patch('platform.machine')
def test_create_on_loop(
@ -123,7 +123,7 @@ class TestFileSystemBuilder:
'target_dir'
)
@patch('kiwi.builder.filesystem.FileSystem')
@patch('kiwi.builder.filesystem.FileSystem.new')
@patch('kiwi.builder.filesystem.DeviceProvider')
@patch('platform.machine')
def test_create_on_file(

View File

@ -30,11 +30,6 @@ class TestLiveImageBuilder:
return_value=self.setup
)
self.filesystem = mock.Mock()
kiwi.builder.live.FileSystem = mock.Mock(
return_value=self.filesystem
)
self.filesystem_setup = mock.Mock()
kiwi.builder.live.FileSystemSetup = mock.Mock(
return_value=self.filesystem_setup
@ -140,11 +135,12 @@ class TestLiveImageBuilder:
@patch('kiwi.builder.live.shutil')
@patch('kiwi.builder.live.Iso.set_media_tag')
@patch('kiwi.builder.live.FileSystemIsoFs')
@patch('kiwi.builder.live.FileSystem.new')
@patch('kiwi.builder.live.SystemSize')
@patch('kiwi.builder.live.Defaults.get_grub_boot_directory_name')
@patch('os.path.exists')
def test_create_overlay_structure(
self, mock_exists, mock_grub_dir, mock_size,
self, mock_exists, mock_grub_dir, mock_size, mock_filesystem,
mock_isofs, mock_tag, mock_shutil, mock_tmpfile, mock_dtemp,
mock_setup_media_loader_directory
):
@ -154,6 +150,8 @@ class TestLiveImageBuilder:
mock_exists.return_value = True
mock_grub_dir.return_value = 'grub2'
tmpdir_name = ['temp-squashfs', 'temp_media_dir']
filesystem = mock.Mock()
mock_filesystem.return_value = filesystem
def side_effect(prefix, dir):
return tmpdir_name.pop()
@ -179,7 +177,7 @@ class TestLiveImageBuilder:
self.setup.import_cdroot_files.assert_called_once_with('temp_media_dir')
assert kiwi.builder.live.FileSystem.call_args_list == [
assert kiwi.builder.live.FileSystem.new.call_args_list == [
call(
device_provider=self.loop, name='ext4',
root_dir='root_dir/',
@ -195,11 +193,11 @@ class TestLiveImageBuilder:
)
]
self.filesystem.create_on_device.assert_called_once_with()
self.filesystem.sync_data.assert_called_once_with(
filesystem.create_on_device.assert_called_once_with()
filesystem.sync_data.assert_called_once_with(
['image', '.profile', '.kconfig', '.buildenv', 'var/cache/kiwi']
)
self.filesystem.create_on_file.assert_called_once_with('tmpfile')
filesystem.create_on_file.assert_called_once_with('tmpfile')
assert mock_shutil.copy.call_args_list == [
call('tmpfile', 'temp-squashfs/LiveOS/rootfs.img'),

View File

@ -1,6 +1,6 @@
from mock import patch
from pytest import raises
import mock
from mock import Mock
from kiwi.filesystem import FileSystem
@ -10,64 +10,68 @@ from kiwi.exceptions import KiwiFileSystemSetupError
class TestFileSystem:
def test_filesystem_not_implemented(self):
with raises(KiwiFileSystemSetupError):
FileSystem('foo', mock.Mock(), 'root_dir')
FileSystem.new('foo', Mock(), 'root_dir')
@patch('kiwi.filesystem.FileSystemExt2')
def test_factory_init(self):
with raises(TypeError):
FileSystem()
@patch('kiwi.filesystem.ext2.FileSystemExt2')
def test_filesystem_ext2(self, mock_ext2):
provider = mock.Mock()
FileSystem('ext2', provider, 'root_dir')
provider = Mock()
FileSystem.new('ext2', provider, 'root_dir')
mock_ext2.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemExt3')
@patch('kiwi.filesystem.ext3.FileSystemExt3')
def test_filesystem_ext3(self, mock_ext3):
provider = mock.Mock()
FileSystem('ext3', provider, 'root_dir')
provider = Mock()
FileSystem.new('ext3', provider, 'root_dir')
mock_ext3.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemExt4')
@patch('kiwi.filesystem.ext4.FileSystemExt4')
def test_filesystem_ext4(self, mock_ext4):
provider = mock.Mock()
FileSystem('ext4', provider, 'root_dir')
provider = Mock()
FileSystem.new('ext4', provider, 'root_dir')
mock_ext4.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemXfs')
@patch('kiwi.filesystem.xfs.FileSystemXfs')
def test_filesystem_xfs(self, mock_xfs):
provider = mock.Mock()
FileSystem('xfs', provider, 'root_dir')
provider = Mock()
FileSystem.new('xfs', provider, 'root_dir')
mock_xfs.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemBtrfs')
@patch('kiwi.filesystem.btrfs.FileSystemBtrfs')
def test_filesystem_btrfs(self, mock_btrfs):
provider = mock.Mock()
FileSystem('btrfs', provider, 'root_dir')
provider = Mock()
FileSystem.new('btrfs', provider, 'root_dir')
mock_btrfs.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemFat16')
@patch('kiwi.filesystem.fat16.FileSystemFat16')
def test_filesystem_fat16(self, mock_fat16):
provider = mock.Mock()
FileSystem('fat16', provider, 'root_dir')
provider = Mock()
FileSystem.new('fat16', provider, 'root_dir')
mock_fat16.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemFat32')
@patch('kiwi.filesystem.fat32.FileSystemFat32')
def test_filesystem_fat32(self, mock_fat32):
provider = mock.Mock()
FileSystem('fat32', provider, 'root_dir')
provider = Mock()
FileSystem.new('fat32', provider, 'root_dir')
mock_fat32.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemSquashFs')
@patch('kiwi.filesystem.squashfs.FileSystemSquashFs')
def test_filesystem_squashfs(self, mock_squashfs):
provider = mock.Mock()
FileSystem('squashfs', provider, 'root_dir')
provider = Mock()
FileSystem.new('squashfs', provider, 'root_dir')
mock_squashfs.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemClicFs')
@patch('kiwi.filesystem.clicfs.FileSystemClicFs')
def test_filesystem_clicfs(self, mock_clicfs):
provider = mock.Mock()
FileSystem('clicfs', provider, 'root_dir')
provider = Mock()
FileSystem.new('clicfs', provider, 'root_dir')
mock_clicfs.assert_called_once_with(provider, 'root_dir', None)
@patch('kiwi.filesystem.FileSystemSwap')
@patch('kiwi.filesystem.swap.FileSystemSwap')
def test_filesystem_swap(self, mock_swap):
provider = mock.Mock()
FileSystem('swap', provider)
provider = Mock()
FileSystem.new('swap', provider)
mock_swap.assert_called_once_with(provider, None, None)

View File

@ -77,7 +77,7 @@ class TestVolumeManagerBtrfs:
@patch('os.path.exists')
@patch('kiwi.volume_manager.btrfs.Command.run')
@patch('kiwi.volume_manager.btrfs.FileSystem')
@patch('kiwi.volume_manager.btrfs.FileSystem.new')
@patch('kiwi.volume_manager.btrfs.MappedDevice')
@patch('kiwi.volume_manager.btrfs.MountManager')
@patch('kiwi.volume_manager.base.mkdtemp')
@ -108,7 +108,7 @@ class TestVolumeManagerBtrfs:
@patch('os.path.exists')
@patch('kiwi.volume_manager.btrfs.Command.run')
@patch('kiwi.volume_manager.btrfs.FileSystem')
@patch('kiwi.volume_manager.btrfs.FileSystem.new')
@patch('kiwi.volume_manager.btrfs.MappedDevice')
@patch('kiwi.volume_manager.btrfs.MountManager')
@patch('kiwi.volume_manager.base.mkdtemp')
@ -150,7 +150,7 @@ class TestVolumeManagerBtrfs:
@patch('os.path.exists')
@patch('kiwi.volume_manager.btrfs.Command.run')
@patch('kiwi.volume_manager.btrfs.FileSystem')
@patch('kiwi.volume_manager.btrfs.FileSystem.new')
@patch('kiwi.volume_manager.btrfs.MappedDevice')
@patch('kiwi.volume_manager.btrfs.MountManager')
@patch('kiwi.volume_manager.base.mkdtemp')
@ -280,7 +280,7 @@ class TestVolumeManagerBtrfs:
@patch('os.path.exists')
@patch('kiwi.volume_manager.btrfs.Command.run')
@patch('kiwi.volume_manager.btrfs.FileSystem')
@patch('kiwi.volume_manager.btrfs.FileSystem.new')
@patch('kiwi.volume_manager.btrfs.MappedDevice')
@patch('kiwi.volume_manager.btrfs.MountManager')
@patch('kiwi.volume_manager.base.mkdtemp')

View File

@ -144,7 +144,7 @@ class TestVolumeManagerLVM:
@patch('kiwi.path.Path.create')
@patch('kiwi.volume_manager.base.SystemSize')
@patch('kiwi.volume_manager.lvm.Command.run')
@patch('kiwi.volume_manager.lvm.FileSystem')
@patch('kiwi.volume_manager.lvm.FileSystem.new')
@patch('kiwi.volume_manager.lvm.MappedDevice')
@patch('kiwi.volume_manager.lvm.MountManager')
@patch('kiwi.volume_manager.base.VolumeManagerBase.apply_attributes_on_volume')