From 1dfcb2755ae98d1e194ad3706e771519a481f46b Mon Sep 17 00:00:00 2001 From: Daniel Wozniak Date: Thu, 7 Mar 2019 11:25:58 -0700 Subject: [PATCH 1/4] Merge pull request #51846 from gtmanfred/develop Allow using salt modules in onlyif and unless --- doc/ref/states/requisites.rst | 42 +++++++- salt/state.py | 68 +++++++++--- tests/integration/modules/test_state.py | 138 ++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 18 deletions(-) diff --git a/doc/ref/states/requisites.rst b/doc/ref/states/requisites.rst index d8b06c3c0f9f..3c77b4dcdb92 100644 --- a/doc/ref/states/requisites.rst +++ b/doc/ref/states/requisites.rst @@ -922,6 +922,22 @@ For example: In the above case, ``some_check`` will be run prior to _each_ name -- once for ``first_deploy_cmd`` and a second time for ``second_deploy_cmd``. +.. versionchanged:: Neon + The ``unless`` requisite can take a module as a dictionary field in unless. + The dictionary must contain an argument ``fun`` which is the module that is + being run, and everything else passed in will be kwargs passed to the module + function. + + .. code-block:: yaml + + install apache on debian based distros: + cmd.run: + - name: make install + - cwd: /path/to/dir/whatever-2.1.5/ + - unless: + - fun: file.file_exists + path: /usr/local/bin/whatever + .. _onlyif-requisite: Onlyif @@ -962,8 +978,30 @@ concept of ``True`` and ``False``. The above example ensures that the stop_volume and delete modules only run if the gluster commands return a 0 ret value. -Listen/Listen_in ----------------- +.. versionchanged:: Neon + The ``onlyif`` requisite can take a module as a dictionary field in onlyif. + The dictionary must contain an argument ``fun`` which is the module that is + being run, and everything else passed in will be kwargs passed to the module + function. + + .. code-block:: yaml + + install apache on redhat based distros: + pkg.latest: + - name: httpd + - onlyif: + - fun: match.grain + tgt: 'os_family: RedHat' + + install apache on debian based distros: + pkg.latest: + - name: apache2 + - onlyif: + - fun: match.grain + tgt: 'os_family: Debian' + +runas +~~~~~ .. versionadded:: 2014.7.0 diff --git a/salt/state.py b/salt/state.py index dfa9c6de5893..182b29fdfebf 100644 --- a/salt/state.py +++ b/salt/state.py @@ -880,20 +880,39 @@ def _run_check_onlyif(self, low_data, cmd_opts): low_data_onlyif = [low_data['onlyif']] else: low_data_onlyif = low_data['onlyif'] - for entry in low_data_onlyif: - if not isinstance(entry, six.string_types): - ret.update({'comment': 'onlyif execution failed, bad type passed', 'result': False}) - return ret - cmd = self.functions['cmd.retcode']( - entry, ignore_retcode=True, python_shell=True, **cmd_opts) - log.debug('Last command return code: %s', cmd) + + def _check_cmd(cmd): if cmd != 0 and ret['result'] is False: ret.update({'comment': 'onlyif condition is false', 'skip_watch': True, 'result': True}) - return ret elif cmd == 0: ret.update({'comment': 'onlyif condition is true', 'result': False}) + + for entry in low_data_onlyif: + if isinstance(entry, six.string_types): + cmd = self.functions['cmd.retcode']( + entry, ignore_retcode=True, python_shell=True, **cmd_opts) + log.debug('Last command return code: %s', cmd) + _check_cmd(cmd) + elif isinstance(entry, dict): + if 'fun' not in entry: + ret['comment'] = 'no `fun` argument in onlyif: {0}'.format(entry) + log.warning(ret['comment']) + return ret + result = self.functions[entry.pop('fun')](**entry) + if self.state_con.get('retcode', 0): + _check_cmd(self.state_con['retcode']) + elif not result: + ret.update({'comment': 'onlyif condition is false', + 'skip_watch': True, + 'result': True}) + else: + ret.update({'comment': 'onlyif condition is true', + 'result': False}) + + else: + ret.update({'comment': 'onlyif execution failed, bad type passed', 'result': False}) return ret def _run_check_unless(self, low_data, cmd_opts): @@ -906,20 +925,37 @@ def _run_check_unless(self, low_data, cmd_opts): low_data_unless = [low_data['unless']] else: low_data_unless = low_data['unless'] - for entry in low_data_unless: - if not isinstance(entry, six.string_types): - ret.update({'comment': 'unless condition is false, bad type passed', 'result': False}) - return ret - cmd = self.functions['cmd.retcode']( - entry, ignore_retcode=True, python_shell=True, **cmd_opts) - log.debug('Last command return code: %s', cmd) + + def _check_cmd(cmd): if cmd == 0 and ret['result'] is False: ret.update({'comment': 'unless condition is true', 'skip_watch': True, 'result': True}) elif cmd != 0: ret.update({'comment': 'unless condition is false', 'result': False}) - return ret + + for entry in low_data_unless: + if isinstance(entry, six.string_types): + cmd = self.functions['cmd.retcode'](entry, ignore_retcode=True, python_shell=True, **cmd_opts) + log.debug('Last command return code: %s', cmd) + _check_cmd(cmd) + elif isinstance(entry, dict): + if 'fun' not in entry: + ret['comment'] = 'no `fun` argument in onlyif: {0}'.format(entry) + log.warning(ret['comment']) + return ret + result = self.functions[entry.pop('fun')](**entry) + if self.state_con.get('retcode', 0): + _check_cmd(self.state_con['retcode']) + elif result: + ret.update({'comment': 'unless condition is true', + 'skip_watch': True, + 'result': True}) + else: + ret.update({'comment': 'unless condition is false', + 'result': False}) + else: + ret.update({'comment': 'unless condition is false, bad type passed', 'result': False}) # No reason to stop, return ret return ret diff --git a/tests/integration/modules/test_state.py b/tests/integration/modules/test_state.py index 987b79a11006..9cc4c3ad85ef 100644 --- a/tests/integration/modules/test_state.py +++ b/tests/integration/modules/test_state.py @@ -1365,6 +1365,144 @@ def test_requisites_use_no_state_module(self): for item, descr in six.iteritems(ret): self.assertEqual(descr['comment'], 'onlyif condition is false') + def test_onlyif_req(self): + ret = self.run_function( + 'state.single', + fun='test.succeed_with_changes', + name='onlyif test', + onlyif=[ + {} + ], + )['test_|-onlyif test_|-onlyif test_|-succeed_with_changes'] + self.assertTrue(ret['result']) + self.assertEqual(ret['comment'], 'Success!') + ret = self.run_function( + 'state.single', + fun='test.fail_with_changes', + name='onlyif test', + onlyif=[ + {'fun': 'test.false'}, + ], + )['test_|-onlyif test_|-onlyif test_|-fail_with_changes'] + self.assertTrue(ret['result']) + self.assertFalse(ret['changes']) + self.assertEqual(ret['comment'], 'onlyif condition is false') + ret = self.run_function( + 'state.single', + fun='test.fail_with_changes', + name='onlyif test', + onlyif=[ + {'fun': 'test.true'}, + ], + )['test_|-onlyif test_|-onlyif test_|-fail_with_changes'] + self.assertFalse(ret['result']) + self.assertTrue(ret['changes']) + self.assertEqual(ret['comment'], 'Failure!') + ret = self.run_function( + 'state.single', + fun='test.succeed_without_changes', + name='onlyif test', + onlyif=[ + {'fun': 'test.true'}, + ], + )['test_|-onlyif test_|-onlyif test_|-succeed_without_changes'] + self.assertTrue(ret['result']) + self.assertFalse(ret['changes']) + self.assertEqual(ret['comment'], 'Success!') + + def test_onlyif_req_retcode(self): + ret = self.run_function( + 'state.single', + fun='test.succeed_with_changes', + name='onlyif test', + onlyif=[ + {'fun': 'test.retcode'}, + ], + )['test_|-onlyif test_|-onlyif test_|-succeed_with_changes'] + self.assertTrue(ret['result']) + self.assertFalse(ret['changes']) + self.assertEqual(ret['comment'], 'onlyif condition is false') + ret = self.run_function( + 'state.single', + fun='test.succeed_with_changes', + name='onlyif test', + onlyif=[ + {'fun': 'test.retcode', 'code': 0}, + ], + )['test_|-onlyif test_|-onlyif test_|-succeed_with_changes'] + self.assertTrue(ret['result']) + self.assertTrue(ret['changes']) + self.assertEqual(ret['comment'], 'Success!') + + def test_unless_req(self): + ret = self.run_function( + 'state.single', + fun='test.succeed_with_changes', + name='unless test', + unless=[ + {} + ], + )['test_|-unless test_|-unless test_|-succeed_with_changes'] + self.assertTrue(ret['result']) + self.assertEqual(ret['comment'], 'Success!') + ret = self.run_function( + 'state.single', + fun='test.fail_with_changes', + name='unless test', + unless=[ + {'fun': 'test.true'}, + ], + )['test_|-unless test_|-unless test_|-fail_with_changes'] + self.assertTrue(ret['result']) + self.assertFalse(ret['changes']) + self.assertEqual(ret['comment'], 'unless condition is true') + ret = self.run_function( + 'state.single', + fun='test.fail_with_changes', + name='unless test', + unless=[ + {'fun': 'test.false'}, + ], + )['test_|-unless test_|-unless test_|-fail_with_changes'] + self.assertFalse(ret['result']) + self.assertTrue(ret['changes']) + self.assertEqual(ret['comment'], 'Failure!') + ret = self.run_function( + 'state.single', + fun='test.succeed_without_changes', + name='unless test', + unless=[ + {'fun': 'test.false'}, + ], + )['test_|-unless test_|-unless test_|-succeed_without_changes'] + self.assertTrue(ret['result']) + self.assertFalse(ret['changes']) + self.assertEqual(ret['comment'], 'Success!') + + def test_unless_req_retcode(self): + ret = self.run_function( + 'state.single', + fun='test.succeed_with_changes', + name='unless test', + unless=[ + {'fun': 'test.retcode'}, + ], + )['test_|-unless test_|-unless test_|-succeed_with_changes'] + self.assertTrue(ret['result']) + self.assertTrue(ret['changes']) + self.assertEqual(ret['comment'], 'Success!') + ret = self.run_function( + 'state.single', + fun='test.succeed_with_changes', + name='unless test', + unless=[ + {'fun': 'test.retcode', 'code': 0}, + ], + )['test_|-unless test_|-unless test_|-succeed_with_changes'] + self.assertTrue(ret['result']) + self.assertFalse(ret['changes']) + self.assertEqual(ret['comment'], 'unless condition is true') + def test_get_file_from_env_in_top_match(self): tgt = os.path.join(TMP, 'prod-cheese-file') try: From bbc7c737c84ad069e91ff74f112de5a8e71e691e Mon Sep 17 00:00:00 2001 From: Megan Wilhite Date: Tue, 14 May 2019 12:31:23 -0400 Subject: [PATCH 2/4] Merge pull request #52969 from mchugh19/unless_doc support args in unless and onlyif --- doc/ref/states/requisites.rst | 31 +++++++-- doc/topics/releases/neon.rst | 31 +++++++++ salt/state.py | 14 +++- tests/unit/test_state.py | 122 ++++++++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 7 deletions(-) diff --git a/doc/ref/states/requisites.rst b/doc/ref/states/requisites.rst index 3c77b4dcdb92..dc7b370ce323 100644 --- a/doc/ref/states/requisites.rst +++ b/doc/ref/states/requisites.rst @@ -925,8 +925,8 @@ In the above case, ``some_check`` will be run prior to _each_ name -- once for .. versionchanged:: Neon The ``unless`` requisite can take a module as a dictionary field in unless. The dictionary must contain an argument ``fun`` which is the module that is - being run, and everything else passed in will be kwargs passed to the module - function. + being run, and everything else must be passed in under the args key or will + be passed as individual kwargs to the module function. .. code-block:: yaml @@ -938,6 +938,18 @@ In the above case, ``some_check`` will be run prior to _each_ name -- once for - fun: file.file_exists path: /usr/local/bin/whatever + .. code-block:: yaml + + set mysql root password: + debconf.set: + - name: mysql-server-5.7 + - data: + 'mysql-server/root_password': {'type': 'password', 'value': {{pillar['mysql.pass']}} } + - unless: + - fun: pkg.version + args: + - mysql-server-5.7 + .. _onlyif-requisite: Onlyif @@ -981,8 +993,8 @@ if the gluster commands return a 0 ret value. .. versionchanged:: Neon The ``onlyif`` requisite can take a module as a dictionary field in onlyif. The dictionary must contain an argument ``fun`` which is the module that is - being run, and everything else passed in will be kwargs passed to the module - function. + being run, and everything else must be passed in under the args key or will + be passed as individual kwargs to the module function. .. code-block:: yaml @@ -1000,6 +1012,17 @@ if the gluster commands return a 0 ret value. - fun: match.grain tgt: 'os_family: Debian' + .. code-block:: yaml + + arbitrary file example: + file.touch: + - name: /path/to/file + - onlyif: + - fun: file.search + args: + - /etc/crontab + - 'entry1' + runas ~~~~~ diff --git a/doc/topics/releases/neon.rst b/doc/topics/releases/neon.rst index 780a8d9e429b..c945cad842c0 100644 --- a/doc/topics/releases/neon.rst +++ b/doc/topics/releases/neon.rst @@ -3,3 +3,34 @@ ================================== Salt Release Notes - Codename Neon ================================== + + +Unless and onlyif Enhancements +============================== + +The ``unless`` and ``onlyif`` requisites can now be operated with salt modules. +The dictionary must contain an argument ``fun`` which is the module that is +being run, and everything else must be passed in under the args key or will be +passed as individual kwargs to the module function. + +.. code-block:: yaml + + install apache on debian based distros: + cmd.run: + - name: make install + - cwd: /path/to/dir/whatever-2.1.5/ + - unless: + - fun: file.file_exists + path: /usr/local/bin/whatever + +.. code-block:: yaml + + set mysql root password: + debconf.set: + - name: mysql-server-5.7 + - data: + 'mysql-server/root_password': {'type': 'password', 'value': {{pillar['mysql.pass']}} } + - unless: + - fun: pkg.version + args: + - mysql-server-5.7 diff --git a/salt/state.py b/salt/state.py index 182b29fdfebf..5ac07049565d 100644 --- a/salt/state.py +++ b/salt/state.py @@ -900,7 +900,11 @@ def _check_cmd(cmd): ret['comment'] = 'no `fun` argument in onlyif: {0}'.format(entry) log.warning(ret['comment']) return ret - result = self.functions[entry.pop('fun')](**entry) + + if 'args' in entry: + result = self.functions[entry.pop('fun')](*entry.pop('args'), **entry) + else: + result = self.functions[entry.pop('fun')](**entry) if self.state_con.get('retcode', 0): _check_cmd(self.state_con['retcode']) elif not result: @@ -941,10 +945,14 @@ def _check_cmd(cmd): _check_cmd(cmd) elif isinstance(entry, dict): if 'fun' not in entry: - ret['comment'] = 'no `fun` argument in onlyif: {0}'.format(entry) + ret['comment'] = 'no `fun` argument in unless: {0}'.format(entry) log.warning(ret['comment']) return ret - result = self.functions[entry.pop('fun')](**entry) + + if 'args' in entry: + result = self.functions[entry.pop('fun')](*entry.pop('args'), **entry) + else: + result = self.functions[entry.pop('fun')](**entry) if self.state_con.get('retcode', 0): _check_cmd(self.state_con['retcode']) elif result: diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index d6555a03bb2d..260bcf399fa6 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -73,6 +73,128 @@ def test_render_error_on_invalid_requisite(self): with self.assertRaises(salt.exceptions.SaltRenderError): state_obj.call_high(high_data) + def test_render_requisite_require_disabled(self): + ''' + Test that the state compiler correctly deliver a rendering + exception when a requisite cannot be resolved + ''' + with patch('salt.state.State._gather_pillar') as state_patch: + high_data = { + 'step_one': OrderedDict([ + ('test', [ + OrderedDict([ + ('require', [ + OrderedDict([ + ('test', 'step_two')])])]), + 'succeed_with_changes', {'order': 10000}]), + ('__sls__', 'test.disable_require'), + ('__env__', 'base')]), + 'step_two': {'test': ['succeed_with_changes', + {'order': 10001}], + '__env__': 'base', + '__sls__': 'test.disable_require'}} + + minion_opts = self.get_temp_config('minion') + minion_opts['disabled_requisites'] = ['require'] + state_obj = salt.state.State(minion_opts) + ret = state_obj.call_high(high_data) + run_num = ret['test_|-step_one_|-step_one_|-succeed_with_changes']['__run_num__'] + self.assertEqual(run_num, 0) + + def test_render_requisite_require_in_disabled(self): + ''' + Test that the state compiler correctly deliver a rendering + exception when a requisite cannot be resolved + ''' + with patch('salt.state.State._gather_pillar') as state_patch: + high_data = { + 'step_one': {'test': ['succeed_with_changes', + {'order': 10000}], + '__env__': 'base', + '__sls__': 'test.disable_require_in'}, + 'step_two': OrderedDict([ + ('test', [ + OrderedDict([ + ('require_in', [ + OrderedDict([ + ('test', 'step_one')])])]), + 'succeed_with_changes', {'order': 10001}]), + ('__sls__', 'test.disable_require_in'), + ('__env__', 'base')])} + + minion_opts = self.get_temp_config('minion') + minion_opts['disabled_requisites'] = ['require_in'] + state_obj = salt.state.State(minion_opts) + ret = state_obj.call_high(high_data) + run_num = ret['test_|-step_one_|-step_one_|-succeed_with_changes']['__run_num__'] + self.assertEqual(run_num, 0) + + def test_verify_onlyif_parse(self): + low_data = { + "onlyif": [ + { + "fun": "file.search", + "args": [ + "/etc/crontab", + "run-parts" + ] + } + ], + "name": "mysql-server-5.7", + "state": "debconf", + "__id__": "set root password", + "fun": "set", + "__env__": "base", + "__sls__": "debconf", + "data": { + "mysql-server/root_password": { + "type": "password", + "value": "temp123" + } + }, + "order": 10000 + } + expected_result = {'comment': 'onlyif condition is true', 'result': False} + + with patch('salt.state.State._gather_pillar') as state_patch: + minion_opts = self.get_temp_config('minion') + state_obj = salt.state.State(minion_opts) + return_result = state_obj._run_check_onlyif(low_data, '') + self.assertEqual(expected_result, return_result) + + def test_verify_unless_parse(self): + low_data = { + "unless": [ + { + "fun": "file.search", + "args": [ + "/etc/crontab", + "run-parts" + ] + } + ], + "name": "mysql-server-5.7", + "state": "debconf", + "__id__": "set root password", + "fun": "set", + "__env__": "base", + "__sls__": "debconf", + "data": { + "mysql-server/root_password": { + "type": "password", + "value": "temp123" + } + }, + "order": 10000 + } + expected_result = {'comment': 'unless condition is true', 'result': True, 'skip_watch': True} + + with patch('salt.state.State._gather_pillar') as state_patch: + minion_opts = self.get_temp_config('minion') + state_obj = salt.state.State(minion_opts) + return_result = state_obj._run_check_unless(low_data, '') + self.assertEqual(expected_result, return_result) + class HighStateTestCase(TestCase, AdaptedConfigurationTestCaseMixin): def setUp(self): From 486df12ce320038cad9c9fbf841812534d2a66f8 Mon Sep 17 00:00:00 2001 From: Christian McHugh Date: Wed, 13 Nov 2019 06:57:23 +0000 Subject: [PATCH 3/4] Remove unrelated tests --- tests/unit/test_state.py | 56 ---------------------------------------- 1 file changed, 56 deletions(-) diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index e77158fbd903..3d4aec2eae23 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -73,62 +73,6 @@ def test_render_error_on_invalid_requisite(self): with self.assertRaises(salt.exceptions.SaltRenderError): state_obj.call_high(high_data) - def test_render_requisite_require_disabled(self): - ''' - Test that the state compiler correctly deliver a rendering - exception when a requisite cannot be resolved - ''' - with patch('salt.state.State._gather_pillar') as state_patch: - high_data = { - 'step_one': OrderedDict([ - ('test', [ - OrderedDict([ - ('require', [ - OrderedDict([ - ('test', 'step_two')])])]), - 'succeed_with_changes', {'order': 10000}]), - ('__sls__', 'test.disable_require'), - ('__env__', 'base')]), - 'step_two': {'test': ['succeed_with_changes', - {'order': 10001}], - '__env__': 'base', - '__sls__': 'test.disable_require'}} - - minion_opts = self.get_temp_config('minion') - minion_opts['disabled_requisites'] = ['require'] - state_obj = salt.state.State(minion_opts) - ret = state_obj.call_high(high_data) - run_num = ret['test_|-step_one_|-step_one_|-succeed_with_changes']['__run_num__'] - self.assertEqual(run_num, 0) - - def test_render_requisite_require_in_disabled(self): - ''' - Test that the state compiler correctly deliver a rendering - exception when a requisite cannot be resolved - ''' - with patch('salt.state.State._gather_pillar') as state_patch: - high_data = { - 'step_one': {'test': ['succeed_with_changes', - {'order': 10000}], - '__env__': 'base', - '__sls__': 'test.disable_require_in'}, - 'step_two': OrderedDict([ - ('test', [ - OrderedDict([ - ('require_in', [ - OrderedDict([ - ('test', 'step_one')])])]), - 'succeed_with_changes', {'order': 10001}]), - ('__sls__', 'test.disable_require_in'), - ('__env__', 'base')])} - - minion_opts = self.get_temp_config('minion') - minion_opts['disabled_requisites'] = ['require_in'] - state_obj = salt.state.State(minion_opts) - ret = state_obj.call_high(high_data) - run_num = ret['test_|-step_one_|-step_one_|-succeed_with_changes']['__run_num__'] - self.assertEqual(run_num, 0) - def test_verify_onlyif_parse(self): low_data = { "onlyif": [ From 134a839c17368439721617f36f23634d427b703d Mon Sep 17 00:00:00 2001 From: Christian McHugh Date: Wed, 11 Dec 2019 21:53:53 +0000 Subject: [PATCH 4/4] Correct tests to not be debian specific --- tests/unit/test_state.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index 3d4aec2eae23..2a2a768a7e20 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -77,10 +77,10 @@ def test_verify_onlyif_parse(self): low_data = { "onlyif": [ { - "fun": "file.search", + "fun": "test.arg", "args": [ - "/etc/crontab", - "run-parts" + "arg1", + "arg2" ] } ], @@ -110,10 +110,10 @@ def test_verify_unless_parse(self): low_data = { "unless": [ { - "fun": "file.search", + "fun": "test.arg", "args": [ - "/etc/crontab", - "run-parts" + "arg1", + "arg2" ] } ],