From 152c43c843c8baa15af4a97d0f6303d57e5934f5 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Thu, 15 Mar 2018 17:45:42 -0700 Subject: [PATCH] Accounting for a case when multiple onfails are used along with requires. Previously if you have multiple states using 'onfail' and two of those states using a 'require' against the first one state, the last two will run even if the 'onfail' isn't met because the 'require' is met because the first state returns true even though it didn't excute. This change adds an additional hidden variable that is used when checking requisities to determine if the state actually ran. --- salt/state.py | 5 +- .../requisites/onfail_multiple_required.sls | 25 ++++++++++ .../onfail_multiple_required_no_run.sls | 25 ++++++++++ tests/integration/modules/test_state.py | 50 +++++++++++++++++++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 tests/integration/files/file/base/requisites/onfail_multiple_required.sls create mode 100644 tests/integration/files/file/base/requisites/onfail_multiple_required_no_run.sls diff --git a/salt/state.py b/salt/state.py index 62b3b60104..2cf0c2a36f 100644 --- a/salt/state.py +++ b/salt/state.py @@ -2207,7 +2207,8 @@ class State(object): if r_state == 'prereq' and not run_dict[tag]['result'] is None: fun_stats.add('pre') else: - fun_stats.add('met') + if run_dict[tag].get('__state_ran__', True): + fun_stats.add('met') if 'unmet' in fun_stats: status = 'unmet' @@ -2462,6 +2463,7 @@ class State(object): 'duration': duration, 'start_time': start_time, 'comment': 'State was not run because onfail req did not change', + '__state_ran__': False, '__run_num__': self.__run_num, '__sls__': low['__sls__']} self.__run_num += 1 @@ -2472,6 +2474,7 @@ class State(object): 'duration': duration, 'start_time': start_time, 'comment': 'State was not run because none of the onchanges reqs changed', + '__state_ran__': False, '__run_num__': self.__run_num, '__sls__': low['__sls__']} self.__run_num += 1 diff --git a/tests/integration/files/file/base/requisites/onfail_multiple_required.sls b/tests/integration/files/file/base/requisites/onfail_multiple_required.sls new file mode 100644 index 0000000000..d4c49518e9 --- /dev/null +++ b/tests/integration/files/file/base/requisites/onfail_multiple_required.sls @@ -0,0 +1,25 @@ +a: + cmd.run: + - name: exit 1 + +b: + cmd.run: + - name: echo b + - onfail: + - cmd: a + +c: + cmd.run: + - name: echo c + - onfail: + - cmd: a + - require: + - cmd: b + +d: + cmd.run: + - name: echo d + - onfail: + - cmd: a + - require: + - cmd: c diff --git a/tests/integration/files/file/base/requisites/onfail_multiple_required_no_run.sls b/tests/integration/files/file/base/requisites/onfail_multiple_required_no_run.sls new file mode 100644 index 0000000000..fbf43b6618 --- /dev/null +++ b/tests/integration/files/file/base/requisites/onfail_multiple_required_no_run.sls @@ -0,0 +1,25 @@ +a: + cmd.run: + - name: exit 0 + +b: + cmd.run: + - name: echo b + - onfail: + - cmd: a + +c: + cmd.run: + - name: echo c + - onfail: + - cmd: a + - require: + - cmd: b + +d: + cmd.run: + - name: echo d + - onfail: + - cmd: a + - require: + - cmd: c diff --git a/tests/integration/modules/test_state.py b/tests/integration/modules/test_state.py index 6206ef1054..c6d816e4fb 100644 --- a/tests/integration/modules/test_state.py +++ b/tests/integration/modules/test_state.py @@ -1095,6 +1095,56 @@ class StateModuleTest(ModuleCase, SaltReturnAssertsMixin): test_data = state_run['cmd_|-test_non_failing_state_|-echo "Should not run"_|-run'] self.assertIn('duration', test_data) + def test_multiple_onfail_requisite_with_required(self): + ''' + test to ensure multiple states are run + when specified as onfails for a single state. + This is a test for the issue: + https://github.com/saltstack/salt/issues/46552 + ''' + + state_run = self.run_function('state.sls', mods='requisites.onfail_multiple_required') + + retcode = state_run['cmd_|-b_|-echo b_|-run']['changes']['retcode'] + self.assertEqual(retcode, 0) + + retcode = state_run['cmd_|-c_|-echo c_|-run']['changes']['retcode'] + self.assertEqual(retcode, 0) + + retcode = state_run['cmd_|-d_|-echo d_|-run']['changes']['retcode'] + self.assertEqual(retcode, 0) + + stdout = state_run['cmd_|-b_|-echo b_|-run']['changes']['stdout'] + self.assertEqual(stdout, 'b') + + stdout = state_run['cmd_|-c_|-echo c_|-run']['changes']['stdout'] + self.assertEqual(stdout, 'c') + + stdout = state_run['cmd_|-d_|-echo d_|-run']['changes']['stdout'] + self.assertEqual(stdout, 'd') + + def test_multiple_onfail_requisite_with_required_no_run(self): + ''' + test to ensure multiple states are not run + when specified as onfails for a single state + which fails. + This is a test for the issue: + https://github.com/saltstack/salt/issues/46552 + ''' + + state_run = self.run_function('state.sls', mods='requisites.onfail_multiple_required_no_run') + + expected = 'State was not run because onfail req did not change' + + stdout = state_run['cmd_|-b_|-echo b_|-run']['comment'] + self.assertEqual(stdout, expected) + + stdout = state_run['cmd_|-c_|-echo c_|-run']['comment'] + self.assertEqual(stdout, expected) + + stdout = state_run['cmd_|-d_|-echo d_|-run']['comment'] + self.assertEqual(stdout, expected) + # listen tests def test_listen_requisite(self):