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

Make salt-client aware of edge-case where saltutil might be broken #35446

Merged
merged 3 commits into from
Aug 16, 2016

Conversation

cachedout
Copy link
Contributor

Closes #34161

except KeyError as exc:
# This is a safe pass. We're just using the try/except to avoid having to deep-check for keys
log.debug('Passing on saltutil error. This may be an error in saltclient. {0}'.format(exc))
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass is not needed here, I guess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops. Forgot to remove that from when I was debugging. Thanks.

@rallytime rallytime added the pending-changes The pull request needs additional changes before it can be merged label Aug 15, 2016
@cachedout cachedout removed the pending-changes The pull request needs additional changes before it can be merged label Aug 16, 2016
@rallytime rallytime merged commit ee525b1 into saltstack:2016.3 Aug 16, 2016
gitebra pushed a commit to gitebra/salt that referenced this pull request Aug 17, 2016
* upstream/develop: (25 commits)
  Remove "env" keword arg references from cp module functions
  Update blockdev references to 'disk' in blockdev state unit test
  Add docs for pillar_raise_on_missing option
  Update blockdev state to use disk execution module instead of blockdev
  Add blockdev.py module back in to develop.
  Remove the env and activate kwargs from pip install/ed functions
  Update bootstrap script to latest stable (2016.08.16) (saltstack#35486)
  Make salt-client aware of edge-case where saltutil might be broken (saltstack#35446)
  Fixup SSH bug where sudo without sudo user would break
  Fix silly error
  win_pkg: Fix traceback when package is not installed
  fix clrf
  commit fix
  add a test to check existing functionality is broken
  fixes saltstack#34279
  [2015.8] Update bootstrap script to latest stable (2016.08.15) (saltstack#35460)
  Some environments refuse to return the command output
  Fix Unit test for suppressing the exception removal on non-modified repos
  Remove zypper's raise exception if mod_repo has no arguments and/or no changes
  Add ignore_repo_failure option to suppress zypper's exit code 106 on unavailable repos
  ...
@terminalmage
Copy link
Contributor

@cachedout This causes error logging when there is no retcode in ret['data'], like cases when saltutil.find_job is executed during a run. For example:

{'data': {'_stamp': '2016-08-30T04:42:45.195486',
          'arg': ['20160829234240087757'],
          'fun': 'saltutil.find_job',
          'jid': '20160829234245192978',
          'minions': ['cent7-upgrade-test', 'tardis'],
          'tgt': '*',
          'tgt_type': 'glob',
          'user': 'root'},
 'tag': 'salt/job/20160829234245192978/new'}

I had to obtain that by launching a debugger when the exception was triggered. This particular case happened while I was running the state.orchestrate runner to execute a state (i.e. a salt.state orchestration job).

Are we simply missing a retcode here, or does this need to be refined?

@cachedout
Copy link
Contributor Author

Ah, K. Apologies for the headache there. I think we should just refine this to only enter the block if a retcode is present. I'll make the change if you concur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants