diff --git a/kiwi/builder/disk.py b/kiwi/builder/disk.py index a8d6afd9..1b2b8a2b 100644 --- a/kiwi/builder/disk.py +++ b/kiwi/builder/disk.py @@ -43,6 +43,7 @@ from kiwi.system.kernel import Kernel from kiwi.storage.subformat import DiskFormat from kiwi.system.result import Result from kiwi.utils.block import BlockID +from kiwi.utils.fstab import Fstab from kiwi.path import Path from kiwi.runtime_config import RuntimeConfig from kiwi.partitioner import Partitioner @@ -139,7 +140,7 @@ class DiskBuilder: self.boot_is_crypto = True if self.luks and not \ self.disk_setup.need_boot_partition() else False self.install_media = self._install_image_requested() - self.generic_fstab_entries = [] + self.fstab = Fstab() # an instance of a class with the sync_data capability # representing the entire image system except for the boot/ area @@ -893,8 +894,7 @@ class DiskBuilder: self.persistency_type, self.requested_filesystem ) for volume_fstab_entry in volume_fstab_entries: - if volume_fstab_entry not in self.generic_fstab_entries: - self.generic_fstab_entries.append(volume_fstab_entry) + self.fstab.add_entry(volume_fstab_entry) if device_map.get('spare') and \ self.spare_part_fs and self.spare_part_mountpoint: self._add_generic_fstab_entry( @@ -910,7 +910,7 @@ class DiskBuilder: device_map['swap'].get_device(), 'swap' ) setup.create_fstab( - self.generic_fstab_entries + self.fstab ) def _add_simple_fstab_entry( @@ -923,10 +923,7 @@ class DiskBuilder: device, mount_point, filesystem, ','.join(options), check ] ) - if fstab_entry not in self.generic_fstab_entries: - self.generic_fstab_entries.append( - fstab_entry - ) + self.fstab.add_entry(fstab_entry) def _add_generic_fstab_entry( self, device, mount_point, options=None, check='0 0' @@ -942,10 +939,7 @@ class DiskBuilder: block_operation.get_filesystem(), ','.join(options), check ] ) - if fstab_entry not in self.generic_fstab_entries: - self.generic_fstab_entries.append( - fstab_entry - ) + self.fstab.add_entry(fstab_entry) def _write_image_identifier_to_system_image(self): log.info('Creating image identifier: %s', self.mbrid.get_id()) diff --git a/kiwi/system/setup.py b/kiwi/system/setup.py index 95ae7598..ca276e93 100644 --- a/kiwi/system/setup.py +++ b/kiwi/system/setup.py @@ -32,7 +32,6 @@ from kiwi.system.root_init import RootInit from kiwi.command import Command from kiwi.command_process import CommandProcess from kiwi.utils.sync import DataSync -from kiwi.utils.fstab import Fstab from kiwi.defaults import Defaults from kiwi.system.users import Users from kiwi.system.shell import Shell @@ -585,9 +584,9 @@ class SystemSetup: working_directory=working_directory ) - def create_fstab(self, entries): + def create_fstab(self, fstab): """ - Create etc/fstab from given list of entries + Create etc/fstab from given Fstab object Custom fstab modifications are possible and handled in the following order: @@ -608,17 +607,17 @@ class SystemSetup: file in the image rootfs. Once called the fstab.script file will be deleted - :param list entries: list of line entries for fstab + :param list fstab: instance of Fstab """ fstab_file = self.root_dir + '/etc/fstab' fstab_append_file = self.root_dir + '/etc/fstab.append' fstab_patch_file = self.root_dir + '/etc/fstab.patch' fstab_script_file = self.root_dir + '/etc/fstab.script' - with open(fstab_file, 'w') as fstab: - for entry in entries: - fstab.write(entry + os.linesep) - if os.path.exists(fstab_append_file): + fstab.export(fstab_file) + + if os.path.exists(fstab_append_file): + with open(fstab_file, 'a') as fstab: with open(fstab_append_file, 'r') as append: fstab.write(append.read()) Path.wipe(fstab_append_file) @@ -635,13 +634,6 @@ class SystemSetup: ) Path.wipe(fstab_script_file) - # rewrite fstab after initial creation and after all - # optional modifications to make sure the canonical - # mount order is correct and no garbage exists. - fstab_final = Fstab() - fstab_final.read(fstab_file) - fstab_final.export(fstab_file) - def create_init_link_from_linuxrc(self): """ kiwi boot images provides the linuxrc script, however the kernel diff --git a/kiwi/utils/fstab.py b/kiwi/utils/fstab.py index 0f79963f..7a12be7d 100644 --- a/kiwi/utils/fstab.py +++ b/kiwi/utils/fstab.py @@ -15,12 +15,15 @@ # You should have received a copy of the GNU General Public License # along with kiwi. If not, see # +import logging import os from collections import namedtuple # project from kiwi.path import Path +log = logging.getLogger('kiwi') + class Fstab: """ @@ -38,42 +41,28 @@ class Fstab: """ Import specified fstab file + Read the given fstab file and initialize a new entry list + :param string filename: path to a fstab file """ self.fstab = [] with open(filename) as fstab: for line in fstab.readlines(): - mount_record = line.split() - if not mount_record or mount_record[0].startswith('#'): - continue - device = mount_record[0] - mountpoint = mount_record[1] - fstype = mount_record[2] - options = mount_record[3] - if device.startswith('UUID'): - device_path = ''.join( - ['/dev/disk/by-uuid/', device.split('=')[1]] - ) - elif device.startswith('LABEL'): - device_path = ''.join( - ['/dev/disk/by-label/', device.split('=')[1]] - ) - elif device.startswith('PARTUUID'): - device_path = ''.join( - ['/dev/disk/by-partuuid/', device.split('=')[1]] - ) - else: - device_path = device + self.add_entry(line) - self.fstab.append( - self.fstab_entry_type( - fstype=fstype, - mountpoint=mountpoint, - device_path=device_path, - device_spec=device, - options=options + def add_entry(self, line): + new_entry = self._parse_entry(line) + if new_entry: + for entry in self.fstab: + if entry.mountpoint == new_entry.mountpoint: + log.warning( + 'Mountpoint for "{0}" in use by "{1}", skipped'.format( + self._file_entry(new_entry), + self._file_entry(entry) + ) ) - ) + return + self.fstab.append(new_entry) def get_devices(self): return self.fstab @@ -94,9 +83,42 @@ class Fstab: ): entry = fstab_entries_by_path[device_path] fstab.write( - '{0} {1} {2} {3} 0 0{4}'.format( - entry.device_spec, entry.mountpoint, - entry.fstype, entry.options, - os.linesep - ) + self._file_entry(entry) + os.linesep ) + + def _file_entry(self, entry): + return '{0} {1} {2} {3} 0 0'.format( + entry.device_spec, entry.mountpoint, + entry.fstype, entry.options + ) + + def _parse_entry(self, line): + data_record = line.split() + if data_record and len(data_record) >= 4 \ + and not data_record[0].startswith('#'): + device = data_record[0] + mountpoint = data_record[1] + fstype = data_record[2] + options = data_record[3] + if device.startswith('UUID'): + device_path = ''.join( + ['/dev/disk/by-uuid/', device.split('=')[1]] + ) + elif device.startswith('LABEL'): + device_path = ''.join( + ['/dev/disk/by-label/', device.split('=')[1]] + ) + elif device.startswith('PARTUUID'): + device_path = ''.join( + ['/dev/disk/by-partuuid/', device.split('=')[1]] + ) + else: + device_path = device + + return self.fstab_entry_type( + fstype=fstype, + mountpoint=mountpoint, + device_path=device_path, + device_spec=device, + options=options + ) diff --git a/test/data/fstab b/test/data/fstab index ff6696d5..dadea8cc 100644 --- a/test/data/fstab +++ b/test/data/fstab @@ -5,5 +5,8 @@ LABEL=BOOT /boot xfs defaults 0 0 LABEL=foo /home ext4 defaults 0 0 PARTUUID=3c8bd108-01 /bar ext4 defaults 0 0 /dev/mynode /foo ext4 defaults 0 0 +# entry with the same mountpoint, expected to be skipped +LABEL=bar /home xfs defaults 0 0 + # this comment line and the line above should be ignored by the parser diff --git a/test/unit/builder/disk_test.py b/test/unit/builder/disk_test.py index e24a2120..7bbcbd26 100644 --- a/test/unit/builder/disk_test.py +++ b/test/unit/builder/disk_test.py @@ -188,6 +188,10 @@ class TestDiskBuilder: kiwi.builder.disk.LuksDevice = mock.Mock( return_value=self.luks_root ) + self.fstab = mock.Mock() + kiwi.builder.disk.Fstab = mock.Mock( + return_value=self.fstab + ) self.disk_builder = DiskBuilder( XMLState(description.load()), 'target_dir', 'root_dir', custom_args={'signing_keys': ['key_file_a', 'key_file_b']} @@ -754,22 +758,10 @@ class TestDiskBuilder: volume_manager.umount_volumes.call_args_list[0].assert_called_once_with( ) self.setup.create_fstab.assert_called_once_with( - [ - 'UUID=blkid_result / blkid_result_fs ro 0 0', - 'UUID=blkid_result /boot blkid_result_fs defaults 0 0', - 'UUID=blkid_result /boot/efi blkid_result_fs defaults 0 0', - 'fstab_volume_entries', - '/dev/systemVG/LVSwap swap swap defaults 0 0' - ] + self.disk_builder.fstab ) self.boot_image_task.setup.create_fstab.assert_called_once_with( - [ - 'UUID=blkid_result / blkid_result_fs ro 0 0', - 'UUID=blkid_result /boot blkid_result_fs defaults 0 0', - 'UUID=blkid_result /boot/efi blkid_result_fs defaults 0 0', - 'fstab_volume_entries', - '/dev/systemVG/LVSwap swap swap defaults 0 0' - ] + self.disk_builder.fstab ) @patch('kiwi.builder.disk.FileSystem') @@ -855,8 +847,13 @@ class TestDiskBuilder: 'boot/*', 'boot/.*', 'boot/efi/*', 'boot/efi/.*' ] ) - assert 'UUID=blkid_result /var blkid_result_fs defaults 0 0' in \ - self.disk_builder.generic_fstab_entries + assert [ + call('UUID=blkid_result / blkid_result_fs ro 0 0'), + call('UUID=blkid_result /boot blkid_result_fs defaults 0 0'), + call('UUID=blkid_result /boot/efi blkid_result_fs defaults 0 0'), + call('UUID=blkid_result /var blkid_result_fs defaults 0 0'), + call('UUID=blkid_result swap blkid_result_fs defaults 0 0'), + ] in self.disk_builder.fstab.add_entry.call_args_list self.disk.create_root_partition.reset_mock() self.disk.create_spare_partition.reset_mock() diff --git a/test/unit/system/setup_test.py b/test/unit/system/setup_test.py index 3909ef59..b9ddd18b 100644 --- a/test/unit/system/setup_test.py +++ b/test/unit/system/setup_test.py @@ -692,24 +692,23 @@ class TestSystemSetup: @patch('os.path.exists') @patch('kiwi.system.setup.Path.wipe') @patch('kiwi.command.Command.run') - @patch('kiwi.system.setup.Fstab') def test_create_fstab( - self, mock_Fstab, mock_command, mock_wipe, mock_exists + self, mock_command, mock_wipe, mock_exists ): - fstab_final = Mock() - mock_Fstab.return_value = fstab_final + fstab = Mock() mock_exists.return_value = True m_open = mock_open(read_data='append_entry') with patch('builtins.open', m_open, create=True): - self.setup.create_fstab(['fstab_entry']) + self.setup.create_fstab(fstab) + + fstab.export.assert_called_once_with('root_dir/etc/fstab') assert m_open.call_args_list == [ - call('root_dir/etc/fstab', 'w'), + call('root_dir/etc/fstab', 'a'), call('root_dir/etc/fstab.append', 'r') ] assert m_open.return_value.write.call_args_list == [ - call('fstab_entry\n'), call('append_entry') ] assert mock_command.call_args_list == [ @@ -721,8 +720,6 @@ class TestSystemSetup: call('root_dir/etc/fstab.patch'), call('root_dir/etc/fstab.script') ] - fstab_final.read.assert_called_once_with('root_dir/etc/fstab') - fstab_final.export.assert_called_once_with('root_dir/etc/fstab') @patch('kiwi.command.Command.run') @patch('kiwi.system.setup.NamedTemporaryFile') diff --git a/test/unit/utils/fstab_test.py b/test/unit/utils/fstab_test.py index 55c1fcba..2109554d 100644 --- a/test/unit/utils/fstab_test.py +++ b/test/unit/utils/fstab_test.py @@ -1,4 +1,6 @@ import io +import logging +from pytest import fixture from unittest.mock import ( MagicMock, patch, call ) @@ -6,9 +8,18 @@ from kiwi.utils.fstab import Fstab class TestFstab(object): + @fixture(autouse=True) + def inject_fixtures(self, caplog): + self._caplog = caplog + def setup(self): self.fstab = Fstab() - self.fstab.read('../data/fstab') + with self._caplog.at_level(logging.WARNING): + self.fstab.read('../data/fstab') + assert format( + 'Mountpoint for "LABEL=bar /home xfs defaults 0 0" ' + 'in use by "LABEL=foo /home ext4 defaults 0 0", skipped' + ) in self._caplog.text def test_get_devices(self): assert self.fstab.get_devices() == [