Allow to access disk root after sync_data

In preparation to allow a chroot operation into the loop
mounted disk this commit refactors the process when filesystems
gets umounted and also fixes the canonical order for calling
the destructors. Related to Issue #1464
This commit is contained in:
Marcus Schäfer 2020-05-29 17:41:15 +02:00
parent 08d92631d0
commit c7ed1cfc29
No known key found for this signature in database
GPG Key ID: AD11DD02B44996EF
13 changed files with 84 additions and 78 deletions

View File

@ -56,8 +56,6 @@ class BootImageBase:
self.initrd_filename = None
self.boot_xml_state = None
self.setup = None
self.temp_directories = []
self.call_destructor = True
self.signing_keys = signing_keys
self.boot_root_directory = root_dir
@ -151,24 +149,11 @@ class BootImageBase:
try:
with open(filename, 'wb') as boot_image:
pickle.dump(self, boot_image)
self.disable_cleanup()
except Exception as e:
raise KiwiBootImageDumpError(
'Failed to pickle dump boot image: %s' % format(e)
)
def disable_cleanup(self):
"""
Deactivate cleanup(deletion) of boot root directory
"""
self.call_destructor = False
def enable_cleanup(self):
"""
Activate cleanup(deletion) of boot root directory
"""
self.call_destructor = True
def get_boot_names(self):
"""
Provides kernel and initrd names for the boot image
@ -383,6 +368,12 @@ class BootImageBase:
boot_description
return boot_description
def cleanup(self):
"""
Cleanup temporary boot image data if any
"""
pass
def _get_boot_image_output_file_format(self, kernel_version):
"""
The initrd output file format varies between
@ -451,10 +442,3 @@ class BootImageBase:
return os.path.basename(initrd_file).replace(
kernel_version, '{kernel_version}'
)
def __del__(self):
if self.call_destructor:
log.info('Cleaning up %s instance', type(self).__name__)
for directory in self.temp_directories:
if directory and os.path.exists(directory):
Path.wipe(directory)

View File

@ -49,17 +49,18 @@ class BootImageKiwi(BootImageBase):
root filesystem which is a separate image to create
the initrd from
"""
self.temp_directories = []
def prepare(self):
"""
Prepare new root system suitable to create a kiwi initrd from it
"""
self.boot_root_directory = mkdtemp(
prefix='kiwi_boot_root.', dir=self.target_dir
)
self.temp_directories.append(
self.boot_root_directory
)
def prepare(self):
"""
Prepare new root system suitable to create a kiwi initrd from it
"""
self.load_boot_xml_description()
boot_image_name = self.boot_xml_state.xml_data.get_name()
@ -180,3 +181,8 @@ class BootImageKiwi(BootImageBase):
['--check=crc32', '--lzma2=dict=1MiB', '--threads=0']
)
self.initrd_filename = compress.compressed_filename
def cleanup(self):
for directory in self.temp_directories:
if directory and os.path.exists(directory):
Path.wipe(directory)

View File

@ -234,13 +234,13 @@ class DiskBuilder:
# create the disk
log.info('Creating raw disk image %s', self.diskname)
loop_provider = LoopDevice(
self.loop_provider = LoopDevice(
self.diskname, disksize_mbytes, self.blocksize
)
loop_provider.create()
self.loop_provider.create()
self.disk = Disk(
self.firmware.get_partition_table_type(), loop_provider,
self.firmware.get_partition_table_type(), self.loop_provider,
self.xml_state.get_disk_start_sector()
)
@ -249,7 +249,7 @@ class DiskBuilder:
self.bootloader, self.xml_state, root_dir=self.root_dir,
boot_dir=self.root_dir, custom_args={
'targetbase':
loop_provider.get_device(),
self.loop_provider.get_device(),
'grub_directory_name':
Defaults.get_grub_boot_directory_name(self.root_dir),
'boot_is_crypto':
@ -320,22 +320,22 @@ class DiskBuilder:
'image_type':
self.xml_state.get_build_type_name()
}
volume_manager = VolumeManager(
self.volume_manager = VolumeManager(
self.volume_manager_name, device_map,
self.root_dir + '/',
self.volumes,
volume_manager_custom_parameters
)
volume_manager.setup(
self.volume_manager.setup(
self.volume_group_name
)
volume_manager.create_volumes(
self.volume_manager.create_volumes(
self.requested_filesystem
)
volume_manager.mount_volumes()
self.system = volume_manager
device_map['root'] = volume_manager.get_device().get('root')
device_map['swap'] = volume_manager.get_device().get('swap')
self.volume_manager.mount_volumes()
self.system = self.volume_manager
device_map['root'] = self.volume_manager.get_device().get('root')
device_map['swap'] = self.volume_manager.get_device().get('swap')
else:
log.info(
'Creating root(%s) filesystem on %s',
@ -603,7 +603,6 @@ class DiskBuilder:
try:
with open(boot_image_dump_file, 'rb') as boot_image_dump:
boot_image = pickle.load(boot_image_dump)
boot_image.enable_cleanup()
Path.wipe(boot_image_dump_file)
except Exception as e:
raise KiwiInstallMediaError(

View File

@ -238,6 +238,7 @@ class InstallImageBuilder:
custom_args=self.custom_iso_args
)
iso_image.create_on_file(self.isoname)
self.boot_image_task.cleanup()
def create_install_pxe_archive(self):
"""
@ -345,6 +346,7 @@ class InstallImageBuilder:
archive = ArchiveTar(self.pxetarball)
archive.create(self.pxe_dir)
self.boot_image_task.cleanup()
def _create_pxe_install_kernel_and_initrd(self):
kernelname = 'pxeboot.{0}.kernel'.format(self.pxename)

View File

@ -122,6 +122,19 @@ class FileSystemBase:
"""
raise NotImplementedError
def get_mountpoint(self):
"""
Provides mount point directory
Effective use of the directory is guaranteed only after sync_data
:return: directory path name
:rtype: string
"""
if self.filesystem_mount:
return self.filesystem_mount.mountpoint
def sync_data(self, exclude=None):
"""
Copy root data tree into filesystem
@ -150,7 +163,6 @@ class FileSystemBase:
options=['-a', '-H', '-X', '-A', '--one-file-system'],
exclude=exclude
)
self.filesystem_mount.umount()
def _apply_attributes(self):
"""

View File

@ -312,6 +312,18 @@ class VolumeManagerBase(DeviceProvider):
)
return mbsize
def get_mountpoint(self):
"""
Provides mount point directory
Effective use of the directory is guaranteed only after sync_data
:return: directory path name
:rtype: string
"""
return self.mountpoint
def sync_data(self, exclude=None):
"""
Implements sync of root directory to mounted volumes
@ -327,7 +339,6 @@ class VolumeManagerBase(DeviceProvider):
options=['-a', '-H', '-X', '-A', '--one-file-system'],
exclude=exclude
)
self.umount_volumes()
def set_property_readonly_root(self):
"""
@ -343,11 +354,3 @@ class VolumeManagerBase(DeviceProvider):
the mounts of all volumes
"""
self.mountpoint = mkdtemp(prefix='kiwi_volumes.')
def __del__(self):
"""
Implements destructor to cleanup all volume subsystems
and mount processes. overwrite in specialized volume
management class
"""
pass

View File

@ -92,7 +92,7 @@ class VolumeManagerBtrfs(VolumeManagerBase):
filesystem = FileSystem(
name='btrfs',
device_provider=MappedDevice(
device=self.device, device_provider=self
device=self.device, device_provider=self.device_provider_root
),
custom_args=self.custom_filesystem_args
)

View File

@ -81,7 +81,7 @@ class VolumeManagerLVM(VolumeManagerBase):
# the same key to put them on the same level
volume_name = 'swap'
device_map[volume_name] = MappedDevice(
device=volume_node, device_provider=self
device=volume_node, device_provider=self.device_provider_root
)
return device_map
@ -296,7 +296,7 @@ class VolumeManagerLVM(VolumeManagerBase):
filesystem = FileSystem(
name=filesystem_name,
device_provider=MappedDevice(
device=device_node, device_provider=self
device=device_node, device_provider=self.device_provider_root
),
custom_args=self.custom_filesystem_args
)

View File

@ -69,8 +69,7 @@ class TestBootImageBase:
self.boot_image.dump('filename')
@patch('pickle.dump')
@patch('kiwi.boot.image.base.BootImageBase.disable_cleanup')
def test_dump(self, mock_disable_cleanup, mock_dump):
def test_dump(self, mock_dump):
with patch('builtins.open', create=True) as mock_open:
mock_open.return_value = MagicMock(spec=io.IOBase)
file_handle = mock_open.return_value.__enter__.return_value
@ -79,15 +78,6 @@ class TestBootImageBase:
mock_dump.assert_called_once_with(
self.boot_image, file_handle
)
mock_disable_cleanup.assert_called_once_with()
def test_disable_cleanup(self):
self.boot_image.disable_cleanup()
assert self.boot_image.call_destructor is False
def test_enable_cleanup(self):
self.boot_image.enable_cleanup()
assert self.boot_image.call_destructor is True
@patch('os.listdir')
def test_is_prepared(self, mock_listdir):
@ -198,3 +188,4 @@ class TestBootImageBase:
self.boot_image.include_module('module')
self.boot_image.omit_module('module')
self.boot_image.write_system_config_file({'config_key': 'value'})
self.boot_image.cleanup()

View File

@ -53,7 +53,9 @@ class TestBootImageKiwi:
self.boot_image.include_file('/root/a')
@patch('kiwi.defaults.Defaults.get_boot_image_description_path')
def test_prepare(self, mock_boot_path):
@patch('kiwi.boot.image.builtin_kiwi.mkdtemp')
def test_prepare(self, mock_mkdtemp, mock_boot_path):
mock_mkdtemp.return_value = 'boot-root-directory'
mock_boot_path.return_value = '../data'
self.boot_image.prepare()
self.system_prepare.setup_repositories.assert_called_once_with(
@ -82,7 +84,11 @@ class TestBootImageKiwi:
self.setup.call_image_script.assert_called_once_with()
@patch('os.path.exists')
def test_prepare_no_boot_description_found(self, mock_os_path):
@patch('kiwi.boot.image.builtin_kiwi.mkdtemp')
def test_prepare_no_boot_description_found(
self, mock_mkdtemp, mock_os_path
):
mock_mkdtemp.return_value = 'boot-root-directory'
mock_os_path.return_value = False
with raises(KiwiConfigFileNotFound):
self.boot_image.prepare()
@ -102,6 +108,7 @@ class TestBootImageKiwi:
mock_sync.return_value = data
mock_mkdtemp.return_value = 'temp-boot-directory'
mock_prepared.return_value = True
self.boot_image.boot_root_directory = 'boot-root-directory'
mbrid = mock.Mock()
mbrid.write = mock.Mock()
cpio = mock.Mock()
@ -158,11 +165,12 @@ class TestBootImageKiwi:
self.boot_image.target_dir + '/foo'
)
@patch('kiwi.boot.image.base.Path.wipe')
@patch('kiwi.boot.image.builtin_kiwi.Path.wipe')
@patch('os.path.exists')
def test_destructor(self, mock_path, mock_wipe):
def test_cleanup(self, mock_path, mock_wipe):
mock_path.return_value = True
self.boot_image.__del__()
self.boot_image.temp_directories.append('boot-root-directory')
self.boot_image.cleanup()
mock_wipe.assert_called_once_with('boot-root-directory')
def teardown(self):

View File

@ -178,6 +178,7 @@ class TestInstallImageBuilder:
self.boot_image_task.create_initrd.assert_called_once_with(
self.mbrid, 'initrd_kiwi_install', install_initrd=True
)
self.boot_image_task.cleanup.assert_called_once_with()
self.kernel.copy_kernel.assert_called_once_with(
'temp_media_dir/boot/x86_64/loader', '/linux'
)
@ -354,6 +355,7 @@ class TestInstallImageBuilder:
self.boot_image_task.create_initrd.assert_called_once_with(
self.mbrid, 'initrd_kiwi_install', install_initrd=True
)
self.boot_image_task.cleanup.assert_called_once_with()
assert mock_command.call_args_list[1] == call(
[
'mv', 'initrd',

View File

@ -36,6 +36,9 @@ class TestFileSystemBase:
with raises(NotImplementedError):
self.fsbase.create_on_file('myimage')
def test_get_mountpoint(self):
assert self.fsbase.get_mountpoint() is None
@patch('kiwi.filesystem.base.MountManager')
@patch('kiwi.filesystem.base.DataSync')
@patch('kiwi.filesystem.base.Command.run')
@ -66,7 +69,7 @@ class TestFileSystemBase:
['chattr', '+C', 'tmpdir']
)
filesystem_mount.mount.assert_called_once_with([])
filesystem_mount.umount.assert_called_once_with()
assert self.fsbase.get_mountpoint() == 'tmpdir'
def test_destructor_valid_mountpoint(self):
self.fsbase.filesystem_mount = mock.Mock()

View File

@ -206,13 +206,13 @@ class TestVolumeManagerBase:
with raises(NotImplementedError):
self.volume_manager.get_volumes()
def test_get_mountpoint(self):
assert self.volume_manager.get_mountpoint() is None
@patch('kiwi.volume_manager.base.DataSync')
@patch('kiwi.volume_manager.base.MountManager.is_mounted')
@patch('kiwi.volume_manager.base.VolumeManagerBase.mount_volumes')
@patch('kiwi.volume_manager.base.VolumeManagerBase.umount_volumes')
def test_sync_data(
self, mock_umount_volumes, mock_mount_volumes, mock_mounted, mock_sync
):
def test_sync_data(self, mock_mount_volumes, mock_mounted, mock_sync):
data_sync = Mock()
mock_sync.return_value = data_sync
mock_mounted.return_value = False
@ -226,7 +226,7 @@ class TestVolumeManagerBase:
exclude=['exclude_me'],
options=['-a', '-H', '-X', '-A', '--one-file-system']
)
mock_umount_volumes.assert_called_once_with()
assert self.volume_manager.get_mountpoint() == 'mountpoint'
@patch('kiwi.volume_manager.base.mkdtemp')
def test_setup_mountpoint(self, mock_mkdtemp):
@ -250,7 +250,3 @@ class TestVolumeManagerBase:
mock_command.assert_called_once_with(
['chattr', '+C', 'toplevel/etc']
)
def test_destructor(self):
# does nothing by default, just pass
self.volume_manager.__del__()