Report failures and error information.

Replace stdout in cases of success with known value for state to evaluate.
Update response handling in states.

Fixes saltstack#60500

changelog/60500.fixed

@@ -0,0 +1 @@
+Handle failure and error information from tuned module/state

salt/modules/tuned.py

@@ -7,11 +7,12 @@
 :platform:      Linux
 """
 
-
 import re
 
 import salt.utils.path
 
+TUNED_OFF_RETURN_NAME = "No current active profile."
+
 __func_alias__ = {
     "list_": "list",
 }
@@ -63,7 +64,7 @@ def list_():
 
 def active():
     """
-    Return current active profile
+    Return current active profile in stdout key if retcode is 0, otherwise raw result
 
     CLI Example:
 
@@ -72,13 +73,16 @@ def active():
         salt '*' tuned.active
     """
 
-    # turn off all profiles
+    # determine the active profile
     result = __salt__["cmd.run_all"]("tuned-adm active", ignore_retcode=True)
-    if result["retcode"] != 0:
-        return "none"
-    pattern = re.compile(r"""(?P<stmt>Current active profile:) (?P<profile>\w+.*)""")
-    match = re.match(pattern, result["stdout"])
-    return "{}".format(match.group("profile"))
+    if result["retcode"] == 0:
+        pattern = re.compile(
+            r"""(?P<stmt>Current active profile:) (?P<profile>\w+.*)"""
+        )
+        match = re.match(pattern, result["stdout"])
+        if match:
+            result["stdout"] = "{}".format(match.group("profile"))
+    return result
 
 
 def off():
@@ -93,10 +97,7 @@ def off():
     """
 
     # turn off all profiles
-    result = __salt__["cmd.retcode"]("tuned-adm off")
-    if int(result) != 0:
-        return False
-    return True
+    return __salt__["cmd.run_all"]("tuned-adm off")
 
 
 def profile(profile_name):
@@ -110,8 +111,10 @@ def profile(profile_name):
         salt '*' tuned.profile virtual-guest
     """
 
-    # run tuned-adm with the profile specified
-    result = __salt__["cmd.retcode"]("tuned-adm profile {}".format(profile_name))
-    if int(result) != 0:
-        return False
-    return "{}".format(profile_name)
+    # run tuned-adm with the profile specified, upon success replace stdout with the profile_name
+    result = __salt__["cmd.run_all"](
+        "tuned-adm profile {}".format(profile_name), ignore_retcode=True
+    )
+    if result["retcode"] == 0:
+        result["stdout"] = profile_name
+    return result

salt/states/tuned.py

@@ -7,8 +7,8 @@
 :platform:      Linux
 """
 
-
 import salt.exceptions
+from salt.modules.tuned import TUNED_OFF_RETURN_NAME
 
 
 def profile(name):
@@ -36,46 +36,56 @@ def profile(name):
     profile = name
 
     # get the current state of tuned-adm
-    current_state = __salt__["tuned.active"]()
-
-    valid_profiles = __salt__["tuned.list"]()
-
-    # check valid profiles, and return error if profile name is not valid
-    if profile not in valid_profiles:
-        raise salt.exceptions.SaltInvocationError("Invalid Profile Name")
+    current_state_dict = __salt__["tuned.active"]()
+
+    # Off is returned as retcode 1, stdout == TUNED_OFF_RETURN_NAME
+    # any other type of error will be returned here
+    if (
+        current_state_dict["retcode"] != 0
+        and current_state_dict["stdout"] != TUNED_OFF_RETURN_NAME
+    ):
+        ret["comment"] = current_state_dict["stderr"]
+        return ret
 
     # if current state is same as requested state, return without doing much
-    if profile in current_state:
+    if current_state_dict["retcode"] == 0 and profile == current_state_dict["stdout"]:
         ret["result"] = True
         ret["comment"] = "System already in the correct state"
         return ret
 
     # test mode
     if __opts__["test"] is True:
-        ret["comment"] = 'The state of "{}" will be changed.'.format(current_state)
+        # Only perform the valid profile test if it is a test,
+        #   tuned-adm command will fail and return message
+        valid_profiles = __salt__["tuned.list"]()
+        if profile not in valid_profiles:
+            raise salt.exceptions.SaltInvocationError("Invalid Profile Name")
+        ret["comment"] = 'The state of "{}" will be changed.'.format(
+            current_state_dict["stdout"]
+        )
         ret["changes"] = {
-            "old": current_state,
+            "old": current_state_dict["stdout"],
             "new": "Profile will be set to {}".format(profile),
         }
-
         # return None when testing
         ret["result"] = None
         return ret
 
-    # we come to this stage if current state is different that requested state
+    # we come to this stage if current state was determined and is different that requested state
     # we there have to set the new state request
-    new_state = __salt__["tuned.profile"](profile)
-
-    # create the comment data structure
-    ret["comment"] = 'The state of "{}" was changed!'.format(profile)
-
-    # fill in the ret data structure
-    ret["changes"] = {
-        "old": current_state,
-        "new": new_state,
-    }
-
-    ret["result"] = True
+    new_state_dict = __salt__["tuned.profile"](profile)
+
+    if new_state_dict["retcode"] != 0:
+        ret["comment"] = new_state_dict["stderr"]
+    else:
+        # create the comment data structure
+        ret["comment"] = 'Tunings changed to "{}"'.format(profile)
+        # add the changes specifics
+        ret["changes"] = {
+            "old": current_state_dict["stdout"],
+            "new": new_state_dict["stdout"],
+        }
+        ret["result"] = True
 
     # return with the dictionary data structure
     return ret
@@ -99,31 +109,42 @@ def off(name=None):
     ret = {"name": "off", "changes": {}, "result": False, "comment": "off"}
 
     # check the current state of tuned
-    current_state = __salt__["tuned.active"]()
-
-    # if profile is already off, then don't do anything
-    if current_state == "off":
-        ret["result"] = True
-        ret["comment"] = "System already in the correct state"
+    current_state_dict = __salt__["tuned.active"]()
+
+    # Off is returned as retcode 1, stdout == TUNED_OFF_RETURN_NAME
+    if current_state_dict["retcode"] != 0:
+        if current_state_dict["stdout"] == TUNED_OFF_RETURN_NAME:
+            ret["result"] = True
+            ret["comment"] = "System already in the correct state"
+            return ret
+        ret["comment"] = current_state_dict["stderr"]
         return ret
 
     # test mode
     if __opts__["test"] is True:
-        ret["comment"] = 'The state of "{}" will be changed.'.format(current_state)
+        ret["comment"] = 'The state of "{}" will be turned off.'.format(
+            current_state_dict["stdout"]
+        )
         ret["changes"] = {
-            "old": current_state,
+            "old": current_state_dict["stdout"],
             "new": "Profile will be set to off",
         }
-
         # return None when testing
         ret["result"] = None
         return ret
 
-    # actually execute the off statement
-    if __salt__["tuned.off"]():
-        ret["result"] = True
+    # execute the tuned.off module
+    off_result_dict = __salt__["tuned.off"]()
+
+    if off_result_dict["retcode"] != 0:
+        ret["comment"] = off_result_dict["stderr"]
+    else:
+        ret["comment"] = "Tunings have been turned off"
         ret["changes"] = {
-            "old": current_state,
+            "old": current_state_dict["stdout"],
             "new": "off",
         }
-        return ret
+        ret["result"] = True
+
+    # return with the dictionary data structure
+    return ret

tests/pytests/unit/modules/test_tuned.py

@@ -132,3 +132,70 @@ def test_none():
     mock_cmd = MagicMock(return_value=ret)
     with patch.dict(tuned.__salt__, {"cmd.run_all": mock_cmd}):
         assert tuned.active() == "none"
+
+
+def test_active_balanced(self):
+    ret = {
+        "pid": 12345,
+        "retcode": 0,
+        "stderr": "",
+        "stdout": "Current active profile: balanced",
+    }
+    mock_cmd = MagicMock(return_value=ret)
+    with patch.dict(tuned.__salt__, {"cmd.run_all": mock_cmd}):
+        self.assertEqual(tuned.active()["stdout"], "balanced")
+
+
+def test_off(self):
+    ret = {
+        "pid": 12345,
+        "retcode": 0,
+        "stderr": "",
+        "stdout": "",
+    }
+    mock_cmd = MagicMock(return_value=ret)
+    with patch.dict(tuned.__salt__, {"cmd.run_all": mock_cmd}):
+        self.assertEqual(tuned.off()["retcode"], 0)
+
+
+def test_profile_valid(self):
+    ret = {
+        "pid": 12345,
+        "retcode": 0,
+        "stderr": "",
+        "stdout": "",
+    }
+    mock_cmd = MagicMock(return_value=ret)
+    with patch.dict(tuned.__salt__, {"cmd.run_all": mock_cmd}):
+        self.assertEqual(tuned.profile("balanced")["stdout"], "balanced")
+
+
+def test_profile_noexist(self):
+    ret = {
+        "pid": 12345,
+        "retcode": 1,
+        "stderr": "Requested profile 'noexist' doesn't exist.",
+        "stdout": "",
+    }
+    mock_cmd = MagicMock(return_value=ret)
+    with patch.dict(tuned.__salt__, {"cmd.run_all": mock_cmd}):
+        self.assertEqual(
+            tuned.profile("noexist")["stderr"],
+            "Requested profile 'noexist' doesn't exist.",
+        )
+
+
+def test_profile_invalid(self):
+    ret = {
+        "pid": 12345,
+        "retcode": 1,
+        "stderr": """Cannot load profile(s) 'invalid':
+         ("Cannot parse '/usr/lib/tuned/invalid/tuned.conf'.",
+          DuplicateError('Duplicate keyword name at line 4.'))""",
+        "stdout": "",
+    }
+    mock_cmd = MagicMock(return_value=ret)
+    with patch.dict(tuned.__salt__, {"cmd.run_all": mock_cmd}):
+        self.assertTrue(
+            tuned.profile("invalid")["stderr"].startswith("Cannot load profile")
+        )