diff options
| -rw-r--r-- | changelogs/fragments/package_facts_warnings.yml | 3 | ||||
| -rw-r--r-- | lib/ansible/module_utils/facts/packages.py | 56 | ||||
| -rw-r--r-- | lib/ansible/modules/package_facts.py | 86 | ||||
| -rw-r--r-- | test/integration/targets/package_facts/aliases | 1 | ||||
| -rw-r--r-- | test/integration/targets/package_facts/files/apk | 3 | ||||
| -rwxr-xr-x | test/integration/targets/package_facts/runme.sh | 15 | ||||
| -rw-r--r-- | test/integration/targets/package_facts/runme.yml | 4 | ||||
| -rw-r--r-- | test/integration/targets/package_facts/test_warning_failed.yml | 26 | ||||
| -rw-r--r-- | test/integration/targets/package_facts/test_warning_unusable.yml | 12 |
9 files changed, 132 insertions, 74 deletions
diff --git a/changelogs/fragments/package_facts_warnings.yml b/changelogs/fragments/package_facts_warnings.yml new file mode 100644 index 0000000000..0edb03f052 --- /dev/null +++ b/changelogs/fragments/package_facts_warnings.yml @@ -0,0 +1,3 @@ +bugfixes: + - package_facts - returns the correct warning when package listing fails. + - package_facts - no longer fails silently when the selected package manager is unable to list packages. diff --git a/lib/ansible/module_utils/facts/packages.py b/lib/ansible/module_utils/facts/packages.py index 21be56fab2..b5b9bcb35e 100644 --- a/lib/ansible/module_utils/facts/packages.py +++ b/lib/ansible/module_utils/facts/packages.py @@ -3,24 +3,29 @@ from __future__ import annotations +import ansible.module_utils.compat.typing as t + from abc import ABCMeta, abstractmethod from ansible.module_utils.six import with_metaclass +from ansible.module_utils.basic import missing_required_lib from ansible.module_utils.common.process import get_bin_path +from ansible.module_utils.common.respawn import has_respawned, probe_interpreters_for_module, respawn_module from ansible.module_utils.common._utils import get_all_subclasses def get_all_pkg_managers(): - return {obj.__name__.lower(): obj for obj in get_all_subclasses(PkgMgr) if obj not in (CLIMgr, LibMgr)} + return {obj.__name__.lower(): obj for obj in get_all_subclasses(PkgMgr) if obj not in (CLIMgr, LibMgr, RespawningLibMgr)} class PkgMgr(with_metaclass(ABCMeta, object)): # type: ignore[misc] @abstractmethod - def is_available(self): + def is_available(self, handle_exceptions): # This method is supposed to return True/False if the package manager is currently installed/usable # It can also 'prep' the required systems in the process of detecting availability + # If handle_exceptions is false it should raise exceptions related to manager discovery instead of handling them. pass @abstractmethod @@ -58,16 +63,50 @@ class LibMgr(PkgMgr): self._lib = None super(LibMgr, self).__init__() - def is_available(self): + def is_available(self, handle_exceptions=True): found = False try: self._lib = __import__(self.LIB) found = True except ImportError: - pass + if not handle_exceptions: + raise Exception(missing_required_lib(self.LIB)) return found +class RespawningLibMgr(LibMgr): + + CLI_BINARIES = [] # type: t.List[str] + INTERPRETERS = ['/usr/bin/python3'] + + def is_available(self, handle_exceptions=True): + if super(RespawningLibMgr, self).is_available(): + return True + + for binary in self.CLI_BINARIES: + try: + bin_path = get_bin_path(binary) + except ValueError: + # Not an interesting exception to raise, just a speculative probe + continue + else: + # It looks like this package manager is installed + if not has_respawned(): + # See if respawning will help + interpreter_path = probe_interpreters_for_module(self.INTERPRETERS, self.LIB) + if interpreter_path: + respawn_module(interpreter_path) + # The module will exit when the respawned copy completes + + if not handle_exceptions: + raise Exception(f'Found executable at {bin_path}. {missing_required_lib(self.LIB)}') + + if not handle_exceptions: + raise Exception(missing_required_lib(self.LIB)) + + return False + + class CLIMgr(PkgMgr): CLI = None # type: str | None @@ -77,9 +116,12 @@ class CLIMgr(PkgMgr): self._cli = None super(CLIMgr, self).__init__() - def is_available(self): + def is_available(self, handle_exceptions=True): + found = False try: self._cli = get_bin_path(self.CLI) + found = True except ValueError: - return False - return True + if not handle_exceptions: + raise + return found diff --git a/lib/ansible/modules/package_facts.py b/lib/ansible/modules/package_facts.py index 820d292bea..bec6c34260 100644 --- a/lib/ansible/modules/package_facts.py +++ b/lib/ansible/modules/package_facts.py @@ -19,7 +19,7 @@ options: - The V(portage) and V(pkg) options were added in version 2.8. - The V(apk) option was added in version 2.11. - The V(pkg_info)' option was added in version 2.13. - - Aliases were added in 2.18, to support using C(auto={{ansible_facts['pkg_mgr']}}) + - Aliases were added in 2.18, to support using C(manager={{ansible_facts['pkg_mgr']}}) default: ['auto'] choices: auto: Depending on O(strategy), will match the first or all package managers provided, in order @@ -253,11 +253,9 @@ ansible_facts: import re from ansible.module_utils.common.text.converters import to_native, to_text -from ansible.module_utils.basic import AnsibleModule, missing_required_lib +from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.common.locale import get_best_parsable_locale -from ansible.module_utils.common.process import get_bin_path -from ansible.module_utils.common.respawn import has_respawned, probe_interpreters_for_module, respawn_module -from ansible.module_utils.facts.packages import LibMgr, CLIMgr, get_all_pkg_managers +from ansible.module_utils.facts.packages import CLIMgr, RespawningLibMgr, get_all_pkg_managers ALIASES = { @@ -267,9 +265,14 @@ ALIASES = { } -class RPM(LibMgr): +class RPM(RespawningLibMgr): LIB = 'rpm' + CLI_BINARIES = ['rpm'] + INTERPRETERS = [ + '/usr/libexec/platform-python', + '/usr/bin/python3', + ] def list_installed(self): return self._lib.TransactionSet().dbMatch() @@ -281,34 +284,11 @@ class RPM(LibMgr): epoch=package[self._lib.RPMTAG_EPOCH], arch=package[self._lib.RPMTAG_ARCH],) - def is_available(self): - ''' we expect the python bindings installed, but this gives warning if they are missing and we have rpm cli''' - we_have_lib = super(RPM, self).is_available() - try: - get_bin_path('rpm') - - if not we_have_lib and not has_respawned(): - # try to locate an interpreter with the necessary lib - interpreters = ['/usr/libexec/platform-python', - '/usr/bin/python3', - '/usr/bin/python2'] - interpreter_path = probe_interpreters_for_module(interpreters, self.LIB) - if interpreter_path: - respawn_module(interpreter_path) - # end of the line for this process; this module will exit when the respawned copy completes - - if not we_have_lib: - module.warn('Found "rpm" but %s' % (missing_required_lib(self.LIB))) - except ValueError: - pass - - return we_have_lib - - -class APT(LibMgr): +class APT(RespawningLibMgr): LIB = 'apt' + CLI_BINARIES = ['apt', 'apt-get', 'aptitude'] def __init__(self): self._cache = None @@ -322,30 +302,6 @@ class APT(LibMgr): self._cache = self._lib.Cache() return self._cache - def is_available(self): - ''' we expect the python bindings installed, but if there is apt/apt-get give warning about missing bindings''' - we_have_lib = super(APT, self).is_available() - if not we_have_lib: - for exe in ('apt', 'apt-get', 'aptitude'): - try: - get_bin_path(exe) - except ValueError: - continue - else: - if not has_respawned(): - # try to locate an interpreter with the necessary lib - interpreters = ['/usr/bin/python3', - '/usr/bin/python2'] - interpreter_path = probe_interpreters_for_module(interpreters, self.LIB) - if interpreter_path: - respawn_module(interpreter_path) - # end of the line for this process; this module will exit here when respawned copy completes - - module.warn('Found "%s" but %s' % (exe, missing_required_lib('apt'))) - break - - return we_have_lib - def list_installed(self): # Store the cache to avoid running pkg_cache() for each item in the comprehension, which is very slow cache = self.pkg_cache @@ -551,22 +507,18 @@ def main(): continue seen.add(pkgmgr) + + manager = PKG_MANAGERS[pkgmgr]() try: - try: - # manager throws exception on init (calls self.test) if not usable. - manager = PKG_MANAGERS[pkgmgr]() - if manager.is_available(): - found += 1 + if manager.is_available(handle_exceptions=False): + found += 1 + try: packages.update(manager.get_packages()) - - except Exception as e: - if pkgmgr in module.params['manager']: - module.warn('Requested package manager %s was not usable by this module: %s' % (pkgmgr, to_text(e))) - continue - + except Exception as e: + module.warn('Failed to retrieve packages with %s: %s' % (pkgmgr, to_text(e))) except Exception as e: if pkgmgr in module.params['manager']: - module.warn('Failed to retrieve packages with %s: %s' % (pkgmgr, to_text(e))) + module.warn('Requested package manager %s was not usable by this module: %s' % (pkgmgr, to_text(e))) if found == 0: msg = ('Could not detect a supported package manager from the following list: %s, ' diff --git a/test/integration/targets/package_facts/aliases b/test/integration/targets/package_facts/aliases index f5edf4b117..eedfe259b6 100644 --- a/test/integration/targets/package_facts/aliases +++ b/test/integration/targets/package_facts/aliases @@ -1,2 +1,3 @@ +destructive shippable/posix/group2 skip/macos diff --git a/test/integration/targets/package_facts/files/apk b/test/integration/targets/package_facts/files/apk new file mode 100644 index 0000000000..2bb8d868bd --- /dev/null +++ b/test/integration/targets/package_facts/files/apk @@ -0,0 +1,3 @@ +#!/bin/sh + +exit 1 diff --git a/test/integration/targets/package_facts/runme.sh b/test/integration/targets/package_facts/runme.sh new file mode 100755 index 0000000000..e1b21599ce --- /dev/null +++ b/test/integration/targets/package_facts/runme.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash + +set -eux + +ansible-playbook -i ../../inventory runme.yml -v "$@" + +ansible-playbook -i ../../inventory test_warning_unusable.yml -v "$@" 2>&1 | tee output.log +if ! grep -q "Conditional result was False" output.log; then + grep "Requested package manager apk was not usable by this module" output.log +fi + +ansible-playbook -i ../../inventory test_warning_failed.yml -v "$@" 2>&1 | tee output.log +if ! grep -q "Conditional result was False" output.log; then + grep "Failed to retrieve packages with apk: Unable to list packages" output.log +fi diff --git a/test/integration/targets/package_facts/runme.yml b/test/integration/targets/package_facts/runme.yml new file mode 100644 index 0000000000..4724d7639c --- /dev/null +++ b/test/integration/targets/package_facts/runme.yml @@ -0,0 +1,4 @@ +- hosts: all + gather_facts: true + roles: + - { role: ../package_facts } diff --git a/test/integration/targets/package_facts/test_warning_failed.yml b/test/integration/targets/package_facts/test_warning_failed.yml new file mode 100644 index 0000000000..1246bda206 --- /dev/null +++ b/test/integration/targets/package_facts/test_warning_failed.yml @@ -0,0 +1,26 @@ +- hosts: all + tasks: + - name: Check for apk + ansible.builtin.command: apk info + ignore_errors: true + register: apk_exists + + - when: apk_exists is failed + block: + - name: Create a mock apk + ansible.builtin.copy: + dest: /usr/bin/apk + src: apk + mode: "0755" + become: true + + - name: Elicit a warning about failing to list packages + ansible.builtin.package_facts: + manager: apk + failed_when: false + + - name: Remove the mock + ansible.builtin.file: + dest: /usr/bin/apk + state: absent + become: true diff --git a/test/integration/targets/package_facts/test_warning_unusable.yml b/test/integration/targets/package_facts/test_warning_unusable.yml new file mode 100644 index 0000000000..3379f98bd0 --- /dev/null +++ b/test/integration/targets/package_facts/test_warning_unusable.yml @@ -0,0 +1,12 @@ +- hosts: all + tasks: + - name: Check for apk + ansible.builtin.command: apk info + ignore_errors: true + register: apk_exists + + - name: Elicit a warning about the missing binary + ansible.builtin.package_facts: + manager: apk + when: apk_exists is failed + failed_when: false |