diff --git a/changelog/60500.fixed b/changelog/60500.fixed new file mode 100644 index 000000000000..1daf48c1a11c --- /dev/null +++ b/changelog/60500.fixed @@ -0,0 +1 @@ +Handle failure and error information from tuned module/state diff --git a/salt/modules/tuned.py b/salt/modules/tuned.py index 73742a9d54e1..96e9da61334a 100644 --- a/salt/modules/tuned.py +++ b/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"""(?PCurrent active profile:) (?P\w+.*)""") - match = re.match(pattern, result["stdout"]) - return "{}".format(match.group("profile")) + if result["retcode"] == 0: + pattern = re.compile( + r"""(?PCurrent active profile:) (?P\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 diff --git a/salt/states/tuned.py b/salt/states/tuned.py index d158f2ad1046..91c73e05b69f 100644 --- a/salt/states/tuned.py +++ b/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 diff --git a/tests/pytests/unit/modules/test_tuned.py b/tests/pytests/unit/modules/test_tuned.py index 01240df03cce..d36a860c0657 100644 --- a/tests/pytests/unit/modules/test_tuned.py +++ b/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") + ) \ No newline at end of file