From f88b721cf6988e0a18945c7c25be82cdad7c3c86 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 26 Feb 2019 13:24:35 -0600 Subject: [PATCH 1/7] implement using salt modules in onlyif and unless Closes #51604 Closes #5050 --- salt/state.py | 71 ++++++++++++------- tests/integration/modules/test_state.py | 91 +++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 24 deletions(-) diff --git a/salt/state.py b/salt/state.py index fb9f943304ed..3d640c9714f7 100644 --- a/salt/state.py +++ b/salt/state.py @@ -883,19 +883,31 @@ def _run_check_onlyif(self, low_data, cmd_opts): else: low_data_onlyif = low_data['onlyif'] for entry in low_data_onlyif: - if not isinstance(entry, six.string_types): + 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) + if cmd != 0 and ret['result'] is False: + ret.update({'comment': 'onlyif condition is false', + 'skip_watch': True, + 'result': True}) + elif cmd == 0: + ret.update({'comment': 'onlyif condition is true', 'result': False}) + 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 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 - cmd = self.functions['cmd.retcode']( - entry, ignore_retcode=True, python_shell=True, **cmd_opts) - log.debug('Last command return code: %s', 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}) return ret def _run_check_unless(self, low_data, cmd_opts): @@ -909,19 +921,30 @@ def _run_check_unless(self, low_data, cmd_opts): else: low_data_unless = low_data['unless'] for entry in low_data_unless: - if not isinstance(entry, six.string_types): + 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) + 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}) + 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 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}) - return ret - cmd = self.functions['cmd.retcode']( - entry, ignore_retcode=True, python_shell=True, **cmd_opts) - log.debug('Last command return code: %s', 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 # 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 150e4c3b940f..e6b1d06d6a50 100644 --- a/tests/integration/modules/test_state.py +++ b/tests/integration/modules/test_state.py @@ -1424,6 +1424,97 @@ 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_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_get_file_from_env_in_top_match(self): tgt = os.path.join(RUNTIME_VARS.TMP, 'prod-cheese-file') try: From c1fc4b7319f8783e8733ec1bb1d98f9c74bf0e41 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 26 Feb 2019 13:40:17 -0600 Subject: [PATCH 2/7] add documentation about new unless and onlyif feature --- doc/ref/states/requisites.rst | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/doc/ref/states/requisites.rst b/doc/ref/states/requisites.rst index 18dcb9a0ab42..e640bba54506 100644 --- a/doc/ref/states/requisites.rst +++ b/doc/ref/states/requisites.rst @@ -832,6 +832,24 @@ 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``. +.. versionadded: 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 @@ -872,6 +890,29 @@ 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. +.. versionadded: 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 ~~~~~ From 7c4006cfef8cbebff54bafc5a0aab564b19ee502 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 26 Feb 2019 13:45:52 -0600 Subject: [PATCH 3/7] use versionchanged instead of versionadded --- doc/ref/states/requisites.rst | 63 +++++++++++++++++------------------ 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/doc/ref/states/requisites.rst b/doc/ref/states/requisites.rst index e640bba54506..df7f99db6380 100644 --- a/doc/ref/states/requisites.rst +++ b/doc/ref/states/requisites.rst @@ -832,23 +832,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``. -.. versionadded: Neon +.. 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. + 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 + .. 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: @@ -890,28 +889,28 @@ 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. -.. versionadded: Neon +.. 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. + 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 + .. code-block:: yaml - install apache on redhat based distros: - pkg.latest: - - name: httpd - - onlyif: - - fun: match.grain - tgt: 'os_family: RedHat' + 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' + install apache on debian based distros: + pkg.latest: + - name: apache2 + - onlyif: + - fun: match.grain + tgt: 'os_family: Debian' runas ~~~~~ From ee438e43654d0652739fdd5a70ec65207d3f20ae Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 26 Feb 2019 13:46:50 -0600 Subject: [PATCH 4/7] remove extra whitespace --- doc/ref/states/requisites.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/ref/states/requisites.rst b/doc/ref/states/requisites.rst index df7f99db6380..ec68006313a4 100644 --- a/doc/ref/states/requisites.rst +++ b/doc/ref/states/requisites.rst @@ -833,7 +833,6 @@ 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 @@ -890,7 +889,6 @@ The above example ensures that the stop_volume and delete modules only run 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 From abb4541bfd525a8ca31a934b21ee635317dd1bb6 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 26 Feb 2019 14:09:56 -0600 Subject: [PATCH 5/7] fix lint --- tests/integration/modules/test_state.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/modules/test_state.py b/tests/integration/modules/test_state.py index e6b1d06d6a50..56abd5997266 100644 --- a/tests/integration/modules/test_state.py +++ b/tests/integration/modules/test_state.py @@ -1514,7 +1514,6 @@ def test_unless_req(self): self.assertFalse(ret['changes']) self.assertEqual(ret['comment'], 'Success!') - def test_get_file_from_env_in_top_match(self): tgt = os.path.join(RUNTIME_VARS.TMP, 'prod-cheese-file') try: From 4bc7b375fbcde277e6c98e2335a842ecb2c74948 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Thu, 28 Feb 2019 16:21:35 -0600 Subject: [PATCH 6/7] use retcode when it is set to something besides 0 --- salt/state.py | 37 +++++++++++-------- tests/integration/modules/test_state.py | 48 +++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 14 deletions(-) diff --git a/salt/state.py b/salt/state.py index 3d640c9714f7..a32ceff49465 100644 --- a/salt/state.py +++ b/salt/state.py @@ -882,30 +882,35 @@ def _run_check_onlyif(self, low_data, cmd_opts): low_data_onlyif = [low_data['onlyif']] else: low_data_onlyif = low_data['onlyif'] + def _check_cmd(cmd): + if cmd != 0 and ret['result'] is False: + ret.update({'comment': 'onlyif condition is false', + 'skip_watch': True, + 'result': True}) + 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) - if cmd != 0 and ret['result'] is False: - ret.update({'comment': 'onlyif condition is false', - 'skip_watch': True, - 'result': True}) - elif cmd == 0: - ret.update({'comment': 'onlyif condition is true', 'result': False}) + _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 not result: + 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 @@ -920,23 +925,27 @@ def _run_check_unless(self, low_data, cmd_opts): low_data_unless = [low_data['unless']] else: low_data_unless = low_data['unless'] + 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}) 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) - 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}) + _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 result: + 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}) diff --git a/tests/integration/modules/test_state.py b/tests/integration/modules/test_state.py index 56abd5997266..247211d4d343 100644 --- a/tests/integration/modules/test_state.py +++ b/tests/integration/modules/test_state.py @@ -1469,6 +1469,30 @@ def test_onlyif_req(self): 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', @@ -1514,6 +1538,30 @@ def test_unless_req(self): 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(RUNTIME_VARS.TMP, 'prod-cheese-file') try: From 6489e353eb2d6b1ef02bf13cc19e37ca7fef0c04 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Thu, 28 Feb 2019 16:24:24 -0600 Subject: [PATCH 7/7] fix pylint --- salt/state.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/salt/state.py b/salt/state.py index a32ceff49465..9c275c43b394 100644 --- a/salt/state.py +++ b/salt/state.py @@ -882,6 +882,7 @@ def _run_check_onlyif(self, low_data, cmd_opts): low_data_onlyif = [low_data['onlyif']] else: low_data_onlyif = low_data['onlyif'] + def _check_cmd(cmd): if cmd != 0 and ret['result'] is False: ret.update({'comment': 'onlyif condition is false', @@ -889,6 +890,7 @@ def _check_cmd(cmd): 'result': True}) 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']( @@ -925,6 +927,7 @@ def _run_check_unless(self, low_data, cmd_opts): low_data_unless = [low_data['unless']] else: low_data_unless = low_data['unless'] + def _check_cmd(cmd): if cmd == 0 and ret['result'] is False: ret.update({'comment': 'unless condition is true', @@ -932,6 +935,7 @@ def _check_cmd(cmd): 'result': True}) elif cmd != 0: ret.update({'comment': 'unless condition is false', 'result': False}) + 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)