From a984315026e14e3f9c4655e9ca28edbe3fac87d2 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 1 Jul 2025 08:41:33 -0600 Subject: [PATCH] refactor: support Ansible 2.19 Ansible 2.19 introduces some big changes https://docs.ansible.com/ansible/devel/porting_guides/porting_guide_core_2.19.html One big change is that data structures are no longer mutable by the use of python methods such as `__setitem__`, `setdefault`, `update`, etc. in Jinja constructs. Instead, items must use filters or other Jinja operations. One common idiom is to mutate each element in a list. Since we cannot do this "in-place" anymore, a common way to do this is: ```yaml - name: Construct a new list from an existing list and mutate each element set_fact: __new_list: "{{ __new_list | d([]) + [mutated_item] }}" loop: "{{ old_list }}" mutated_item: "{{ some value based on item from old list }}" - name: Reset original old list set_fact: old_list: "{{ __new_list }}" ``` Similarly with `dict` items: ```yaml - name: Construct a new dict from an existing dict and mutate each element set_fact: __new_dict: "{{ __new_dict | d({}) | combine(mutated_item) }}" loop: "{{ old_dict | dict2items }}" mutated_item: "{{ {item.key: mutation of item.value} }}" - name: Reset original old dict set_fact: old_dict: "{{ __new_dict }}" ``` Another big change is that a boolean expression in a `when` or similar construct must be converted to a boolean - we cannot rely on the implicit evaluation in a boolean context. For example, if `var` is some iterable, like a `dict`, `list`, or `string`, you used to be able to evaluate an empty value in a boolean context: ```yaml when: var # do this only if var is not empty ``` You now have to explicitly test for empty using `length`: ```yaml when: var | length > 0 # do this only if var is not empty ``` Signed-off-by: Rich Megginson --- tasks/main.yml | 104 +++++++++++------- templates/kernel_settings.j2 | 54 +++++---- .../assert_kernel_settings_conf_files.yml | 6 +- tests/tasks/check_header.yml | 4 +- tests/vars/vars_simple_settings.yml | 3 +- 5 files changed, 98 insertions(+), 73 deletions(-) diff --git a/tasks/main.yml b/tasks/main.yml index 1d6f1446..902f2d1b 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -120,63 +120,87 @@ path: "{{ __kernel_settings_profile_filename }}" register: __kernel_settings_profile_contents -- name: Apply kernel settings - template: - src: "{{ __kernel_settings_profile_src }}.j2" - dest: "{{ __kernel_settings_profile_filename }}" - mode: "0644" - vars: - __sysctl_old: "{{ __kernel_settings_profile_contents.data.get('sysctl', {}) +- name: Initialize new sysctl + set_fact: + __kernel_settings_new_sysctl: "{{ + __kernel_settings_profile_contents.data.get('sysctl', {}) if not kernel_settings_purge and kernel_settings_sysctl != __kernel_settings_state_empty and not __kernel_settings_previous_replaced in kernel_settings_sysctl else {} }}" - __sysfs_old: "{{ __kernel_settings_profile_contents.data.get('sysfs', {}) + +- name: Set new sysctl + set_fact: + __kernel_settings_new_sysctl: "{{ __kernel_settings_new_sysctl | combine(__new_item) }}" + loop: "{{ [] if kernel_settings_sysctl == __kernel_settings_state_empty + else kernel_settings_sysctl | rejectattr('previous', 'defined') | list }}" + vars: + __new_item: "{{ {item.name: __new_value} }}" + __new_value: "{{ __kernel_settings_state_absent + if item.state | d('present') == 'absent' + else item.value | d(__kernel_settings_state_absent) }}" + +- name: Initialize new sysfs + set_fact: + __kernel_settings_new_sysfs: "{{ + __kernel_settings_profile_contents.data.get('sysfs', {}) if not kernel_settings_purge and kernel_settings_sysfs != __kernel_settings_state_empty and not __kernel_settings_previous_replaced in kernel_settings_sysfs else {} }}" + +- name: Set new sysfs + set_fact: + __kernel_settings_new_sysfs: "{{ __kernel_settings_new_sysfs | combine(__new_item) }}" + loop: "{{ [] if kernel_settings_sysfs == __kernel_settings_state_empty + else kernel_settings_sysfs | rejectattr('previous', 'defined') | list }}" + vars: + __new_item: "{{ {item.name: __new_value} }}" + __new_value: "{{ __kernel_settings_state_absent + if item.state | d('present') == 'absent' + else item.value | d(__kernel_settings_state_absent) }}" + +- name: Apply kernel settings + template: + src: "{{ __kernel_settings_profile_src }}.j2" + dest: "{{ __kernel_settings_profile_filename }}" + mode: "0644" + vars: + # we don't have a way to compare an item to a dict - eq not available in el7 + # so assume if the value is a dict, it is the {"state": "absent"} dict + # because "real" values should be scalars like strings, int, bool + __sysctl_has_values: "{{ __kernel_settings_new_sysctl | dict2items | rejectattr('value', 'mapping') | list | length > 0 }}" + __sysfs_has_values: "{{ __kernel_settings_new_sysfs | dict2items | rejectattr('value', 'mapping') | list | length > 0 }}" __systemd_old: "{{ - __kernel_settings_profile_contents.data.get('systemd', {}).get('cpu_affinity') + __kernel_settings_profile_contents.data.get('systemd', {}).get('cpu_affinity', '') if not kernel_settings_purge and kernel_settings_systemd_cpu_affinity != __kernel_settings_state_absent - else none }}" + else '' }}" + __systemd_new: "{{ kernel_settings_systemd_cpu_affinity + if kernel_settings_systemd_cpu_affinity is not none and + kernel_settings_systemd_cpu_affinity != __kernel_settings_state_absent and + kernel_settings_systemd_cpu_affinity | length > 0 + else __systemd_old }}" __trans_huge_old: "{{ - __kernel_settings_profile_contents.data.get('vm', {}).get('transparent_hugepages') + __kernel_settings_profile_contents.data.get('vm', {}).get('transparent_hugepages', '') if not kernel_settings_purge and kernel_settings_transparent_hugepages != __kernel_settings_state_absent - else none }}" + else '' }}" + __trans_huge_new: "{{ kernel_settings_transparent_hugepages + if kernel_settings_transparent_hugepages is not none and + kernel_settings_transparent_hugepages != __kernel_settings_state_absent and + kernel_settings_transparent_hugepages | length > 0 + else __trans_huge_old }}" __trans_defrag_old: "{{ - __kernel_settings_profile_contents.data.get('vm', {}).get('transparent_hugepage.defrag') + __kernel_settings_profile_contents.data.get('vm', {}).get('transparent_hugepage.defrag', '') if not kernel_settings_purge and kernel_settings_transparent_hugepages_defrag != __kernel_settings_state_absent - else none }}" - __sections: - - name: sysctl - new: "{{ kernel_settings_sysctl | difference([__kernel_settings_previous_replaced]) | list - if kernel_settings_sysctl != __kernel_settings_state_empty - else [] }}" - old: "{{ __sysctl_old }}" - - name: sysfs - new: "{{ kernel_settings_sysfs | difference([__kernel_settings_previous_replaced]) | list - if kernel_settings_sysfs != __kernel_settings_state_empty - else [] }}" - old: "{{ __sysfs_old }}" - - name: systemd - new: - - name: cpu_affinity - value: "{{ kernel_settings_systemd_cpu_affinity }}" - old: - cpu_affinity: "{{ __systemd_old }}" - - name: vm - new: - - name: transparent_hugepages - value: "{{ kernel_settings_transparent_hugepages }}" - - name: transparent_hugepage.defrag - value: "{{ kernel_settings_transparent_hugepages_defrag }}" - old: - transparent_hugepages: "{{ __trans_huge_old }}" - transparent_hugepage.defrag: "{{ __trans_defrag_old }}" + else '' }}" + __trans_defrag_new: "{{ kernel_settings_transparent_hugepages_defrag + if kernel_settings_transparent_hugepages_defrag is not none and + kernel_settings_transparent_hugepages_defrag != __kernel_settings_state_absent and + kernel_settings_transparent_hugepages_defrag | length > 0 + else __trans_defrag_old }}" register: __kernel_settings_register_apply # this will also apply the kernel_settings profile, so we diff --git a/templates/kernel_settings.j2 b/templates/kernel_settings.j2 index 9db83076..9ecb587f 100644 --- a/templates/kernel_settings.j2 +++ b/templates/kernel_settings.j2 @@ -1,33 +1,31 @@ {{ ansible_managed | comment }} {{ "system_role:kernel_settings" | comment(prefix="", postfix="") }} -[main] -summary = kernel settings -{% set __settings = {} %} -{% for section in __sections %} -{% set section_name = section["name"] %} -{% for item in section["new"] %} -{% if item.state | d() == "absent" %} -{% set _ = __settings.setdefault(section_name, {}).__setitem__(item.name, __kernel_settings_state_absent) %} -{% elif item.value != none and item.value != "" %} -{% set _ = __settings.setdefault(section_name, {}).__setitem__(item.name, item.value) %} -{% endif %} -{% endfor %} -{% for key, value in section["old"].items() %} -{% if not __settings.get(section_name, {}).__contains__(key) and value != none and value != "" %} -{% set _ = __settings.setdefault(section_name, {}).__setitem__(key, value) %} -{% endif %} -{% endfor %} -{% endfor %} -{% set seen_sections = {} %} -{% for section_name in __settings.keys() | sort %} -{% set section = __settings[section_name] %} -{% for key in section.keys() | sort %} -{% if section[key] != __kernel_settings_state_absent %} -{% if not seen_sections.__contains__(section_name) %} -{% set _ = seen_sections.__setitem__(section_name, true) %} +{% macro write_section(section_name, settings) %} [{{ section_name }}] -{% endif %} -{{ key }} = {{ section[key] }} +{% for key, val in settings.items() %} +{% if val != {"state": "absent"} %} +{{ key }} = {{ val }} {% endif %} {% endfor %} -{% endfor %} +{% endmacro %} +[main] +summary = kernel settings +{% if __sysctl_has_values %} +{{ write_section("sysctl", __kernel_settings_new_sysctl) -}} +{% endif %} +{% if __sysfs_has_values %} +{{ write_section("sysfs", __kernel_settings_new_sysfs) -}} +{% endif %} +{% if __systemd_new | length > 0 %} +[systemd] +cpu_affinity = {{ __systemd_new }} +{% endif %} +{% if __trans_huge_new | length > 0 or __trans_defrag_new | length > 0 %} +[vm] +{% if __trans_huge_new | length > 0 %} +transparent_hugepages = {{ __trans_huge_new }} +{% endif %} +{% if __trans_defrag_new | length > 0 %} +transparent_hugepage.defrag = {{ __trans_defrag_new }} +{% endif %} +{% endif %} diff --git a/tests/tasks/assert_kernel_settings_conf_files.yml b/tests/tasks/assert_kernel_settings_conf_files.yml index b6fbb756..f3635bea 100644 --- a/tests/tasks/assert_kernel_settings_conf_files.yml +++ b/tests/tasks/assert_kernel_settings_conf_files.yml @@ -80,14 +80,16 @@ ignore_errors: true register: __kernel_settings_register_verify_bl_cmdline when: - - __kernel_settings_blcmdline_value | d() + - __kernel_settings_blcmdline_value | d() is not none + - __kernel_settings_blcmdline_value | d("") | length > 0 changed_when: false - name: Verify bootloader settings value set_fact: __kernel_settings_success: false when: - - __kernel_settings_blcmdline_value | d() + - __kernel_settings_blcmdline_value | d() is not none + - __kernel_settings_blcmdline_value | d("") | length > 0 - __kernel_settings_register_verify_bl_cmdline is defined - __kernel_settings_register_verify_bl_cmdline.stdout is defined - __kernel_settings_register_verify_bl_cmdline is failed or diff --git a/tests/tasks/check_header.yml b/tests/tasks/check_header.yml index 607320f0..efcac838 100644 --- a/tests/tasks/check_header.yml +++ b/tests/tasks/check_header.yml @@ -9,8 +9,8 @@ - name: Check for presence of ansible managed header, fingerprint assert: that: - - ansible_managed in content + - __ansible_managed in content - __fingerprint in content vars: content: "{{ (__file_content | d(__content)).content | b64decode }}" - ansible_managed: "{{ lookup('template', 'get_ansible_managed.j2') }}" + __ansible_managed: "{{ lookup('template', 'get_ansible_managed.j2') }}" diff --git a/tests/vars/vars_simple_settings.yml b/tests/vars/vars_simple_settings.yml index 5407e1c4..d3ff679c 100644 --- a/tests/vars/vars_simple_settings.yml +++ b/tests/vars/vars_simple_settings.yml @@ -53,7 +53,8 @@ __kernel_settings_profile_file: | /sys/kernel/debug/x86/ibrs_enabled = 0 /sys/kernel/debug/x86/pti_enabled = 0 /sys/kernel/debug/x86/retp_enabled = 0 - {% if __kernel_settings_blcmdline_value | d() %} + {% if __kernel_settings_blcmdline_value | d() is not none and + __kernel_settings_blcmdline_value | d("") | length > 0 %} [bootloader] cmdline = {{ __kernel_settings_blcmdline_value }} {% endif %}