From 49692e98eb0111ced7470562d66e248832aca8ea Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Wed, 14 Sep 2016 13:26:09 -0700 Subject: [PATCH] Fix python 3 issues with apt* modules. (#4848) - Use range instead of xrange. - Use python3-apt package for python 3. - Eliminate unsupported for/else/raise usage. - Use list on dict.items when modifying dict. - Update requirements documentation. Also made non-intrustive style fixes (adding blank lines). --- lib/ansible/modules/packaging/os/apt.py | 41 +++++++++++++++---- lib/ansible/modules/packaging/os/apt_key.py | 9 +++- .../modules/packaging/os/apt_repository.py | 26 ++++++++---- 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/lib/ansible/modules/packaging/os/apt.py b/lib/ansible/modules/packaging/os/apt.py index 9dd54adfaf9..29b8fb8f84e 100644 --- a/lib/ansible/modules/packaging/os/apt.py +++ b/lib/ansible/modules/packaging/os/apt.py @@ -116,8 +116,10 @@ options: required: false default: false version_added: "2.1" - -requirements: [ python-apt, aptitude ] +requirements: + - python-apt (python 2) + - python3-apt (python 3) + - aptitude author: "Matthew Williams (@mgwilliams)" notes: - Three of the upgrade modes (C(full), C(safe) and its alias C(yes)) require C(aptitude), otherwise @@ -197,6 +199,7 @@ import os import datetime import fnmatch import itertools +import sys from ansible.module_utils._text import to_native @@ -226,6 +229,12 @@ try: except ImportError: HAS_PYTHON_APT = False +if sys.version_info[0] < 3: + PYTHON_APT = 'python-apt' +else: + PYTHON_APT = 'python3-apt' + + def package_split(pkgspec): parts = pkgspec.split('=', 1) if len(parts) > 1: @@ -233,6 +242,7 @@ def package_split(pkgspec): else: return parts[0], None + def package_versions(pkgname, pkg, pkg_cache): try: versions = set(p.version for p in pkg.versions) @@ -245,12 +255,14 @@ def package_versions(pkgname, pkg, pkg_cache): return versions + def package_version_compare(version, other_version): try: return apt_pkg.version_compare(version, other_version) except AttributeError: return apt_pkg.VersionCompare(version, other_version) + def package_status(m, pkgname, version, cache, state): try: # get the package from the cache, as well as the @@ -327,6 +339,7 @@ def package_status(m, pkgname, version, cache, state): return package_is_installed, package_is_upgradable, has_files + def expand_dpkg_options(dpkg_options_compressed): options_list = dpkg_options_compressed.split(',') dpkg_options = "" @@ -335,6 +348,7 @@ def expand_dpkg_options(dpkg_options_compressed): % (dpkg_options, dpkg_option) return dpkg_options.strip() + def expand_pkgspec_from_fnmatches(m, pkgspec, cache): # Note: apt-get does implicit regex matching when an exact package name # match is not found. Something like this: @@ -373,6 +387,7 @@ def expand_pkgspec_from_fnmatches(m, pkgspec, cache): new_pkgspec.append(pkgspec_pattern) return new_pkgspec + def parse_diff(output): diff = to_native(output).splitlines() try: @@ -394,6 +409,7 @@ def parse_diff(output): diff_end += 1 return {'prepared': '\n'.join(diff[diff_start:diff_end])} + def install(m, pkgspec, cache, upgrade=False, default_release=None, install_recommends=None, force=False, dpkg_options=expand_dpkg_options(DPKG_OPTIONS), @@ -471,6 +487,7 @@ def install(m, pkgspec, cache, upgrade=False, default_release=None, else: return (True, dict(changed=False)) + def get_field_of_deb(m, deb_file, field="Version"): cmd_dpkg = m.get_bin_path("dpkg", True) cmd = cmd_dpkg + " --field %s %s" % (deb_file, field) @@ -479,6 +496,7 @@ def get_field_of_deb(m, deb_file, field="Version"): m.fail_json(msg="%s failed" % cmd, stdout=stdout, stderr=stderr) return to_native(stdout).strip('\n') + def install_deb(m, debs, cache, force, install_recommends, allow_unauthenticated, dpkg_options): changed=False deps_to_install = [] @@ -553,6 +571,7 @@ def install_deb(m, debs, cache, force, install_recommends, allow_unauthenticated else: m.exit_json(changed=changed, stdout=retvals.get('stdout',''), stderr=retvals.get('stderr',''), diff=retvals.get('diff', '')) + def remove(m, pkgspec, cache, purge=False, force=False, dpkg_options=expand_dpkg_options(DPKG_OPTIONS), autoremove=False): pkg_list = [] @@ -598,6 +617,7 @@ def remove(m, pkgspec, cache, purge=False, force=False, m.fail_json(msg="'apt-get remove %s' failed: %s" % (packages, err), stdout=out, stderr=err) m.exit_json(changed=True, stdout=out, stderr=err, diff=diff) + def upgrade(m, mode="yes", force=False, default_release=None, dpkg_options=expand_dpkg_options(DPKG_OPTIONS)): if m.check_mode: @@ -648,6 +668,7 @@ def upgrade(m, mode="yes", force=False, default_release=None, m.exit_json(changed=False, msg=out, stdout=out, stderr=err) m.exit_json(changed=True, msg=out, stdout=out, stderr=err, diff=diff) + def download(module, deb): tempdir = os.path.dirname(__file__) package = os.path.join(tempdir, str(deb.rsplit('/', 1)[1])) @@ -674,6 +695,7 @@ def download(module, deb): return deb + def main(): module = AnsibleModule( argument_spec = dict( @@ -701,16 +723,18 @@ def main(): if not HAS_PYTHON_APT: if module.check_mode: - module.fail_json(msg="python-apt must be installed to use check mode. If run normally this module can autoinstall it") + module.fail_json(msg="%s must be installed to use check mode. " + "If run normally this module can auto-install it." % PYTHON_APT) try: - module.run_command('apt-get update', check_rc=True) - module.run_command('apt-get install python-apt -y -q', check_rc=True) + module.run_command(['apt-get', 'update'], check_rc=True) + module.run_command(['apt-get', 'install', PYTHON_APT, '-y', '-q'], check_rc=True) global apt, apt_pkg import apt import apt.debfile import apt_pkg except ImportError: - module.fail_json(msg="Could not import python modules: apt, apt_pkg. Please install python-apt package.") + module.fail_json(msg="Could not import python modules: apt, apt_pkg. " + "Please install %s package." % PYTHON_APT) global APTITUDE_CMD APTITUDE_CMD = module.get_bin_path("aptitude", False) @@ -772,15 +796,14 @@ def main(): updated_cache_time = int(time.mktime(mtimestamp.timetuple())) if cache_valid is not True: - for retry in xrange(3): + for retry in range(3): try: cache.update() break except apt.cache.FetchFailedException: pass else: - #out of retries, pass on the exception - raise + module.fail_json(msg='Failed to update apt cache.') cache.open(progress=None) updated_cache = True updated_cache_time = int(time.mktime(now.timetuple())) diff --git a/lib/ansible/modules/packaging/os/apt_key.py b/lib/ansible/modules/packaging/os/apt_key.py index c2993be1af7..129c467e7f7 100644 --- a/lib/ansible/modules/packaging/os/apt_key.py +++ b/lib/ansible/modules/packaging/os/apt_key.py @@ -123,6 +123,7 @@ def check_missing_binaries(module): if len(missing): module.fail_json(msg="binaries are missing", names=missing) + def all_keys(module, keyring, short_format): if keyring: cmd = "apt-key --keyring %s adv --list-public-keys --keyid-format=long" % keyring @@ -141,6 +142,7 @@ def all_keys(module, keyring, short_format): results = shorten_key_ids(results) return results + def shorten_key_ids(key_id_list): """ Takes a list of key ids, and converts them to the 'short' format, @@ -151,6 +153,7 @@ def shorten_key_ids(key_id_list): short.append(key[-8:]) return short + def download_key(module, url): # FIXME: move get_url code to common, allow for in-memory D/L, support proxies # and reuse here @@ -166,12 +169,13 @@ def download_key(module, url): except Exception: module.fail_json(msg="error getting key id from url: %s" % url, traceback=format_exc()) + def import_key(module, keyring, keyserver, key_id): if keyring: cmd = "apt-key --keyring %s adv --keyserver %s --recv %s" % (keyring, keyserver, key_id) else: cmd = "apt-key adv --keyserver %s --recv %s" % (keyserver, key_id) - for retry in xrange(5): + for retry in range(5): (rc, out, err) = module.run_command(cmd) if rc == 0: break @@ -181,6 +185,7 @@ def import_key(module, keyring, keyserver, key_id): rc=rc, stdout=out, stderr=err) return True + def add_key(module, keyfile, keyring, data=None): if data is not None: if keyring: @@ -196,6 +201,7 @@ def add_key(module, keyfile, keyring, data=None): (rc, out, err) = module.run_command(cmd, check_rc=True) return True + def remove_key(module, key_id, keyring): # FIXME: use module.run_command, fail at point of error and don't discard useful stdin/stdout if keyring: @@ -205,6 +211,7 @@ def remove_key(module, key_id, keyring): (rc, out, err) = module.run_command(cmd, check_rc=True) return True + def main(): module = AnsibleModule( argument_spec=dict( diff --git a/lib/ansible/modules/packaging/os/apt_repository.py b/lib/ansible/modules/packaging/os/apt_repository.py index cecd94ac322..eae6228034f 100644 --- a/lib/ansible/modules/packaging/os/apt_repository.py +++ b/lib/ansible/modules/packaging/os/apt_repository.py @@ -28,7 +28,7 @@ short_description: Add and remove APT repositories description: - Add or remove an APT repositories in Ubuntu and Debian. notes: - - This module works on Debian and Ubuntu and requires C(python-apt). + - This module works on Debian and Ubuntu. - This module supports Debian Squeeze (version 6) as well as its successors. - This module treats Debian and Ubuntu distributions separately. So PPA could be installed only on Ubuntu machines. options: @@ -72,7 +72,9 @@ options: required: false author: "Alexander Saltanov (@sashka)" version_added: "0.7" -requirements: [ python-apt ] +requirements: + - python-apt (python 2) + - python3-apt (python 3) ''' EXAMPLES = ''' @@ -96,6 +98,7 @@ apt_repository: repo='ppa:nginx/stable' import glob import os import re +import sys import tempfile try: @@ -108,10 +111,16 @@ except ImportError: distro = None HAVE_PYTHON_APT = False +if sys.version_info[0] < 3: + PYTHON_APT = 'python-apt' +else: + PYTHON_APT = 'python3-apt' + DEFAULT_SOURCES_PERM = int('0644', 8) VALID_SOURCE_TYPES = ('deb', 'deb-src') + def install_python_apt(module): if not module.check_mode: @@ -119,8 +128,8 @@ def install_python_apt(module): if apt_get_path: rc, so, se = module.run_command([apt_get_path, 'update']) if rc != 0: - module.fail_json(msg="Failed to auto-install python-apt. Error was: '%s'" % se.strip()) - rc, so, se = module.run_command([apt_get_path, 'install', 'python-apt', '-y', '-q']) + module.fail_json(msg="Failed to auto-install %s. Error was: '%s'" % (PYTHON_APT, se.strip())) + rc, so, se = module.run_command([apt_get_path, 'install', PYTHON_APT, '-y', '-q']) if rc == 0: global apt, apt_pkg, aptsources_distro, distro, HAVE_PYTHON_APT import apt @@ -129,9 +138,10 @@ def install_python_apt(module): distro = aptsources_distro.get_distro() HAVE_PYTHON_APT = True else: - module.fail_json(msg="Failed to auto-install python-apt. Error was: '%s'" % se.strip()) + module.fail_json(msg="Failed to auto-install %s. Error was: '%s'" % (PYTHON_APT, se.strip())) else: - module.fail_json(msg="python-apt must be installed to use check mode") + module.fail_json(msg="%s must be installed to use check mode" % PYTHON_APT) + class InvalidSource(Exception): pass @@ -255,7 +265,7 @@ class SourcesList(object): self.files[file] = group def save(self): - for filename, sources in self.files.items(): + for filename, sources in list(self.files.items()): if sources: d, fn = os.path.split(filename) fd, tmp_path = tempfile.mkstemp(prefix=".%s-" % fn, dir=d) @@ -475,7 +485,7 @@ def main(): if params['install_python_apt']: install_python_apt(module) else: - module.fail_json(msg='python-apt is not installed, and install_python_apt is False') + module.fail_json(msg='%s is not installed, and install_python_apt is False' % PYTHON_APT) if isinstance(distro, aptsources_distro.UbuntuDistribution): sourceslist = UbuntuSourcesList(module,