Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] tuned state does not report error #60500

Closed
leeclemens opened this issue Jul 7, 2021 · 2 comments · Fixed by #65720
Closed

[BUG] tuned state does not report error #60500

leeclemens opened this issue Jul 7, 2021 · 2 comments · Fixed by #65720
Labels
Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@leeclemens
Copy link
Contributor

Description
If tuned-adm profile xyz fails, the output is ignored. If setting the state, the returned new value is False and the state is reported as changed and Succeeded.

If the profile does not exist at all, a Traceback is returned. This could perhaps be formatted nicer, at least for this known error condition.

Minor: The state example does not appear to be formatted correctly.

Setup

tunedtest_invalid:
  tuned.profile:
    - name: invalid
tunedtest_noexist:
  tuned.profile:
    - name: noexist

testit.sls

include:
  - tunedtest.invalid
  - tunedtest.noexist

Steps to Reproduce the behavior
Create a malformed custom tuned profile and use the state to set it. I had two include keywords in [main].

Expected behavior
The state's response could include useful information (not currently returned by module) and at a minimum indicate an error occurred (not Succeeded/changed).

Screenshots

tuned-adm output
# tuned-adm profile invalid
Cannot load profile(s) 'invalid': (u"Cannot parse '/usr/lib/tuned/invalid/tuned.conf'.", DuplicateError('Duplicate keyword name at line 4.',))
# tuned-adm profile noexist
Requested profile 'noexist' doesn't exist.
tuned.profile module
# salt `hostname` tuned.profile invalid
minion:
    False
ERROR: Minions returned with non-zero exit code
# salt `hostname` tuned.profile noexist
minion:
    False
state.sls
minion:
----------
          ID: tunedtest_invalid
    Function: tuned.profile
        Name: invalid
      Result: True
     Comment: The state of "invalid" was changed!
     Started: 17:20:49.600399
    Duration: 2837.899 ms
     Changes:   
              ----------
              new:
                  False
              old:
                  virtual-guest
----------
          ID: tunedtest_noexist
    Function: tuned.profile
        Name: noexist
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python3.6/site-packages/salt/state.py", line 2172, in call
                  *cdata["args"], **cdata["kwargs"]
                File "/usr/lib/python3.6/site-packages/salt/loader.py", line 1241, in __call__
                  return self.loader.run(run_func, *args, **kwargs)
                File "/usr/lib/python3.6/site-packages/salt/loader.py", line 2274, in run
                  return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
                File "/usr/lib/python3.6/site-packages/contextvars/__init__.py", line 38, in run
                  return callable(*args, **kwargs)
                File "/usr/lib/python3.6/site-packages/salt/loader.py", line 2289, in _run_as
                  return _func_or_method(*args, **kwargs)
                File "/usr/lib/python3.6/site-packages/salt/loader.py", line 2322, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python3.6/site-packages/salt/states/tuned.py", line 49, in profile
                  raise salt.exceptions.SaltInvocationError("Invalid Profile Name")
              salt.exceptions.SaltInvocationError: Invalid Profile Name
     Started: 17:20:52.440581
    Duration: 1464.783 ms
     Changes:   

Summary for minion
------------
Succeeded: 1 (changed=1)
Failed:    1
------------
Total states run:     2
Total run time:   4.303 s

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3003.1
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: unknown
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.11.1
       libgit2: Not Installed
      M2Crypto: 0.35.2
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: 2.6.1
  pycryptodome: Not Installed
        pygit2: Not Installed
        Python: 3.6.8 (default, Nov 16 2020, 16:55:22)
  python-gnupg: Not Installed
        PyYAML: 3.13
         PyZMQ: 17.0.0
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.1.4
 
System Versions:
          dist: centos 7 Core
        locale: UTF-8
       machine: x86_64
       release: 3.10.0-1160.31.1.el7.x86_64
        system: Linux
       version: CentOS Linux 7 Core

Additional context
N/A

@leeclemens leeclemens added Bug broken, incorrect, or confusing behavior needs-triage labels Jul 7, 2021
@OrangeDog OrangeDog added severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around and removed needs-triage labels Jul 9, 2021
@OrangeDog OrangeDog added this to the Approved milestone Jul 9, 2021
@leeclemens
Copy link
Contributor Author

I have a fix for this in progress; tested the modules but am still working on confirming the state functionality (and docs/tests/etc).

https://github.com/leeclemens/salt/tree/60500/tuned

@leeclemens

This comment has been minimized.

leeclemens added a commit to leeclemens/salt that referenced this issue Jul 13, 2021
Replace stdout in cases of success with known value for state to evaluate.
Update response handling in states.

Fixes saltstack#60500
leeclemens added a commit to leeclemens/salt that referenced this issue Dec 17, 2023
Replace stdout in cases of success with known value for state to evaluate.
Update response handling in states.

Fixes saltstack#60500
dwoz pushed a commit to leeclemens/salt that referenced this issue Dec 18, 2023
Replace stdout in cases of success with known value for state to evaluate.
Update response handling in states.

Fixes saltstack#60500
leeclemens added a commit to leeclemens/salt that referenced this issue Dec 19, 2023
Replace stdout in cases of success with known value for state to evaluate.
Update response handling in states.

Fixes saltstack#60500
@dwoz dwoz closed this as completed in a47eeb9 Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
2 participants