Refactor and fix tests and linters

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`.

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:

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,14 +209,15 @@ 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:
             kernel_action = "modify" if state == "present" else "remove"
             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,25 +303,29 @@ 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 + " "
     if len(boot_args) > 0:
         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
 

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:

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):