Skip to content

Commit c4e6795

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. Create a task file tests/tasks/run_role_with_clear_facts.yml to do the tasks to clear the facts and run the role. Note that this means we don't need to use `gather_facts` for the tests. Some 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`. This is typically done for blocks that have a `when` condition that uses `ansible_facts` and the block has a role invocation using run_role_with_clear_facts.yml These have been rewritten to define the `when` condition using `set_fact`. 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. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
1 parent a38861d commit c4e6795

14 files changed

+65
-50
lines changed
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
# Task file: clear_facts, run linux-system-roles.ad_integration
3+
# Include this with include_tasks or import_tasks
4+
# Input:
5+
# - __sr_tasks_from: tasks_from to run - same as tasks_from in include_role
6+
# - __sr_public: export private vars from role - same as public in include_role
7+
# - __sr_failed_when: set to false to ignore role errors - same as failed_when in include_role
8+
- name: Clear facts
9+
meta: clear_facts
10+
11+
# note that you can use failed_when with import_role but not with include_role
12+
# so this simulates the __sr_failed_when false case
13+
# Q: Why do we need a separate task to run the role normally? Why not just
14+
# run the role in the block and rethrow the error in the rescue block?
15+
# A: Because you cannot rethrow the error in exactly the same way as the role does.
16+
# It might be possible to exactly reconstruct ansible_failed_result but it's not worth the effort.
17+
- name: Run the role with __sr_failed_when false
18+
when:
19+
- __sr_failed_when is defined
20+
- not __sr_failed_when
21+
block:
22+
- name: Run the role
23+
include_role:
24+
name: linux-system-roles.ad_integration
25+
tasks_from: "{{ __sr_tasks_from | default('main') }}"
26+
public: "{{ __sr_public | default(false) }}"
27+
rescue:
28+
- name: Ignore the failure when __sr_failed_when is false
29+
debug:
30+
msg: Ignoring failure when __sr_failed_when is false
31+
32+
- name: Run the role normally
33+
include_role:
34+
name: linux-system-roles.ad_integration
35+
tasks_from: "{{ __sr_tasks_from | default('main') }}"
36+
public: "{{ __sr_public | default(false) }}"
37+
when: __sr_failed_when | d(true)

tests/tests_basic_join.yml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
---
33
- name: Ensure that the role runs with mandatory parameters populated
44
hosts: all,!ad
5-
gather_facts: false # test that role works in this case
65
vars:
76
# sample realm that will skip joining realm step
87
ad_integration_realm: "{{ __ad_integration_sample_realm }}"
@@ -14,8 +13,7 @@
1413
__ad_integration_sample_realm: MiXeD.cAsE
1514

1615
- name: Test - Run the system role
17-
include_role:
18-
name: linux-system-roles.ad_integration
16+
include_tasks: tasks/run_role_with_clear_facts.yml
1917

2018
- name: Test - Check packages are present
2119
block:

tests/tests_default.yml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@
22
---
33
- name: Ensure role behaviour with default parameters
44
hosts: all,!ad
5-
gather_facts: false # test that role works in this case
65
tasks:
76
- name: Verify that the role fails without ad_integration_realm variable
87
block:
98
- name: Include the role
10-
include_role:
11-
name: linux-system-roles.ad_integration
9+
include_tasks: tasks/run_role_with_clear_facts.yml
1210

1311
rescue:
1412
- name: Assert that user is notified about missing variables

tests/tests_dyndns.yml

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
- name: Ensure that the role configures dynamic dns
2121
hosts: all,!ad
22-
gather_facts: true
2322
vars:
2423
# if we don't have a real AD server, just verify the config
2524
# file is written properly
@@ -38,10 +37,9 @@
3837
when: __ad_integration_test_sssd_config_only
3938

4039
- name: Test - Run the system role with bogus vars
41-
include_role:
42-
name: linux-system-roles.ad_integration
43-
public: true
40+
include_tasks: tasks/run_role_with_clear_facts.yml
4441
vars:
42+
__sr_public: true
4543
ad_dyndns_iface: TESTING
4644
ad_dyndns_server: 127.0.0.1
4745
ad_dyndns_auth: GSS-TSIG
@@ -82,8 +80,7 @@
8280
when: sssd_log.stdout | length > 0
8381

8482
- name: Test - Re-run the system role to remove vars
85-
include_role:
86-
name: linux-system-roles.ad_integration
83+
include_tasks: tasks/run_role_with_clear_facts.yml
8784
vars:
8885
ad_dyndns_iface: null
8986
ad_dyndns_server: null

tests/tests_full_integration.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@
6262
block:
6363

6464
- name: Test run the system role
65-
include_role:
66-
name: linux-system-roles.ad_integration
67-
public: true
65+
include_tasks: tasks/run_role_with_clear_facts.yml
66+
vars:
67+
__sr_public: true
6868
register: role_run
6969

7070
- name: Debug role_run

tests/tests_full_integration_dc.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@
6969
block:
7070

7171
- name: Test run the system role
72-
include_role:
73-
name: linux-system-roles.ad_integration
74-
public: true
72+
include_tasks: tasks/run_role_with_clear_facts.yml
73+
vars:
74+
__sr_public: true
7575
register: role_run
7676

7777
- name: Debug role_run

tests/tests_full_integration_dyndns.yml

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030

3131
- name: Configure DynDNS on AD
3232
hosts: ad
33-
gather_facts: true
3433
tasks:
3534
- name: Get AD address
3635
set_fact:
@@ -62,7 +61,6 @@
6261

6362
- name: Ensure that the role configures dynamic dns
6463
hosts: client
65-
gather_facts: true
6664
tasks:
6765
- name: Run the integration
6866
when: "'ad' in groups and groups['ad']"
@@ -73,10 +71,9 @@
7371
Interface {{ _ad_dyndns_iface | default(ansible_facts['default_ipv4']['interface']) }} and server
7472
: {{ hostvars[groups['ad'][0]].ansible_host }}
7573
- name: Run the system role with proper config
76-
include_role:
77-
name: linux-system-roles.ad_integration
78-
public: true
74+
include_tasks: tasks/run_role_with_clear_facts.yml
7975
vars:
76+
__sr_public: true
8077
ad_dyndns_server: "{{ hostvars[groups['ad'][0]].ansible_host }}"
8178
ad_dyndns_iface: "{{ _ad_dyndns_iface | default(ansible_facts['default_ipv4']['interface']) }}"
8279
ad_dyndns_auth: "none"
@@ -126,7 +123,6 @@
126123

127124
- name: Check hosts on AD
128125
hosts: ad
129-
gather_facts: true
130126
tasks:
131127
- name: Get network for zone
132128
set_fact:

tests/tests_full_integration_force_rejoin.yml

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@
6565
block:
6666

6767
- name: Test run the system role
68-
include_role:
69-
name: linux-system-roles.ad_integration
70-
public: true
68+
include_tasks: tasks/run_role_with_clear_facts.yml
69+
vars:
70+
__sr_public: true
7171
register: role_run
7272

7373
- name: Debug role_run
@@ -98,8 +98,7 @@
9898
changed_when: false
9999

100100
- name: Test run the system role with force rejoin
101-
include_role:
102-
name: linux-system-roles.ad_integration
101+
include_tasks: tasks/run_role_with_clear_facts.yml
103102
vars:
104103
ad_integration_force_rejoin: true
105104
register: role_run_rejoin

tests/tests_include_vars_from_parent.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
---
22
- name: Test role inclusion variable overrides
33
hosts: all,!ad
4-
gather_facts: true
54
tasks:
65
- name: Create var file in caller that can override the one in called role
76
delegate_to: localhost

tests/tests_manage_packages.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@
3030

3131
# role will fail if ad_integration_manage_packages is not handled correctly
3232
- name: Run the role with ad_integration_manage_packages
33-
include_role:
34-
name: linux-system-roles.ad_integration
33+
include_tasks: tasks/run_role_with_clear_facts.yml
3534
vars:
3635
ad_integration_manage_packages: false
3736

0 commit comments

Comments
 (0)