From fedc28ff6494877eb38bfa5b790fa394a0d59627 Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Thu, 21 Dec 2023 14:03:08 +0100 Subject: [PATCH 01/10] feat: Add the ability to create kernels --- library/bootloader_facts.py | 4 +- library/bootloader_settings.py | 313 ++++++++++++++++++++----- tests/tests_create.yml | 63 +++++ tests/tests_settings.yml | 25 +- tests/unit/test_bootloader_settings.py | 312 ++++++++++++++++++++++-- 5 files changed, 610 insertions(+), 107 deletions(-) create mode 100644 tests/tests_create.yml diff --git a/library/bootloader_facts.py b/library/bootloader_facts.py index 17b3924..05d93f9 100644 --- a/library/bootloader_facts.py +++ b/library/bootloader_facts.py @@ -109,11 +109,9 @@ def get_facts(kernels_info, default_kernel): for line in kernels_info_lines: index = re.search(r"index=(\d+)", line) if index: - is_default = False + is_default = index.group(1) == default_kernel.strip() index_count += 1 kernels.append({}) - if index.group(1) == default_kernel.strip(): - is_default = True search = re.search(r"(.*?)=(.*)", line) if search: key = search.group(1).strip('"') diff --git a/library/bootloader_settings.py b/library/bootloader_settings.py index 5e50c2b..5f67ffc 100644 --- a/library/bootloader_settings.py +++ b/library/bootloader_settings.py @@ -24,7 +24,22 @@ required: true type: list elements: dict - + suboptions: + kernel: + description: Kernels to operate on. Can be a string DEFAULT or ALL, or dict.clear + required: true + type: str or dict + state: + description: State of the kernel. + required: false + type: str + choices: ["absent", "present"] + default: "present" + options: + description: list bootloader arguments to apply + required: false + type: list + elements: dict author: - Sergei Petrosian (@spetrosi) """ @@ -37,16 +52,11 @@ RETURN = r""" # These are examples of possible return values, and in general should use other names for return values. -# original_message: -# description: The original name param that was passed in. -# type: str -# returned: always -# sample: 'hello world' -# message: -# description: The output message that the test module generates. -# type: str -# returned: always -# sample: 'goodbye' +actions: + description: Commands that the module runs + type: str + returned: always + # sample: 'hello world' """ import re @@ -58,34 +68,177 @@ import ansible.module_utils.six.moves as ansible_six_moves -def escapeval(val): - """make sure val is quoted as in shell""" - return ansible_six_moves.shlex_quote(str(val)) +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: + is_default = index.group(1) == default_kernel.strip() + index_count += 1 + kernels.append({}) + search = re.search(r"(.*?)=(.*)", line) + if search: + key = search.group(1).strip('"') + value = search.group(2).strip('"') + kernels[index_count - 1].update({key: value}) + else: + kernels[index_count - 1].update({"kernel": line}) + kernels[index_count - 1].update({"default": is_default}) + return kernels -def get_kernels(bootloader_setting_kernel, kernels_keys): - kernels = [] - for kernel_key in kernels_keys: - if ( - kernel_key in bootloader_setting_kernel - and kernel_key != bootloader_setting_kernel - ): - kernel_key_prefix = "" - if kernel_key == "kernel_title": - kernel_key_prefix = "TITLE=" - if not isinstance(bootloader_setting_kernel[kernel_key], list): - kernels.append( - kernel_key_prefix + str(bootloader_setting_kernel[kernel_key]) +def dict_compare(d1, d2): + d1_keys = set(d1.keys()) + d2_keys = set(d2.keys()) + shared_keys = d1_keys.intersection(d2_keys) + diff = {o: (d1[o], d2[o]) for o in shared_keys if d1[o] != d2[o]} + same = set(o for o in shared_keys if d1[o] == d2[o]) + return diff, same + + +def validate_kernel_initrd(bootloader_setting_kernel, kernel_mod_keys): + """Validate that initrd is not provided as a single key when not creating a kernel""" + if ( + len(bootloader_setting_kernel) == 1 + and "initrd" in bootloader_setting_kernel.keys() + ): + err = ( + "You can use 'initrd' as a kernel key only when you must create a kernel. To modify an existing kernel, use one of %s" + % ", ".join(kernel_mod_keys) + ) + return err + err = "" + return err + + +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} + + +def get_single_kernel(bootloader_setting_kernel): + """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 = "" + if kernel_key == "title": + kernel_key_prefix = "TITLE=" + return kernel_key_prefix + escapeval(kernel_val) + + +def get_create_kernel(bootloader_setting_kernel): + """Get kernel in the format expected by 'grubby --add-kernel=' from a multiple-element dict""" + kernel = "" + for key, value in bootloader_setting_kernel.items(): + if key == "path": + kernel += " --add-kernel=" + escapeval(value) + elif key == "title": + kernel += " --title=" + escapeval(value) + elif key == "initrd": + kernel += " --initrd=" + escapeval(value) + return kernel.strip() + + +def validate_kernels(bootloader_setting, bootloader_facts): + """Validate that user passes bootloader_setting correctly""" + err = "" + create_kernel = False + kernel = "" + kernel_str_values = ["DEFAULT", "ALL"] + kernel_keys = ["path", "index", "title", "initrd"] + kernel_create_keys = ["path", "title", "initrd"] + kernel_mod_keys = ["path", "title", "index"] + states = ["present", "absent"] + if "state" in bootloader_setting and bootloader_setting["state"] not in states: + err = "State must be one of '%s'" % ", ".join(states) + return err, create_kernel, kernel + if (not isinstance(bootloader_setting["kernel"], dict)) and ( + not isinstance(bootloader_setting["kernel"], str) + ): + err = ( + "kernel value in %s must be of type str or dict" + % bootloader_setting["kernel"] + ) + return err, create_kernel, kernel + if (isinstance(bootloader_setting["kernel"], str)) and ( + bootloader_setting["kernel"] not in kernel_str_values + ): + err = "kernel %s is of type str, it must be one of '%s'" % ( + bootloader_setting["kernel"], + ", ".join(kernel_str_values), + ) + return err, create_kernel, kernel + if isinstance(bootloader_setting["kernel"], str): + kernel = escapeval(bootloader_setting["kernel"]) + return err, create_kernel, kernel + + """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'" % ( + key, + value, + ", ".join(kernel_keys), + ) + return err, create_kernel, kernel + 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, create_kernel, kernel + + if len(bootloader_setting["kernel"]) == 1: + err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys) + if not err: + kernel = get_single_kernel(bootloader_setting["kernel"]) + return err, create_kernel, kernel + + """Validate with len(bootloader_setting["kernel"]) > 1""" + for fact in bootloader_facts: + # Rename kernel to path in fact dict + if "kernel" in fact: + fact["path"] = fact.pop("kernel") + diff, same = dict_compare(bootloader_setting["kernel"], fact) + if diff and same: + err = ( + "A kernel with provided %s already exists and it's other fields are different %s" + % (same, diff) + ) + return err, create_kernel, kernel + elif not same and diff: + if len(bootloader_setting["kernel"]) != 3 and sorted( + bootloader_setting["kernel"].keys() + ) != sorted(kernel_create_keys): + err = ( + "To create a kernel, you must provide 3 kernel keys - '%s'" + % ", ".join(kernel_create_keys) ) - else: - for kernel_entry in bootloader_setting_kernel[kernel_key]: - kernels.append(kernel_key_prefix + str(kernel_entry)) - elif kernel_key == bootloader_setting_kernel: - kernels.append(bootloader_setting_kernel) - return kernels + return err, create_kernel, kernel + create_kernel = True + break + elif not diff and same: + create_kernel = False + break + if not create_kernel: + err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys) + if err: + return err, create_kernel, 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, create_kernel, kernel + + +def escapeval(val): + """Make sure val is quoted as in shell""" + return ansible_six_moves.shlex_quote(str(val)) def get_boot_args(kernel_info): + """Get arguments from kernel info""" args = re.search(r'args="(.*)"', kernel_info) if args is None: return "" @@ -93,6 +246,7 @@ def get_boot_args(kernel_info): def get_rm_boot_args_cmd(kernel_info, kernel): + """Build cmd to rm all existing args for a kernel""" bootloader_args = get_boot_args(kernel_info) if bootloader_args: return ( @@ -103,18 +257,36 @@ def get_rm_boot_args_cmd(kernel_info, kernel): ) +def get_setting_name(kernel_setting): + """Get setting name based on whether it is with or without a value""" + if kernel_setting == {"previous": "replaced"}: + return "" + if "value" in kernel_setting: + return kernel_setting["name"] + "=" + str(kernel_setting["value"]) + else: + return kernel_setting["name"] + + +def get_add_kernel_cmd(bootloader_setting_options, kernel): + """Build cmd to add a kernel with specified args""" + boot_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()) + # Need to add ability to set --copy-default + return "grubby %s %s" % (kernel, args) + + def get_mod_boot_args_cmd(bootloader_setting_options, kernel, kernel_info): + """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: - if {"previous": "replaced"} == kernel_setting: - continue - if "value" in kernel_setting: - setting_name = kernel_setting["name"] + "=" + str(kernel_setting["value"]) - else: - setting_name = kernel_setting["name"] + setting_name = get_setting_name(kernel_setting) if "state" in kernel_setting and kernel_setting["state"] == "absent": if re.search(r"(^|$| )" + setting_name + r"(^|$| )", bootloader_args): boot_absent_args += setting_name + " " @@ -147,39 +319,50 @@ def run_module(): # args/params passed to the execution, as well as if the module # supports check mode module = AnsibleModule(argument_spec=module_args, supports_check_mode=True) - kernels_keys = ["kernel_index", "kernel_path", "kernel_title", "DEFAULT", "ALL"] for bootloader_setting in module.params["bootloader_settings"]: - kernels = get_kernels(bootloader_setting["kernel"], kernels_keys) - if not kernels: - module.fail_json( - msg="bootloader_settings.kernel must contain one of %s" - % ", ".join(kernels_keys), - **result - ) - for kernel in kernels: - kernel = escapeval(kernel) - # Remove all existing boot settings - if {"previous": "replaced"} in bootloader_setting["options"]: - rc, stdout, stderr = module.run_command("grubby --info=" + kernel) - rm_boot_args_cmd = get_rm_boot_args_cmd(stdout, kernel) - if rm_boot_args_cmd: - rc, stdout, stderr = module.run_command(str(rm_boot_args_cmd)) - result["changed"] = True - result["actions"].append(rm_boot_args_cmd) - rc, stdout, stderr = module.run_command("grubby --info=" + kernel) - # Configure boot settings + rc, kernels_info, stderr = module.run_command("grubby --info=ALL") + if "Permission denied" in stderr: + module.fail_json(msg="You must run this as sudo", **result) + + rc, default_kernel, stderr = module.run_command("grubby --default-index") + bootloader_facts = get_facts(kernels_info, default_kernel) + + err, create_kernel, kernel = validate_kernels( + bootloader_setting, bootloader_facts + ) + if err: + module.fail_json(msg=err, **result) + + # Remove all existing boot settings + if ({"previous": "replaced"} in bootloader_setting["options"]) and ( + not create_kernel + ): + rc, kernel_info, stderr = module.run_command("grubby --info=" + kernel) + rm_boot_args_cmd = get_rm_boot_args_cmd(kernel_info, kernel) + if rm_boot_args_cmd: + rc, stdout, stderr = module.run_command(rm_boot_args_cmd) + result["changed"] = True + result["actions"].append(rm_boot_args_cmd) + + # Create a kernel with provided options + if create_kernel: + add_kernel_cmd = get_add_kernel_cmd(bootloader_setting["options"], kernel) + rc, stdout, stderr = module.run_command(add_kernel_cmd) + result["changed"] = True + result["actions"].append(add_kernel_cmd) + + # Modify boot settings + else: + rc, kernel_info, stderr = module.run_command("grubby --info=" + kernel) mod_boot_args_cmd = get_mod_boot_args_cmd( - bootloader_setting["options"], kernel, stdout + bootloader_setting["options"], kernel, kernel_info ) if mod_boot_args_cmd: rc, stdout, stderr = module.run_command(mod_boot_args_cmd) result["changed"] = True result["actions"].append(mod_boot_args_cmd) - # if the user is working with this module in only check mode we do not - # want to make any changes to the environment, just return the current - # state with no modifications - # if module.check_mode: - # module.exit_json(**result) + else: + result["changed"] = False # in the event of a successful module execution, you will want to # simple AnsibleModule.exit_json(), passing the key/value results diff --git a/tests/tests_create.yml b/tests/tests_create.yml new file mode 100644 index 0000000..cad6466 --- /dev/null +++ b/tests/tests_create.yml @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: MIT +--- +- name: Test creating, modifying, and removing kernels + hosts: all + gather_facts: false + tags: + - tests::reboot + vars: + bootloader_reboot_ok: true + tasks: + - name: Get bootloader_facts + vars: + bootloader_gather_facts: true + include_role: + name: linux-system-roles.bootloader + + - name: Clone the oldest kernel and initrd for test purposes + copy: + src: "{{ item }}" + remote_src: true + dest: "{{ item }}_clone" + mode: preserve + loop: + # - "{{ (bootloader_facts | selectattr('index', 'search', '0') | first).kernel }}" + # - "{{ (bootloader_facts | selectattr('index', 'search', '0') | first).initrd }}" + - "{{ (bootloader_facts | last).kernel }}" + - "{{ (bootloader_facts | last).initrd | regex_replace(' .*$', '') }}" + + - name: Create kernel + vars: + bootloader_settings: + - kernel: + path: "{{ (bootloader_facts | last).kernel }}_clone" + initrd: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}_clone" + title: Test kernel + options: + - name: console + value: tty0 + state: present + # - copy_default: true + include_role: + name: linux-system-roles.bootloader + + - name: Flush handlers + meta: flush_handlers + + - name: Ensure bootloader_reboot_required is not set to true + assert: + that: not bootloader_reboot_required + + - name: Get bootloader_facts + vars: + bootloader_gather_facts: true + include_role: + name: linux-system-roles.bootloader + + - name: Verify settings + assert: + that: >- + (bootloader_facts | selectattr('title', 'search', 'Test kernel') | + first).args + | regex_search('^console=tty0$') diff --git a/tests/tests_settings.yml b/tests/tests_settings.yml index 653ca30..b98a697 100644 --- a/tests/tests_settings.yml +++ b/tests/tests_settings.yml @@ -20,22 +20,19 @@ - name: Verify that default bootloader is correct in bootloader_gather_facts vars: default_bootloader: "{{ - (bootloader_facts | selectattr('index', 'search', '0') | first).kernel - }}" - shell: >- - set -euo pipefail; - grubby --info=DEFAULT; - grubby --info=DEFAULT | - grep -P - 'kernel=("|){{ default_bootloader }}("|)' + (bootloader_facts | selectattr('default', 'equalto', True) | + first).kernel }}" + command: grubby --default-kernel changed_when: false + register: __default_kernel_cmd + failed_when: default_bootloader != __default_kernel_cmd.stdout - - name: Replace configuration with settings using kernel_index + - name: Replace configuration with settings using kernel index vars: bootloader_gather_facts: true bootloader_settings: - kernel: - kernel_index: "{{ + index: "{{ (bootloader_facts | selectattr('index', 'search', '0') | first).index }}" options: @@ -76,12 +73,12 @@ register: __bootloader_default_grub_content changed_when: false - - name: Change some settings using kernel_title + - name: Change some settings using kernel title vars: bootloader_gather_facts: true bootloader_settings: - kernel: - kernel_title: "{{ + title: "{{ (bootloader_facts | selectattr('index', 'search', '0') | first).title }}" options: @@ -114,11 +111,11 @@ register: __bootloader_default_grub_content changed_when: false - - name: Set existing variable using kernel_path, should report not changed + - name: Set existing variable using kernel path, should report not changed vars: bootloader_settings: - kernel: - kernel_path: "{{ + path: "{{ (bootloader_facts | selectattr('index', 'search', '0') | first).kernel }}" options: diff --git a/tests/unit/test_bootloader_settings.py b/tests/unit/test_bootloader_settings.py index 7292ee3..6beb859 100644 --- a/tests/unit/test_bootloader_settings.py +++ b/tests/unit/test_bootloader_settings.py @@ -23,6 +23,237 @@ {"previous": "replaced"}, ] +# SETTINGS_EXAMPLE = [ +# { +# "kernel": { +# "kernel_index": [ +# 0, +# 1 +# ] +# }, +# "options": [ +# { +# "name": "console", +# "value": "tty0" +# }, +# { +# "name": "print-fatal-signals", +# "value": 1 +# }, +# { +# "name": "no_timer_check", +# "state": "present" +# }, +# { +# "name": "quiet" +# }, +# { +# "name": "debug" +# }, +# { +# "previous": "replaced" +# } +# ] +# }, +# { +# "kernel": { +# "kernel_path": "path1" +# }, +# "options": [ +# { +# "name": "console", +# "value": "tty0" +# }, +# { +# "name": "print-fatal-signals", +# "value": 1 +# }, +# { +# "name": "no_timer_check", +# "state": "present" +# }, +# { +# "name": "quiet" +# }, +# { +# "name": "debug" +# } +# ] +# }, +# { +# "kernel": "ALL", +# "options": [ +# { +# "name": "console", +# "value": "tty0" +# }, +# { +# "name": "print-fatal-signals", +# "value": 1 +# }, +# { +# "name": "no_timer_check", +# "state": "present" +# }, +# { +# "name": "quiet" +# }, +# { +# "name": "debug" +# } +# ] +# }, +# { +# "kernel": "INCORRECT_STRING", +# "options": [ +# { +# "name": "console", +# "value": "tty0" +# }, +# { +# "name": "print-fatal-signals", +# "value": 1 +# }, +# { +# "name": "no_timer_check", +# "state": "present" +# }, +# { +# "name": "quiet" +# }, +# { +# "name": "debug" +# } +# ] +# } +# ] + +SETTINGS = [ + { + "kernel": "DEFAULT", + "options": OPTIONS + }, + { + "kernel": "ALL", + "options": OPTIONS + }, + { + "kernel": "INCORRECT_STRING", + "options": OPTIONS + }, + { + "kernel": { + "index": 1 + }, + "options": OPTIONS + }, + { + "kernel": { + "index": [ + 0, + 1 + ] + }, + "options": OPTIONS + }, + { + "kernel": { + "kernel_index": [ + 0, + 1 + ] + }, + "options": OPTIONS + }, + { + "kernel": { + "title": "Fedora Linux", + "path": "/boot/vmlinuz-6.5.12-100.fc37.x86_64" + }, + "options": OPTIONS + }, + { + "kernel": { + "title": "Fedora Linux", + "path": "/boot/vmlinuz-6" + }, + "options": OPTIONS + }, + { + "kernel": { + "title": "Fedora Linux", + "path": "/boot/vmlinuz-6", + "initrd": "/boot/initramfs-6.6.img" + }, + "options": OPTIONS + }, + { + "kernel": { + "initrd": "/boot/initramfs-6.6.img" + }, + "options": OPTIONS + }, + { + "kernel": { + "initrd": "/boot/initramfs-6.6.img" + }, + "options": OPTIONS, + "state": "test_state" + }, + { + "kernel": [ + { + "initrd": "/boot/initramfs-6.6.img" + } + ], + "options": OPTIONS + }, +] + +FACTS = [ + { + "args": "$tuned_params ro rootflags=subvol=root rd.luks.uuid=luks-9da1fdf5-14ac-49fd-a388-8b1ee48f3df1 rhgb quiet", + "id": "c44543d15b2c4e898912c2497f734e67-6.5.12-100.fc37.x86_64", + "index": "0", + "initrd": "/boot/initramfs-6.5.12-100.fc37.x86_64.img $tuned_initrd", + "kernel": "/boot/vmlinuz-6.5.12-100.fc37.x86_64", + "root": "UUID=65c70529-e9ad-4778-9001-18fe8c525285", + "title": "Fedora Linux (6.5.12-100.fc37.x86_64) 37 (Workstation Edition)", + "default": False, + }, + { + "args": "ro rootflags=subvol=root rd.luks.uuid=luks-9da1fdf5-14ac-49fd-a388-8b1ee48f3df1 rhgb quiet $tuned_params", + "id": "c44543d15b2c4e898912c2497f734e67-6.5.10-100.fc37.x86_64", + "index": "1", + "initrd": "/boot/initramfs-6.5.10-100.fc37.x86_64.img $tuned_initrd", + "kernel": "/boot/vmlinuz-6.5.10-100.fc37.x86_64", + "root": "UUID=65c70529-e9ad-4778-9001-18fe8c525285", + "title": "Fedora Linux (6.5.10-100.fc37.x86_64) 37 (Workstation Edition)", + "default": False, + }, + { + "args": "ro rootflags=subvol=root rd.luks.uuid=luks-9da1fdf5-14ac-49fd-a388-8b1ee48f3df1 rhgb quiet $tuned_params", + "id": "c44543d15b2c4e898912c2497f734e67-6.5.7-100.fc37.x86_64", + "index": "2", + "initrd": "/boot/initramfs-6.5.7-100.fc37.x86_64.img $tuned_initrd", + "kernel": "/boot/vmlinuz-6.5.7-100.fc37.x86_64", + "root": "UUID=65c70529-e9ad-4778-9001-18fe8c525285", + "title": "Fedora Linux (6.5.7-100.fc37.x86_64) 37 (Workstation Edition)", + "default": True, + }, + { + "args": "ro rootflags=subvol=root rd.luks.uuid=luks-9da1fdf5-14ac-49fd-a388-8b1ee48f3df1 rhgb quiet", + "id": "c44543d15b2c4e898912c2497f734e67-0-rescue", + "index": "3", + "initrd": "/boot/initramfs-0-rescue-c44543d15b2c4e898912c2497f734e67.img", + "kernel": "/boot/vmlinuz-0-rescue-c44543d15b2c4e898912c2497f734e67", + "root": "UUID=65c70529-e9ad-4778-9001-18fe8c525285", + "title": "Fedora Linux (0-rescue-c44543d15b2c4e898912c2497f734e67) 36 (Workstation Edition)", + "default": False, + }, + {"index": "4", "kernel": "non linux entry", "default": False}, +] + KERNELS = [ {"kernel": {"kernel_index": [0, 1]}}, {"kernel": {"kernel_index": 2}}, @@ -70,33 +301,64 @@ class InputValidator(unittest.TestCase): """test functions that process bootloader_settings argument""" - - def test_get_kernels(self): - kernels = bootloader_settings.get_kernels(KERNELS[0]["kernel"], kernels_keys) - self.assertEqual(["0", "1"], kernels) - kernels = bootloader_settings.get_kernels(KERNELS[1]["kernel"], kernels_keys) - self.assertEqual(["2"], kernels) - kernels = bootloader_settings.get_kernels(KERNELS[2]["kernel"], kernels_keys) - self.assertEqual(["/path/1", "/path/2"], kernels) - kernels = bootloader_settings.get_kernels(KERNELS[3]["kernel"], kernels_keys) - self.assertEqual(["/path/3"], kernels) - kernels = bootloader_settings.get_kernels(KERNELS[4]["kernel"], kernels_keys) + def test_validate_kernels(self): + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[0], FACTS) + self.assertEqual(err, "",) + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "DEFAULT") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[1], FACTS) + self.assertEqual(err, "") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "ALL") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[2], FACTS) + self.assertEqual(err, "kernel INCORRECT_STRING is of type str, it must be one of 'DEFAULT, ALL'") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[3], FACTS) + self.assertEqual(err, "") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "1") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[4], FACTS) + self.assertEqual(err, "kernel value in 'index: [0, 1]' must be of type str or int") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[5], FACTS) + # initrd can be provided ONLY when creating a kernel + self.assertEqual(err, "kernel key in 'kernel_index: [0, 1]' must be one of 'path, index, title, initrd'") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[6], FACTS) self.assertEqual( - [ - "TITLE=Fedora Linux (1.1.11-100.fc37.x86_64) 37 (Workstation Edition)", - "TITLE=Fedora Linux (1.1.11-200.fc37.x86_64) 37 (Workstation Edition)", - ], - kernels, + 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)')}" ) - kernels = bootloader_settings.get_kernels(KERNELS[5]["kernel"], kernels_keys) - self.assertEqual( - ["TITLE=Fedora Linux (1.1.11-300.fc37.x86_64) 37 (Workstation Edition)"], - kernels, - ) - kernels = bootloader_settings.get_kernels(KERNELS[6]["kernel"], kernels_keys) - self.assertEqual(["DEFAULT"], kernels) - kernels = bootloader_settings.get_kernels(KERNELS[7]["kernel"], kernels_keys) - self.assertEqual(["ALL"], kernels) + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[7], FACTS) + self.assertEqual(err, "To create a kernel, you must provide 3 kernel keys - 'path, title, initrd'") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[8], FACTS) + self.assertEqual(err, "") + self.assertEqual(create_kernel, True) + self.assertEqual(kernel, "--title='Fedora Linux' --add-kernel=/boot/vmlinuz-6 --initrd=/boot/initramfs-6.6.img") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[9], FACTS) + self.assertEqual(err, "You can use 'initrd' as a kernel key only when you must create a kernel. To modify an existing kernel, use one of path, title, index") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[10], FACTS) + self.assertEqual(err, "State must be one of 'present, absent'") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "") + err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[11], FACTS) + self.assertEqual(err, "kernel value in [{'initrd': '/boot/initramfs-6.6.img'}] must be of type str or dict") + self.assertEqual(create_kernel, False) + self.assertEqual(kernel, "") + + def test_get_add_kernel_cmd(self): + kernel = bootloader_settings.get_create_kernel(SETTINGS[8]["kernel"]) + add_kernel_cmd = bootloader_settings.get_add_kernel_cmd(SETTINGS[8]["options"], kernel) + 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'") def test_get_boot_args(self): bootloader_args = bootloader_settings.get_boot_args(INFO) From 46e35621412d558e0fc075a8595b4fbf7d6edded Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Thu, 21 Dec 2023 19:45:15 +0100 Subject: [PATCH 02/10] Add ability to add/remove kernels, and do --copy-defaults when adding --- library/bootloader_settings.py | 114 ++++++--- tests/tests_add_rm.yml | 145 +++++++++++ tests/tests_create.yml | 63 ----- tests/unit/test_bootloader_settings.py | 329 ++++++++++--------------- 4 files changed, 347 insertions(+), 304 deletions(-) create mode 100644 tests/tests_add_rm.yml delete mode 100644 tests/tests_create.yml diff --git a/library/bootloader_settings.py b/library/bootloader_settings.py index 5f67ffc..6a1772a 100644 --- a/library/bootloader_settings.py +++ b/library/bootloader_settings.py @@ -69,6 +69,7 @@ def get_facts(kernels_info, default_kernel): + """Get kernel facts""" kernels_info_lines = kernels_info.strip().split("\n") kernels = [] index_count = 0 @@ -89,12 +90,22 @@ def get_facts(kernels_info, default_kernel): return kernels -def dict_compare(d1, d2): - d1_keys = set(d1.keys()) - d2_keys = set(d2.keys()) - shared_keys = d1_keys.intersection(d2_keys) - diff = {o: (d1[o], d2[o]) for o in shared_keys if d1[o] != d2[o]} - same = set(o for o in shared_keys if d1[o] == d2[o]) +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 + + +def compare_dicts(dict1, dict2): + """Compare dict1 to dict2 and return same and different entries""" + dict1_keys = set(dict1.keys()) + dict2_keys = set(dict2.keys()) + shared_keys = dict1_keys.intersection(dict2_keys) + diff = {o: (dict1[o], dict2[o]) for o in shared_keys if dict1[o] != dict2[o]} + same = set(o for o in shared_keys if dict1[o] == dict2[o]) return diff, same @@ -145,16 +156,18 @@ def get_create_kernel(bootloader_setting_kernel): def validate_kernels(bootloader_setting, bootloader_facts): """Validate that user passes bootloader_setting correctly""" err = "" - create_kernel = False + kernel_action = "" kernel = "" + state = "" kernel_str_values = ["DEFAULT", "ALL"] kernel_keys = ["path", "index", "title", "initrd"] kernel_create_keys = ["path", "title", "initrd"] 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, create_kernel, kernel + return err, kernel_action, kernel if (not isinstance(bootloader_setting["kernel"], dict)) and ( not isinstance(bootloader_setting["kernel"], str) ): @@ -162,7 +175,7 @@ def validate_kernels(bootloader_setting, bootloader_facts): "kernel value in %s must be of type str or dict" % bootloader_setting["kernel"] ) - return err, create_kernel, kernel + return err, kernel_action, kernel if (isinstance(bootloader_setting["kernel"], str)) and ( bootloader_setting["kernel"] not in kernel_str_values ): @@ -170,10 +183,11 @@ def validate_kernels(bootloader_setting, bootloader_facts): bootloader_setting["kernel"], ", ".join(kernel_str_values), ) - return err, create_kernel, kernel + 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, create_kernel, kernel + return err, kernel_action, kernel """Process bootloader_setting["kernel"] being dict""" """Validate kernel key and value""" @@ -184,52 +198,55 @@ def validate_kernels(bootloader_setting, bootloader_facts): value, ", ".join(kernel_keys), ) - return err, create_kernel, kernel + return err, kernel_action, kernel 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, create_kernel, kernel - + 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, create_kernel, kernel + return err, kernel_action, kernel """Validate with len(bootloader_setting["kernel"]) > 1""" for fact in bootloader_facts: # Rename kernel to path in fact dict if "kernel" in fact: fact["path"] = fact.pop("kernel") - diff, same = dict_compare(bootloader_setting["kernel"], fact) + fact_trunc = get_dict_same_keys(bootloader_setting["kernel"], fact) + diff, same = compare_dicts(bootloader_setting["kernel"], fact_trunc) + # diff, same = compare_dicts(bootloader_setting["kernel"], fact) if diff and same: err = ( "A kernel with provided %s already exists and it's other fields are different %s" % (same, diff) ) - return err, create_kernel, kernel - elif not same and diff: - if len(bootloader_setting["kernel"]) != 3 and sorted( - bootloader_setting["kernel"].keys() - ) != sorted(kernel_create_keys): - err = ( - "To create a kernel, you must provide 3 kernel keys - '%s'" - % ", ".join(kernel_create_keys) - ) - return err, create_kernel, kernel - create_kernel = True - break + return err, kernel_action, kernel elif not diff and same: - create_kernel = False + kernel_action = "modify" if state == "present" else "remove" break - if not create_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) + ): + err = ( + "To create a kernel, you must provide 3 kernel keys - '%s'" + % ", ".join(kernel_create_keys) + ) + return err, kernel_action, kernel + kernel_action = "create" if state == "present" else "remove" + + if not kernel_action: err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys) if err: - return err, create_kernel, kernel + 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, create_kernel, kernel + return err, kernel_action, kernel def escapeval(val): @@ -259,7 +276,10 @@ def get_rm_boot_args_cmd(kernel_info, kernel): def get_setting_name(kernel_setting): """Get setting name based on whether it is with or without a value""" - if kernel_setting == {"previous": "replaced"}: + if ( + kernel_setting == {"previous": "replaced"} + or "copy_default" in kernel_setting.keys() + ): return "" if "value" in kernel_setting: return kernel_setting["name"] + "=" + str(kernel_setting["value"]) @@ -275,7 +295,8 @@ def get_add_kernel_cmd(bootloader_setting_options, kernel): boot_args += setting_name + " " if len(boot_args) > 0: args = "--args=" + escapeval(boot_args.strip()) - # Need to add ability to set --copy-default + if {"copy_default": True} in bootloader_setting_options: + args += " --copy-default" return "grubby %s %s" % (kernel, args) @@ -301,6 +322,11 @@ 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): + """Build cmd to remove a kernel""" + return "grubby --remove-kernel=%s" % kernel + + def run_module(): # define available arguments/parameters a user can pass to the module module_args = dict( @@ -327,16 +353,17 @@ def run_module(): rc, default_kernel, stderr = module.run_command("grubby --default-index") bootloader_facts = get_facts(kernels_info, default_kernel) - err, create_kernel, kernel = validate_kernels( + err, kernel_action, kernel = validate_kernels( bootloader_setting, bootloader_facts ) if err: module.fail_json(msg=err, **result) # Remove all existing boot settings - if ({"previous": "replaced"} in bootloader_setting["options"]) and ( - not create_kernel - ): + if ( + "options" in bootloader_setting + and {"previous": "replaced"} in bootloader_setting["options"] + ) and (kernel_action != "remove"): rc, kernel_info, stderr = module.run_command("grubby --info=" + kernel) rm_boot_args_cmd = get_rm_boot_args_cmd(kernel_info, kernel) if rm_boot_args_cmd: @@ -345,14 +372,14 @@ def run_module(): result["actions"].append(rm_boot_args_cmd) # Create a kernel with provided options - if create_kernel: + if kernel_action == "create": add_kernel_cmd = get_add_kernel_cmd(bootloader_setting["options"], kernel) rc, stdout, stderr = module.run_command(add_kernel_cmd) result["changed"] = True result["actions"].append(add_kernel_cmd) # Modify boot settings - else: + if kernel_action == "modify": rc, kernel_info, stderr = module.run_command("grubby --info=" + kernel) mod_boot_args_cmd = get_mod_boot_args_cmd( bootloader_setting["options"], kernel, kernel_info @@ -364,6 +391,13 @@ def run_module(): else: result["changed"] = False + # Remove a kernel + if kernel_action == "remove": + rm_kernel_cmd = get_rm_kernel_cmd(kernel) + rc, stdout, stderr = module.run_command(rm_kernel_cmd) + result["changed"] = True + result["actions"].append(rm_kernel_cmd) + # in the event of a successful module execution, you will want to # simple AnsibleModule.exit_json(), passing the key/value results module.exit_json(**result) diff --git a/tests/tests_add_rm.yml b/tests/tests_add_rm.yml new file mode 100644 index 0000000..219a9c0 --- /dev/null +++ b/tests/tests_add_rm.yml @@ -0,0 +1,145 @@ +# SPDX-License-Identifier: MIT +--- +- name: Test creating, modifying, and removing kernels + hosts: all + gather_facts: false + tags: + - tests::reboot + vars: + bootloader_reboot_ok: true + tasks: + - name: Get bootloader_facts + vars: + bootloader_gather_facts: true + include_role: + name: linux-system-roles.bootloader + + - name: Clone the oldest kernel and initrd for test purposes + copy: + src: "{{ item.src }}" + remote_src: true + dest: "{{ item.dest }}" + mode: preserve + 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).kernel }}" + dest: "{{ (bootloader_facts | last).kernel }}_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: + bootloader_settings: + - kernel: + path: "{{ (bootloader_facts | last).kernel }}_clone1" + initrd: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}_clone1" + title: Clone1 + options: + - name: console + value: tty0 + state: present + - copy_default: true + include_role: + name: linux-system-roles.bootloader + + - name: Flush handlers + meta: flush_handlers + + - name: Ensure bootloader_reboot_required is not set to true + assert: + that: not bootloader_reboot_required + + - name: Get bootloader_facts + vars: + bootloader_gather_facts: true + include_role: + name: linux-system-roles.bootloader + + - name: Verify settings + vars: + __default_args: "{{ + (bootloader_facts | selectattr('default', 'equalto', True) | + first).args }}" + assert: + that: >- + (bootloader_facts | selectattr('title', 'search', 'Clone1') | + first).args == __default_args ~ ' console=tty0' + + - name: Remove Clone1 kernel with 3 kernel keys + vars: + bootloader_gather_facts: true + bootloader_settings: + - kernel: + path: "{{ (bootloader_facts | last).kernel }}_clone1" + initrd: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}_clone1" + title: Clone1 + options: + - name: console + value: tty0 + state: present + - copy_default: true + state: absent + include_role: + name: linux-system-roles.bootloader + + - name: Verify that Clone1 kernel is removed + assert: + that: bootloader_facts | selectattr('title', 'search', 'Clone1') | + list | length == 0 + + - name: Create clone2 kernel without copy_defaults=true + vars: + bootloader_settings: + - kernel: + path: "{{ (bootloader_facts | last).kernel }}_clone2" + initrd: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}_clone2" + title: Clone2 + options: + - name: console + value: tty0 + state: present + include_role: + name: linux-system-roles.bootloader + + - name: Flush handlers + meta: flush_handlers + + - name: Ensure bootloader_reboot_required is not set to true + assert: + that: not bootloader_reboot_required + + - name: Get bootloader_facts + vars: + bootloader_gather_facts: true + include_role: + name: linux-system-roles.bootloader + + - name: Verify settings + assert: + that: >- + (bootloader_facts | selectattr('title', 'search', 'Clone2') | + first).args == 'console=tty0' + + - name: Remove Clone2 kernel with kernel path + vars: + bootloader_gather_facts: true + bootloader_settings: + - kernel: + path: "{{ (bootloader_facts | last).kernel }}_clone2" + initrd: "{{ (bootloader_facts | last).initrd | + regex_replace(' .*$', '') }}_clone2" + title: Clone2 + state: absent + include_role: + name: linux-system-roles.bootloader + + - name: Verify that Clone2 kernel is removed + assert: + that: bootloader_facts | selectattr('title', 'search', 'Clone2') | + list | length == 0 diff --git a/tests/tests_create.yml b/tests/tests_create.yml deleted file mode 100644 index cad6466..0000000 --- a/tests/tests_create.yml +++ /dev/null @@ -1,63 +0,0 @@ -# SPDX-License-Identifier: MIT ---- -- name: Test creating, modifying, and removing kernels - hosts: all - gather_facts: false - tags: - - tests::reboot - vars: - bootloader_reboot_ok: true - tasks: - - name: Get bootloader_facts - vars: - bootloader_gather_facts: true - include_role: - name: linux-system-roles.bootloader - - - name: Clone the oldest kernel and initrd for test purposes - copy: - src: "{{ item }}" - remote_src: true - dest: "{{ item }}_clone" - mode: preserve - loop: - # - "{{ (bootloader_facts | selectattr('index', 'search', '0') | first).kernel }}" - # - "{{ (bootloader_facts | selectattr('index', 'search', '0') | first).initrd }}" - - "{{ (bootloader_facts | last).kernel }}" - - "{{ (bootloader_facts | last).initrd | regex_replace(' .*$', '') }}" - - - name: Create kernel - vars: - bootloader_settings: - - kernel: - path: "{{ (bootloader_facts | last).kernel }}_clone" - initrd: "{{ (bootloader_facts | last).initrd | - regex_replace(' .*$', '') }}_clone" - title: Test kernel - options: - - name: console - value: tty0 - state: present - # - copy_default: true - include_role: - name: linux-system-roles.bootloader - - - name: Flush handlers - meta: flush_handlers - - - name: Ensure bootloader_reboot_required is not set to true - assert: - that: not bootloader_reboot_required - - - name: Get bootloader_facts - vars: - bootloader_gather_facts: true - include_role: - name: linux-system-roles.bootloader - - - name: Verify settings - assert: - that: >- - (bootloader_facts | selectattr('title', 'search', 'Test kernel') | - first).args - | regex_search('^console=tty0$') diff --git a/tests/unit/test_bootloader_settings.py b/tests/unit/test_bootloader_settings.py index 6beb859..839287c 100644 --- a/tests/unit/test_bootloader_settings.py +++ b/tests/unit/test_bootloader_settings.py @@ -21,192 +21,49 @@ {"name": "arg_with_int_value_absent", "value": 1, "state": "absent"}, {"name": "arg_without_val_absent", "state": "absent"}, {"previous": "replaced"}, + {"copy_default": True}, ] -# SETTINGS_EXAMPLE = [ -# { -# "kernel": { -# "kernel_index": [ -# 0, -# 1 -# ] -# }, -# "options": [ -# { -# "name": "console", -# "value": "tty0" -# }, -# { -# "name": "print-fatal-signals", -# "value": 1 -# }, -# { -# "name": "no_timer_check", -# "state": "present" -# }, -# { -# "name": "quiet" -# }, -# { -# "name": "debug" -# }, -# { -# "previous": "replaced" -# } -# ] -# }, -# { -# "kernel": { -# "kernel_path": "path1" -# }, -# "options": [ -# { -# "name": "console", -# "value": "tty0" -# }, -# { -# "name": "print-fatal-signals", -# "value": 1 -# }, -# { -# "name": "no_timer_check", -# "state": "present" -# }, -# { -# "name": "quiet" -# }, -# { -# "name": "debug" -# } -# ] -# }, -# { -# "kernel": "ALL", -# "options": [ -# { -# "name": "console", -# "value": "tty0" -# }, -# { -# "name": "print-fatal-signals", -# "value": 1 -# }, -# { -# "name": "no_timer_check", -# "state": "present" -# }, -# { -# "name": "quiet" -# }, -# { -# "name": "debug" -# } -# ] -# }, -# { -# "kernel": "INCORRECT_STRING", -# "options": [ -# { -# "name": "console", -# "value": "tty0" -# }, -# { -# "name": "print-fatal-signals", -# "value": 1 -# }, -# { -# "name": "no_timer_check", -# "state": "present" -# }, -# { -# "name": "quiet" -# }, -# { -# "name": "debug" -# } -# ] -# } -# ] - SETTINGS = [ - { - "kernel": "DEFAULT", - "options": OPTIONS - }, - { - "kernel": "ALL", - "options": OPTIONS - }, - { - "kernel": "INCORRECT_STRING", - "options": OPTIONS - }, - { - "kernel": { - "index": 1 - }, - "options": OPTIONS - }, - { - "kernel": { - "index": [ - 0, - 1 - ] - }, - "options": OPTIONS - }, - { - "kernel": { - "kernel_index": [ - 0, - 1 - ] - }, - "options": OPTIONS - }, + {"kernel": "DEFAULT", "options": OPTIONS}, + {"kernel": "ALL", "options": OPTIONS}, + {"kernel": "INCORRECT_STRING", "options": OPTIONS}, + {"kernel": {"index": 1}, "options": OPTIONS}, + {"kernel": {"index": [0, 1]}, "options": OPTIONS}, + {"kernel": {"kernel_index": [0, 1]}, "options": OPTIONS}, { "kernel": { "title": "Fedora Linux", - "path": "/boot/vmlinuz-6.5.12-100.fc37.x86_64" + "path": "/boot/vmlinuz-6.5.12-100.fc37.x86_64", }, - "options": OPTIONS + "options": OPTIONS, }, { - "kernel": { - "title": "Fedora Linux", - "path": "/boot/vmlinuz-6" - }, - "options": OPTIONS + "kernel": {"title": "Fedora Linux", "path": "/boot/vmlinuz-6"}, + "options": OPTIONS, }, { "kernel": { "title": "Fedora Linux", "path": "/boot/vmlinuz-6", - "initrd": "/boot/initramfs-6.6.img" + "initrd": "/boot/initramfs-6.6.img", }, - "options": OPTIONS + "options": OPTIONS, }, + {"kernel": {"initrd": "/boot/initramfs-6.6.img"}, "options": OPTIONS}, { - "kernel": { - "initrd": "/boot/initramfs-6.6.img" - }, - "options": OPTIONS + "kernel": {"initrd": "/boot/initramfs-6.6.img"}, + "options": OPTIONS, + "state": "test_state", }, + {"kernel": [{"initrd": "/boot/initramfs-6.6.img"}], "options": OPTIONS}, { "kernel": { - "initrd": "/boot/initramfs-6.6.img" + "title": "Fedora Linux (6.5.12-100.fc37.x86_64) 37 (Workstation Edition)", + "path": "/boot/vmlinuz-6.5.12-100.fc37.x86_64", + "initrd": "/boot/initramfs-6.5.12-100.fc37.x86_64.img $tuned_initrd", }, "options": OPTIONS, - "state": "test_state" - }, - { - "kernel": [ - { - "initrd": "/boot/initramfs-6.6.img" - } - ], - "options": OPTIONS }, ] @@ -301,64 +158,134 @@ class InputValidator(unittest.TestCase): """test functions that process bootloader_settings argument""" + def test_validate_kernels(self): - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[0], FACTS) - self.assertEqual(err, "",) - self.assertEqual(create_kernel, False) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[0], FACTS + ) + self.assertEqual( + err, + "", + ) + self.assertEqual(kernel_action, "modify") self.assertEqual(kernel, "DEFAULT") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[1], FACTS) - self.assertEqual(err, "") - self.assertEqual(create_kernel, False) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[1], FACTS + ) + self.assertEqual(err, "") + self.assertEqual(kernel_action, "modify") self.assertEqual(kernel, "ALL") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[2], FACTS) - self.assertEqual(err, "kernel INCORRECT_STRING is of type str, it must be one of 'DEFAULT, ALL'") - self.assertEqual(create_kernel, False) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[2], FACTS + ) + self.assertEqual( + err, + "kernel INCORRECT_STRING is of type str, it must be one of 'DEFAULT, ALL'", + ) + self.assertEqual(kernel_action, "") self.assertEqual(kernel, "") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[3], FACTS) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[3], FACTS + ) self.assertEqual(err, "") - self.assertEqual(create_kernel, False) + self.assertEqual(kernel_action, "modify") self.assertEqual(kernel, "1") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[4], FACTS) - self.assertEqual(err, "kernel value in 'index: [0, 1]' must be of type str or int") - self.assertEqual(create_kernel, False) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[4], FACTS + ) + self.assertEqual( + err, "kernel value in 'index: [0, 1]' must be of type str or int" + ) + self.assertEqual(kernel_action, "") self.assertEqual(kernel, "") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[5], FACTS) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[5], FACTS + ) # initrd can be provided ONLY when creating a kernel - self.assertEqual(err, "kernel key in 'kernel_index: [0, 1]' must be one of 'path, index, title, initrd'") - self.assertEqual(create_kernel, False) + self.assertEqual( + err, + "kernel key in 'kernel_index: [0, 1]' must be one of 'path, index, title, initrd'", + ) + self.assertEqual(kernel_action, "") self.assertEqual(kernel, "") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[6], FACTS) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[6], FACTS + ) 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(create_kernel, False) + self.assertEqual(kernel_action, "") self.assertEqual(kernel, "") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[7], FACTS) - self.assertEqual(err, "To create a kernel, you must provide 3 kernel keys - 'path, title, initrd'") - self.assertEqual(create_kernel, False) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[7], FACTS + ) + self.assertEqual( + err, + "To create a kernel, you must provide 3 kernel keys - 'path, title, initrd'", + ) + self.assertEqual(kernel_action, "") self.assertEqual(kernel, "") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[8], FACTS) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[8], FACTS + ) self.assertEqual(err, "") - self.assertEqual(create_kernel, True) - self.assertEqual(kernel, "--title='Fedora Linux' --add-kernel=/boot/vmlinuz-6 --initrd=/boot/initramfs-6.6.img") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[9], FACTS) - self.assertEqual(err, "You can use 'initrd' as a kernel key only when you must create a kernel. To modify an existing kernel, use one of path, title, index") - self.assertEqual(create_kernel, False) + self.assertEqual(kernel_action, "create") + self.assertEqual( + kernel, + "--title='Fedora Linux' --add-kernel=/boot/vmlinuz-6 --initrd=/boot/initramfs-6.6.img", + ) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[9], FACTS + ) + self.assertEqual( + err, + "You can use 'initrd' as a kernel key only when you must create a kernel. To modify an existing kernel, use one of path, title, index", + ) + self.assertEqual(kernel_action, "") self.assertEqual(kernel, "") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[10], FACTS) + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[10], FACTS + ) self.assertEqual(err, "State must be one of 'present, absent'") - self.assertEqual(create_kernel, False) + self.assertEqual(kernel_action, "") + self.assertEqual(kernel, "") + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[11], FACTS + ) + self.assertEqual( + err, + "kernel value in [{'initrd': '/boot/initramfs-6.6.img'}] must be of type str or dict", + ) + self.assertEqual(kernel_action, "") self.assertEqual(kernel, "") - err, create_kernel, kernel = bootloader_settings.validate_kernels(SETTINGS[11], FACTS) - self.assertEqual(err, "kernel value in [{'initrd': '/boot/initramfs-6.6.img'}] must be of type str or dict") - self.assertEqual(create_kernel, False) - self.assertEqual(kernel, "") + err, kernel_action, kernel = bootloader_settings.validate_kernels( + SETTINGS[12], FACTS + ) + self.assertEqual(err, "") + self.assertEqual(kernel_action, "modify") def test_get_add_kernel_cmd(self): kernel = bootloader_settings.get_create_kernel(SETTINGS[8]["kernel"]) - add_kernel_cmd = bootloader_settings.get_add_kernel_cmd(SETTINGS[8]["options"], kernel) - 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'") + add_kernel_cmd = bootloader_settings.get_add_kernel_cmd( + SETTINGS[8]["options"], kernel + ) + 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", + ) + + def test_get_rm_kernel_cmd(self): + kernel = bootloader_settings.get_single_kernel(SETTINGS[3]["kernel"]) + self.assertEqual( + kernel, + "1", + ) + rm_kernel_cmd = bootloader_settings.get_rm_kernel_cmd(kernel) + self.assertEqual( + rm_kernel_cmd, + "grubby --remove-kernel=1", + ) def test_get_boot_args(self): bootloader_args = bootloader_settings.get_boot_args(INFO) From cbdf2d4179aaf566e0253efcdedba9d0b27f4332 Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Thu, 21 Dec 2023 20:13:20 +0100 Subject: [PATCH 03/10] Update README --- README.md | 98 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 50 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index cb874be..88f82f5 100644 --- a/README.md +++ b/README.md @@ -25,60 +25,39 @@ With this variable, list kernels and their command line parameters that you want Required keys: 1. `kernel` - with this, specify the kernel to update settings for. -You can specify one or more kernels using the following criteria, you can use only single criteria at a time: +Each list should specify the same kernel using one or multiple keys. - * `kernel_path` - a specific kernel path, can be a list of paths - * `kernel_index` - a specific kernel index, can be a list of indexes - * `kernel_title` - a specific kernel title, can be a list of titles + If you want to you add a kernel, you must specify three keys - `path`, `title`, `initrd`. + + If you want to modify or remove a kerne, you can specify one or more key. + + You can also specify `DEFAULT` or `ALL` to update the default or all kernels. + + Available keys: + * `path` - kernel path + * `index` - kernel index + * `title` - kernel title + * `initrd` - kernel initrd image + + Available strings: * `DEFAULT` - to update the default entry * `ALL` - to update all of the entries +2. `state` - state of the kernel. + + Available values: `present`, `absent` + + Default: `present` + 2. `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`. * `state` - `present` (default) or `absent`. The value `absent` means to remove a setting with `name` name - name must be provided. * `previous` - Optional - the only value is `replaced` - this is used to specify that the previous settings should be replaced with the given settings. + * `copy_default` - Optional - when you create a kernel, you can specify `copy_default: true` to copy the default arguments to the created kernel -Example: - -```yaml -bootloader_settings: - - kernel: - kernel_path: /boot/vmlinuz-0-rescue-1 - options: - - name: console - value: tty0 - state: present - - previous: replaced - - kernel: - kernel_index: [1, 2, 3] - options: - - name: print-fatal-signals - value: 1 - - kernel: - kernel_title: Red Hat Enterprise Linux (4.1.1.1.el8.x86_64) 8 - options: - - name: no_timer_check - - kernel: - kernel_path: /boot/vmlinuz-0-rescue-1 - options: - - name: console - value: tty0 - - name: print-fatal-signals - value: 1 - - name: no_timer_check - state: present - - previous: replaced - - kernel: ALL - options: - - name: debug - state: present - - kernel: DEFAULT - options: - - name: quiet - state: present -``` +For an example, see [Example Playbook](#example-playbook). Default: `{}` @@ -195,24 +174,31 @@ For example: - hosts: all vars: bootloader_settings: + # Update an existing kernel using path and replacing previous settings - kernel: - kernel_path: /boot/vmlinuz-0-rescue-1 + path: /boot/vmlinuz-6.5.7-100.fc37.x86_64 options: - name: console value: tty0 state: present - previous: replaced + # Update an existing kernel using index - kernel: - kernel_index: [1, 2, 3] + index: 1 options: - name: print-fatal-signals value: 1 + # Update an existing kernel using title - kernel: - kernel_title: Red Hat Enterprise Linux (4.1.1.1.el8.x86_64) 8 + title: Red Hat Enterprise Linux (4.1.1.1.el8.x86_64) 8 options: - name: no_timer_check + state: present + # Add a kernel with arguments - kernel: - kernel_path: /boot/vmlinuz-0-rescue-1 + path: /boot/vmlinuz-6.5.7-100.fc37.x86_64 + initrd: /boot/initramfs-6.5.7-100.fc37.x86_64.img + title: My kernel options: - name: console value: tty0 @@ -220,11 +206,27 @@ For example: value: 1 - name: no_timer_check state: present - - previous: replaced + state: present + # Add a kernel with arguments and copying default arguments + - kernel: + path: /boot/vmlinuz-6.5.7-100.fc37.x86_64 + initrd: /boot/initramfs-6.5.7-100.fc37.x86_64.img + title: My kernel + options: + - name: console + value: tty0 + - copy_default: true + state: present + # Remove a kernel + - kernel: + title: My kernel + state: absent + # Update all kernels - kernel: ALL options: - name: debug state: present + # Update the default kernel - kernel: DEFAULT options: - name: quiet From b2e4ae0905337fe3fcfd693d82cc3b0e96df02cf Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Fri, 22 Dec 2023 13:59:58 +0100 Subject: [PATCH 04/10] Refactor and fix tests and linters --- README.md | 2 +- library/bootloader_facts.py | 2 + library/bootloader_settings.py | 61 ++++++++++++++++---------- tests/tests_add_rm.yml | 12 +++-- tests/unit/test_bootloader_facts.py | 6 +++ tests/unit/test_bootloader_settings.py | 9 ++-- 6 files changed, 61 insertions(+), 31 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..a1782fd 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): @@ -116,7 +113,7 @@ def validate_kernel_initrd(bootloader_setting_kernel, kernel_mod_keys): and "initrd" in bootloader_setting_kernel.keys() ): err = ( - "You can use 'initrd' as a kernel key only when you must create a kernel. To modify an existing kernel, use one of %s" + "You can use 'initrd' as a kernel key only when you must create a kernel. To modify or remove an existing kernel, use one of %s" % ", ".join(kernel_mod_keys) ) return err @@ -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,14 +205,16 @@ 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 + + # Validate with len(bootloader_setting["kernel"]) == 1 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"]) + kernel_action = "modify" if state == "present" else "remove" 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 +231,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) @@ -238,14 +244,19 @@ def validate_kernels(bootloader_setting, bootloader_facts): return err, kernel_action, kernel kernel_action = "create" if state == "present" else "remove" - if not kernel_action: - err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys) - if err: - return err, kernel_action, kernel + if kernel_action == "create": + kernel = get_create_kernel(bootloader_setting["kernel"]) + + err = validate_kernel_initrd(bootloader_setting["kernel"], kernel_mod_keys) + if err: + return err, kernel_action, kernel + + if 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": 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 +268,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 +301,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 +309,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 +318,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 +333,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_facts.py b/tests/unit/test_bootloader_facts.py index 8a77d06..26319a5 100644 --- a/tests/unit/test_bootloader_facts.py +++ b/tests/unit/test_bootloader_facts.py @@ -12,6 +12,7 @@ import unittest import bootloader_facts +import bootloader_settings # non linux entry: RHEL 7 might print such a message INFO = """ @@ -101,3 +102,8 @@ def test_get_facts(self): FACTS, kernels, ) + kernels = bootloader_settings.get_facts(INFO, "2") + self.assertEqual( + FACTS, + kernels, + ) diff --git a/tests/unit/test_bootloader_settings.py b/tests/unit/test_bootloader_settings.py index 839287c..8dba7c8 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, "") @@ -240,7 +241,7 @@ def test_validate_kernels(self): ) self.assertEqual( err, - "You can use 'initrd' as a kernel key only when you must create a kernel. To modify an existing kernel, use one of path, title, index", + "You can use 'initrd' as a kernel key only when you must create a kernel. To modify or remove an existing kernel, use one of path, title, index", ) 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): From 01b556d81146392db665d19e2379dd1bb21a76ce Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Tue, 2 Jan 2024 15:32:17 +0100 Subject: [PATCH 05/10] Attempt to fix tests --- handlers/main.yml | 3 +- tests/tests_add_rm.yml | 70 +++++++++++++++++++++++----------------- tests/tests_settings.yml | 3 ++ 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/handlers/main.yml b/handlers/main.yml index 4d46b2c..0839c0f 100644 --- a/handlers/main.yml +++ b/handlers/main.yml @@ -13,7 +13,8 @@ changed_when: true when: >- (ansible_distribution in ['CentOS', 'RedHat'] and - ansible_facts.distribution_major_version is version('7', '=')) or + (ansible_facts.distribution_major_version is version('7', '=') or + ansible_facts.distribution_major_version is version('9', '='))) or ansible_distribution == 'Fedora' - name: Rebuild grub config diff --git a/tests/tests_add_rm.yml b/tests/tests_add_rm.yml index cb5dcca..2aa587c 100644 --- a/tests/tests_add_rm.yml +++ b/tests/tests_add_rm.yml @@ -14,37 +14,44 @@ include_role: name: linux-system-roles.bootloader - - name: Clone the oldest kernel and initrd for test purposes + # Images in CI might have a grub timeout set to a different other than the + # default 5 value. + # In this case, the above invocation require handlers to be flushed. + - name: Flush handlers + meta: flush_handlers + + - name: Set fact with the default kernel to use for clones + set_fact: + test_kernel: "{{ bootloader_facts | rejectattr('initrd', 'undefined') + | selectattr('default', 'equalto', True) | first }}" + + - name: Clone test_kernel kernel and initrd for test purposes copy: src: "{{ item.src }}" remote_src: true dest: "{{ item.dest }}" mode: preserve 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).kernel }}" - dest: "{{ (bootloader_facts | last).kernel }}_clone2" - - src: "{{ (bootloader_facts | last).initrd | - regex_replace(' .*$', '') }}" - dest: "{{ (bootloader_facts | last).initrd | - regex_replace(' .*$', '') }}_clone2" + - src: "{{ test_kernel.kernel }}" + dest: "{{ test_kernel.kernel }}_clone1" + - src: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}" + dest: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}_clone1" + - src: "{{ test_kernel.kernel }}" + dest: "{{ test_kernel.kernel }}_clone2" + - src: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}" + dest: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}_clone2" - name: Create Clone1 kernel with copy_defaults=true vars: bootloader_settings: - kernel: - path: "{{ (bootloader_facts | last).kernel }}_clone1" - initrd: "{{ (bootloader_facts | last).initrd | + path: "{{ test_kernel.kernel }}_clone1" + initrd: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}_clone1" title: Clone1 options: - - name: console - value: tty0 + - name: test + value: setting state: present - copy_default: true include_role: @@ -66,20 +73,22 @@ - name: Verify settings vars: __default_args: "{{ - (bootloader_facts | selectattr('default', 'equalto', True) | + (bootloader_facts | selectattr('title', 'defined') | + selectattr('default', 'equalto', True) | first).args }}" assert: that: >- - (bootloader_facts | selectattr('title', 'search', 'Clone1') | - first).args == __default_args ~ ' console=tty0' + (bootloader_facts | selectattr('title', 'defined') | + selectattr('title', 'search', 'Clone1') | + first).args == __default_args ~ ' test=setting' - name: Remove Clone1 kernel with 3 kernel keys vars: bootloader_gather_facts: true bootloader_settings: - kernel: - path: "{{ (bootloader_facts | last).kernel }}_clone1" - initrd: "{{ (bootloader_facts | last).initrd | + path: "{{ test_kernel.kernel }}_clone1" + initrd: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}_clone1" title: Clone1 options: @@ -93,15 +102,16 @@ - name: Verify that Clone1 kernel is removed assert: - that: bootloader_facts | selectattr('title', 'search', 'Clone1') | + that: bootloader_facts | selectattr('title', 'defined') | + selectattr('title', 'search', 'Clone1') | list | length == 0 - name: Create clone2 kernel without copy_defaults=true vars: bootloader_settings: - kernel: - path: "{{ (bootloader_facts | last).kernel }}_clone2" - initrd: "{{ (bootloader_facts | last).initrd | + path: "{{ test_kernel.kernel }}_clone2" + initrd: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}_clone2" title: Clone2 options: @@ -127,7 +137,8 @@ - name: Verify settings assert: that: >- - (bootloader_facts | selectattr('title', 'search', 'Clone2') | + (bootloader_facts | selectattr('title', 'defined') | + selectattr('title', 'search', 'Clone2') | first).args == 'console=tty0' - name: Remove Clone2 kernel with kernel path @@ -135,8 +146,8 @@ bootloader_gather_facts: true bootloader_settings: - kernel: - path: "{{ (bootloader_facts | last).kernel }}_clone2" - initrd: "{{ (bootloader_facts | last).initrd | + path: "{{ test_kernel.kernel }}_clone2" + initrd: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}_clone2" title: Clone2 state: absent @@ -145,5 +156,6 @@ - name: Verify that Clone2 kernel is removed assert: - that: bootloader_facts | selectattr('title', 'search', 'Clone2') | + that: bootloader_facts | selectattr('title', 'defined') | + selectattr('title', 'search', 'Clone2') | list | length == 0 diff --git a/tests/tests_settings.yml b/tests/tests_settings.yml index b98a697..2bd48dd 100644 --- a/tests/tests_settings.yml +++ b/tests/tests_settings.yml @@ -14,6 +14,9 @@ include_role: name: linux-system-roles.bootloader + # Images in CI might have a grub timeout set to a different other than the + # default 5 value. + # In this case, the above invocation require handlers to be flushed. - name: Flush handlers meta: flush_handlers From 542cad4fe7ddae7a76e45eba8d349ad7e8c961b4 Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Thu, 4 Jan 2024 10:30:58 +0100 Subject: [PATCH 06/10] Attempt removing condition for bug workaround --- handlers/main.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/handlers/main.yml b/handlers/main.yml index 0839c0f..7da327f 100644 --- a/handlers/main.yml +++ b/handlers/main.yml @@ -11,11 +11,6 @@ {{ __bootloader_default_grub }} cat {{ __bootloader_default_grub }} changed_when: true - when: >- - (ansible_distribution in ['CentOS', 'RedHat'] and - (ansible_facts.distribution_major_version is version('7', '=') or - ansible_facts.distribution_major_version is version('9', '='))) or - ansible_distribution == 'Fedora' - name: Rebuild grub config command: grub2-mkconfig -o {{ __bootloader_grub_conf }} From b09868047559dba059c4cff203b216b436ddac96 Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Mon, 8 Jan 2024 17:21:34 +0100 Subject: [PATCH 07/10] Add conf dirs for SLES it is required by sap roles --- tasks/main.yml | 13 +++++-------- vars/main.yml | 7 +++++++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tasks/main.yml b/tasks/main.yml index cbf9a0d..dfc7aae 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -29,17 +29,14 @@ - name: Set boot loader configuration files vars: - efi_path: "{{ - ansible_distribution in ['CentOS', 'RedHat'] and - ansible_facts.distribution_major_version | int < 9 and - __bootloader_efi_dir.stat.exists }}" + efi_path: "{{ __bootloader_efi_dir.stat.exists }}" set_fact: __bootloader_grub_conf: >- - {{ efi_path | ternary('/boot/efi/EFI/redhat/grub.cfg', - '/boot/grub2/grub.cfg') }} + {{ efi_path | ternary(__bootloader_uefi_conf_dir ~ 'grub.cfg', + /boot/grub2/ ~ 'grub.cfg') }} __bootloader_user_conf: >- - {{ efi_path | ternary('/boot/efi/EFI/redhat/user.cfg', - '/boot/grub2/user.cfg') }} + {{ efi_path | ternary(__bootloader_uefi_conf_dir ~ 'user.cfg', + /boot/grub2/ ~ 'user.cfg') }} - name: Update boot loader password when: bootloader_password is not none diff --git a/vars/main.yml b/vars/main.yml index 2b2aa6d..df6c2c7 100644 --- a/vars/main.yml +++ b/vars/main.yml @@ -14,3 +14,10 @@ __bootloader_required_facts_subsets: "{{ ['!all', '!min'] + __bootloader_packages: - grubby __bootloader_default_grub: /etc/default/grub +__bootloader_uefi_conf_dir: >- + {% if ansible_distribution == 'RedHat' %} + /boot/efi/EFI/redhat/ + {% elif ansible_distribution in ['SLES', 'SLES_SAP'] %} + /boot/efi/EFI/BOOT/ + {% endif %} +__bootloader_bios_conf_dir: /boot/grub2/ From bea2321b8e135afe81f7254a0e2a4de14bee57df Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Mon, 8 Jan 2024 17:21:54 +0100 Subject: [PATCH 08/10] Fix Variable defined multiple times warning --- README.md | 2 +- library/bootloader_settings.py | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ce93804..c24b9ae 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Role Name +# bootloader [![ansible-lint.yml](https://github.com/linux-system-roles/bootloader/actions/workflows/ansible-lint.yml/badge.svg)](https://github.com/linux-system-roles/bootloader/actions/workflows/ansible-lint.yml) [![ansible-test.yml](https://github.com/linux-system-roles/bootloader/actions/workflows/ansible-test.yml/badge.svg)](https://github.com/linux-system-roles/bootloader/actions/workflows/ansible-test.yml) [![codeql.yml](https://github.com/linux-system-roles/bootloader/actions/workflows/codeql.yml/badge.svg)](https://github.com/linux-system-roles/bootloader/actions/workflows/codeql.yml) [![markdownlint.yml](https://github.com/linux-system-roles/bootloader/actions/workflows/markdownlint.yml/badge.svg)](https://github.com/linux-system-roles/bootloader/actions/workflows/markdownlint.yml) [![python-unit-test.yml](https://github.com/linux-system-roles/bootloader/actions/workflows/python-unit-test.yml/badge.svg)](https://github.com/linux-system-roles/bootloader/actions/workflows/python-unit-test.yml) [![woke.yml](https://github.com/linux-system-roles/bootloader/actions/workflows/woke.yml/badge.svg)](https://github.com/linux-system-roles/bootloader/actions/workflows/woke.yml) diff --git a/library/bootloader_settings.py b/library/bootloader_settings.py index a1782fd..ec3a596 100644 --- a/library/bootloader_settings.py +++ b/library/bootloader_settings.py @@ -361,11 +361,11 @@ def run_module(): # supports check mode module = AnsibleModule(argument_spec=module_args, supports_check_mode=True) for bootloader_setting in module.params["bootloader_settings"]: - rc, kernels_info, stderr = module.run_command("grubby --info=ALL") + _unused, kernels_info, stderr = module.run_command("grubby --info=ALL") if "Permission denied" in stderr: module.fail_json(msg="You must run this as sudo", **result) - rc, default_kernel, stderr = module.run_command("grubby --default-index") + _unused, default_kernel, _unused = module.run_command("grubby --default-index") bootloader_facts = get_facts(kernels_info, default_kernel) err, kernel_action, kernel = validate_kernels( @@ -382,25 +382,27 @@ def run_module(): rc, kernel_info, stderr = module.run_command("grubby --info=" + kernel) rm_boot_args_cmd = get_rm_boot_args_cmd(kernel_info, kernel) if rm_boot_args_cmd: - rc, stdout, stderr = module.run_command(rm_boot_args_cmd) + _unused, stdout, _unused = module.run_command(rm_boot_args_cmd) result["changed"] = True result["actions"].append(rm_boot_args_cmd) # Create a kernel with provided options if kernel_action == "create": add_kernel_cmd = get_add_kernel_cmd(bootloader_setting["options"], kernel) - rc, stdout, stderr = module.run_command(add_kernel_cmd) + _unused, stdout, _unused = module.run_command(add_kernel_cmd) result["changed"] = True result["actions"].append(add_kernel_cmd) # Modify boot settings if kernel_action == "modify": - rc, kernel_info, stderr = module.run_command("grubby --info=" + kernel) + _unused, kernel_info, _unused = module.run_command( + "grubby --info=" + kernel + ) mod_boot_args_cmd = get_mod_boot_args_cmd( bootloader_setting["options"], kernel, kernel_info ) if mod_boot_args_cmd: - rc, stdout, stderr = module.run_command(mod_boot_args_cmd) + _unused, stdout, _unused = module.run_command(mod_boot_args_cmd) result["changed"] = True result["actions"].append(mod_boot_args_cmd) else: @@ -409,7 +411,7 @@ def run_module(): # Remove a kernel if kernel_action == "remove": rm_kernel_cmd = get_rm_kernel_cmd(kernel) - rc, stdout, stderr = module.run_command(rm_kernel_cmd) + _unused, stdout, _unused = module.run_command(rm_kernel_cmd) result["changed"] = True result["actions"].append(rm_kernel_cmd) From 051cd3199cee83eabfec779943c4857deb27d41b Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Mon, 8 Jan 2024 17:23:25 +0100 Subject: [PATCH 09/10] selectattr evaluates as boolean by default --- tests/tests_add_rm.yml | 4 ++-- tests/tests_settings.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/tests_add_rm.yml b/tests/tests_add_rm.yml index 2aa587c..cec9f42 100644 --- a/tests/tests_add_rm.yml +++ b/tests/tests_add_rm.yml @@ -23,7 +23,7 @@ - name: Set fact with the default kernel to use for clones set_fact: test_kernel: "{{ bootloader_facts | rejectattr('initrd', 'undefined') - | selectattr('default', 'equalto', True) | first }}" + | selectattr('default') | first }}" - name: Clone test_kernel kernel and initrd for test purposes copy: @@ -74,7 +74,7 @@ vars: __default_args: "{{ (bootloader_facts | selectattr('title', 'defined') | - selectattr('default', 'equalto', True) | + selectattr('default') | first).args }}" assert: that: >- diff --git a/tests/tests_settings.yml b/tests/tests_settings.yml index 2bd48dd..09888eb 100644 --- a/tests/tests_settings.yml +++ b/tests/tests_settings.yml @@ -23,7 +23,7 @@ - name: Verify that default bootloader is correct in bootloader_gather_facts vars: default_bootloader: "{{ - (bootloader_facts | selectattr('default', 'equalto', True) | + (bootloader_facts | selectattr('default') | first).kernel }}" command: grubby --default-kernel changed_when: false From 94b54215956397ab18d37c33069eb9787bc97448 Mon Sep 17 00:00:00 2001 From: Sergei Petrosian Date: Mon, 8 Jan 2024 17:27:20 +0100 Subject: [PATCH 10/10] Fix typos --- tasks/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks/main.yml b/tasks/main.yml index dfc7aae..c15579c 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -33,10 +33,10 @@ set_fact: __bootloader_grub_conf: >- {{ efi_path | ternary(__bootloader_uefi_conf_dir ~ 'grub.cfg', - /boot/grub2/ ~ 'grub.cfg') }} + '/boot/grub2/grub.cfg') }} __bootloader_user_conf: >- {{ efi_path | ternary(__bootloader_uefi_conf_dir ~ 'user.cfg', - /boot/grub2/ ~ 'user.cfg') }} + '/boot/grub2/user.cfg') }} - name: Update boot loader password when: bootloader_password is not none