From 530f6d75828c69b23f370e63d35b77ee11f1095a Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 21 Nov 2023 17:47:48 -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 | 7 ++++- meta/collection-requirements.yml | 1 - tasks/enable-package-repositories.yml | 2 +- tasks/main.yml | 4 +++ tasks/set_vars.yml | 18 ++++-------- tasks/test_cleanup_qnetd.yml | 4 +-- tasks/test_setup.yml | 20 +++++-------- tasks/test_setup_qnetd.yml | 2 ++ tests/tasks/fixture_psks.yml | 2 ++ ...ests_cluster_basic_custom_fence_agents.yml | 6 ++-- tests/tests_cluster_basic_custom_packages.yml | 2 +- tests/tests_qdevice_tls_kaptb_options.yml | 20 +++++-------- 14 files changed, 61 insertions(+), 58 deletions(-) diff --git a/.ansible-lint b/.ansible-lint index 73cc1507..8ff1ec7e 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -22,5 +22,3 @@ exclude_paths: - examples/roles/ mock_roles: - linux-system-roles.ha_cluster -mock_modules: - - ansible.utils.update_fact 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/README.md b/README.md index b4411379..537d12de 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,8 @@ If the `ha_cluster` is a role from the `fedora.linux_system_roles` collection or from the Fedora RPM package, the requirement is already satisfied. -Otherwise, please run the following command line to install the collection. +If you need to manage `rpm-ostree` systems, you will need to install additional +collections. Please run the following command line to install the collections. ```bash ansible-galaxy collection install -r meta/collection-requirements.yml @@ -1854,6 +1855,10 @@ Note that you cannot run a quorum device on a cluster node. - linux-system-roles.ha_cluster ``` +## rpm-ostree + +See README-ostree.md + ## License MIT diff --git a/meta/collection-requirements.yml b/meta/collection-requirements.yml index 08257d28..26e594f0 100644 --- a/meta/collection-requirements.yml +++ b/meta/collection-requirements.yml @@ -2,6 +2,5 @@ --- collections: - ansible.posix - - ansible.utils - community.general - fedora.linux_system_roles diff --git a/tasks/enable-package-repositories.yml b/tasks/enable-package-repositories.yml index 8eec01b5..43e3f47c 100644 --- a/tasks/enable-package-repositories.yml +++ b/tasks/enable-package-repositories.yml @@ -26,4 +26,4 @@ when: - ha_cluster_enable_repos - __ha_cluster_enable_repo_tasks_file is defined - - ansible_facts.pkg_mgr | d() != "ansible.posix.rhel_rpm_ostree" + - not __ha_cluster_is_ostree | d(false) diff --git a/tasks/main.yml b/tasks/main.yml index f0a43bce..0c57d292 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -15,6 +15,8 @@ + ha_cluster_extra_packages }}" state: present + use: "{{ (__ha_cluster_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Check and prepare role variables include_tasks: shell_{{ ha_cluster_pacemaker_shell }}/check-and-prepare-role-variables.yml # yamllint disable-line rule:line-length @@ -49,6 +51,8 @@ ha_cluster_fence_agent_packages }}" state: present + use: "{{ (__ha_cluster_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Configure firewall include_tasks: firewall.yml diff --git a/tasks/set_vars.yml b/tasks/set_vars.yml index 7fab9e6e..13d0c69c 100644 --- a/tasks/set_vars.yml +++ b/tasks/set_vars.yml @@ -5,23 +5,17 @@ when: __ha_cluster_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 __ha_cluster_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: + __ha_cluster_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" - name: Set platform/version specific variables include_vars: "{{ __vars_file }}" diff --git a/tasks/test_cleanup_qnetd.yml b/tasks/test_cleanup_qnetd.yml index 4f513607..5e148f84 100644 --- a/tasks/test_cleanup_qnetd.yml +++ b/tasks/test_cleanup_qnetd.yml @@ -11,10 +11,10 @@ name: - corosync-qnetd state: absent - when: ansible_facts.pkg_mgr | d() != "ansible.posix.rhel_rpm_ostree" + when: not __ha_cluster_is_ostree | d(false) - name: Make sure qnetd config files are not present file: path: /etc/corosync/qnetd state: absent - when: ansible_facts.pkg_mgr | d() != "ansible.posix.rhel_rpm_ostree" + when: not __ha_cluster_is_ostree | d(false) diff --git a/tasks/test_setup.yml b/tasks/test_setup.yml index 51126d35..38590307 100644 --- a/tasks/test_setup.yml +++ b/tasks/test_setup.yml @@ -16,23 +16,17 @@ gather_subset: min when: "'distribution' not in ansible_facts" -- 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 __ha_cluster_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: + __ha_cluster_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" # Test systems may not have repositories available through subscriptions - name: Do not try to enable RHEL repositories @@ -53,5 +47,5 @@ grep "^$username" /usr/lib/passwd >> /etc/passwd fi done - when: ansible_facts.pkg_mgr | d() == "ansible.posix.rhel_rpm_ostree" + when: __ha_cluster_is_ostree | d(false) changed_when: true diff --git a/tasks/test_setup_qnetd.yml b/tasks/test_setup_qnetd.yml index b2ec7471..471eb057 100644 --- a/tasks/test_setup_qnetd.yml +++ b/tasks/test_setup_qnetd.yml @@ -12,6 +12,8 @@ - corosync-qnetd - pcs state: present + use: "{{ (__ha_cluster_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Set up qnetd command: diff --git a/tests/tasks/fixture_psks.yml b/tests/tasks/fixture_psks.yml index cf182e15..422a9400 100644 --- a/tests/tasks/fixture_psks.yml +++ b/tests/tasks/fixture_psks.yml @@ -19,6 +19,8 @@ package: name: openssl state: present + use: "{{ (__ha_cluster_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: "'openssl' not in ansible_facts.packages" - name: Generate a self signed pcsd cert and the pcsd key diff --git a/tests/tests_cluster_basic_custom_fence_agents.yml b/tests/tests_cluster_basic_custom_fence_agents.yml index 8055834d..488e4cc7 100644 --- a/tests/tests_cluster_basic_custom_fence_agents.yml +++ b/tests/tests_cluster_basic_custom_fence_agents.yml @@ -21,7 +21,7 @@ package: name: fence-agents-all state: absent - when: ansible_facts.pkg_mgr | d() != "ansible.posix.rhel_rpm_ostree" + when: not __ha_cluster_is_ostree | d(false) - name: Run HA Cluster role include_role: @@ -30,14 +30,14 @@ - name: Get packages status package_facts: - when: ansible_facts.pkg_mgr | d() != "ansible.posix.rhel_rpm_ostree" + when: not __ha_cluster_is_ostree | d(false) - name: Check installed fence-agents packages assert: that: - "'fence-agents-all' not in ansible_facts.packages" - "'fence-agents-ipmilan' in ansible_facts.packages" - when: ansible_facts.pkg_mgr | d() != "ansible.posix.rhel_rpm_ostree" + when: not __ha_cluster_is_ostree | d(false) - name: Check cluster status include_tasks: tasks/assert_cluster_running.yml diff --git a/tests/tests_cluster_basic_custom_packages.yml b/tests/tests_cluster_basic_custom_packages.yml index 4cc3f8c8..23df815d 100644 --- a/tests/tests_cluster_basic_custom_packages.yml +++ b/tests/tests_cluster_basic_custom_packages.yml @@ -20,7 +20,7 @@ - name: Skip test on ostree systems meta: end_host - when: ansible_facts.pkg_mgr | d() == "ansible.posix.rhel_rpm_ostree" + when: __ha_cluster_is_ostree | d(false) - name: Ensure extra package is not installed package: diff --git a/tests/tests_qdevice_tls_kaptb_options.yml b/tests/tests_qdevice_tls_kaptb_options.yml index d3c39117..58d3b493 100644 --- a/tests/tests_qdevice_tls_kaptb_options.yml +++ b/tests/tests_qdevice_tls_kaptb_options.yml @@ -13,29 +13,25 @@ (ansible_play_hosts_all | length == 1) | ternary('localhost', ansible_play_hosts[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 __ha_cluster_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: + __ha_cluster_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" # Install pcs so we can detect whether it supports tls and kaptb options - name: Install pcs package: name: pcs state: present + use: "{{ (__ha_cluster_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Fetch versions of cluster components include_tasks: tasks/fetch_versions.yml