Skip to content

Commit 4bb5734

Browse files
committed
test: ensure role gathers the facts it uses by having test clear_facts before include_role
The role gathers the facts it uses. For example, if the user uses `ANSIBLE_GATHERING=explicit`, the role uses the `setup` module with the facts and subsets it requires. This change allows us to test this. Before every role invocation, the test will use `meta: clear_facts` so that the role starts with no facts. However, since the tests use facts, we save and restore the facts used by the test. Create a task file tests/tasks/run_role_with_clear_facts.yml to do the tasks to save, clear, run the role, and restore the facts. This uses an action plugin merge_ansible_facts to merge the saved facts with any changes by the role. Any vars defined using `ansible_facts` have been changed to be defined with `set_fact` instead. This is because of the fact that `vars` are lazily evaluated - the var might be referenced when the facts have been cleared, and will issue an error like `ansible_facts["distribution"] is undefined`. Any blocks with a `when` condition that uses `ansible_facts` has been rewritten. This is because the `when` condition is evaluated every time a task is invoked in the block, and if the facts are cleared, this will raise an undefined variable error.
1 parent 5a25e68 commit 4bb5734

File tree

57 files changed

+485
-693
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+485
-693
lines changed

tasks/main-blivet.yml

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,10 @@
5555
extra_pkgs:
5656
- kpartx
5757

58-
- name: Get service facts
59-
service_facts:
60-
when: storage_skip_checks is not defined or
61-
not "service_facts" in storage_skip_checks
62-
63-
# rejectattr required because the fix to service_facts is on Ansible > 2.12 only
64-
# https://github.com/ansible/ansible/pull/75326
65-
- name: Set storage_cryptsetup_services
66-
set_fact:
58+
- name: Manage storage devices and check for errors
59+
vars:
60+
# rejectattr required because the fix to service_facts is on Ansible > 2.12 only
61+
# https://github.com/ansible/ansible/pull/75326
6762
storage_cryptsetup_services: "{{
6863
ansible_facts.services.values() |
6964
selectattr('name', 'defined') |
@@ -72,10 +67,15 @@
7267
rejectattr('status', 'match', '^failed$') |
7368
map(attribute='name') |
7469
select('match', '^systemd-cryptsetup@') |
75-
list }}"
76-
77-
- name: Manage storage devices and check for errors
70+
list
71+
if 'services' in ansible_facts else [] }}"
7872
block:
73+
- name: Get service facts
74+
service_facts:
75+
when:
76+
- not "services" in ansible_facts
77+
- not "cryptsetup_services" in storage_skip_checks | d([])
78+
7979
- name: Mask the systemd cryptsetup services
8080
systemd:
8181
name: "{{ item }}"
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# -*- coding: utf-8 -*-
2+
#
3+
# Ansible action plugin merge_ansible_facts.
4+
# Merges saved facts with current Ansible facts on the controller and returns
5+
# ansible_facts (saved_ansible_facts with current_ansible_facts overlaid).
6+
#
7+
# Copyright: (c) 2025, Linux System Roles
8+
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
9+
10+
from __future__ import absolute_import, division, print_function
11+
12+
__metaclass__ = type
13+
14+
from ansible.plugins.action import ActionBase
15+
16+
17+
class ActionModule(ActionBase):
18+
19+
TRANSFERS_FILES = False
20+
21+
def run(self, tmp=None, task_vars=None):
22+
if task_vars is None:
23+
task_vars = {}
24+
25+
result = super(ActionModule, self).run(tmp, task_vars)
26+
result["changed"] = False
27+
28+
current = self._task.args.get("current_ansible_facts")
29+
saved = self._task.args.get("saved_ansible_facts")
30+
31+
if current is None:
32+
result["failed"] = True
33+
result["msg"] = "current_ansible_facts is required"
34+
return result
35+
if saved is None:
36+
result["failed"] = True
37+
result["msg"] = "saved_ansible_facts is required"
38+
return result
39+
40+
# Template in case args were passed as raw Jinja
41+
current = self._templar.template(current)
42+
saved = self._templar.template(saved)
43+
44+
if not isinstance(current, dict):
45+
result["failed"] = True
46+
result["msg"] = "current_ansible_facts must be a dict"
47+
return result
48+
if not isinstance(saved, dict):
49+
result["failed"] = True
50+
result["msg"] = "saved_ansible_facts must be a dict"
51+
return result
52+
53+
# Merge: start with a copy of saved, overlay current (same behavior as module)
54+
merged = dict(saved)
55+
merged.update(current)
56+
result["ansible_facts"] = merged
57+
58+
return result
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# -*- coding: utf-8 -*-
2+
#
3+
# Ansible module to merge saved facts with current Ansible facts.
4+
# The result is saved_facts with any keys/values overridden by current_ansible_facts.
5+
#
6+
# Created by Claude AI running in Cursor
7+
8+
from __future__ import absolute_import, division, print_function
9+
10+
__metaclass__ = type
11+
12+
13+
DOCUMENTATION = r"""
14+
---
15+
module: merge_ansible_facts
16+
short_description: Merge saved facts with current Ansible facts
17+
version_added: "1.0.0"
18+
description:
19+
- Does O(saved_ansible_facts.update(current_ansible_facts)) and returns the result.
20+
author:
21+
- Linux System Roles
22+
options:
23+
current_ansible_facts:
24+
description: Current Ansible facts.
25+
type: dict
26+
required: true
27+
saved_ansible_facts:
28+
description: Previously saved facts.
29+
type: dict
30+
required: true
31+
"""
32+
33+
EXAMPLES = r"""
34+
- name: Update saved facts with current facts
35+
merge_ansible_facts:
36+
current_ansible_facts: "{{ ansible_facts }}"
37+
saved_ansible_facts: "{{ saved_ansible_facts }}"
38+
register: merged
39+
40+
- name: Use updated facts
41+
set_fact:
42+
ansible_facts: "{{ merged.ansible_facts }}"
43+
"""
44+
45+
RETURN = r"""
46+
ansible_facts:
47+
description: saved_facts with keys/values overridden by current_ansible_facts.
48+
returned: success
49+
type: dict
50+
"""

tests/setup-snapshot.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
hosts: all
44
tasks:
55
- name: Set platform/version specific variables
6-
include_role:
7-
name: linux-system-roles.storage
6+
include_tasks: tasks/run_role_with_clear_facts.yml
7+
vars:
88
tasks_from: set_vars.yml
99
public: true
1010

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
# Task file: save facts, clear_facts, run linux-system-roles.storage, then restore facts.
3+
# Include this with include_tasks or import_tasks; ensure tests/library is in module search path.
4+
5+
- name: Save current ansible facts
6+
set_fact:
7+
saved_facts: "{{ ansible_facts }}"
8+
9+
- name: Clear facts
10+
meta: clear_facts
11+
12+
- name: Run the role
13+
include_role:
14+
name: linux-system-roles.storage
15+
tasks_from: "{{ tasks_from | default('main') }}"
16+
public: "{{ public | default(false) }}"
17+
18+
- name: Reset ansible_facts (saved base with current overrides)
19+
merge_ansible_facts:
20+
current_ansible_facts: "{{ ansible_facts }}"
21+
saved_ansible_facts: "{{ saved_facts }}"

tests/tests_change_disk_fs.yml

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@
1212
else 'ext4' }}"
1313
tasks:
1414
- name: Run the role
15-
include_role:
16-
name: linux-system-roles.storage
15+
include_tasks: tasks/run_role_with_clear_facts.yml
1716

1817
- name: Mark tasks to be skipped
1918
set_fact:
@@ -22,7 +21,6 @@
2221
- "{{ (lookup('env',
2322
'SYSTEM_ROLES_REMOVE_CLOUD_INIT') in ['', 'false']) |
2423
ternary('packages_installed', '') }}"
25-
- service_facts
2624

2725
- name: Get unused disks
2826
include_tasks: get_unused_disk.yml
@@ -31,8 +29,7 @@
3129
max_return: 1
3230

3331
- name: Create a disk device with the default file system type
34-
include_role:
35-
name: linux-system-roles.storage
32+
include_tasks: tasks/run_role_with_clear_facts.yml
3633
vars:
3734
storage_volumes:
3835
- name: test1
@@ -44,8 +41,7 @@
4441
include_tasks: verify-role-results.yml
4542

4643
- name: Change the disk device file system type to {{ fs_type_after }}
47-
include_role:
48-
name: linux-system-roles.storage
44+
include_tasks: tasks/run_role_with_clear_facts.yml
4945
vars:
5046
storage_volumes:
5147
- name: test1
@@ -58,8 +54,7 @@
5854
include_tasks: verify-role-results.yml
5955

6056
- name: Repeat the previous invocation to verify idempotence
61-
include_role:
62-
name: linux-system-roles.storage
57+
include_tasks: tasks/run_role_with_clear_facts.yml
6358
vars:
6459
storage_volumes:
6560
- name: test1
@@ -72,8 +67,7 @@
7267
include_tasks: verify-role-results.yml
7368

7469
- name: Clean up
75-
include_role:
76-
name: linux-system-roles.storage
70+
include_tasks: tasks/run_role_with_clear_facts.yml
7771
vars:
7872
storage_volumes:
7973
- name: test1

tests/tests_change_disk_mount.yml

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@
2020
changed_when: false
2121

2222
- name: Run the role
23-
include_role:
24-
name: linux-system-roles.storage
23+
include_tasks: tasks/run_role_with_clear_facts.yml
2524

2625
- name: Mark tasks to be skipped
2726
set_fact:
@@ -30,7 +29,6 @@
3029
- "{{ (lookup('env',
3130
'SYSTEM_ROLES_REMOVE_CLOUD_INIT') in ['', 'false']) |
3231
ternary('packages_installed', '') }}"
33-
- service_facts
3432

3533
- name: Get unused disks
3634
include_tasks: get_unused_disk.yml
@@ -39,8 +37,7 @@
3937
max_return: 1
4038

4139
- name: Create a disk device mounted at "{{ mount_location_before }}"
42-
include_role:
43-
name: linux-system-roles.storage
40+
include_tasks: tasks/run_role_with_clear_facts.yml
4441
vars:
4542
storage_volumes:
4643
- name: test1
@@ -52,8 +49,7 @@
5249
include_tasks: verify-role-results.yml
5350

5451
- name: Change the disk device mount location to {{ mount_location_after }}
55-
include_role:
56-
name: linux-system-roles.storage
52+
include_tasks: tasks/run_role_with_clear_facts.yml
5753
vars:
5854
storage_volumes:
5955
- name: test1
@@ -65,8 +61,7 @@
6561
include_tasks: verify-role-results.yml
6662

6763
- name: Repeat the previous invocation to verify idempotence
68-
include_role:
69-
name: linux-system-roles.storage
64+
include_tasks: tasks/run_role_with_clear_facts.yml
7065
vars:
7166
storage_volumes:
7267
- name: test1
@@ -78,8 +73,7 @@
7873
include_tasks: verify-role-results.yml
7974

8075
- name: Clean up
81-
include_role:
82-
name: linux-system-roles.storage
76+
include_tasks: tasks/run_role_with_clear_facts.yml
8377
vars:
8478
storage_volumes:
8579
- name: test1

tests/tests_change_fs.yml

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@
1414

1515
tasks:
1616
- name: Run the role
17-
include_role:
18-
name: linux-system-roles.storage
17+
include_tasks: tasks/run_role_with_clear_facts.yml
1918

2019
- name: Mark tasks to be skipped
2120
set_fact:
@@ -24,7 +23,6 @@
2423
- "{{ (lookup('env',
2524
'SYSTEM_ROLES_REMOVE_CLOUD_INIT') in ['', 'false']) |
2625
ternary('packages_installed', '') }}"
27-
- service_facts
2826

2927
- name: Get unused disks
3028
include_tasks: get_unused_disk.yml
@@ -33,8 +31,7 @@
3331
max_return: 1
3432

3533
- name: Create a LVM logical volume with default fs_type
36-
include_role:
37-
name: linux-system-roles.storage
34+
include_tasks: tasks/run_role_with_clear_facts.yml
3835
vars:
3936
storage_pools:
4037
- name: foo
@@ -48,8 +45,7 @@
4845
include_tasks: verify-role-results.yml
4946

5047
- name: Change the file system signature on the logical volume created above
51-
include_role:
52-
name: linux-system-roles.storage
48+
include_tasks: tasks/run_role_with_clear_facts.yml
5349
vars:
5450
storage_pools:
5551
- name: foo
@@ -64,8 +60,7 @@
6460
include_tasks: verify-role-results.yml
6561

6662
- name: Re-run the role on the same volume without specifying fs_type
67-
include_role:
68-
name: linux-system-roles.storage
63+
include_tasks: tasks/run_role_with_clear_facts.yml
6964
vars:
7065
storage_pools:
7166
- name: foo
@@ -86,8 +81,7 @@
8681
include_tasks: verify-role-results.yml
8782

8883
- name: Repeat the previous invocation to verify idempotence
89-
include_role:
90-
name: linux-system-roles.storage
84+
include_tasks: tasks/run_role_with_clear_facts.yml
9185
vars:
9286
storage_pools:
9387
- name: foo
@@ -102,8 +96,7 @@
10296
include_tasks: verify-role-results.yml
10397

10498
- name: Remove the FS
105-
include_role:
106-
name: linux-system-roles.storage
99+
include_tasks: tasks/run_role_with_clear_facts.yml
107100
vars:
108101
storage_pools:
109102
- name: foo
@@ -116,10 +109,8 @@
116109
- name: Verify role results - 5
117110
include_tasks: verify-role-results.yml
118111

119-
120112
- name: Clean up
121-
include_role:
122-
name: linux-system-roles.storage
113+
include_tasks: tasks/run_role_with_clear_facts.yml
123114
vars:
124115
storage_pools:
125116
- name: foo

0 commit comments

Comments
 (0)