From 3cae9290c857e665b89302bb4de6210a381f1d0d Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Fri, 22 Dec 2023 13:59:58 +0100 Subject: [PATCH] Refactor and fix tests and linters --- README.md | 2 +- library/bootloader_facts.py | 2 ++ library/bootloader_settings.py | 49 ++++++++++++++++---------- tests/tests_add_rm.yml | 12 ++++--- tests/unit/test_bootloader_settings.py | 7 ++-- 5 files changed, 46 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 88f82f5..ce93804 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ Each list should specify the same kernel using one or multiple keys. Default: `present` -2. `options` - with this, specify settings to update +3. `options` - with this, specify settings to update * `name` - The name of the setting. `name` is omitted when using `replaced`. * `value` - The value for the setting. You must omit `value` if the setting has no value, e.g. `quiet`. diff --git a/library/bootloader_facts.py b/library/bootloader_facts.py index 05d93f9..e0f7a8f 100644 --- a/library/bootloader_facts.py +++ b/library/bootloader_facts.py @@ -103,9 +103,11 @@ def get_facts(kernels_info, default_kernel): + """Get kernel facts""" kernels_info_lines = kernels_info.strip().split("\n") kernels = [] index_count = 0 + for line in kernels_info_lines: index = re.search(r"index=(\d+)", line) if index: diff --git a/library/bootloader_settings.py b/library/bootloader_settings.py index 6a1772a..6f45293 100644 --- a/library/bootloader_settings.py +++ b/library/bootloader_settings.py @@ -28,7 +28,7 @@ kernel: description: Kernels to operate on. Can be a string DEFAULT or ALL, or dict.clear required: true - type: str or dict + type: dict state: description: State of the kernel. required: false @@ -73,6 +73,7 @@ def get_facts(kernels_info, default_kernel): kernels_info_lines = kernels_info.strip().split("\n") kernels = [] index_count = 0 + for line in kernels_info_lines: index = re.search(r"index=(\d+)", line) if index: @@ -92,11 +93,7 @@ def get_facts(kernels_info, default_kernel): def get_dict_same_keys(dict1, dict2): """Shorten dict2 to the same keys as in dict1""" - result = {} - for key1 in dict1: - if key1 in dict2: - result.update({key1: dict2[key1]}) - return result + return {key1: dict2[key1] for key1 in dict1 if key1 in dict2} def compare_dicts(dict1, dict2): @@ -126,9 +123,11 @@ def validate_kernel_initrd(bootloader_setting_kernel, kernel_mod_keys): def get_kernel_to_mod(bootloader_setting_kernel, kernel_mod_keys): """From a list of kernels, select not initrd kernel dict to use it for modifying options""" - for key, value in bootloader_setting_kernel.items(): - if key in kernel_mod_keys: - return {key: value} + return { + key: value + for key, value in bootloader_setting_kernel.items() + if key in kernel_mod_keys + } def get_single_kernel(bootloader_setting_kernel): @@ -165,9 +164,11 @@ def validate_kernels(bootloader_setting, bootloader_facts): kernel_mod_keys = ["path", "title", "index"] states = ["present", "absent"] state = bootloader_setting["state"] if "state" in bootloader_setting else "present" + if "state" in bootloader_setting and bootloader_setting["state"] not in states: err = "State must be one of '%s'" % ", ".join(states) return err, kernel_action, kernel + if (not isinstance(bootloader_setting["kernel"], dict)) and ( not isinstance(bootloader_setting["kernel"], str) ): @@ -176,6 +177,7 @@ def validate_kernels(bootloader_setting, bootloader_facts): % bootloader_setting["kernel"] ) return err, kernel_action, kernel + if (isinstance(bootloader_setting["kernel"], str)) and ( bootloader_setting["kernel"] not in kernel_str_values ): @@ -184,13 +186,14 @@ def validate_kernels(bootloader_setting, bootloader_facts): ", ".join(kernel_str_values), ) return err, kernel_action, kernel + if isinstance(bootloader_setting["kernel"], str): kernel_action = "modify" if state == "present" else "remove" kernel = escapeval(bootloader_setting["kernel"]) return err, kernel_action, kernel - """Process bootloader_setting["kernel"] being dict""" - """Validate kernel key and value""" + # Process bootloader_setting["kernel"] being dict + # Validate kernel key and value for key, value in bootloader_setting["kernel"].items(): if key not in kernel_keys: err = "kernel key in '%s: %s' must be one of '%s'" % ( @@ -202,6 +205,7 @@ def validate_kernels(bootloader_setting, bootloader_facts): if (not isinstance(value, str)) and (not isinstance(value, int)): err = "kernel value in '%s: %s' must be of type str or int" % (key, value) return err, kernel_action, kernel + if len(bootloader_setting["kernel"]) == 1: err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys) if not err: @@ -209,7 +213,7 @@ def validate_kernels(bootloader_setting, bootloader_facts): kernel = get_single_kernel(bootloader_setting["kernel"]) return err, kernel_action, kernel - """Validate with len(bootloader_setting["kernel"]) > 1""" + # Validate with len(bootloader_setting["kernel"]) > 1 for fact in bootloader_facts: # Rename kernel to path in fact dict if "kernel" in fact: @@ -226,7 +230,8 @@ def validate_kernels(bootloader_setting, bootloader_facts): elif not diff and same: kernel_action = "modify" if state == "present" else "remove" break - """Process kernel_action when none of the facts had same keys with bootloader_setting["kernel"]""" + + # Process kernel_action when none of the facts had same keys with bootloader_setting["kernel"] if not kernel_action: if len(bootloader_setting["kernel"]) != 3 and ( sorted(bootloader_setting["kernel"].keys()) != sorted(kernel_create_keys) @@ -237,15 +242,17 @@ def validate_kernels(bootloader_setting, bootloader_facts): ) return err, kernel_action, kernel kernel_action = "create" if state == "present" else "remove" - - if not kernel_action: + if kernel_action == "create": + kernel = get_create_kernel(bootloader_setting["kernel"]) + elif kernel_action == "remove": + kernel_to_mod = get_kernel_to_mod(bootloader_setting["kernel"], kernel_mod_keys) + kernel = get_single_kernel(kernel_to_mod) + elif kernel_action == "modify": err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys) if err: return err, kernel_action, kernel kernel_to_mod = get_kernel_to_mod(bootloader_setting["kernel"], kernel_mod_keys) kernel = get_single_kernel(kernel_to_mod) - else: - kernel = get_create_kernel(bootloader_setting["kernel"]) return err, kernel_action, kernel @@ -257,7 +264,7 @@ def escapeval(val): def get_boot_args(kernel_info): """Get arguments from kernel info""" args = re.search(r'args="(.*)"', kernel_info) - if args is None: + if not args: return "" return args.group(1).strip() @@ -290,6 +297,7 @@ def get_setting_name(kernel_setting): def get_add_kernel_cmd(bootloader_setting_options, kernel): """Build cmd to add a kernel with specified args""" boot_args = "" + args = "" for kernel_setting in bootloader_setting_options: setting_name = get_setting_name(kernel_setting) boot_args += setting_name + " " @@ -297,7 +305,7 @@ def get_add_kernel_cmd(bootloader_setting_options, kernel): args = "--args=" + escapeval(boot_args.strip()) if {"copy_default": True} in bootloader_setting_options: args += " --copy-default" - return "grubby %s %s" % (kernel, args) + return "grubby %s %s" % (kernel, args.strip()) def get_mod_boot_args_cmd(bootloader_setting_options, kernel, kernel_info): @@ -306,6 +314,7 @@ def get_mod_boot_args_cmd(bootloader_setting_options, kernel, kernel_info): boot_present_args = "" boot_mod_args = "" bootloader_args = get_boot_args(kernel_info) + for kernel_setting in bootloader_setting_options: setting_name = get_setting_name(kernel_setting) if "state" in kernel_setting and kernel_setting["state"] == "absent": @@ -320,6 +329,8 @@ def get_mod_boot_args_cmd(bootloader_setting_options, kernel, kernel_info): boot_mod_args += " --args=" + escapeval(boot_present_args.strip()) if boot_mod_args: return "grubby --update-kernel=" + kernel + boot_mod_args + else: + return None def get_rm_kernel_cmd(kernel): diff --git a/tests/tests_add_rm.yml b/tests/tests_add_rm.yml index 219a9c0..cb5dcca 100644 --- a/tests/tests_add_rm.yml +++ b/tests/tests_add_rm.yml @@ -23,12 +23,16 @@ loop: - src: "{{ (bootloader_facts | last).kernel }}" dest: "{{ (bootloader_facts | last).kernel }}_clone1" - - src: "{{ (bootloader_facts | last).initrd | regex_replace(' .*$', '') }}" - dest: "{{ (bootloader_facts | last).initrd | regex_replace(' .*$', '') }}_clone1" + - src: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}" + dest: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}_clone1" - src: "{{ (bootloader_facts | last).kernel }}" dest: "{{ (bootloader_facts | last).kernel }}_clone2" - - src: "{{ (bootloader_facts | last).initrd | regex_replace(' .*$', '') }}" - dest: "{{ (bootloader_facts | last).initrd | regex_replace(' .*$', '') }}_clone2" + - src: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}" + dest: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}_clone2" - name: Create Clone1 kernel with copy_defaults=true vars: diff --git a/tests/unit/test_bootloader_settings.py b/tests/unit/test_bootloader_settings.py index 839287c..ddac133 100644 --- a/tests/unit/test_bootloader_settings.py +++ b/tests/unit/test_bootloader_settings.py @@ -213,7 +213,8 @@ def test_validate_kernels(self): ) self.assertEqual( err, - "A kernel with provided {'path'} already exists and it's other fields are different {'title': ('Fedora Linux', 'Fedora Linux (6.5.12-100.fc37.x86_64) 37 (Workstation Edition)')}", + "A kernel with provided {'path'} already exists and it's other fields are different " + + "{'title': ('Fedora Linux', 'Fedora Linux (6.5.12-100.fc37.x86_64) 37 (Workstation Edition)')}", ) self.assertEqual(kernel_action, "") self.assertEqual(kernel, "") @@ -272,7 +273,9 @@ def test_get_add_kernel_cmd(self): ) self.assertEqual( add_kernel_cmd, - "grubby --title='Fedora Linux' --add-kernel=/boot/vmlinuz-6 --initrd=/boot/initramfs-6.6.img --args='arg_with_str_value=test_value arg_with_int_value=1 arg_without_val arg_with_str_value_absent=test_value arg_with_int_value_absent=1 arg_without_val_absent' --copy-default", + "grubby --title='Fedora Linux' --add-kernel=/boot/vmlinuz-6 --initrd=/boot/initramfs-6.6.img " + + "--args='arg_with_str_value=test_value arg_with_int_value=1 arg_without_val arg_with_str_value_absent=test_value " + + "arg_with_int_value_absent=1 arg_without_val_absent' --copy-default", ) def test_get_rm_kernel_cmd(self):