From 99be52baec3ff3a93f569abdaaad55e691bddbb6 Mon Sep 17 00:00:00 2001 From: David Cassany Date: Tue, 15 Dec 2020 12:38:33 +0100 Subject: [PATCH] Refactor container factory classes This commit refactors the container related classes to turn them into proper factory classes and also includes type hints to facilitate it's use from an API POV. Related to #1498 --- kiwi/builder/container.py | 4 +-- kiwi/container/__init__.py | 45 +++++++++++++++--------- kiwi/container/oci.py | 2 +- kiwi/container/setup/__init__.py | 38 +++++++++++++------- kiwi/oci_tools/__init__.py | 36 +++++++++++++------ kiwi/system/root_import/oci.py | 2 +- test/unit/builder/container_test.py | 12 +++---- test/unit/container/init_test.py | 20 +++++------ test/unit/container/oci_test.py | 4 +-- test/unit/container/setup/init_test.py | 14 ++++---- test/unit/oci_tools/init_test.py | 10 +++--- test/unit/system/root_import/oci_test.py | 12 +++---- 12 files changed, 118 insertions(+), 81 deletions(-) diff --git a/kiwi/builder/container.py b/kiwi/builder/container.py index c16f9ec7..6894b6f7 100644 --- a/kiwi/builder/container.py +++ b/kiwi/builder/container.py @@ -113,7 +113,7 @@ class ContainerBuilder: log.info( 'Setting up %s container', self.requested_container_type ) - container_setup = ContainerSetup( + container_setup = ContainerSetup.new( self.requested_container_type, self.root_dir, self.container_config ) @@ -130,7 +130,7 @@ class ContainerBuilder: log.info( '--> Creating container image' ) - container_image = ContainerImage( + container_image = ContainerImage.new( self.requested_container_type, self.root_dir, self.container_config ) self.filename = container_image.create( diff --git a/kiwi/container/__init__.py b/kiwi/container/__init__.py index eb515790..c6403161 100644 --- a/kiwi/container/__init__.py +++ b/kiwi/container/__init__.py @@ -15,16 +15,18 @@ # You should have received a copy of the GNU General Public License # along with kiwi. If not, see # -# project -from kiwi.container.oci import ContainerImageOCI -from kiwi.container.appx import ContainerImageAppx +import importlib +from abc import ( + ABCMeta, + abstractmethod +) from kiwi.exceptions import ( KiwiContainerImageSetupError ) -class ContainerImage: +class ContainerImage(metaclass=ABCMeta): """ **Container Image factory** @@ -32,20 +34,29 @@ class ContainerImage: :param string root_dir: root directory path name :param dict custom_args: custom arguments """ - def __new__(self, name, root_dir, custom_args=None): - if name == 'docker': - return ContainerImageOCI( - root_dir, 'docker-archive', custom_args=custom_args + @abstractmethod + def __init__(self) -> None: + return None # pragma: no cover + + @staticmethod + def new(name: str, root_dir: str, custom_args: dict = None): + name_map = { + 'docker': 'OCI', + 'oci': 'OCI', + 'appx': 'Appx' + } + args_map = { + 'docker': [root_dir, 'docker-archive', custom_args], + 'oci': [root_dir, 'oci-archive', custom_args], + 'appx': [root_dir, custom_args] + } + try: + container_image = importlib.import_module( + 'kiwi.container.{}'.format('oci' if name == 'docker' else name) ) - elif name == 'oci': - return ContainerImageOCI( - root_dir, 'oci-archive', custom_args=custom_args - ) - elif name == 'appx': - return ContainerImageAppx( - root_dir, custom_args=custom_args - ) - else: + module_name = 'ContainerImage{}'.format(name_map[name]) + return container_image.__dict__[module_name](*args_map[name]) + except Exception: raise KiwiContainerImageSetupError( 'Support for {0} container not implemented'.format(name) ) diff --git a/kiwi/container/oci.py b/kiwi/container/oci.py index 7ad5f38e..403ac9f4 100644 --- a/kiwi/container/oci.py +++ b/kiwi/container/oci.py @@ -118,7 +118,7 @@ class ContainerImageOCI: exclude_list.append('sys/*') exclude_list.append('proc/*') - oci = OCI() + oci = OCI.new() if base_image: oci.import_container_image( 'oci-archive:{0}:{1}'.format( diff --git a/kiwi/container/setup/__init__.py b/kiwi/container/setup/__init__.py index 43e7b37b..f84b4dfa 100644 --- a/kiwi/container/setup/__init__.py +++ b/kiwi/container/setup/__init__.py @@ -15,28 +15,40 @@ # You should have received a copy of the GNU General Public License # along with kiwi. If not, see # -# project -from kiwi.container.setup.docker import ContainerSetupDocker -from kiwi.container.setup.oci import ContainerSetupOCI -from kiwi.container.setup.appx import ContainerSetupAppx +import importlib +from abc import ( + ABCMeta, + abstractmethod +) +# project from kiwi.exceptions import ( KiwiContainerSetupError ) -class ContainerSetup: +class ContainerSetup(metaclass=ABCMeta): """ container setup factory """ - def __new__(self, name, root_dir, custom_args=None): - if name == 'docker': - return ContainerSetupDocker(root_dir, custom_args) - elif name == 'oci': - return ContainerSetupOCI(root_dir, custom_args) - elif name == 'appx': - return ContainerSetupAppx(root_dir, custom_args) - else: + @abstractmethod + def __init__(self) -> None: + return None # pragma: no cover + + @staticmethod + def new(name: str, root_dir: str, custom_args: dict = None): + name_map = { + 'docker': 'Docker', + 'oci': 'OCI', + 'appx': 'Appx' + } + try: + container_setup = importlib.import_module( + 'kiwi.container.setup.{}'.format(name) + ) + module_name = 'ContainerSetup{}'.format(name_map[name]) + return container_setup.__dict__[module_name](root_dir, custom_args) + except Exception: raise KiwiContainerSetupError( 'Support for {0} container not implemented'.format(name) ) diff --git a/kiwi/oci_tools/__init__.py b/kiwi/oci_tools/__init__.py index a5d0be46..8951d967 100644 --- a/kiwi/oci_tools/__init__.py +++ b/kiwi/oci_tools/__init__.py @@ -15,28 +15,42 @@ # You should have received a copy of the GNU General Public License # along with kiwi. If not, see # -# project -from kiwi.oci_tools.umoci import OCIUmoci -from kiwi.oci_tools.buildah import OCIBuildah -from kiwi.runtime_config import RuntimeConfig +import importlib +from abc import ( + ABCMeta, + abstractmethod +) +# project +from kiwi.runtime_config import RuntimeConfig from kiwi.exceptions import ( KiwiOCIArchiveToolError ) -class OCI: +class OCI(metaclass=ABCMeta): """ **OCI Factory** """ - def __new__(self): + @abstractmethod + def __init__(self) -> None: + return None # pragma: no cover + + @staticmethod + def new(): + name_map = { + 'umoci': 'Umoci', + 'buildah': 'Buildah' + } runtime_config = RuntimeConfig() tool_name = runtime_config.get_oci_archive_tool() - if tool_name == 'umoci': - return OCIUmoci() - elif tool_name == 'buildah': - return OCIBuildah() - else: + try: + oci_tool = importlib.import_module( + 'kiwi.oci_tools.{}'.format(tool_name) + ) + module_name = 'OCI{}'.format(name_map[tool_name]) + return oci_tool.__dict__[module_name]() + except Exception: raise KiwiOCIArchiveToolError( 'No support for {0} tool available'.format(tool_name) ) diff --git a/kiwi/system/root_import/oci.py b/kiwi/system/root_import/oci.py index 5eb07b30..6d8a3670 100644 --- a/kiwi/system/root_import/oci.py +++ b/kiwi/system/root_import/oci.py @@ -55,7 +55,7 @@ class RootImportOCI(RootImportBase): log.warning('Bypassing base image URI to OCI tools') image_uri = self.unknown_uri - oci = OCI() + oci = OCI.new() oci.import_container_image(image_uri) oci.unpack() oci.import_rootfs(self.root_dir) diff --git a/test/unit/builder/container_test.py b/test/unit/builder/container_test.py index 2a139232..ace87c86 100644 --- a/test/unit/builder/container_test.py +++ b/test/unit/builder/container_test.py @@ -112,22 +112,22 @@ class TestContainerBuilder: @patch('kiwi.builder.container.ContainerImage') def test_create(self, mock_image, mock_setup): container_setup = mock.Mock() - mock_setup.return_value = container_setup + mock_setup.new.return_value = container_setup container_image = mock.Mock() container_image.create = mock.Mock( return_value='target_dir/image_name.x86_64-1.2.3.docker.tar.xz' ) - mock_image.return_value = container_image + mock_image.new.return_value = container_image self.setup.export_package_verification.return_value = '.verified' self.setup.export_package_changes.return_value = '.changes' self.setup.export_package_list.return_value = '.packages' self.container.base_image = None self.container.create() - mock_setup.assert_called_once_with( + mock_setup.new.assert_called_once_with( 'docker', 'root_dir', self.container_config ) container_setup.setup.assert_called_once_with() - mock_image.assert_called_once_with( + mock_image.new.assert_called_once_with( 'docker', 'root_dir', self.container_config ) container_image.create.assert_called_once_with( @@ -188,7 +188,7 @@ class TestContainerBuilder: container_image.create = mock.Mock( return_value='target_dir/image_name.x86_64-1.2.3.docker.tar.xz' ) - mock_image.return_value = container_image + mock_image.new.return_value = container_image container = ContainerBuilder( self.xml_state, 'target_dir', 'root_dir' @@ -212,7 +212,7 @@ class TestContainerBuilder: 'checksumvalue', 'root_dir/image/imported_root.md5' ) - mock_image.assert_called_once_with( + mock_image.new.assert_called_once_with( 'docker', 'root_dir', self.container_config ) container_image.create.assert_called_once_with( diff --git a/test/unit/container/init_test.py b/test/unit/container/init_test.py index 7b9fa310..04eebbc8 100644 --- a/test/unit/container/init_test.py +++ b/test/unit/container/init_test.py @@ -9,25 +9,25 @@ from kiwi.exceptions import KiwiContainerImageSetupError class TestContainerImage: def test_container_image_not_implemented(self): with raises(KiwiContainerImageSetupError): - ContainerImage('foo', 'root_dir') + ContainerImage.new('foo', 'root_dir') - @patch('kiwi.container.ContainerImageOCI') + @patch('kiwi.container.oci.ContainerImageOCI') def test_container_image_docker(self, mock_docker): - ContainerImage('docker', 'root_dir') + ContainerImage.new('docker', 'root_dir') mock_docker.assert_called_once_with( - 'root_dir', 'docker-archive', custom_args=None + 'root_dir', 'docker-archive', None ) - @patch('kiwi.container.ContainerImageOCI') + @patch('kiwi.container.oci.ContainerImageOCI') def test_container_image_oci(self, mock_oci): - ContainerImage('oci', 'root_dir') + ContainerImage.new('oci', 'root_dir') mock_oci.assert_called_once_with( - 'root_dir', 'oci-archive', custom_args=None + 'root_dir', 'oci-archive', None ) - @patch('kiwi.container.ContainerImageAppx') + @patch('kiwi.container.appx.ContainerImageAppx') def test_container_image_appx(self, mock_appx): - ContainerImage('appx', 'root_dir') + ContainerImage.new('appx', 'root_dir') mock_appx.assert_called_once_with( - 'root_dir', custom_args=None + 'root_dir', None ) diff --git a/test/unit/container/oci_test.py b/test/unit/container/oci_test.py index 01f0d82d..761f1b55 100644 --- a/test/unit/container/oci_test.py +++ b/test/unit/container/oci_test.py @@ -120,7 +120,7 @@ class TestContainerImageOCI: def test_create_oci_archive(self, mock_cache, mock_OCI): mock_cache.return_value = 'var/cache/kiwi' mock_oci = mock.Mock() - mock_OCI.return_value = mock_oci + mock_OCI.new.return_value = mock_oci self.oci.runtime_config.get_container_compression = mock.Mock( return_value=None @@ -164,7 +164,7 @@ class TestContainerImageOCI: ): mock_cache.return_value = 'var/cache/kiwi' mock_oci = mock.Mock() - mock_OCI.return_value = mock_oci + mock_OCI.new.return_value = mock_oci self.oci.create('result.tar', 'root_dir/image/image_file') diff --git a/test/unit/container/setup/init_test.py b/test/unit/container/setup/init_test.py index 40ce4200..a57caca6 100644 --- a/test/unit/container/setup/init_test.py +++ b/test/unit/container/setup/init_test.py @@ -9,19 +9,19 @@ from kiwi.exceptions import KiwiContainerSetupError class TestContainerSetup: def test_container_not_implemented(self): with raises(KiwiContainerSetupError): - ContainerSetup('foo', 'root_dir') + ContainerSetup.new('foo', 'root_dir') - @patch('kiwi.container.setup.ContainerSetupDocker') + @patch('kiwi.container.setup.docker.ContainerSetupDocker') def test_container_docker(self, mock_docker): - ContainerSetup('docker', 'root_dir') + ContainerSetup.new('docker', 'root_dir') mock_docker.assert_called_once_with('root_dir', None) - @patch('kiwi.container.setup.ContainerSetupOCI') + @patch('kiwi.container.setup.oci.ContainerSetupOCI') def test_container_oci(self, mock_oci): - ContainerSetup('oci', 'root_dir') + ContainerSetup.new('oci', 'root_dir') mock_oci.assert_called_once_with('root_dir', None) - @patch('kiwi.container.setup.ContainerSetupAppx') + @patch('kiwi.container.setup.appx.ContainerSetupAppx') def test_container_appx(self, mock_appx): - ContainerSetup('appx', 'root_dir') + ContainerSetup.new('appx', 'root_dir') mock_appx.assert_called_once_with('root_dir', None) diff --git a/test/unit/oci_tools/init_test.py b/test/unit/oci_tools/init_test.py index f61f6ea2..7f303c6b 100644 --- a/test/unit/oci_tools/init_test.py +++ b/test/unit/oci_tools/init_test.py @@ -15,24 +15,24 @@ class TestOCI: self.runtime_config = Mock() self.runtime_config.get_oci_archive_tool = Mock() - @patch('kiwi.oci_tools.OCIUmoci') + @patch('kiwi.oci_tools.umoci.OCIUmoci') @patch('kiwi.oci_tools.RuntimeConfig') def test_oci_tool_umoci( self, mock_RuntimeConfig, mock_OCIUmoci ): self.runtime_config.get_oci_archive_tool.return_value = 'umoci' mock_RuntimeConfig.return_value = self.runtime_config - OCI() + OCI.new() mock_OCIUmoci.assert_called_once_with() - @patch('kiwi.oci_tools.OCIBuildah') + @patch('kiwi.oci_tools.buildah.OCIBuildah') @patch('kiwi.oci_tools.RuntimeConfig') def test_oci_tool_buildah( self, mock_RuntimeConfig, mock_OCIBuildah ): self.runtime_config.get_oci_archive_tool.return_value = 'buildah' mock_RuntimeConfig.return_value = self.runtime_config - OCI() + OCI.new() mock_OCIBuildah.assert_called_once_with() @patch('kiwi.oci_tools.RuntimeConfig') @@ -40,4 +40,4 @@ class TestOCI: self.runtime_config.get_oci_archive_tool.return_value = 'foo' mock_RuntimeConfig.return_value = self.runtime_config with raises(KiwiOCIArchiveToolError): - OCI() + OCI.new() diff --git a/test/unit/system/root_import/oci_test.py b/test/unit/system/root_import/oci_test.py index 71646d81..0981b2b2 100644 --- a/test/unit/system/root_import/oci_test.py +++ b/test/unit/system/root_import/oci_test.py @@ -42,7 +42,7 @@ class TestRootImportOCI: @patch('kiwi.system.root_import.oci.OCI') def test_sync_data(self, mock_OCI, mock_path, mock_md5, mock_compress): oci = Mock() - mock_OCI.return_value = oci + mock_OCI.new.return_value = oci md5 = Mock() mock_md5.return_value = Mock() @@ -52,7 +52,7 @@ class TestRootImportOCI: self.oci_import.sync_data() - mock_OCI.assert_called_once_with() + mock_OCI.new.assert_called_once_with() oci.unpack.assert_called_once_with() oci.import_rootfs.assert_called_once_with( @@ -70,7 +70,7 @@ class TestRootImportOCI: self, mock_OCI, mock_path, mock_md5, mock_compress ): oci = Mock() - mock_OCI.return_value = oci + mock_OCI.new.return_value = oci md5 = Mock() mock_md5.return_value = Mock() @@ -80,7 +80,7 @@ class TestRootImportOCI: self.oci_import.sync_data() - mock_OCI.assert_called_once_with() + mock_OCI.new.assert_called_once_with() oci.unpack.assert_called_once_with() oci.import_rootfs.assert_called_once_with( @@ -100,7 +100,7 @@ class TestRootImportOCI: ): mock_exists.return_value = True oci = Mock() - mock_OCI.return_value = oci + mock_OCI.new.return_value = oci md5 = Mock() mock_md5.return_value = Mock() with patch.dict('os.environ', {'HOME': '../data'}): @@ -111,7 +111,7 @@ class TestRootImportOCI: with self._caplog.at_level(logging.WARNING): oci_import.sync_data() - mock_OCI.assert_called_once_with() + mock_OCI.new.assert_called_once_with() oci.import_container_image.assert_called_once_with( 'docker:image:tag' )