Skip to content

Commit becee02

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 0539fe0 commit becee02

6 files changed

Lines changed: 59 additions & 37 deletions

File tree

tasks/main.yml

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,6 @@
2424
path: "{{ __kdump_conf_file }}"
2525
register: __kdump_conf
2626

27-
- name: Debug variables used by kdump.conf.j2 template
28-
debug:
29-
msg:
30-
kdump_target: "{{ kdump_target }}"
31-
kdump_ssh_user: "{{ kdump_ssh_user }}"
32-
kdump_ssh_server: "{{ kdump_ssh_server }}"
33-
kdump_sshkey: "{{ kdump_sshkey }}"
34-
kdump_path: "{{ kdump_path }}"
35-
kdump_core_collector: "{{ kdump_core_collector }}"
36-
ansible_facts_distribution: "{{ ansible_facts['distribution'] }}"
37-
ansible_facts_distribution_major_version: "{{ ansible_facts['distribution_major_version'] }}"
38-
kdump_auto_reset_crashkernel: "{{ kdump_auto_reset_crashkernel }}"
39-
kdump_system_action: "{{ kdump_system_action }}"
40-
kdump_dracut_args: "{{ kdump_dracut_args }}"
41-
4227
- name: Generate kdump configuration file
4328
template:
4429
src: "{{ __kdump_conf_file | basename }}.j2"
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.kdump.
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.kdump
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.kdump
35+
tasks_from: "{{ __sr_tasks_from | default('main') }}"
36+
public: "{{ __sr_public | default(false) }}"
37+
when: __sr_failed_when | d(true)

tests/tests_default.yml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
---
22
- name: Ensure that the rule runs with default parameters
33
hosts: all
4-
gather_facts: false
54
tasks:
65
- name: >-
76
The role requires reboot only on specific systems. Hence running the
87
role in a rescue block to catch when it fails with reboot.
98
block:
109
- name: Run the role. If reboot is not required - the play succeeds.
11-
include_role:
12-
name: linux-system-roles.kdump
10+
include_tasks: tasks/run_role_with_clear_facts.yml
1311
rescue:
1412
- name: If reboot is required - assert the expected fail message
1513
assert:

tests/tests_default_reboot.yml

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@
99
- tests::reboot
1010
tasks:
1111
- name: Run the role and reboot if necessary
12-
include_role:
13-
name: linux-system-roles.kdump
14-
public: true
12+
include_tasks: tasks/run_role_with_clear_facts.yml
13+
vars:
14+
__sr_public: true
1515

1616
- name: Notify and run handlers
1717
meta: flush_handlers
@@ -80,8 +80,7 @@
8080
kdump_dracut_args: --print-cmdline
8181

8282
- name: Run the role again with new parameters
83-
include_role:
84-
name: linux-system-roles.kdump
83+
include_tasks: tasks/run_role_with_clear_facts.yml
8584

8685
- name: Notify and run handlers again
8786
meta: flush_handlers

tests/tests_ssh.yml

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@
1414
# but is less realistic, as in practice a host can not dump core
1515
# to itself.
1616
kdump_test_ssh_server_outside: "{{ inventory_hostname }}"
17-
kdump_test_ssh_source: "{{
18-
ansible_facts['env']['SSH_CONNECTION'].split()[0]
19-
if 'SSH_CONNECTION' in ansible_facts['env']
20-
else '127.0.0.1' }}"
2117

2218
# this is the address at which the ssh dump server can be reached
2319
# from the managed host. Dumps will be uploaded there.
@@ -78,13 +74,22 @@
7874
- ansible_facts['distribution_major_version'] == '6'
7975
- kdump_test_ssh_server_outside == inventory_hostname
8076

77+
# because this relies on ansible_facts['env'], it must be set before
78+
# the role is run because ansible_facts['env'] is cleared by the test
79+
# and the role does not gather that fact.
80+
- name: Set kdump_test_ssh_source
81+
set_fact:
82+
kdump_test_ssh_source: "{{
83+
ansible_facts['env']['SSH_CONNECTION'].split()[0]
84+
if 'SSH_CONNECTION' in ansible_facts['env']
85+
else '127.0.0.1' }}"
86+
8187
- name: >-
8288
The role requires reboot only on specific systems. Hence running the
8389
role in a rescue block to catch when it fails with reboot.
8490
block:
8591
- name: Run the role. If reboot is not required - the play succeeds.
86-
include_role:
87-
name: linux-system-roles.kdump
92+
include_tasks: tasks/run_role_with_clear_facts.yml
8893
rescue:
8994
- name: If reboot is required - assert the expected fail message
9095
assert:

tests/tests_ssh_reboot.yml

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@
1414
# but is less realistic, as in practice a host can not dump core
1515
# to itself.
1616
kdump_test_ssh_server_outside: "{{ inventory_hostname }}"
17-
kdump_test_ssh_source: "{{
18-
ansible_facts['env']['SSH_CONNECTION'].split()[0]
19-
if 'SSH_CONNECTION' in ansible_facts['env']
20-
else '127.0.0.1' }}"
2117

2218
# this is the address at which the ssh dump server can be reached
2319
# from the managed host. Dumps will be uploaded there.
@@ -77,6 +73,10 @@
7773
__user_info.home ~ '/.ssh' }}"
7874
__authorized_keys_path: "{{
7975
__user_info.home ~ '/.ssh/authorized_keys' }}"
76+
kdump_test_ssh_source: "{{
77+
ansible_facts['env']['SSH_CONNECTION'].split()[0]
78+
if 'SSH_CONNECTION' in ansible_facts['env']
79+
else '127.0.0.1' }}"
8080

8181
- name: Print message that this test is skipped on EL 6
8282
debug:
@@ -112,8 +112,7 @@
112112
when: __kdump_is_ostree | bool
113113

114114
- name: Run the role and reboot if necessary
115-
include_role:
116-
name: linux-system-roles.kdump
115+
include_tasks: tasks/run_role_with_clear_facts.yml
117116

118117
- name: Flush handlers
119118
meta: flush_handlers
@@ -137,8 +136,7 @@
137136
delegate_to: "{{ kdump_ssh_server }}"
138137

139138
- name: Run the role again
140-
include_role:
141-
name: linux-system-roles.kdump
139+
include_tasks: tasks/run_role_with_clear_facts.yml
142140

143141
- name: Notify and run handlers again
144142
meta: flush_handlers

0 commit comments

Comments
 (0)