Merge pull request #2450 from OSInside/nested_build_process

Move Raid Luks and Integrity to context manager

kiwi/builder/disk.py

@@ -349,26 +349,33 @@ def create_disk(self) -> Result:
                     disk.public_partition_id_map['kiwi_RootPart'] = \
                         disk.public_partition_id_map['kiwi_ROPart']
 
-                # create raid on current root device if requested
-                if self.mdraid:
-                    raid_root = self._raid_instance(device_map)
-                    device_map = self._map_raid(device_map, disk, raid_root)
-
-                # create integrity on current root device if requested
-                if self.integrity:
-                    integrity_root = self._integrity_instance(device_map)
-                    device_map = self._map_integrity(device_map, integrity_root)
-
-                # create luks on current root device if requested
-                if self.luks is not None:
-                    luks_root = self._luks_instance(device_map)
-                    device_map = self._map_luks(device_map, luks_root)
-
-                # create system layout for root system
-                self._create_system_instance(device_map)
-
-                # sync system data, configure system, setup loader and initrd
-                self._process_build(device_map, disk)
+                with ExitStack() as stack:
+                    if self.mdraid:
+                        # create raid on current root device
+                        raid_root = self._raid_instance(device_map)
+                        stack.push(raid_root)
+                        device_map = self._map_raid(
+                            device_map, disk, raid_root
+                        )
+
+                    if self.integrity:
+                        # create integrity on current root device
+                        integrity_root = self._integrity_instance(device_map)
+                        stack.push(integrity_root)
+                        device_map = self._map_integrity(
+                            device_map, integrity_root
+                        )
+
+                    if self.luks is not None:
+                        # create luks on current root device
+                        luks_root = self._luks_instance(device_map)
+                        stack.push(luks_root)
+                        device_map = self._map_luks(
+                            device_map, luks_root
+                        )
+
+                    # build bootable disk
+                    self._process_build(device_map, disk)
 
         # store image bundle_format in result
         if self.bundle_format:
@@ -703,6 +710,9 @@ 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]] = \

kiwi/storage/integrity_device.py

@@ -99,6 +99,9 @@ def __init__(
         self.integrity_metadata_file: Optional[IO[bytes]] = None
         self.credentials = credentials
 
+    def __enter__(self):
+        return self
+
     def get_device(self) -> Optional[MappedDevice]:
         """
         Instance of MappedDevice providing the dm_integrity device
@@ -298,15 +301,15 @@ def _get_integrity_superblock(self) -> Dict[str, Union[str, List[str]]]:
                     integrity[entry[0]] = entry[1]
         return integrity
 
-    def __del__(self):
+    def __exit__(self, exc_type, exc_value, traceback):
         if self.integrity_device:
-            log.info('Cleaning up %s instance', type(self).__name__)
             try:
                 Command.run(
                     ['integritysetup', 'close', self.integrity_name]
                 )
-            except Exception:
-                log.warning(
-                    'Shutdown of integrity map %s failed, %s still busy',
-                    self.integrity_name, self.integrity_device
+            except Exception as issue:
+                log.error(
+                    'Shutdown of integrity map {0}:{1} failed with {2}'.format(
+                        self.integrity_name, self.integrity_device, issue
+                    )
                 )

kiwi/storage/luks_device.py

@@ -57,6 +57,9 @@ def __init__(self, storage_provider: DeviceProvider) -> None:
             ]
         }
 
+    def __enter__(self):
+        return self
+
     def get_device(self) -> Optional[MappedDevice]:
         """
         Instance of MappedDevice providing the luks device
@@ -208,15 +211,15 @@ def create_random_keyfile(filename: str) -> None:
             keyfile.write(os.urandom(Defaults.get_luks_key_length()))
         os.chmod(filename, 0o600)
 
-    def __del__(self):
+    def __exit__(self, exc_type, exc_value, traceback):
         if self.luks_device:
-            log.info('Cleaning up %s instance', type(self).__name__)
             try:
                 Command.run(
                     ['cryptsetup', 'luksClose', self.luks_name]
                 )
-            except Exception:
-                log.warning(
-                    'Shutdown of luks map %s failed, %s still busy',
-                    self.luks_name, self.luks_device
+            except Exception as issue:
+                log.error(
+                    'Shutdown of luks map {0}:{1} failed with: {2}'.format(
+                        self.luks_name, self.luks_device, issue
+                    )
                 )

kiwi/storage/raid_device.py

@@ -49,6 +49,9 @@ def __init__(self, storage_provider: DeviceProvider):
         }
         self.raid_device = None
 
+    def __enter__(self):
+        return self
+
     def get_device(self) -> Optional[MappedDevice]:
         """
         Instance of MappedDevice providing the raid device
@@ -123,15 +126,15 @@ def is_loop(self) -> bool:
         """
         return self.storage_provider.is_loop()
 
-    def __del__(self):
+    def __exit__(self, exc_type, exc_value, traceback):
         if self.raid_device:
-            log.info('Cleaning up %s instance', type(self).__name__)
             try:
                 Command.run(
                     ['mdadm', '--stop', self.raid_device]
                 )
-            except Exception:
-                log.warning(
-                    'Shutdown of raid device failed, %s still busy',
-                    self.raid_device
+            except Exception as issue:
+                log.error(
+                    'Shutdown of raid device {0} failed with: {1}'.format(
+                        self.raid_device, issue
+                    )
                 )

test/unit/storage/integrity_device_test.py

@@ -1,6 +1,9 @@
+import logging
 import io
 from textwrap import dedent
-from pytest import raises
+from pytest import (
+    raises, fixture
+)
 from mock import (
     patch, Mock, call, MagicMock
 )
@@ -15,6 +18,10 @@
 
 
 class TestIntegrityDevice:
+    @fixture(autouse=True)
+    def inject_fixtures(self, caplog):
+        self._caplog = caplog
+
     @patch('os.path.getsize')
     def setup(self, mock_os_path_getsize):
         mock_os_path_getsize.return_value = 42
@@ -222,11 +229,12 @@ def test_create_integritytab_with_keyfile(self, mock_BlockID):
     @patch('kiwi.storage.integrity_device.Command.run')
     @patch('kiwi.storage.integrity_device.log.warning')
     def test_destructor(self, mock_log_warn, mock_Command_run):
-        self.integrity.integrity_device = '/dev/mapper/integrityRoot'
         mock_Command_run.side_effect = Exception
-        self.integrity.__del__()
+        with self._caplog.at_level(logging.ERROR):
+            with IntegrityDevice(
+                Mock(), defaults.INTEGRITY_ALGORITHM
+            ) as integrity:
+                integrity.integrity_device = '/dev/mapper/integrityRoot'
         mock_Command_run.assert_called_once_with(
             ['integritysetup', 'close', 'integrityRoot']
         )
-        assert mock_log_warn.called
-        self.integrity.integrity_device = None

test/unit/storage/luks_device_test.py

@@ -1,14 +1,21 @@
+import logging
 import io
 from mock import (
     patch, call, MagicMock, Mock
 )
-from pytest import raises
+from pytest import (
+    raises, fixture
+)
 
 from kiwi.exceptions import KiwiLuksSetupError
 from kiwi.storage.luks_device import LuksDevice
 
 
 class TestLuksDevice:
+    @fixture(autouse=True)
+    def inject_fixtures(self, caplog):
+        self._caplog = caplog
+
     def setup(self):
         storage_device = Mock()
         storage_device.get_byte_size = Mock(
@@ -175,12 +182,11 @@ def test_is_loop(self):
 
     @patch('kiwi.storage.luks_device.Command.run')
     @patch('kiwi.storage.luks_device.log.warning')
-    def test_destructor(self, mock_log_warn, mock_command):
-        self.luks.luks_device = '/dev/mapper/luksRoot'
+    def test_context_manager_exit(self, mock_log_warn, mock_command):
         mock_command.side_effect = Exception
-        self.luks.__del__()
+        with self._caplog.at_level(logging.ERROR):
+            with LuksDevice(Mock()) as luks:
+                luks.luks_device = '/dev/mapper/luksRoot'
         mock_command.assert_called_once_with(
             ['cryptsetup', 'luksClose', 'luksRoot']
         )
-        assert mock_log_warn.called
-        self.luks.luks_device = None

test/unit/storage/raid_device_test.py

@@ -1,14 +1,21 @@
+import logging
 from mock import (
     patch, Mock, mock_open
 )
-from pytest import raises
+from pytest import (
+    raises, fixture
+)
 
 from kiwi.storage.raid_device import RaidDevice
 
 from kiwi.exceptions import KiwiRaidSetupError
 
 
 class TestRaidDevice:
+    @fixture(autouse=True)
+    def inject_fixtures(self, caplog):
+        self._caplog = caplog
+
     def setup(self):
         storage_device = Mock()
         storage_device.get_device = Mock(
@@ -78,12 +85,11 @@ def test_is_loop(self):
 
     @patch('kiwi.storage.raid_device.Command.run')
     @patch('kiwi.storage.raid_device.log.warning')
-    def test_destructor(self, mock_log_warn, mock_command):
-        self.raid.raid_device = '/dev/md0'
+    def test_context_manager_exit(self, mock_log_warn, mock_command):
         mock_command.side_effect = Exception
-        self.raid.__del__()
+        with self._caplog.at_level(logging.ERROR):
+            with RaidDevice(Mock()) as raid:
+                raid.raid_device = '/dev/md0'
         mock_command.assert_called_once_with(
             ['mdadm', '--stop', '/dev/md0']
         )
-        assert mock_log_warn.called
-        self.raid.raid_device = None