From 07fdd9f8ed17d55a47c7bb6f60a75a72f32bbc40 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Wed, 22 Nov 2023 10:11:22 -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 --- .ansible-lint | 2 -- .ostree/get_ostree_data.sh | 29 +++++++++++++++++++---------- README.md | 2 +- meta/collection-requirements.yml | 1 - tasks/main.yml | 2 ++ tasks/set_vars.yml | 18 ++++++------------ 6 files changed, 28 insertions(+), 26 deletions(-) diff --git a/.ansible-lint b/.ansible-lint index cc85baf..9dd0b7a 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -22,5 +22,3 @@ exclude_paths: - examples/roles/ mock_roles: - linux-system-roles.journald -mock_modules: - - ansible.utils.update_fact diff --git a/.ostree/get_ostree_data.sh b/.ostree/get_ostree_data.sh index 7c32524..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,7 +28,7 @@ if [ "$pkgtype" = testing ]; then fi get_rolepath() { - local ostree_dir role rolesdir roles_parent_dir + local ostree_dir role rolesdir roles_parent_dir coll_path pth ostree_dir="$1" role="$2" roles_parent_dir="$(dirname "$(dirname "$ostree_dir")")" @@ -47,16 +46,22 @@ get_rolepath() { fi done # look elsewhere - 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 + 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() { @@ -75,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/README.md b/README.md index 5d648dc..1213e47 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ See below If you want to manage `rpm-ostree` systems with this role, you will need to install additional collections. Please run the following command line to -install the collection. +install them: ```bash ansible-galaxy collection install -vv -r meta/collection-requirements.yml diff --git a/meta/collection-requirements.yml b/meta/collection-requirements.yml index 4f4842b..0110bbe 100644 --- a/meta/collection-requirements.yml +++ b/meta/collection-requirements.yml @@ -2,4 +2,3 @@ --- collections: - ansible.posix - - ansible.utils diff --git a/tasks/main.yml b/tasks/main.yml index 19824d0..2e5b473 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -7,6 +7,8 @@ package: name: "{{ __journald_packages }}" state: present + use: "{{ (__journald_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Ensure required services are enabled service: diff --git a/tasks/set_vars.yml b/tasks/set_vars.yml index 222219d..0329da9 100644 --- a/tasks/set_vars.yml +++ b/tasks/set_vars.yml @@ -5,20 +5,14 @@ when: __journald_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 __journald_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: + __journald_is_ostree: "{{ __ostree_booted_stat.stat.exists }}"