Skip to content

Commit

Permalink
Refactor and fix tests and linters
Browse files Browse the repository at this point in the history
  • Loading branch information
spetrosi committed Dec 22, 2023
1 parent e9c83d6 commit 3cae929
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 26 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 2 additions & 0 deletions library/bootloader_facts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
49 changes: 30 additions & 19 deletions library/bootloader_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -73,6 +73,7 @@ def get_facts(kernels_info, default_kernel):
kernels_info_lines = kernels_info.strip().split("\n")
kernels = []
index_count = 0

Check warning on line 75 in library/bootloader_settings.py

View check run for this annotation

Codecov / codecov/patch

library/bootloader_settings.py#L73-L75

Added lines #L73 - L75 were not covered by tests

for line in kernels_info_lines:
index = re.search(r"index=(\d+)", line)
if index:
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
):
Expand All @@ -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
):
Expand All @@ -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'" % (
Expand All @@ -202,14 +205,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:
Expand All @@ -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)
Expand All @@ -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)

Check warning on line 249 in library/bootloader_settings.py

View check run for this annotation

Codecov / codecov/patch

library/bootloader_settings.py#L248-L249

Added lines #L248 - L249 were not covered by tests
elif kernel_action == "modify":
err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys)
if err:
return err, kernel_action, kernel

Check warning on line 253 in library/bootloader_settings.py

View check run for this annotation

Codecov / codecov/patch

library/bootloader_settings.py#L253

Added line #L253 was not covered by tests
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


Expand All @@ -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()

Expand Down Expand Up @@ -290,14 +297,15 @@ 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 + " "
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):
Expand All @@ -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":
Expand All @@ -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

Check warning on line 333 in library/bootloader_settings.py

View check run for this annotation

Codecov / codecov/patch

library/bootloader_settings.py#L333

Added line #L333 was not covered by tests


def get_rm_kernel_cmd(kernel):
Expand Down
12 changes: 8 additions & 4 deletions tests/tests_add_rm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
7 changes: 5 additions & 2 deletions tests/unit/test_bootloader_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 3cae929

Please sign in to comment.