Skip to content

Commit

Permalink
refactor: improve support for ostree systems
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
richm committed Nov 29, 2023
1 parent 3242b07 commit 530f6d7
Show file tree
Hide file tree
Showing 14 changed files with 61 additions and 58 deletions.
2 changes: 0 additions & 2 deletions .ansible-lint
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,3 @@ exclude_paths:
- examples/roles/
mock_roles:
- linux-system-roles.ha_cluster
mock_modules:
- ansible.utils.update_fact
29 changes: 19 additions & 10 deletions .ostree/get_ostree_data.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")")"
Expand All @@ -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() {
Expand All @@ -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
Expand Down
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion meta/collection-requirements.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@
---
collections:
- ansible.posix
- ansible.utils
- community.general
- fedora.linux_system_roles
2 changes: 1 addition & 1 deletion tasks/enable-package-repositories.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
4 changes: 4 additions & 0 deletions tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 6 additions & 12 deletions tasks/set_vars.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}"
Expand Down
4 changes: 2 additions & 2 deletions tasks/test_cleanup_qnetd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
20 changes: 7 additions & 13 deletions tasks/test_setup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
2 changes: 2 additions & 0 deletions tasks/test_setup_qnetd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions tests/tasks/fixture_psks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tests/tests_cluster_basic_custom_fence_agents.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
2 changes: 1 addition & 1 deletion tests/tests_cluster_basic_custom_packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
20 changes: 8 additions & 12 deletions tests/tests_qdevice_tls_kaptb_options.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 530f6d7

Please sign in to comment.