From 2290ce8367e5c8bd50198ab3027536054025946d Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 9 Nov 2012 10:50:16 +0000 Subject: [PATCH 1/6] Fix `salt.modules.yumpkg`. * Don't assume the `yum` module is available(`_YumErrorLogger`) * Indentation fix. * PEP-8 fixes. --- salt/modules/yumpkg.py | 121 ++++++++++++++++++++++++----------------- 1 file changed, 71 insertions(+), 50 deletions(-) diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index 21682199cf..dc6cbb29ec 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -16,6 +16,7 @@ import re log = logging.getLogger(__name__) + def __virtual__(): ''' Confine this module to yum based systems @@ -40,37 +41,37 @@ def __virtual__(): else: return False - -class _YumErrorLogger(yum.rpmtrans.NoOutputCallBack): - ''' - A YUM callback handler that logs failed packages with their associated - script output. - ''' - def __init__(self): - self.messages = {} - self.failed = [] - - def log_accumulated_errors(self): +if has_yumdeps: + class _YumErrorLogger(yum.rpmtrans.NoOutputCallBack): ''' - Convenience method for logging all messages from failed packages + A YUM callback handler that logs failed packages with their associated + script output. ''' - for pkg in self.failed: - log.error('{0} {1}'.format(pkg, self.messages[pkg])) + def __init__(self): + self.messages = {} + self.failed = [] - def errorlog(self, msg): - # Log any error we receive - log.error(msg) + def log_accumulated_errors(self): + ''' + Convenience method for logging all messages from failed packages + ''' + for pkg in self.failed: + log.error('{0} {1}'.format(pkg, self.messages[pkg])) - def filelog(self, package, action): - # TODO: extend this for more conclusive transaction handling for - # installs and removes VS. the pkg list compare method used now. - if action == yum.constants.TS_FAILED: - self.failed.append(package) + def errorlog(self, msg): + # Log any error we receive + log.error(msg) - def scriptout(self, package, msgs): - # This handler covers ancillary messages coming from the RPM script. - # Will sometimes contain more detailed error messages. - self.messages[package] = msgs + def filelog(self, package, action): + # TODO: extend this for more conclusive transaction handling for + # installs and removes VS. the pkg list compare method used now. + if action == yum.constants.TS_FAILED: + self.failed.append(package) + + def scriptout(self, package, msgs): + # This handler covers ancillary messages coming from the RPM script + # Will sometimes contain more detailed error messages. + self.messages[package] = msgs def _list_removed(old, new): @@ -96,22 +97,23 @@ def _parse_pkg_meta(path): if result['retcode'] == 0: for line in result['stdout'].splitlines(): if not name: - m = re.match('^Name\s*:\s*(.+)\s*$',line) + m = re.match('^Name\s*:\s*(.+)\s*$', line) if m: name = m.group(1) continue if not version: - m = re.match('^Version\s*:\s*(.+)\s*$',line) + m = re.match('^Version\s*:\s*(.+)\s*$', line) if m: version = m.group(1) continue if not rel: - m = re.match('^Release\s*:\s*(.+)\s*$',line) + m = re.match('^Release\s*:\s*(.+)\s*$', line) if m: version = m.group(1) continue - if rel: version += '-{0}'.format(rel) - return name,version + if rel: + version += '-{0}'.format(rel) + return name, version def _compare_versions(old, new): @@ -135,6 +137,7 @@ def _compare_versions(old, new): 'new': new[npkg]} return pkgs + def list_upgrades(*args): ''' Check whether or not an upgrade is available for all packages @@ -143,19 +146,24 @@ def list_upgrades(*args): salt '*' pkg.list_upgrades ''' - pkgs=list_pkgs() + pkgs = list_pkgs() - yb=yum.YumBase() - versions_list={} + yb = yum.YumBase() + versions_list = {} for pkgtype in ['updates']: - pl=yb.doPackageLists(pkgtype) + pl = yb.doPackageLists(pkgtype) for pkg in pkgs: - exactmatch, matched, unmatched = yum.packages.parsePackages(pl, [pkg]) + exactmatch, matched, unmatched = yum.packages.parsePackages( + pl, [pkg] + ) for pkg in exactmatch: if pkg.arch == getBaseArch() or pkg.arch == 'noarch': - versions_list[pkg['name']] = '-'.join([pkg['version'],pkg['release']]) + versions_list[pkg['name']] = '-'.join( + [pkg['version'], pkg['release']] + ) return versions_list + def available_version(name): ''' The available version of the package in the repository @@ -190,6 +198,7 @@ def available_version(name): # remove the duplicate items from the list and return the first one return list(set(versions_list))[0] + def upgrade_available(name): ''' Check whether or not an upgrade is available for a given package @@ -200,6 +209,7 @@ def upgrade_available(name): ''' return available_version(name) + def version(name): ''' Returns a version if the package is installed, else returns an empty string @@ -214,6 +224,7 @@ def version(name): else: return '' + def list_pkgs(*args): ''' List the packages currently installed in a dict:: @@ -226,15 +237,15 @@ def list_pkgs(*args): ''' ts = rpm.TransactionSet() pkgs = {} - # In order to support specific package versions, we are going to use the yum - # libraries to handle pattern matching + # In order to support specific package versions, we are going to use the + # yum libraries to handle pattern matching yb = yum.YumBase() setattr(yb.conf, 'assumeyes', True) # if no args are passed in get all packages if len(args) == 0: for h in ts.dbMatch(): - pkgs[h['name']] = '-'.join([h['version'],h['release']]) + pkgs[h['name']] = '-'.join([h['version'], h['release']]) else: # get package version for each package in *args for arg in args: @@ -242,13 +253,14 @@ def list_pkgs(*args): a = yb.pkgSack.returnPackages(patterns=[arg], ignore_case=False) # make sure there is an a if len(a) > 0: - arg = a[0].name + arg = a[0].name # use the name from yum to do an rpm lookup for h in ts.dbMatch('name', arg): - pkgs[h['name']] = '-'.join([h['version'],h['release']]) + pkgs[h['name']] = '-'.join([h['version'], h['release']]) return pkgs + def refresh_db(): ''' Since yum refreshes the database automatically, this runs a yum clean, @@ -262,6 +274,7 @@ def refresh_db(): yb.cleanMetadata() return True + def clean_metadata(): ''' Cleans local yum metadata. @@ -272,6 +285,7 @@ def clean_metadata(): ''' return refresh_db() + def install(pkgs, refresh=False, repo='', skip_verify=False, sources=None, **kwargs): ''' @@ -319,9 +333,11 @@ def install(pkgs, refresh=False, repo='', skip_verify=False, sources=None, '({1})'.format(len(srcsplit), len(pkgs))) return {} - sources = [__salt__['cp.cache_file'](x) - if __salt__['config.valid_fileproto'](x) else x - for x in srcsplit] + sources = [ + __salt__['cp.cache_file'](x) + if __salt__['config.valid_fileproto'](x) else x + for x in srcsplit + ] # Check metadata to make sure the name passed matches the source for i in range(0, len(pkgs)): @@ -332,7 +348,6 @@ def install(pkgs, refresh=False, repo='', skip_verify=False, sources=None, '({2})'.format(sources[i], pname, pkgs[i])) return {} - old = list_pkgs(*pkgs) yb = yum.YumBase() @@ -340,13 +355,16 @@ def install(pkgs, refresh=False, repo='', skip_verify=False, sources=None, setattr(yb.conf, 'gpgcheck', not skip_verify) if repo: - log.info("Enabling repo '{0}'".format(repo)) + log.info('Enabling repo \'{0}\''.format(repo)) yb.repos.enableRepo(repo) - for i in range(0,len(pkgs)): + + for i in range(0, len(pkgs)): try: if sources is not None: target = sources[i] - log.info("Selecting '{0}' for local installation".format(target)) + log.info( + 'Selecting \'{0}\' for local installation'.format(target) + ) a = yb.installLocal(target) # if yum didn't install anything, maybe its a downgrade? log.debug('Added {0} transactions'.format(len(a))) @@ -355,7 +373,7 @@ def install(pkgs, refresh=False, repo='', skip_verify=False, sources=None, a = yb.downgradeLocal(target) else: target = pkgs[i] - log.info("Selecting '{0}' for installation".format(target)) + log.info('Selecting \'{0}\' for installation'.format(target)) # Changed to pattern to allow specific package versions a = yb.install(pattern=target) # if yum didn't install anything, maybe its a downgrade? @@ -382,6 +400,7 @@ def install(pkgs, refresh=False, repo='', skip_verify=False, sources=None, return _compare_versions(old, new) + def upgrade(): ''' Run a full system upgrade, a yum upgrade @@ -416,6 +435,7 @@ def upgrade(): new = list_pkgs() return _compare_versions(old, new) + def remove(pkgs): ''' Removes packages with yum remove @@ -448,6 +468,7 @@ def remove(pkgs): return _list_removed(old, new) + def purge(pkgs): ''' Yum does not have a purge, this function calls remove From 28510df4db55ad155a9a631036dd2d03c4513a2d Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 9 Nov 2012 11:04:49 +0000 Subject: [PATCH 2/6] Don't assume! * Only add '*' and 'sys.doc' on a `salt -d` cli call if those arguments aren't already there. --- salt/utils/parsers.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/salt/utils/parsers.py b/salt/utils/parsers.py index 7dcbf30cc9..f55929e25c 100644 --- a/salt/utils/parsers.py +++ b/salt/utils/parsers.py @@ -728,9 +728,11 @@ class SaltCMDOptionParser(OptionParser, ConfigDirMixIn, TimeoutMixIn, if self.options.doc: # Include the target - self.args.insert(0, '*') - # Include the function - self.args.insert(1, 'sys.doc') + if self.args[0] != '*': + self.args.insert(0, '*') + if self.args[1] != 'sys.doc': + # Include the function + self.args.insert(1, 'sys.doc') if self.options.list: self.config['tgt'] = self.args[0].split(',') From 75f70e49f381cdada62ded5018b5bf33c5710540 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 9 Nov 2012 11:23:30 +0000 Subject: [PATCH 3/6] Include a test case for the previous "Don't assume!" commit. --- tests/integration/shell/matcher.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/integration/shell/matcher.py b/tests/integration/shell/matcher.py index 5f88153b3f..3dc0916003 100644 --- a/tests/integration/shell/matcher.py +++ b/tests/integration/shell/matcher.py @@ -150,6 +150,15 @@ class MatchTest(integration.ShellCase, integration.ShellCaseCommonTestsMixIn): data = self.run_salt('-d user.add') self.assertIn('user.add:', data) + def test_salt_documentation_arguments_not_assumed(self): + ''' + Test to see if we're not auto-adding '*' and 'sys.doc' to the call + ''' + data = self.run_salt('\'*\' -d user.add') + self.assertIn('user.add:', data) + data = self.run_salt('\'*\' sys.doc -d user.add') + self.assertIn('user.add:', data) + if __name__ == "__main__": loader = TestLoader() From 527cf4ee38e5832d64ef5118e0060d192f21dcfc Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 9 Nov 2012 11:32:36 +0000 Subject: [PATCH 4/6] Don't assume again and expand test. * When introducing the "Don't assume!" changes I also assumed that there were always more than one argument from the shell. Expanded the testcase to include this check. --- salt/utils/parsers.py | 2 +- tests/integration/shell/matcher.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/salt/utils/parsers.py b/salt/utils/parsers.py index f55929e25c..41ebec4a83 100644 --- a/salt/utils/parsers.py +++ b/salt/utils/parsers.py @@ -730,7 +730,7 @@ class SaltCMDOptionParser(OptionParser, ConfigDirMixIn, TimeoutMixIn, # Include the target if self.args[0] != '*': self.args.insert(0, '*') - if self.args[1] != 'sys.doc': + if len(self.args) < 2 or self.args[1] != 'sys.doc': # Include the function self.args.insert(1, 'sys.doc') diff --git a/tests/integration/shell/matcher.py b/tests/integration/shell/matcher.py index 3dc0916003..a6d0ce7316 100644 --- a/tests/integration/shell/matcher.py +++ b/tests/integration/shell/matcher.py @@ -154,10 +154,15 @@ class MatchTest(integration.ShellCase, integration.ShellCaseCommonTestsMixIn): ''' Test to see if we're not auto-adding '*' and 'sys.doc' to the call ''' + data = self.run_salt('\'*\' -d') + self.assertIn('user.add:', data) data = self.run_salt('\'*\' -d user.add') self.assertIn('user.add:', data) data = self.run_salt('\'*\' sys.doc -d user.add') self.assertIn('user.add:', data) + data = self.run_salt('\'*\' sys.doc user.add') + self.assertIn('user.add:', data) + if __name__ == "__main__": From 755201a93b438077751ed2d5eab82fe13b7cb6c8 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 9 Nov 2012 11:50:12 +0000 Subject: [PATCH 5/6] Sort `salt-run` documentation output. PEP-8 Fixes. --- salt/cli/__init__.py | 10 ++++++++-- salt/runner.py | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/salt/cli/__init__.py b/salt/cli/__init__.py index 5a1ca05941..91929a4769 100644 --- a/salt/cli/__init__.py +++ b/salt/cli/__init__.py @@ -18,7 +18,11 @@ import salt.key from salt.utils import parsers from salt.utils.verify import verify_env -from salt.exceptions import SaltInvocationError, SaltClientError, EauthAuthenticationError +from salt.exceptions import ( + SaltInvocationError, + SaltClientError, + EauthAuthenticationError +) class SaltCMD(parsers.SaltCMDOptionParser): @@ -33,7 +37,9 @@ class SaltCMD(parsers.SaltCMDOptionParser): self.parse_args() try: - local = salt.client.LocalClient(self.get_config_file_path('master')) + local = salt.client.LocalClient( + self.get_config_file_path('master') + ) except SaltClientError as exc: self.exit(2, '{0}\n'.format(exc)) return diff --git a/salt/runner.py b/salt/runner.py index 56328ab282..1ba3065087 100644 --- a/salt/runner.py +++ b/salt/runner.py @@ -64,8 +64,8 @@ class Runner(RunnerClient): ''' ret = super(Runner, self).get_docs() - for fun, doc in ret.items(): - print("{0}:\n{1}\n".format(fun, doc)) + for fun in sorted(ret): + print("{0}:\n{1}\n".format(fun, ret[fun])) def run(self): ''' From d3e0d6406872c4c1b9dea7011f844a0a9ec44224 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 9 Nov 2012 15:25:16 +0000 Subject: [PATCH 6/6] Show a nice clock counter while we wait for the sync job to finish. --- tests/integration/__init__.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py index 092e0efee7..3a786121c3 100644 --- a/tests/integration/__init__.py +++ b/tests/integration/__init__.py @@ -11,6 +11,7 @@ import shutil import subprocess import tempfile import time +from datetime import timedelta try: import pwd except ImportError: @@ -326,6 +327,33 @@ class TestDaemon(object): if os.path.isdir(TMP): shutil.rmtree(TMP) + def wait_for_jid(self, targets, jid, timeout=120): + while timeout > 0: + running = self.__client_job_running(targets, jid) + sys.stdout.write('\r' + ' ' * PNUM + '\r') + if not running: + return True + sys.stdout.write( + ' * [Quit in {0}] Waiting for {1}'.format( + timedelta(seconds=timeout), ', '.join(running) + ) + ) + sys.stdout.flush() + timeout -= 1 + time.sleep(1) + else: + sys.stdout.write('\n * ERROR: Failed to get information back\n') + sys.stdout.flush() + return False + + def __client_job_running(self, targets, jid): + running = self.client.cmd( + ','.join(targets), 'saltutil.running', expr_form='list' + ) + return [ + k for (k, v) in running.iteritems() if v and v[0]['jid'] == jid + ] + def __wait_for_minions_connections(self, evt, targets): print_header( 'Waiting at most {0} secs for local minions to connect ' @@ -362,6 +390,10 @@ class TestDaemon(object): timeout=9999999999999999, ) + if self.wait_for_jid(targets, jid_info['jid']) is False: + evt.set() + return + while syncing: rdata = self.client.get_returns(jid_info['jid'], syncing, 1) if rdata: