File backport-of-upstream-pr59492-to-3002.2-404.patch of Package venv-salt-minion

Overview Repositories Revisions Requests Users Attributes Meta

File backport-of-upstream-pr59492-to-3002.2-404.patch of Package venv-salt-minion (Revision a345df66821511333c33807f5c05adb5)

Currently displaying revision a345df66821511333c33807f5c05adb5 , Show latest

From fba6631e0a66a5f8ea76a104e9acf385ce06471c Mon Sep 17 00:00:00 2001
From: Victor Zhestkov <35733135+vzhestkov@users.noreply.github.com>
Date: Wed, 18 Aug 2021 15:05:30 +0300
Subject: [PATCH] Backport of upstream PR59492 to 3002.2 (#404)

* Fix failing integration tests

* Fix unless logic and failing tests

* Revert some of the changes in the onlyif code

Co-authored-by: twangboy <slee@saltstack.com>
---
 salt/state.py                                 | 24 +++++++++------
 .../files/file/base/issue-35384.sls           |  7 +++++
 tests/unit/test_state.py                      | 30 ++++++++++++++-----
 3 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/salt/state.py b/salt/state.py
index 070a914636..64c5225728 100644
--- a/salt/state.py
+++ b/salt/state.py
@@ -929,7 +929,8 @@ class State:
 
     def _run_check_onlyif(self, low_data, cmd_opts):
         """
-        Check that unless doesn't return 0, and that onlyif returns a 0.
+        Make sure that all commands return True for the state to run. If any
+        command returns False (non 0), the state will not run
         """
         ret = {"result": False}
 
@@ -938,7 +939,9 @@ class State:
         else:
             low_data_onlyif = low_data["onlyif"]
 
+        # If any are False the state will NOT run
         def _check_cmd(cmd):
+            # Don't run condition (False)
             if cmd != 0 and ret["result"] is False:
                 ret.update(
                     {
@@ -1001,7 +1004,8 @@ class State:
 
     def _run_check_unless(self, low_data, cmd_opts):
         """
-        Check that unless doesn't return 0, and that onlyif returns a 0.
+        Check if any of the commands return False (non 0). If any are False the
+        state will run.
         """
         ret = {"result": False}
 
@@ -1010,8 +1014,10 @@ class State:
         else:
             low_data_unless = low_data["unless"]
 
+        # If any are False the state will run
         def _check_cmd(cmd):
-            if cmd == 0 and ret["result"] is False:
+            # Don't run condition
+            if cmd == 0:
                 ret.update(
                     {
                         "comment": "unless condition is true",
@@ -1020,9 +1026,10 @@ class State:
                     }
                 )
                 return False
-            elif cmd != 0:
+            else:
+                ret.pop("skip_watch", None)
                 ret.update({"comment": "unless condition is false", "result": False})
-            return True
+                return True
 
         for entry in low_data_unless:
             if isinstance(entry, str):
@@ -1034,7 +1041,7 @@ class State:
                 except CommandExecutionError:
                     # Command failed, so notify unless to skip the item
                     cmd = 0
-                if not _check_cmd(cmd):
+                if _check_cmd(cmd):
                     return ret
             elif isinstance(entry, dict):
                 if "fun" not in entry:
@@ -1047,7 +1054,7 @@ class State:
                 if get_return:
                     result = salt.utils.data.traverse_dict_and_list(result, get_return)
                 if self.state_con.get("retcode", 0):
-                    if not _check_cmd(self.state_con["retcode"]):
+                    if _check_cmd(self.state_con["retcode"]):
                         return ret
                 elif result:
                     ret.update(
@@ -1057,11 +1064,11 @@ class State:
                             "result": True,
                         }
                     )
-                    return ret
                 else:
                     ret.update(
                         {"comment": "unless condition is false", "result": False}
                     )
+                    return ret
             else:
                 ret.update(
                     {
@@ -1069,7 +1076,6 @@ class State:
                         "result": False,
                     }
                 )
-                return ret
 
         # No reason to stop, return ret
         return ret
diff --git a/tests/integration/files/file/base/issue-35384.sls b/tests/integration/files/file/base/issue-35384.sls
index 3c41617ca8..2aa436bb37 100644
--- a/tests/integration/files/file/base/issue-35384.sls
+++ b/tests/integration/files/file/base/issue-35384.sls
@@ -2,5 +2,12 @@ cmd_run_unless_multiple:
   cmd.run:
     - name: echo "hello"
     - unless:
+  {% if grains["os"] ==  "Windows" %}
+      - "exit 0"
+      - "exit 1"
+      - "exit 0"
+  {% else %}
       - "$(which true)"
       - "$(which false)"
+      - "$(which true)"
+  {% endif %}
diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py
index 95018a9cf3..79a261d837 100644
--- a/tests/unit/test_state.py
+++ b/tests/unit/test_state.py
@@ -142,7 +142,7 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
     def test_verify_onlyif_cmd_error(self):
         """
         Simulates a failure in cmd.retcode from onlyif
-        This could occur is runas is specified with a user that does not exist
+        This could occur if runas is specified with a user that does not exist
         """
         low_data = {
             "onlyif": "somecommand",
@@ -175,7 +175,7 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
     def test_verify_unless_cmd_error(self):
         """
         Simulates a failure in cmd.retcode from unless
-        This could occur is runas is specified with a user that does not exist
+        This could occur if runas is specified with a user that does not exist
         """
         low_data = {
             "unless": "somecommand",
@@ -206,6 +206,10 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
                 self.assertEqual(expected_result, return_result)
 
     def test_verify_unless_list_cmd(self):
+        """
+        If any of the unless commands return False (non 0) then the state should
+        run (no skip_watch).
+        """
         low_data = {
             "state": "cmd",
             "name": 'echo "something"',
@@ -217,9 +221,8 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
             "fun": "run",
         }
         expected_result = {
-            "comment": "unless condition is true",
-            "result": True,
-            "skip_watch": True,
+            "comment": "unless condition is false",
+            "result": False,
         }
         with patch("salt.state.State._gather_pillar") as state_patch:
             minion_opts = self.get_temp_config("minion")
@@ -228,6 +231,10 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
             self.assertEqual(expected_result, return_result)
 
     def test_verify_unless_list_cmd_different_order(self):
+        """
+        If any of the unless commands return False (non 0) then the state should
+        run (no skip_watch). The order shouldn't matter.
+        """
         low_data = {
             "state": "cmd",
             "name": 'echo "something"',
@@ -239,9 +246,8 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
             "fun": "run",
         }
         expected_result = {
-            "comment": "unless condition is true",
-            "result": True,
-            "skip_watch": True,
+            "comment": "unless condition is false",
+            "result": False,
         }
         with patch("salt.state.State._gather_pillar") as state_patch:
             minion_opts = self.get_temp_config("minion")
@@ -272,6 +278,10 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
             self.assertEqual(expected_result, return_result)
 
     def test_verify_unless_list_cmd_valid(self):
+        """
+        If any of the unless commands return False (non 0) then the state should
+        run (no skip_watch). This tests all commands return False.
+        """
         low_data = {
             "state": "cmd",
             "name": 'echo "something"',
@@ -308,6 +318,10 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
             self.assertEqual(expected_result, return_result)
 
     def test_verify_unless_list_cmd_invalid(self):
+        """
+        If any of the unless commands return False (non 0) then the state should
+        run (no skip_watch). This tests all commands return True
+        """
         low_data = {
             "state": "cmd",
             "name": 'echo "something"',
-- 
2.32.0

Places

File backport-of-upstream-pr59492-to-3002.2-404.patch of Package venv-salt-minion (Revision a345df66821511333c33807f5c05adb5)

Places