Allow explicit reboot on transactional minions

salt/modules/transactional_update.py

@@ -985,13 +985,40 @@ def call(function, *args, **kwargs):
             else:
                 return local
         except ValueError:
-            return {"result": False, "retcode": 1, "comment": ret_stdout}
+            local = {"result": False, "retcode": 1, "comment": ret_stdout}
+            return local
     finally:
         # Check if reboot is needed
-        if activate_transaction and pending_transaction():
+        if (activate_transaction and pending_transaction()) or (
+            not kwargs.get("test", False) and _user_specified_reboot(local, function)
+        ):
             reboot()
 
 
+def _user_specified_reboot(local, function):
+    if function != "state.highstate" and function != "state.sls":
+        return False
+
+    if not isinstance(local, dict):
+        return False
+
+    explicit_reboot_cmds = set(["reboot", "system.reboot"])
+    explicit_reboot_modules = ["cmd_", "module_"]
+    names = set()
+    for full_module_name, module_result in local.items():
+        if not isinstance(full_module_name, str):
+            continue
+
+        module = full_module_name.split("|")[0]
+        if module not in explicit_reboot_modules:
+            continue
+
+        if isinstance(module_result, dict) and "name" in module_result:
+            names.add(module_result["name"])
+
+    return bool(explicit_reboot_cmds.intersection(names))
+
+
 def apply_(mods=None, **kwargs):
     """Apply an state inside a transaction.
 

tests/pytests/unit/modules/test_transactional_update.py

@@ -178,9 +178,11 @@ def test_commands_with_global_params():
                     "--non-interactive",
                     "--drop-if-no-change",
                     "--no-selfupdate",
-                    cmd.replace("_", ".")
-                    if cmd.startswith("grub")
-                    else cmd.replace("_", "-"),
+                    (
+                        cmd.replace("_", ".")
+                        if cmd.startswith("grub")
+                        else cmd.replace("_", "-")
+                    ),
                 ]
             )
 
@@ -501,6 +503,137 @@ def test_call_success_parameters():
         )
 
 
+def test_call_success_explicit_reboot():
+    """Test transactional_update.call executes reboot when user specifies 'reboot' in sls"""
+    return_json = {
+        "local": {
+            "cmd_|-reboot_test_|-reboot_|-run": {
+                "name": "reboot",
+                "changes": {},
+                "result": True,
+            }
+        }
+    }
+    utils_mock = {
+        "json.find_json": MagicMock(return_value=return_json),
+    }
+    salt_mock = {
+        "cmd.run_all": MagicMock(return_value={"retcode": 0, "stdout": ""}),
+    }
+    reboot_mock = MagicMock()
+    with patch.dict(tu.__utils__, utils_mock), patch.dict(
+        tu.__salt__, salt_mock
+    ), patch("salt.modules.transactional_update.reboot", reboot_mock):
+        tu.call("state.sls", key="value")
+        reboot_mock.assert_called_once()
+
+
+def test_call_fail_explicit_reboot_test():
+    """Test transactional_update.call does NOT execute reboot when user specifies 'reboot' in sls in test mode"""
+    return_json = {
+        "local": {
+            "cmd_|-reboot_test_|-reboot_|-run": {
+                "name": "reboot",
+                "changes": {},
+                "result": True,
+            }
+        }
+    }
+    utils_mock = {
+        "json.find_json": MagicMock(return_value=return_json),
+    }
+    salt_mock = {
+        "cmd.run_all": MagicMock(return_value={"retcode": 0, "stdout": ""}),
+    }
+    reboot_mock = MagicMock()
+    with patch.dict(tu.__utils__, utils_mock), patch.dict(
+        tu.__salt__, salt_mock
+    ), patch("salt.modules.transactional_update.reboot", reboot_mock):
+        tu.call("state.sls", test="True")
+        assert not reboot_mock.called
+
+
+def test_call_fail_explicit_reboot():
+    """Test transactional_update.call does NOT execute reboot when the word 'reboot' appears in sls"""
+    return_json = {
+        "local": {
+            "cmd_|-reboot_test_|-reboot_|-run": {
+                "name": "service reboot",
+                "changes": {},
+                "result": True,
+            }
+        }
+    }
+    utils_mock = {
+        "json.find_json": MagicMock(return_value=return_json),
+    }
+    salt_mock = {
+        "cmd.run_all": MagicMock(return_value={"retcode": 0, "stdout": ""}),
+    }
+    reboot_mock = MagicMock()
+    with patch.dict(tu.__utils__, utils_mock), patch.dict(
+        tu.__salt__, salt_mock
+    ), patch("salt.modules.transactional_update.reboot", reboot_mock):
+        tu.call("state.sls", test="True")
+        assert not reboot_mock.called
+
+
+def test_call_fail_explicit_reboot_non_sls():
+    """
+    Test transactional_update.call does NOT execute reboot when the word 'reboot'
+    appears in sls executed with function other than state.sls
+    """
+    return_json = {
+        "local": {
+            "cmd_|-reboot_test_|-reboot_|-run": {
+                "name": "reboot",
+                "changes": {},
+                "result": True,
+            }
+        }
+    }
+    utils_mock = {
+        "json.find_json": MagicMock(return_value=return_json),
+    }
+    salt_mock = {
+        "cmd.run_all": MagicMock(return_value={"retcode": 0, "stdout": ""}),
+    }
+    reboot_mock = MagicMock()
+    with patch.dict(tu.__utils__, utils_mock), patch.dict(
+        tu.__salt__, salt_mock
+    ), patch("salt.modules.transactional_update.reboot", reboot_mock):
+        tu.call("state.test")
+        assert not reboot_mock.called
+
+
+def test_call_fail_explicit_reboot_non_cmd():
+    """
+    Test transactional_update.call does NOT execute reboot when the word 'reboot'
+    appears in sls executed with module other than `cmd` or `module.include`
+    """
+    return_json = {
+        "local": {
+            "test_|-reboot_test_|-reboot_|-run": {
+                "name": "reboot",
+                "changes": {},
+                "result": True,
+            }
+        }
+    }
+    utils_mock = {
+        "json.find_json": MagicMock(return_value=return_json),
+    }
+    salt_mock = {
+        "cmd.run_all": MagicMock(return_value={"retcode": 0, "stdout": ""}),
+    }
+    reboot_mock = MagicMock()
+    with patch.dict(tu.__utils__, utils_mock), patch.dict(
+        tu.__salt__, salt_mock
+    ), patch("salt.modules.transactional_update.reboot", reboot_mock):
+        tu.call("state.sls")
+        assert not reboot_mock.called
+
+
 def test_sls():
     """Test transactional_update.sls"""
     salt_mock = {