From 642811327d2ab88a8014fffdf45c036d46061b1f Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 21 Nov 2023 16:43:53 -0700 Subject: [PATCH] refactor: improve support for ostree systems The dependency on `ansible.utils.update_fact` is causing issue with some users who now must install that collection in order to run the role, even if they do not care about ostree. The fix is to stop trying to set `ansible_facts.pkg_mgr`, and instead force the use of the ostree package manager with the `package:` module `use:` option. The strategy is - on ostree systems, set the flag `__$ROLENAME_is_ostree` if the system is an ostree system. The flag will either be undefined or `false` on non-ostree systems. Then, change every invocation of the `package:` module like this: ```yaml - name: Ensure required packages are present package: name: "{{ __$ROLENAME_packages }}" state: present use: "{{ (__$ROLENAME_is_ostree | d(false)) | ternary('ansible.posix.rhel_rpm_ostree', omit) }}" ``` This should ensure that the `use:` parameter is not used if the system is non-ostree. The goal is to make the ostree support as unobtrusive as possible for non-ostree systems. The user can also set `__$ROLENAME_is_ostree: true` in the inventory or play if the user knows that ostree is being used and wants to skip the check. Or, the user is concerned about the performance hit for ostree detection on non-ostree systems, and sets `__$ROLENAME_is_ostree: false` to skip the check. The flag `__$ROLENAME_is_ostree` can also be used in the role or tests to include or exclude tasks from being run on ostree systems. This fix also improves error reporting in the `get_ostree_data.sh` script when included roles cannot be found. Signed-off-by: Rich Megginson --- .ostree/get_ostree_data.sh | 41 ++++++++++++++++++++++--------- meta/collection-requirements.yml | 1 - tasks/firewalld.yml | 20 ++++++--------- tests/tests_reload_on_reset.yml | 20 ++++++--------- tests/tests_startup_conflicts.yml | 20 ++++++--------- 5 files changed, 52 insertions(+), 50 deletions(-) diff --git a/.ostree/get_ostree_data.sh b/.ostree/get_ostree_data.sh index d0a03a3..cec08b0 100755 --- a/.ostree/get_ostree_data.sh +++ b/.ostree/get_ostree_data.sh @@ -2,7 +2,6 @@ set -euo pipefail -role_collection_dir="${ROLE_COLLECTION_DIR:-fedora/linux_system_roles}" ostree_dir="${OSTREE_DIR:-"$(dirname "$(realpath "$0")")"}" if [ -z "${4:-}" ] || [ "${1:-}" = help ] || [ "${1:-}" = -h ]; then @@ -29,24 +28,40 @@ if [ "$pkgtype" = testing ]; then fi get_rolepath() { - local ostree_dir role rolesdir + local ostree_dir role rolesdir roles_parent_dir coll_path pth ostree_dir="$1" role="$2" - rolesdir="$(dirname "$(dirname "$ostree_dir")")/$role/.ostree" + roles_parent_dir="$(dirname "$(dirname "$ostree_dir")")" + rolesdir="$roles_parent_dir/$role/.ostree" + # assumes collection format if [ -d "$rolesdir" ]; then echo "$rolesdir" return 0 fi - if [ -n "${ANSIBLE_COLLECTIONS_PATHS:-}" ]; then - for pth in ${ANSIBLE_COLLECTIONS_PATHS//:/ }; do - rolesdir="$pth/ansible_collections/$role_collection_dir/roles/$role/.ostree" - if [ -d "$rolesdir" ]; then - echo "$rolesdir" - return 0 - fi + # assumes legacy role format like linux-system-roles.$role/ + for rolesdir in "$roles_parent_dir"/*-system-roles."$role"/.ostree; do + if [ -d "$rolesdir" ]; then + echo "$rolesdir" + return 0 + fi + done + # look elsewhere + coll_path="${ANSIBLE_COLLECTIONS_PATH:-}" + if [ -z "$coll_path" ]; then + coll_path="${ANSIBLE_COLLECTIONS_PATHS:-}" + fi + if [ -n "${coll_path}" ]; then + for pth in ${coll_path//:/ }; do + for rolesdir in "$pth"/ansible_collections/*/*_system_roles/roles/"$role"/.ostree; do + if [ -d "$rolesdir" ]; then + echo "$rolesdir" + return 0 + fi + done done fi - return 1 + 1>&2 echo ERROR - could not find role "$role" - please use ANSIBLE_COLLECTIONS_PATH + exit 2 } get_packages() { @@ -65,6 +80,10 @@ get_packages() { roles="$(cat "$rolefile")" for role in $roles; do rolepath="$(get_rolepath "$ostree_dir" "$role")" + if [ -z "$rolepath" ]; then + 1>&2 echo ERROR - could not find role "$role" - please use ANSIBLE_COLLECTIONS_PATH + exit 2 + fi get_packages "$rolepath" done fi diff --git a/meta/collection-requirements.yml b/meta/collection-requirements.yml index 9ddec21..a0cd255 100644 --- a/meta/collection-requirements.yml +++ b/meta/collection-requirements.yml @@ -1,4 +1,3 @@ --- collections: - name: ansible.posix - - name: ansible.utils diff --git a/tasks/firewalld.yml b/tasks/firewalld.yml index 66416a7..458a66b 100644 --- a/tasks/firewalld.yml +++ b/tasks/firewalld.yml @@ -5,25 +5,21 @@ when: __firewall_required_facts | difference(ansible_facts.keys() | list) | length > 0 -- name: Ensure correct package manager for ostree systems - vars: - ostree_pkg_mgr: ansible.posix.rhel_rpm_ostree - ostree_booted_file: /run/ostree-booted - when: ansible_facts.pkg_mgr | d("") != ostree_pkg_mgr +- name: Determine if system is ostree and set flag + when: not __firewall_is_ostree is defined block: - name: Check if system is ostree stat: - path: "{{ ostree_booted_file }}" + path: /run/ostree-booted register: __ostree_booted_stat - - name: Set package manager to use for ostree - ansible.utils.update_fact: - updates: - - path: ansible_facts.pkg_mgr - value: "{{ ostree_pkg_mgr }}" - when: __ostree_booted_stat.stat.exists + - name: Set flag to indicate system is ostree + set_fact: + __firewall_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" - name: Install firewalld package: name: "{{ __firewall_packages_base }}" state: present + use: "{{ (__firewall_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" diff --git a/tests/tests_reload_on_reset.yml b/tests/tests_reload_on_reset.yml index 93dec5f..1d7c8cc 100644 --- a/tests/tests_reload_on_reset.yml +++ b/tests/tests_reload_on_reset.yml @@ -9,30 +9,24 @@ - ansible_distribution in ['RedHat', 'CentOS'] - ansible_distribution_major_version | int < 8 tasks: - - name: Ensure correct package manager for ostree systems - vars: - ostree_pkg_mgr: ansible.posix.rhel_rpm_ostree - ostree_booted_file: /run/ostree-booted - when: ansible_facts.pkg_mgr | d("") != ostree_pkg_mgr + - name: Determine if system is ostree and set flag + when: not __firewall_is_ostree is defined block: - name: Check if system is ostree stat: - path: "{{ ostree_booted_file }}" + path: /run/ostree-booted register: __ostree_booted_stat - - name: Set package manager to use for ostree + - name: Set flag to indicate system is ostree set_fact: - ansible_facts: "{{ ansible_facts | - combine(new_facts, recursive=True) }}" - vars: - new_facts: - pkg_mgr: "{{ ostree_pkg_mgr }}" - when: __ostree_booted_stat.stat.exists + __firewall_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" - name: Install podman package: name: podman state: present + use: "{{ (__firewall_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Run test script: diff --git a/tests/tests_startup_conflicts.yml b/tests/tests_startup_conflicts.yml index 1e32be1..e91fed5 100644 --- a/tests/tests_startup_conflicts.yml +++ b/tests/tests_startup_conflicts.yml @@ -16,30 +16,24 @@ include_role: name: linux-system-roles.firewall - - name: Ensure correct package manager for ostree systems - vars: - ostree_pkg_mgr: ansible.posix.rhel_rpm_ostree - ostree_booted_file: /run/ostree-booted - when: ansible_facts.pkg_mgr | d("") != ostree_pkg_mgr + - name: Determine if system is ostree and set flag + when: not __firewall_is_ostree is defined block: - name: Check if system is ostree stat: - path: "{{ ostree_booted_file }}" + path: /run/ostree-booted register: __ostree_booted_stat - - name: Set package manager to use for ostree + - name: Set flag to indicate system is ostree set_fact: - ansible_facts: "{{ ansible_facts | - combine(new_facts, recursive=True) }}" - vars: - new_facts: - pkg_mgr: "{{ ostree_pkg_mgr }}" - when: __ostree_booted_stat.stat.exists + __firewall_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" - name: Install conflicting service package: name: nftables state: present + use: "{{ (__firewall_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Enable conflicting service service: