From 852bd295e0ad53aa1dbd4e7775a3f13d14342cbe Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 21 Nov 2023 19:01:30 -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 | 29 ++++++++++++++++--------- meta/collection-requirements.yml | 1 - tasks/enable_coprs.yml | 2 ++ tasks/main-blivet.yml | 4 ++++ tasks/set_vars.yml | 18 +++++---------- tests/get_unused_disk.yml | 2 ++ tests/test-verify-volume-encryption.yml | 2 ++ tests/test-verify-volume-mount.yml | 3 +-- tests/tests_luks.yml | 2 ++ tests/tests_luks_pool.yml | 2 ++ tests/tests_lvm_auto_size_cap.yml | 2 ++ tests/tests_safe_mode_check.yml | 5 +++-- tests/verify-pool-member-encryption.yml | 2 ++ 13 files changed, 47 insertions(+), 27 deletions(-) diff --git a/.ostree/get_ostree_data.sh b/.ostree/get_ostree_data.sh index 7c325241..cec08b0c 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/meta/collection-requirements.yml b/meta/collection-requirements.yml index 9ddec214..a0cd255e 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/enable_coprs.yml b/tasks/enable_coprs.yml index e0481227..3eb084e5 100644 --- a/tasks/enable_coprs.yml +++ b/tasks/enable_coprs.yml @@ -13,6 +13,8 @@ - name: Make sure COPR support packages are present package: name: "{{ _storage_copr_support_packages }}" + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: install_copr | d(false) | bool - name: Enable COPRs diff --git a/tasks/main-blivet.yml b/tasks/main-blivet.yml index d4fedd0c..b773e889 100644 --- a/tasks/main-blivet.yml +++ b/tasks/main-blivet.yml @@ -3,6 +3,8 @@ package: name: "{{ blivet_package_list }}" state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: storage_skip_checks is not defined or not "blivet_available" in storage_skip_checks @@ -40,6 +42,8 @@ package: name: "{{ package_info.packages + extra_pkgs }}" state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: storage_skip_checks is not defined or not "packages_installed" in storage_skip_checks vars: diff --git a/tasks/set_vars.yml b/tasks/set_vars.yml index 99ee4e19..0901a215 100644 --- a/tasks/set_vars.yml +++ b/tasks/set_vars.yml @@ -20,20 +20,14 @@ __vars_file: "{{ role_path }}/vars/{{ item }}" when: __vars_file is file -- 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 __storage_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: + __storage_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" diff --git a/tests/get_unused_disk.yml b/tests/get_unused_disk.yml index 45a850cc..685541ff 100644 --- a/tests/get_unused_disk.yml +++ b/tests/get_unused_disk.yml @@ -3,6 +3,8 @@ package: name: "{{ test_packages }}" state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" vars: # util-linux needed for lsblk, findmnt, etc. test_packages: "{{ ['util-linux-core'] diff --git a/tests/test-verify-volume-encryption.yml b/tests/test-verify-volume-encryption.yml index d8a168fa..da9584a2 100644 --- a/tests/test-verify-volume-encryption.yml +++ b/tests/test-verify-volume-encryption.yml @@ -11,6 +11,8 @@ package: name: cryptsetup state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Collect LUKS info for this volume command: cryptsetup luksDump {{ storage_test_volume._raw_device }} diff --git a/tests/test-verify-volume-mount.yml b/tests/test-verify-volume-mount.yml index 33913cd7..cf86b343 100644 --- a/tests/test-verify-volume-mount.yml +++ b/tests/test-verify-volume-mount.yml @@ -31,8 +31,7 @@ storage_test_volume.fs_type == 'swap' else 0 }}" vars: # assumes /opt which is /var/opt in ostree - mount_prefix: "{{ '/var' - if ansible_facts.pkg_mgr == 'ansible.posix.rhel_rpm_ostree' + mount_prefix: "{{ '/var' if __storage_is_ostree | d(false) and storage_test_volume.mount_point and storage_test_volume.mount_point.startswith('/opt') else '' }}" diff --git a/tests/tests_luks.yml b/tests/tests_luks.yml index 03264ff4..bac4dd10 100644 --- a/tests/tests_luks.yml +++ b/tests/tests_luks.yml @@ -36,6 +36,8 @@ package: name: dracut-fips state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Configure boot for FIPS changed_when: false diff --git a/tests/tests_luks_pool.yml b/tests/tests_luks_pool.yml index d3eb8e6a..b3349f9d 100644 --- a/tests/tests_luks_pool.yml +++ b/tests/tests_luks_pool.yml @@ -39,6 +39,8 @@ package: name: dracut-fips state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Configure boot for FIPS changed_when: false diff --git a/tests/tests_lvm_auto_size_cap.yml b/tests/tests_lvm_auto_size_cap.yml index 3474b27c..0bf46a93 100644 --- a/tests/tests_lvm_auto_size_cap.yml +++ b/tests/tests_lvm_auto_size_cap.yml @@ -39,6 +39,8 @@ package: name: bc state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Run bc 2 * {{ test_disk_size }} command: diff --git a/tests/tests_safe_mode_check.yml b/tests/tests_safe_mode_check.yml index b79298fd..dec2ca6f 100644 --- a/tests/tests_safe_mode_check.yml +++ b/tests/tests_safe_mode_check.yml @@ -36,9 +36,10 @@ block: - name: Install package package: - name: - - nilfs-utils + name: nilfs-utils state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" rescue: - name: Set skip rest true set_fact: diff --git a/tests/verify-pool-member-encryption.yml b/tests/verify-pool-member-encryption.yml index 13c63bfb..f462ad50 100644 --- a/tests/verify-pool-member-encryption.yml +++ b/tests/verify-pool-member-encryption.yml @@ -13,6 +13,8 @@ package: name: cryptsetup state: present + use: "{{ (__storage_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Collect LUKS info for this member command: cryptsetup luksDump {{ _storage_test_member_backing_dev.stdout }}