From 720d34ddd87141f97149e752ea606702ffc7d445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Tue, 9 Apr 2019 12:08:33 +0200 Subject: [PATCH 1/2] Delete check_grub_efi_installed_for_efi_firmware The motivation is nice to check if the required grub module package is part of the package list if the efi firmware is requested, but as long as there is no distribution wide standard for packaging grub this check will cause more trouble than it is of help. Currently it failed for the arm architecture and the grub2-arm64-efi package. We decided to prevent checking against static lists and dropped this runtime check. Missing grub modules will be recognized at the grub stage when we search for them. --- kiwi/runtime_checker.py | 28 -------- kiwi/tasks/system_build.py | 1 - kiwi/tasks/system_prepare.py | 1 - test/unit/runtime_checker_test.py | 112 ------------------------------ 4 files changed, 142 deletions(-) diff --git a/kiwi/runtime_checker.py b/kiwi/runtime_checker.py index ce631e2f65..0877a59794 100644 --- a/kiwi/runtime_checker.py +++ b/kiwi/runtime_checker.py @@ -541,34 +541,6 @@ def check_efi_mode_for_disk_overlay_correctly_setup(self): if overlayroot and firmware == 'uefi': raise KiwiRuntimeError(message) - def check_grub_efi_installed_for_efi_firmware(self): - """ - If the image is being built with efi or uefi firmware setting - we need a grub(2)-...-efi package installed. The check is not 100% - as every distribution has different names and different requirement - but it is a reasonable approximation on the safe side meaning the - user may still get an error but should not receive a false positive - """ - message = dedent('''\n - Firmware set to efi or uefi but not grub*efi* package is - part of the image build. - ''') - firmware = self.xml_state.build_type.get_firmware() - if firmware in ('efi', 'uefi'): - grub_efi_packages = ( - 'grub2-x86_64-efi', # SUSE - 'grub-efi', # Debian based distros have grub-efi-* - 'grub2-efi' # RedHat - ) - package_names = \ - self.xml_state.get_bootstrap_packages() + \ - self.xml_state.get_system_packages() - for grub_package in grub_efi_packages: - for package_name in package_names: - if package_name.startswith(grub_package): - return True - raise KiwiRuntimeError(message) - def check_xen_uniquely_setup_as_server_or_guest(self): """ If the image is classified to be used as Xen image, it can diff --git a/kiwi/tasks/system_build.py b/kiwi/tasks/system_build.py index f0d927dcfc..1f4d1fde32 100644 --- a/kiwi/tasks/system_build.py +++ b/kiwi/tasks/system_build.py @@ -139,7 +139,6 @@ def process(self): # noqa: C901 checks = { 'check_minimal_required_preferences': [], 'check_efi_mode_for_disk_overlay_correctly_setup': [], - 'check_grub_efi_installed_for_efi_firmware': [], 'check_boot_description_exists': [], 'check_consistent_kernel_in_boot_and_system_image': [], 'check_container_tool_chain_installed': [], diff --git a/kiwi/tasks/system_prepare.py b/kiwi/tasks/system_prepare.py index d0d30b90c6..e31efcba0e 100644 --- a/kiwi/tasks/system_prepare.py +++ b/kiwi/tasks/system_prepare.py @@ -124,7 +124,6 @@ def process(self): # noqa: C901 checks = { 'check_minimal_required_preferences': [], 'check_efi_mode_for_disk_overlay_correctly_setup': [], - 'check_grub_efi_installed_for_efi_firmware': [], 'check_boot_description_exists': [], 'check_consistent_kernel_in_boot_and_system_image': [], 'check_container_tool_chain_installed': [], diff --git a/test/unit/runtime_checker_test.py b/test/unit/runtime_checker_test.py index 2a94275a3b..bc3098e0e3 100644 --- a/test/unit/runtime_checker_test.py +++ b/test/unit/runtime_checker_test.py @@ -172,118 +172,6 @@ def test_check_boot_description_exists_does_not_exist(self): runtime_checker = RuntimeChecker(xml_state) runtime_checker.check_boot_description_exists() - @raises(KiwiRuntimeError) - def test_check_grub_efi_installed_for_efi_firmware_is_efi(self): - self.xml_state.build_type.get_firmware = mock.Mock( - return_value='efi' - ) - self.xml_state.get_bootstrap_packages = mock.Mock( - return_value=['foo'] - ) - self.xml_state.get_system_packages = mock.Mock( - return_value=['bar'] - ) - runtime_checker = RuntimeChecker(self.xml_state) - runtime_checker.check_grub_efi_installed_for_efi_firmware() - - def test_check_grub_efi_installed_for_efi_firmware_is_efi_debian(self): - self.xml_state.build_type.get_firmware = mock.Mock( - return_value='efi' - ) - self.xml_state.get_bootstrap_packages = mock.Mock( - return_value=['grub-efi-amd64'] - ) - self.xml_state.get_system_packages = mock.Mock( - return_value=['bar'] - ) - runtime_checker = RuntimeChecker(self.xml_state) - assert runtime_checker.check_grub_efi_installed_for_efi_firmware() \ - is True - - def test_check_grub_efi_installed_for_efi_firmware_is_efi_rhel(self): - self.xml_state.build_type.get_firmware = mock.Mock( - return_value='efi' - ) - self.xml_state.get_bootstrap_packages = mock.Mock( - return_value=['grub2-efi-x64'] - ) - self.xml_state.get_system_packages = mock.Mock( - return_value=['bar'] - ) - runtime_checker = RuntimeChecker(self.xml_state) - assert runtime_checker.check_grub_efi_installed_for_efi_firmware() \ - is True - - def test_check_grub_efi_installed_for_efi_firmware_is_efi_suse(self): - self.xml_state.build_type.get_firmware = mock.Mock( - return_value='efi' - ) - self.xml_state.get_bootstrap_packages = mock.Mock( - return_value=['grub2-x86_64-efi'] - ) - self.xml_state.get_system_packages = mock.Mock( - return_value=['bar'] - ) - runtime_checker = RuntimeChecker(self.xml_state) - assert runtime_checker.check_grub_efi_installed_for_efi_firmware() \ - is True - - @raises(KiwiRuntimeError) - def test_check_grub_efi_installed_for_efi_firmware_is_uefi(self): - self.xml_state.build_type.get_firmware = mock.Mock( - return_value='uefi' - ) - self.xml_state.get_bootstrap_packages = mock.Mock( - return_value=['foo'] - ) - self.xml_state.get_system_packages = mock.Mock( - return_value=['bar'] - ) - runtime_checker = RuntimeChecker(self.xml_state) - runtime_checker.check_grub_efi_installed_for_efi_firmware() - - def test_check_grub_efi_installed_for_efi_firmware_is_uefi_debian(self): - self.xml_state.build_type.get_firmware = mock.Mock( - return_value='uefi' - ) - self.xml_state.get_bootstrap_packages = mock.Mock( - return_value=['grub-efi-amd64'] - ) - self.xml_state.get_system_packages = mock.Mock( - return_value=['bar'] - ) - runtime_checker = RuntimeChecker(self.xml_state) - assert runtime_checker.check_grub_efi_installed_for_efi_firmware() \ - is True - - def test_check_grub_efi_installed_for_efi_firmware_is_uefi_rhel(self): - self.xml_state.build_type.get_firmware = mock.Mock( - return_value='uefi' - ) - self.xml_state.get_bootstrap_packages = mock.Mock( - return_value=['grub2-efi-x64'] - ) - self.xml_state.get_system_packages = mock.Mock( - return_value=['bar'] - ) - runtime_checker = RuntimeChecker(self.xml_state) - assert runtime_checker.check_grub_efi_installed_for_efi_firmware() \ - is True - - def test_check_grub_efi_installed_for_efi_firmware_is_uefi_suse(self): - self.xml_state.build_type.get_firmware = mock.Mock( - return_value='uefi' - ) - self.xml_state.get_bootstrap_packages = mock.Mock( - return_value=['grub2-x86_64-efi'] - ) - self.xml_state.get_system_packages = mock.Mock( - return_value=['bar'] - ) - runtime_checker = RuntimeChecker(self.xml_state) - assert runtime_checker.check_grub_efi_installed_for_efi_firmware() \ - is True - @raises(KiwiRuntimeError) def test_check_xen_uniquely_setup_as_server_or_guest_for_ec2(self): self.xml_state.build_type.get_firmware = mock.Mock( From 7760b2ddfa9e1ebcd3a43ecd2204d50fe02809c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Tue, 9 Apr 2019 14:53:07 +0200 Subject: [PATCH 2/2] Refactor handling of runtime tests check dict Consolidate check list into CliTask base class such that we can avoid duplication of runtime check dicts. Only runtime tests that require stateful information according to the commandline call are handled directly in the task code --- kiwi/tasks/base.py | 22 ++++++++++++++++++++++ kiwi/tasks/system_build.py | 32 +++++++++----------------------- kiwi/tasks/system_create.py | 8 ++++---- kiwi/tasks/system_prepare.py | 31 ++++++++----------------------- 4 files changed, 43 insertions(+), 50 deletions(-) diff --git a/kiwi/tasks/base.py b/kiwi/tasks/base.py index 7768b6687b..6993981cab 100644 --- a/kiwi/tasks/base.py +++ b/kiwi/tasks/base.py @@ -69,6 +69,28 @@ def __init__(self, should_perform_task_setup=True): # get global args self.global_args = self.cli.get_global_args() + # initialize generic runtime check dicts + self.checks_before_command_args = { + 'check_minimal_required_preferences': [], + 'check_efi_mode_for_disk_overlay_correctly_setup': [], + 'check_boot_description_exists': [], + 'check_consistent_kernel_in_boot_and_system_image': [], + 'check_container_tool_chain_installed': [], + 'check_volume_setup_defines_multiple_fullsize_volumes': [], + 'check_volume_setup_has_no_root_definition': [], + 'check_volume_label_used_with_lvm': [], + 'check_xen_uniquely_setup_as_server_or_guest': [], + 'check_mediacheck_only_for_x86_arch': [], + 'check_dracut_module_for_live_iso_in_package_list': [], + 'check_dracut_module_for_disk_overlay_in_package_list': [], + 'check_dracut_module_for_disk_oem_in_package_list': [], + 'check_dracut_module_for_oem_install_in_package_list': [] + } + self.checks_after_command_args = { + 'check_repositories_configured': [], + 'check_image_include_repos_publicly_resolvable': [] + } + if should_perform_task_setup: # set log level if self.global_args['--debug']: diff --git a/kiwi/tasks/system_build.py b/kiwi/tasks/system_build.py index 1f4d1fde32..46f6b9db69 100644 --- a/kiwi/tasks/system_build.py +++ b/kiwi/tasks/system_build.py @@ -136,24 +136,14 @@ def process(self): # noqa: C901 self.command_args['--description'] ) - checks = { - 'check_minimal_required_preferences': [], - 'check_efi_mode_for_disk_overlay_correctly_setup': [], - 'check_boot_description_exists': [], - 'check_consistent_kernel_in_boot_and_system_image': [], - 'check_container_tool_chain_installed': [], - 'check_volume_setup_defines_multiple_fullsize_volumes': [], - 'check_volume_setup_has_no_root_definition': [], - 'check_volume_label_used_with_lvm': [], - 'check_xen_uniquely_setup_as_server_or_guest': [], - 'check_target_directory_not_in_shared_cache': [abs_target_dir_path], - 'check_mediacheck_only_for_x86_arch': [], - 'check_dracut_module_for_live_iso_in_package_list': [], - 'check_dracut_module_for_disk_overlay_in_package_list': [], - 'check_dracut_module_for_disk_oem_in_package_list': [], - 'check_dracut_module_for_oem_install_in_package_list': [] - } - self.run_checks(checks) + build_checks = self.checks_before_command_args + build_checks.update( + { + 'check_target_directory_not_in_shared_cache': + [abs_target_dir_path] + } + ) + self.run_checks(build_checks) if self.command_args['--ignore-repos']: self.xml_state.delete_repository_sections() @@ -192,11 +182,7 @@ def process(self): # noqa: C901 self.command_args['--set-container-derived-from'] ) - checks = { - 'check_repositories_configured': [], - 'check_image_include_repos_publicly_resolvable': [] - } - self.run_checks(checks) + self.run_checks(self.checks_after_command_args) log.info('Preparing new root system') system = SystemPrepare( diff --git a/kiwi/tasks/system_create.py b/kiwi/tasks/system_create.py index 7d55271424..5005a031de 100644 --- a/kiwi/tasks/system_create.py +++ b/kiwi/tasks/system_create.py @@ -79,10 +79,10 @@ def process(self): self.load_xml_description( abs_root_path ) - checks = { - 'check_target_directory_not_in_shared_cache': [abs_root_path] - } - self.run_checks(checks) + + self.run_checks( + {'check_target_directory_not_in_shared_cache': [abs_root_path]} + ) log.info('Creating system image') if not os.path.exists(abs_target_dir_path): diff --git a/kiwi/tasks/system_prepare.py b/kiwi/tasks/system_prepare.py index e31efcba0e..568adbb9fe 100644 --- a/kiwi/tasks/system_prepare.py +++ b/kiwi/tasks/system_prepare.py @@ -121,24 +121,13 @@ def process(self): # noqa: C901 abs_root_path = os.path.abspath(self.command_args['--root']) - checks = { - 'check_minimal_required_preferences': [], - 'check_efi_mode_for_disk_overlay_correctly_setup': [], - 'check_boot_description_exists': [], - 'check_consistent_kernel_in_boot_and_system_image': [], - 'check_container_tool_chain_installed': [], - 'check_volume_setup_defines_multiple_fullsize_volumes': [], - 'check_volume_setup_has_no_root_definition': [], - 'check_volume_label_used_with_lvm': [], - 'check_xen_uniquely_setup_as_server_or_guest': [], - 'check_target_directory_not_in_shared_cache': [abs_root_path], - 'check_mediacheck_only_for_x86_arch': [], - 'check_dracut_module_for_live_iso_in_package_list': [], - 'check_dracut_module_for_disk_overlay_in_package_list': [], - 'check_dracut_module_for_disk_oem_in_package_list': [], - 'check_dracut_module_for_oem_install_in_package_list': [] - } - self.run_checks(checks) + prepare_checks = self.checks_before_command_args + prepare_checks.update( + { + 'check_target_directory_not_in_shared_cache': [abs_root_path] + } + ) + self.run_checks(prepare_checks) if self.command_args['--ignore-repos']: self.xml_state.delete_repository_sections() @@ -177,11 +166,7 @@ def process(self): # noqa: C901 self.command_args['--set-container-derived-from'] ) - checks = { - 'check_repositories_configured': [], - 'check_image_include_repos_publicly_resolvable': [] - } - self.run_checks(checks) + self.run_checks(self.checks_after_command_args) log.info('Preparing system') system = SystemPrepare(