From 149e52e4e5e128261e414f377b74c07c68254168 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 | 4 +- library/bootloader_settings.py | 78 ++++++++++++++++---------- tests/tests_add_rm.yml | 12 ++-- tests/unit/test_bootloader_settings.py | 7 ++- 5 files changed, 66 insertions(+), 37 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..6a93d02 100644 --- a/library/bootloader_facts.py +++ b/library/bootloader_facts.py @@ -102,10 +102,12 @@ from ansible.module_utils.basic import AnsibleModule -def get_facts(kernels_info, default_kernel): +def get_facts(kernels_info: str, default_kernel: str) -> list: + """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..285990a 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 @@ -68,11 +68,12 @@ import ansible.module_utils.six.moves as ansible_six_moves -def get_facts(kernels_info, default_kernel): +def get_facts(kernels_info: str, default_kernel: str) -> list: """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: @@ -90,16 +91,12 @@ def get_facts(kernels_info, default_kernel): return kernels -def get_dict_same_keys(dict1, dict2): +def get_dict_same_keys(dict1: dict, dict2: dict) -> dict: """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): +def compare_dicts(dict1: dict, dict2: dict) -> (dict, set): """Compare dict1 to dict2 and return same and different entries""" dict1_keys = set(dict1.keys()) dict2_keys = set(dict2.keys()) @@ -109,7 +106,9 @@ def compare_dicts(dict1, dict2): return diff, same -def validate_kernel_initrd(bootloader_setting_kernel, kernel_mod_keys): +def validate_kernel_initrd( + bootloader_setting_kernel: dict, kernel_mod_keys: list +) -> str: """Validate that initrd is not provided as a single key when not creating a kernel""" if ( len(bootloader_setting_kernel) == 1 @@ -124,14 +123,16 @@ def validate_kernel_initrd(bootloader_setting_kernel, kernel_mod_keys): return err -def get_kernel_to_mod(bootloader_setting_kernel, kernel_mod_keys): +def get_kernel_to_mod(bootloader_setting_kernel: dict, kernel_mod_keys: list) -> dict: """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): +def get_single_kernel(bootloader_setting_kernel: dict) -> str: """Get kernel in the format expected by 'grubby --update-kernel=' from a one-element dict""" kernel_key, kernel_val = list(bootloader_setting_kernel.items())[0] kernel_key_prefix = "" @@ -140,7 +141,7 @@ def get_single_kernel(bootloader_setting_kernel): return kernel_key_prefix + escapeval(kernel_val) -def get_create_kernel(bootloader_setting_kernel): +def get_create_kernel(bootloader_setting_kernel: dict) -> str: """Get kernel in the format expected by 'grubby --add-kernel=' from a multiple-element dict""" kernel = "" for key, value in bootloader_setting_kernel.items(): @@ -153,7 +154,9 @@ def get_create_kernel(bootloader_setting_kernel): return kernel.strip() -def validate_kernels(bootloader_setting, bootloader_facts): +def validate_kernels( + bootloader_setting: dict, bootloader_facts: list +) -> (str, str, str): """Validate that user passes bootloader_setting correctly""" err = "" kernel_action = "" @@ -165,9 +168,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 +181,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 +190,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 +209,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 +217,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 +234,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,6 +246,13 @@ def validate_kernels(bootloader_setting, bootloader_facts): ) return err, kernel_action, kernel kernel_action = "create" if state == "present" else "remove" + 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) if not kernel_action: err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys) @@ -254,15 +270,15 @@ def escapeval(val): return ansible_six_moves.shlex_quote(str(val)) -def get_boot_args(kernel_info): +def get_boot_args(kernel_info: str) -> str: """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() -def get_rm_boot_args_cmd(kernel_info, kernel): +def get_rm_boot_args_cmd(kernel_info: str, kernel: str) -> str: """Build cmd to rm all existing args for a kernel""" bootloader_args = get_boot_args(kernel_info) if bootloader_args: @@ -274,7 +290,7 @@ def get_rm_boot_args_cmd(kernel_info, kernel): ) -def get_setting_name(kernel_setting): +def get_setting_name(kernel_setting: dict) -> str: """Get setting name based on whether it is with or without a value""" if ( kernel_setting == {"previous": "replaced"} @@ -287,9 +303,10 @@ def get_setting_name(kernel_setting): return kernel_setting["name"] -def get_add_kernel_cmd(bootloader_setting_options, kernel): +def get_add_kernel_cmd(bootloader_setting_options: list, kernel: str) -> str: """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,15 +314,18 @@ 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): +def get_mod_boot_args_cmd( + bootloader_setting_options: list, kernel: str, kernel_info: str +) -> str: """Build cmd to modify args for a kernel""" boot_absent_args = "" 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": @@ -322,7 +342,7 @@ def get_mod_boot_args_cmd(bootloader_setting_options, kernel, kernel_info): return "grubby --update-kernel=" + kernel + boot_mod_args -def get_rm_kernel_cmd(kernel): +def get_rm_kernel_cmd(kernel: str) -> str: """Build cmd to remove a kernel""" return "grubby --remove-kernel=%s" % 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):