From 9c88f9109276ae0ec11144c6c2fe25d8562fa6f1 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Sat, 13 Jun 2015 17:08:32 -0700 Subject: [PATCH 1/4] Yum API is faster than calling out to repoquery. Looking through the commit logs it looks like we weren't previously doing that because of commit 14479e6adca38e2691e7184bf9f1f713ef265ec7 The message there is that Yum API prints an error message if the rhn-plugin is in use and no rhn-certificate is available. So instead of using repoquery in preference always here we use repoquery in preference if the rhn-plugin is enabled. --- packaging/os/yum.py | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/packaging/os/yum.py b/packaging/os/yum.py index 9490a15b15d..61fc9f53333 100644 --- a/packaging/os/yum.py +++ b/packaging/os/yum.py @@ -152,12 +152,6 @@ EXAMPLES = ''' def_qf = "%{name}-%{version}-%{release}.%{arch}" -repoquery='/usr/bin/repoquery' -if not os.path.exists(repoquery): - repoquery = None - -yumbin='/usr/bin/yum' - def log(msg): syslog.openlog('ansible-yum', 0, syslog.LOG_USER) syslog.syslog(syslog.LOG_NOTICE, msg) @@ -177,10 +171,6 @@ def install_yum_utils(module): yum_path = module.get_bin_path('yum') if yum_path: rc, so, se = module.run_command('%s -y install yum-utils' % yum_path) - if rc == 0: - this_path = module.get_bin_path('repoquery') - global repoquery - repoquery = this_path def po_to_nevra(po): @@ -465,10 +455,10 @@ def repolist(module, repoq, qf="%{repoid}"): ret = set([ p for p in out.split('\n') if p.strip() ]) return ret -def list_stuff(module, conf_file, stuff): +def list_stuff(module, repoquerybin, conf_file, stuff): qf = "%{name}|%{epoch}|%{version}|%{release}|%{arch}|%{repoid}" - repoq = [repoquery, '--show-duplicates', '--plugins', '--quiet'] + repoq = [repoquerybin, '--show-duplicates', '--plugins', '--quiet'] if conf_file and os.path.exists(conf_file): repoq += ['-c', conf_file] @@ -779,13 +769,24 @@ def latest(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos): def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo, disable_gpg_check, exclude): + yumbin = module.get_bin_path('yum') # need debug level 2 to get 'Nothing to do' for groupinstall. yum_basecmd = [yumbin, '-d', '2', '-y'] - if not repoquery: - repoq = None - else: - repoq = [repoquery, '--show-duplicates', '--plugins', '--quiet'] + # If rhn-plugin is installed and no rhn-certificate is available on the + # system then users will see an error message using the yum API. Use + # repoquery in those cases. + + my = yum_base(conf_file) + # A sideeffect of accessing conf is that the configuration is + # loaded and plugins are discovered + my.conf + + repoq = None + if 'rhnplugin' in my.plugins._plugins: + repoquery = module.get_bin_path('repoquery', required=False) + if repoquery: + repoq = [repoquery, '--show-duplicates', '--plugins', '--quiet'] if conf_file and os.path.exists(conf_file): yum_basecmd += ['-c', conf_file] @@ -806,7 +807,7 @@ def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo, if exclude: e_cmd = ['--exclude=%s' % exclude] yum_basecmd.extend(e_cmd) - + if state in ['installed', 'present', 'latest']: if module.params.get('update_cache'): @@ -882,10 +883,11 @@ def main(): if params['install_repoquery'] and not repoquery and not module.check_mode: install_yum_utils(module) + repoquerybin = module.get_bin_path('repoquery', required=False) if params['list']: - if not repoquery: + if not repoquerybin: module.fail_json(msg="repoquery is required to use list= with this module. Please install the yum-utils package.") - results = dict(results=list_stuff(module, params['conf_file'], params['list'])) + results = dict(results=list_stuff(module, repoquerybin, params['conf_file'], params['list'])) module.exit_json(**results) else: From ef7a75938a657792ba67d74e686371251b2709ad Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Mon, 15 Jun 2015 09:51:15 -0700 Subject: [PATCH 2/4] Further optimizations pointed out by @kustodian in #1516 * Only install yum-utils if needed (b/c we're going to use repoquery) * Add a warning message explaining that why slower repoquery was used rather than yum API. --- packaging/os/yum.py | 86 ++++++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 41 deletions(-) diff --git a/packaging/os/yum.py b/packaging/os/yum.py index 61fc9f53333..ece8b2407bd 100644 --- a/packaging/os/yum.py +++ b/packaging/os/yum.py @@ -165,12 +165,17 @@ def yum_base(conf_file=None): my.preconf.fn = conf_file return my -def install_yum_utils(module): +def ensure_yum_utils(module): - if not module.check_mode: + repoquerybin = module.get_bin_path('repoquery', required=False) + + if module.params['install_repoquery'] and not repoquerybin and not module.check_mode: yum_path = module.get_bin_path('yum') if yum_path: - rc, so, se = module.run_command('%s -y install yum-utils' % yum_path) + rc, so, se = module.run_command('%s -y install yum-utils' % yum_path) + repoquerybin = module.get_bin_path('repoquery', required=False) + + return repoquerybin def po_to_nevra(po): @@ -313,7 +318,7 @@ def is_update(module, repoq, pkgspec, conf_file, qf=def_qf, en_repos=None, dis_r else: module.fail_json(msg='Error from repoquery: %s: %s' % (cmd, err)) - return [] + return set() def what_provides(module, repoq, req_spec, conf_file, qf=def_qf, en_repos=None, dis_repos=None): if en_repos is None: @@ -365,7 +370,7 @@ def what_provides(module, repoq, req_spec, conf_file, qf=def_qf, en_repos=None, else: module.fail_json(msg='Error from repoquery: %s: %s' % (cmd, err + err2)) - return [] + return set() def transaction_exists(pkglist): """ @@ -626,8 +631,8 @@ def install(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos): shutil.rmtree(tempdir) except Exception, e: module.fail_json(msg="Failure deleting temp directory %s, %s" % (tempdir, e)) - - module.exit_json(**res) + + return res def remove(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos): @@ -681,8 +686,8 @@ def remove(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos): if rc != 0: module.fail_json(**res) - - module.exit_json(**res) + + return res def latest(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos): @@ -764,30 +769,15 @@ def latest(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos): else: res['changed'] = True - module.exit_json(**res) + return res def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo, - disable_gpg_check, exclude): + disable_gpg_check, exclude, repoq): yumbin = module.get_bin_path('yum') # need debug level 2 to get 'Nothing to do' for groupinstall. yum_basecmd = [yumbin, '-d', '2', '-y'] - # If rhn-plugin is installed and no rhn-certificate is available on the - # system then users will see an error message using the yum API. Use - # repoquery in those cases. - - my = yum_base(conf_file) - # A sideeffect of accessing conf is that the configuration is - # loaded and plugins are discovered - my.conf - - repoq = None - if 'rhnplugin' in my.plugins._plugins: - repoquery = module.get_bin_path('repoquery', required=False) - if repoquery: - repoq = [repoquery, '--show-duplicates', '--plugins', '--quiet'] - if conf_file and os.path.exists(conf_file): yum_basecmd += ['-c', conf_file] if repoq: @@ -834,16 +824,19 @@ def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo, if state in ['installed', 'present']: if disable_gpg_check: yum_basecmd.append('--nogpgcheck') - install(module, pkgs, repoq, yum_basecmd, conf_file, en_repos, dis_repos) + res = install(module, pkgs, repoq, yum_basecmd, conf_file, en_repos, dis_repos) elif state in ['removed', 'absent']: - remove(module, pkgs, repoq, yum_basecmd, conf_file, en_repos, dis_repos) + res = remove(module, pkgs, repoq, yum_basecmd, conf_file, en_repos, dis_repos) elif state == 'latest': if disable_gpg_check: yum_basecmd.append('--nogpgcheck') - latest(module, pkgs, repoq, yum_basecmd, conf_file, en_repos, dis_repos) + res = latest(module, pkgs, repoq, yum_basecmd, conf_file, en_repos, dis_repos) + else: + # should be caught by AnsibleModule argument_spec + module.fail_json(msg="we should never get here unless this all + failed", changed=False, results='', errors='unepected state') - # should be caught by AnsibleModule argument_spec - return dict(changed=False, failed=True, results='', errors='unexpected state') + return res def main(): @@ -878,28 +871,39 @@ def main(): supports_check_mode = True ) - # this should not be needed, but exists as a failsafe - params = module.params - if params['install_repoquery'] and not repoquery and not module.check_mode: - install_yum_utils(module) - - repoquerybin = module.get_bin_path('repoquery', required=False) if params['list']: + repoquerybin = ensure_yum_utils(module) if not repoquerybin: module.fail_json(msg="repoquery is required to use list= with this module. Please install the yum-utils package.") results = dict(results=list_stuff(module, repoquerybin, params['conf_file'], params['list'])) - module.exit_json(**results) else: + # If rhn-plugin is installed and no rhn-certificate is available on + # the system then users will see an error message using the yum API. + # Use repoquery in those cases. + + my = yum_base(conf_file) + # A sideeffect of accessing conf is that the configuration is + # loaded and plugins are discovered + my.conf + repoquery = None + if 'rhnplugin' in my.plugins._plugins: + repoquerybin = ensure_yum_utils(module) + if repoquerybin: + repoquery = [repoquerybin, '--show-duplicates', '--plugins', '--quiet'] + pkg = [ p.strip() for p in params['name']] exclude = params['exclude'] state = params['state'] enablerepo = params.get('enablerepo', '') disablerepo = params.get('disablerepo', '') disable_gpg_check = params['disable_gpg_check'] - res = ensure(module, state, pkg, params['conf_file'], enablerepo, - disablerepo, disable_gpg_check, exclude) - module.fail_json(msg="we should never get here unless this all failed", **res) + results = ensure(module, state, pkg, params['conf_file'], enablerepo, + disablerepo, disable_gpg_check, exclude, repoquery) + if repoquery: + results['msg'] = '%s %s' % (results.get('msg',''), 'Warning: Due to potential bad behaviour with rhnplugin and certificates, used slower repoquery calls instead of Yum API.') + + module.exit_json(**results) # import module snippets from ansible.module_utils.basic import * From 08c17814fb48be494e09909fd866d6bec3d61ff8 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Mon, 15 Jun 2015 10:46:59 -0700 Subject: [PATCH 3/4] Fix incorrect line breaking --- packaging/os/yum.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packaging/os/yum.py b/packaging/os/yum.py index ece8b2407bd..6e2b61a189d 100644 --- a/packaging/os/yum.py +++ b/packaging/os/yum.py @@ -833,8 +833,8 @@ def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo, res = latest(module, pkgs, repoq, yum_basecmd, conf_file, en_repos, dis_repos) else: # should be caught by AnsibleModule argument_spec - module.fail_json(msg="we should never get here unless this all - failed", changed=False, results='', errors='unepected state') + module.fail_json(msg="we should never get here unless this all" + " failed", changed=False, results='', errors='unepected state') return res From 7c6c5180037c15d23cf838decc708b55c5d7ddfa Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 16 Jun 2015 06:28:46 -0700 Subject: [PATCH 4/4] Fix bugs found by @kustodian --- packaging/os/yum.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packaging/os/yum.py b/packaging/os/yum.py index 6e2b61a189d..624ae298b18 100644 --- a/packaging/os/yum.py +++ b/packaging/os/yum.py @@ -871,6 +871,8 @@ def main(): supports_check_mode = True ) + params = module.params + if params['list']: repoquerybin = ensure_yum_utils(module) if not repoquerybin: @@ -882,7 +884,7 @@ def main(): # the system then users will see an error message using the yum API. # Use repoquery in those cases. - my = yum_base(conf_file) + my = yum_base(params['conf_file']) # A sideeffect of accessing conf is that the configuration is # loaded and plugins are discovered my.conf