fix: Modify grub timeout in grub config directly (#86)

* fix: Modify grub timeout in grub config directly

* Use replace instead of lineinfile

* Add proper cleanup

handlers/main.yml

@@ -12,9 +12,5 @@
     cat {{ __bootloader_default_grub }}
   changed_when: true
 
-- name: Rebuild grub config
-  command: grub2-mkconfig -o {{ __bootloader_grub_conf }}
-  changed_when: true
-
 - name: Reboot system
   include_tasks: tasks/reboot.yml

tasks/main.yml

@@ -17,33 +17,6 @@
     - Fix default kernel boot parameters
     - Reboot system
 
-- name: Get grub directory stat
-  stat:
-    path: "{{ __bootloader_default_grub | dirname }}"
-  register: __bootloader_grub_dir_stat
-
-- name: Ensure grub directory exists
-  file:
-    path: "{{ __bootloader_default_grub | dirname }}"
-    state: directory
-    mode: "{{ __bootloader_grub_dir_stat.stat.exists |
-      ternary(__bootloader_grub_dir_stat.stat.mode, '0755') }}"
-
-- name: Get grub config stat
-  stat:
-    path: "{{ __bootloader_default_grub }}"
-  register: __bootloader_grub_stat
-
-- name: Update boot loader timeout configuration
-  lineinfile:
-    path: "{{ __bootloader_default_grub }}"
-    regexp: '^GRUB_TIMEOUT=.*'
-    line: 'GRUB_TIMEOUT={{ bootloader_timeout }}'
-    create: true
-    mode: "{{ __bootloader_grub_stat.stat.exists |
-      ternary(__bootloader_grub_stat.stat.mode, '0644') }}"
-  notify: Rebuild grub config
-
 - name: Determine platform type
   stat:
     path: /sys/firmware/efi
@@ -60,6 +33,33 @@
       {{ efi_path | ternary(__bootloader_uefi_conf_dir ~ 'user.cfg',
       '/boot/grub2/user.cfg') }}
 
+- name: Get stat of {{ __bootloader_default_grub }}
+  stat:
+    path: "{{ __bootloader_default_grub }}"
+  register: __bootloader_grub_stat
+
+- name: >-
+    Update boot loader timeout configuration in {{ __bootloader_default_grub }}
+  replace:
+    path: "{{ __bootloader_default_grub }}"
+    regexp: '^GRUB_TIMEOUT=.*'
+    replace: 'GRUB_TIMEOUT={{ bootloader_timeout }}'
+    mode: "{{ __bootloader_grub_stat.stat.exists |
+      ternary(__bootloader_grub_stat.stat.mode, '0644') }}"
+
+- name: Get stat of {{ __bootloader_grub_conf }}
+  stat:
+    path: "{{ __bootloader_grub_conf }}"
+  register: __bootloader_grub_conf_stat
+
+- name: Update boot loader timeout configuration in {{ __bootloader_grub_conf }}
+  replace:
+    path: "{{ __bootloader_grub_conf }}"
+    regexp: 'set timeout=.*'
+    replace: 'set timeout={{ bootloader_timeout }}'
+    mode: "{{ __bootloader_grub_conf_stat.stat.exists |
+      ternary(__bootloader_grub_conf_stat.stat.mode, '0644') }}"
+
 - name: Update boot loader password
   when: bootloader_password is not none
   block:

tests/tasks/clone_kernel.yml

@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: MIT
+---
+- name: Set fact with the default kernel to use for clones
+  set_fact:
+    test_kernel: "{{ bootloader_facts | rejectattr('initrd', 'undefined')
+      | selectattr('default') | 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: "{{ test_kernel.kernel }}"
+      dest: "{{ test_kernel.kernel }}_clone{{ __bootloader_clone_num }}"
+    - src: "{{ test_kernel.initrd | regex_replace(' .*$', '') }}"
+      dest: >-
+        {{ test_kernel.initrd |
+        regex_replace(' .*$', '') }}_clone{{ __bootloader_clone_num }}
+
+- name: Create kernel Clone{{ __bootloader_clone_num }} 
+  vars:
+    bootloader_settings:
+      - kernel:
+          path: "{{ test_kernel.kernel }}_clone{{ __bootloader_clone_num }}"
+          initrd: "{{ test_kernel.initrd |
+            regex_replace(' .*$', '') }}_clone{{ __bootloader_clone_num }}"
+          title: Clone{{ __bootloader_clone_num }}
+        options:
+          - name: test
+            value: kernel
+            state: present
+          - copy_default: "{{ __bootloader_copy_default }}"
+  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

tests/tests_add_rm.yml

@@ -8,158 +8,100 @@
   vars:
     bootloader_reboot_ok: true
   tasks:
-    - name: Get bootloader_facts
-      vars:
-        bootloader_gather_facts: true
-      include_role:
-        name: linux-system-roles.bootloader
-
-    - name: Skip test on ostree systems
-      meta: end_host
-      when: __bootloader_is_ostree
-
-    # 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') | 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: "{{ 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: "{{ test_kernel.kernel }}_clone1"
-              initrd: "{{ test_kernel.initrd |
-                regex_replace(' .*$', '') }}_clone1"
-              title: Clone1
-            options:
-              - name: test
-                value: setting
-                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('title', 'defined') |
-          selectattr('default') |
-          first).args }}"
-      assert:
-        that: >-
-          (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: "{{ test_kernel.kernel }}_clone1"
-              initrd: "{{ test_kernel.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', 'defined') |
-          selectattr('title', 'search', 'Clone1') |
-          list | length == 0
-
-    - name: Create clone2 kernel without copy_defaults=true
-      vars:
-        bootloader_settings:
-          - kernel:
-              path: "{{ test_kernel.kernel }}_clone2"
-              initrd: "{{ test_kernel.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', 'defined') |
-          selectattr('title', 'search', 'Clone2') |
-          first).args == 'console=tty0'
-
-    - name: Remove Clone2 kernel with kernel path
-      vars:
-        bootloader_gather_facts: true
-        bootloader_settings:
-          - kernel:
-              path: "{{ test_kernel.kernel }}_clone2"
-              initrd: "{{ test_kernel.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', 'defined') |
-          selectattr('title', 'search', 'Clone2') |
-          list | length == 0
+    - name: Run in a block to clean up afterwards
+      block:
+        - name: Get bootloader_facts
+          vars:
+            bootloader_gather_facts: true
+          include_role:
+            name: linux-system-roles.bootloader
+
+        - name: Skip test on ostree systems
+          meta: end_host
+          when: __bootloader_is_ostree
+
+        # 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: Clone kernel to Clone1
+          vars:
+            __bootloader_clone_num: 1
+            __bootloader_copy_default: true
+          include_tasks: tasks/clone_kernel.yml
+
+        - name: Verify settings
+          vars:
+            __default_args: "{{
+              (bootloader_facts | selectattr('title', 'defined') |
+              selectattr('default') |
+              first).args }}"
+          assert:
+            that: >-
+              (bootloader_facts | selectattr('title', 'defined') |
+              selectattr('title', 'search', 'Clone1') |
+              first).args == __default_args ~ ' test=kernel'
+
+        - name: Remove Clone1 kernel with 3 kernel keys
+          vars:
+            bootloader_gather_facts: true
+            bootloader_settings:
+              - kernel:
+                  path: "{{ test_kernel.kernel }}_clone1"
+                  initrd: "{{ test_kernel.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', 'defined') |
+              selectattr('title', 'search', 'Clone1') |
+              list | length == 0
+
+        - name: Clone kernel to Clone2
+          vars:
+            __bootloader_clone_num: 2
+            __bootloader_copy_default: false
+          include_tasks: tasks/clone_kernel.yml
+
+        - name: Verify settings
+          assert:
+            that: >-
+              (bootloader_facts | selectattr('title', 'defined') |
+              selectattr('title', 'search', 'Clone2') |
+              first).args == 'test=kernel'
+
+        - name: Remove Clone2 kernel with kernel path
+          vars:
+            bootloader_gather_facts: true
+            bootloader_settings:
+              - kernel:
+                  path: "{{ test_kernel.kernel }}_clone2"
+                state: absent
+          include_role:
+            name: linux-system-roles.bootloader
+
+        - name: Verify that Clone2 kernel is removed
+          assert:
+            that: bootloader_facts | selectattr('title', 'defined') |
+              selectattr('title', 'search', 'Clone2') |
+              list | length == 0
+      always:
+        - name: Remove cloned kernels
+          command: >-
+            grubby --remove-kernel={{ test_kernel.kernel }}_clone{{ item }}
+          loop:
+            - 1
+            - 2
+          changed_when: true
+          failed_when: false