diff --git a/changelogs/fragments/sysctl_fact_fix.yml b/changelogs/fragments/sysctl_fact_fix.yml new file mode 100644 index 00000000000..18933402158 --- /dev/null +++ b/changelogs/fragments/sysctl_fact_fix.yml @@ -0,0 +1,2 @@ +bugfixes: + - setup/gather_facts will now warn and skip missing ``sysctl`` instead of being a fatal error. diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 3d3b7ed8edd..83cbf9e5861 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1348,14 +1348,16 @@ class AnsibleModule(object): # and we don't want to break modules unnecessarily return None - def get_bin_path(self, arg, required=False, opt_dirs=None): + def get_bin_path(self, arg, required=False, opt_dirs=None, warning=None): ''' Find system executable in PATH. :param arg: The executable to find. - :param required: if executable is not found and required is ``True``, fail_json + :param required: if the executable is not found and required is ``True``, fail_json :param opt_dirs: optional list of directories to search in addition to ``PATH`` - :returns: if found return full path; otherwise return None + :param warning: optional message when arg not found, only works when required=False + :returns: if found return full path; otherwise return original arg, unless 'warning' then return None + :raises: Sysexit: if arg is not found and required=True (via fail_json) ''' bin_path = None @@ -1364,8 +1366,8 @@ class AnsibleModule(object): except ValueError as e: if required: self.fail_json(msg=to_text(e)) - else: - return bin_path + elif warning is not None: + self.module.warn("Unable to find %s, %s" % (arg, warning)) return bin_path diff --git a/lib/ansible/module_utils/common/process.py b/lib/ansible/module_utils/common/process.py index 8e62c5f5d6e..85ffd2195e7 100644 --- a/lib/ansible/module_utils/common/process.py +++ b/lib/ansible/module_utils/common/process.py @@ -12,13 +12,18 @@ from ansible.module_utils.common.warnings import deprecate def get_bin_path(arg, opt_dirs=None, required=None): ''' Find system executable in PATH. Raises ValueError if the executable is not found. - Optional arguments: - - required: [Deprecated] Before 2.10, if executable is not found and required is true it raises an Exception. - In 2.10 and later, an Exception is always raised. This parameter will be removed in 2.21. - - opt_dirs: optional list of directories to search in addition to PATH + + :param arg: the executable to find + :type arg: string + :param opt_dirs: optional list of directories to search in addition to PATH + :type opt_dirs: list of strings + :param required: DEPRECATED. This parameter will be removed in 2.21 + :type required: boolean + :returns: path to arg (should be abs path unless PATH or opt_dirs are relative paths) + :raises: ValueError: if arg is not found + In addition to PATH and opt_dirs, this function also looks through /sbin, /usr/sbin and /usr/local/sbin. A lot of modules, especially for gathering facts, depend on this behaviour. - If found return full path, otherwise raise ValueError. ''' if required is not None: deprecate( @@ -27,26 +32,34 @@ def get_bin_path(arg, opt_dirs=None, required=None): collection_name="ansible.builtin", ) + paths = [] + sbin_paths = ['/sbin', '/usr/sbin', '/usr/local/sbin'] opt_dirs = [] if opt_dirs is None else opt_dirs - sbin_paths = ['/sbin', '/usr/sbin', '/usr/local/sbin'] - paths = [] + # Construct possible paths with precedence + # passed in paths for d in opt_dirs: if d is not None and os.path.exists(d): paths.append(d) + # system configured paths paths += os.environ.get('PATH', '').split(os.pathsep) - bin_path = None - # mangle PATH to include /sbin dirs + + # existing /sbin dirs, if not there already for p in sbin_paths: if p not in paths and os.path.exists(p): paths.append(p) + + # Search for binary + bin_path = None for d in paths: if not d: continue path = os.path.join(d, arg) if os.path.exists(path) and not os.path.isdir(path) and is_executable(path): + # fist found wins bin_path = path break + if bin_path is None: raise ValueError('Failed to find required executable "%s" in paths: %s' % (arg, os.pathsep.join(paths))) diff --git a/lib/ansible/module_utils/facts/hardware/aix.py b/lib/ansible/module_utils/facts/hardware/aix.py index db34fe147a6..6afd36d86f1 100644 --- a/lib/ansible/module_utils/facts/hardware/aix.py +++ b/lib/ansible/module_utils/facts/hardware/aix.py @@ -129,7 +129,7 @@ class AIXHardware(Hardware): rc, out, err = self.module.run_command("/usr/sbin/lsattr -El sys0 -a fwversion") data = out.split() dmi_facts['firmware_version'] = data[1].strip('IBM,') - lsconf_path = self.module.get_bin_path("lsconf") + lsconf_path = self.module.get_bin_path("lsconf", warning="dmi facts skipped") if lsconf_path: rc, out, err = self.module.run_command(lsconf_path) if rc == 0 and out: @@ -160,8 +160,9 @@ class AIXHardware(Hardware): """ vgs_facts = {} - lsvg_path = self.module.get_bin_path("lsvg") - xargs_path = self.module.get_bin_path("xargs") + warn = "vgs facts skipped" + lsvg_path = self.module.get_bin_path("lsvg", warning=warn) + xargs_path = self.module.get_bin_path("xargs", warning=warn) cmd = "%s -o | %s %s -p" % (lsvg_path, xargs_path, lsvg_path) if lsvg_path and xargs_path: rc, out, err = self.module.run_command(cmd, use_unsafe_shell=True) @@ -194,35 +195,36 @@ class AIXHardware(Hardware): # AIX does not have mtab but mount command is only source of info (or to use # api calls to get same info) - mount_path = self.module.get_bin_path('mount') - rc, mount_out, err = self.module.run_command(mount_path) - if mount_out: - for line in mount_out.split('\n'): - fields = line.split() - if len(fields) != 0 and fields[0] != 'node' and fields[0][0] != '-' and re.match('^/.*|^[a-zA-Z].*|^[0-9].*', fields[0]): - if re.match('^/', fields[0]): - # normal mount - mount = fields[1] - mount_info = {'mount': mount, - 'device': fields[0], - 'fstype': fields[2], - 'options': fields[6], - 'time': '%s %s %s' % (fields[3], fields[4], fields[5])} - mount_info.update(get_mount_size(mount)) - else: - # nfs or cifs based mount - # in case of nfs if no mount options are provided on command line - # add into fields empty string... - if len(fields) < 8: - fields.append("") - - mount_info = {'mount': fields[2], - 'device': '%s:%s' % (fields[0], fields[1]), - 'fstype': fields[3], - 'options': fields[7], - 'time': '%s %s %s' % (fields[4], fields[5], fields[6])} - - mounts.append(mount_info) + mount_path = self.module.get_bin_path('mount', warning="skipping mount facts") + if mount_path: + rc, mount_out, err = self.module.run_command(mount_path) + if mount_out: + for line in mount_out.split('\n'): + fields = line.split() + if len(fields) != 0 and fields[0] != 'node' and fields[0][0] != '-' and re.match('^/.*|^[a-zA-Z].*|^[0-9].*', fields[0]): + if re.match('^/', fields[0]): + # normal mount + mount = fields[1] + mount_info = {'mount': mount, + 'device': fields[0], + 'fstype': fields[2], + 'options': fields[6], + 'time': '%s %s %s' % (fields[3], fields[4], fields[5])} + mount_info.update(get_mount_size(mount)) + else: + # nfs or cifs based mount + # in case of nfs if no mount options are provided on command line + # add into fields empty string... + if len(fields) < 8: + fields.append("") + + mount_info = {'mount': fields[2], + 'device': '%s:%s' % (fields[0], fields[1]), + 'fstype': fields[3], + 'options': fields[7], + 'time': '%s %s %s' % (fields[4], fields[5], fields[6])} + + mounts.append(mount_info) mount_facts['mounts'] = mounts @@ -232,30 +234,32 @@ class AIXHardware(Hardware): device_facts = {} device_facts['devices'] = {} - lsdev_cmd = self.module.get_bin_path('lsdev', True) - lsattr_cmd = self.module.get_bin_path('lsattr', True) - rc, out_lsdev, err = self.module.run_command(lsdev_cmd) - - for line in out_lsdev.splitlines(): - field = line.split() - - device_attrs = {} - device_name = field[0] - device_state = field[1] - device_type = field[2:] - lsattr_cmd_args = [lsattr_cmd, '-E', '-l', device_name] - rc, out_lsattr, err = self.module.run_command(lsattr_cmd_args) - for attr in out_lsattr.splitlines(): - attr_fields = attr.split() - attr_name = attr_fields[0] - attr_parameter = attr_fields[1] - device_attrs[attr_name] = attr_parameter - - device_facts['devices'][device_name] = { - 'state': device_state, - 'type': ' '.join(device_type), - 'attributes': device_attrs - } + warn = 'device facts are skipped' + lsdev_cmd = self.module.get_bin_path('lsdev', warning=warn) + lsattr_cmd = self.module.get_bin_path('lsattr', warning=warn) + if lsdev_cmd and lsattr_cmd: + rc, out_lsdev, err = self.module.run_command(lsdev_cmd) + + for line in out_lsdev.splitlines(): + field = line.split() + + device_attrs = {} + device_name = field[0] + device_state = field[1] + device_type = field[2:] + lsattr_cmd_args = [lsattr_cmd, '-E', '-l', device_name] + rc, out_lsattr, err = self.module.run_command(lsattr_cmd_args) + for attr in out_lsattr.splitlines(): + attr_fields = attr.split() + attr_name = attr_fields[0] + attr_parameter = attr_fields[1] + device_attrs[attr_name] = attr_parameter + + device_facts['devices'][device_name] = { + 'state': device_state, + 'type': ' '.join(device_type), + 'attributes': device_attrs + } return device_facts diff --git a/lib/ansible/module_utils/facts/hardware/darwin.py b/lib/ansible/module_utils/facts/hardware/darwin.py index b35b8b1658e..fc3c877805e 100644 --- a/lib/ansible/module_utils/facts/hardware/darwin.py +++ b/lib/ansible/module_utils/facts/hardware/darwin.py @@ -94,43 +94,52 @@ class DarwinHardware(Hardware): total_used = 0 page_size = 4096 - vm_stat_command = self.module.get_bin_path('vm_stat') + + vm_stat_command = self.module.get_bin_path( + 'vm_stat', + warning='falling back to sysctl for memtotal_mb, default to 0 for memfree_mb' + ) if vm_stat_command is None: return memory_facts - rc, out, err = self.module.run_command(vm_stat_command) - if rc == 0: - # Free = Total - (Wired + active + inactive) - # Get a generator of tuples from the command output so we can later - # turn it into a dictionary - memory_stats = (line.rstrip('.').split(':', 1) for line in out.splitlines()) - - # Strip extra left spaces from the value - memory_stats = dict((k, v.lstrip()) for k, v in memory_stats) - - for k, v in memory_stats.items(): - try: - memory_stats[k] = int(v) - except ValueError: - # Most values convert cleanly to integer values but if the field does - # not convert to an integer, just leave it alone. - pass - - if memory_stats.get('Pages wired down'): - total_used += memory_stats['Pages wired down'] * page_size - if memory_stats.get('Pages active'): - total_used += memory_stats['Pages active'] * page_size - if memory_stats.get('Pages inactive'): - total_used += memory_stats['Pages inactive'] * page_size - - memory_facts['memfree_mb'] = memory_facts['memtotal_mb'] - (total_used // 1024 // 1024) + if vm_stat_command: + rc, out, err = self.module.run_command(vm_stat_command) + if rc == 0: + # Free = Total - (Wired + active + inactive) + # Get a generator of tuples from the command output so we can later + # turn it into a dictionary + memory_stats = (line.rstrip('.').split(':', 1) for line in out.splitlines()) + + # Strip extra left spaces from the value + memory_stats = dict((k, v.lstrip()) for k, v in memory_stats) + + for k, v in memory_stats.items(): + try: + memory_stats[k] = int(v) + except ValueError: + # Most values convert cleanly to integer values but if the field does + # not convert to an integer, just leave it alone. + pass + + if memory_stats.get('Pages wired down'): + total_used += memory_stats['Pages wired down'] * page_size + if memory_stats.get('Pages active'): + total_used += memory_stats['Pages active'] * page_size + if memory_stats.get('Pages inactive'): + total_used += memory_stats['Pages inactive'] * page_size + + memory_facts['memfree_mb'] = memory_facts['memtotal_mb'] - (total_used // 1024 // 1024) return memory_facts def get_uptime_facts(self): + # On Darwin, the default format is annoying to parse. # Use -b to get the raw value and decode it. - sysctl_cmd = self.module.get_bin_path('sysctl') + sysctl_cmd = self.module.get_bin_path('sysctl', warning='skipping uptime facts') + if not sysctl_cmd: + return {} + cmd = [sysctl_cmd, '-b', 'kern.boottime'] # We need to get raw bytes, not UTF-8. diff --git a/lib/ansible/module_utils/facts/hardware/freebsd.py b/lib/ansible/module_utils/facts/hardware/freebsd.py index d7a3800988f..c7f6c6c48b6 100644 --- a/lib/ansible/module_utils/facts/hardware/freebsd.py +++ b/lib/ansible/module_utils/facts/hardware/freebsd.py @@ -23,7 +23,6 @@ import time from ansible.module_utils.facts.hardware.base import Hardware, HardwareCollector from ansible.module_utils.facts.timeout import TimeoutError, timeout - from ansible.module_utils.facts.utils import get_file_content, get_mount_size diff --git a/lib/ansible/module_utils/facts/hardware/hpux.py b/lib/ansible/module_utils/facts/hardware/hpux.py index abb9dada663..efb63a98c2e 100644 --- a/lib/ansible/module_utils/facts/hardware/hpux.py +++ b/lib/ansible/module_utils/facts/hardware/hpux.py @@ -40,6 +40,9 @@ class HPUXHardware(Hardware): def populate(self, collected_facts=None): hardware_facts = {} + # TODO: very inefficient calls to machinfo, + # should just make one and then deal with finding the data (see facts/sysctl) + # but not going to change unless there is hp/ux for testing cpu_facts = self.get_cpu_facts(collected_facts=collected_facts) memory_facts = self.get_memory_facts() hw_facts = self.get_hw_facts() diff --git a/lib/ansible/module_utils/facts/hardware/netbsd.py b/lib/ansible/module_utils/facts/hardware/netbsd.py index 7d024198392..bcde75d3bcf 100644 --- a/lib/ansible/module_utils/facts/hardware/netbsd.py +++ b/lib/ansible/module_utils/facts/hardware/netbsd.py @@ -161,7 +161,13 @@ class NetBSDHardware(Hardware): def get_uptime_facts(self): # On NetBSD, we need to call sysctl with -n to get this value as an int. - sysctl_cmd = self.module.get_bin_path('sysctl') + sysctl_cmd = self.module.get_bin_path( + 'sysctl', + warning="skipping uptime facts" + ) + if sysctl_cmd is None: + return {} + cmd = [sysctl_cmd, '-n', 'kern.boottime'] rc, out, err = self.module.run_command(cmd) diff --git a/lib/ansible/module_utils/facts/hardware/openbsd.py b/lib/ansible/module_utils/facts/hardware/openbsd.py index 751ee6165dd..909ff444dee 100644 --- a/lib/ansible/module_utils/facts/hardware/openbsd.py +++ b/lib/ansible/module_utils/facts/hardware/openbsd.py @@ -112,7 +112,13 @@ class OpenBSDHardware(Hardware): def get_uptime_facts(self): # On openbsd, we need to call it with -n to get this value as an int. - sysctl_cmd = self.module.get_bin_path('sysctl') + sysctl_cmd = self.module.get_bin_path( + 'sysctl', + warning="skipping uptime facts" + ) + if sysctl_cmd is None: + return {} + cmd = [sysctl_cmd, '-n', 'kern.boottime'] rc, out, err = self.module.run_command(cmd) diff --git a/lib/ansible/module_utils/facts/hardware/sunos.py b/lib/ansible/module_utils/facts/hardware/sunos.py index 62eeafc3c98..d67dc4dc2ff 100644 --- a/lib/ansible/module_utils/facts/hardware/sunos.py +++ b/lib/ansible/module_utils/facts/hardware/sunos.py @@ -172,7 +172,14 @@ class SunOSHardware(Hardware): rc, platform, err = self.module.run_command('/usr/bin/uname -i') platform_sbin = '/usr/platform/' + platform.rstrip() + '/sbin' - prtdiag_path = self.module.get_bin_path("prtdiag", opt_dirs=[platform_sbin]) + prtdiag_path = self.module.get_bin_path( + "prtdiag", + opt_dirs=[platform_sbin], + warning="skipping dmi facts" + ) + if prtdiag_path is None: + return dmi_facts + rc, out, err = self.module.run_command(prtdiag_path) # rc returns 1 if out: diff --git a/lib/ansible/module_utils/facts/network/aix.py b/lib/ansible/module_utils/facts/network/aix.py index 29a679d84b1..9c95c5e906a 100644 --- a/lib/ansible/module_utils/facts/network/aix.py +++ b/lib/ansible/module_utils/facts/network/aix.py @@ -31,21 +31,25 @@ class AIXNetwork(GenericBsdIfconfigNetwork): def get_default_interfaces(self, route_path): interface = dict(v4={}, v6={}) - netstat_path = self.module.get_bin_path('netstat') - - if netstat_path: - rc, out, err = self.module.run_command([netstat_path, '-nr']) - - lines = out.splitlines() - for line in lines: - words = line.split() - if len(words) > 1 and words[0] == 'default': - if '.' in words[1]: - interface['v4']['gateway'] = words[1] - interface['v4']['interface'] = words[5] - elif ':' in words[1]: - interface['v6']['gateway'] = words[1] - interface['v6']['interface'] = words[5] + netstat_path = self.module.get_bin_path( + 'netstat', + warning="skipping default interface facts" + ) + if netstat_path is None: + return interface['v4'], interface['v6'] + + rc, out, err = self.module.run_command([netstat_path, '-nr']) + + lines = out.splitlines() + for line in lines: + words = line.split() + if len(words) > 1 and words[0] == 'default': + if '.' in words[1]: + interface['v4']['gateway'] = words[1] + interface['v4']['interface'] = words[5] + elif ':' in words[1]: + interface['v6']['gateway'] = words[1] + interface['v6']['interface'] = words[5] return interface['v4'], interface['v6'] @@ -58,9 +62,7 @@ class AIXNetwork(GenericBsdIfconfigNetwork): all_ipv6_addresses=[], ) - uname_rc = None - uname_out = None - uname_err = None + uname_rc = uname_out = uname_err = None uname_path = self.module.get_bin_path('uname') if uname_path: uname_rc, uname_out, uname_err = self.module.run_command([uname_path, '-W']) diff --git a/lib/ansible/module_utils/facts/network/fc_wwn.py b/lib/ansible/module_utils/facts/network/fc_wwn.py index f53cc53927d..24ca26a26db 100644 --- a/lib/ansible/module_utils/facts/network/fc_wwn.py +++ b/lib/ansible/module_utils/facts/network/fc_wwn.py @@ -47,7 +47,10 @@ class FcWwnInitiatorFactCollector(BaseFactCollector): elif sys.platform.startswith('sunos'): # on solaris 10 or solaris 11 should use `fcinfo hba-port` # TBD (not implemented): on solaris 9 use `prtconf -pv` - cmd = module.get_bin_path('fcinfo') + cmd = module.get_bin_path( + 'fcinfo', + warning="skipping fibre wwn initiator facts" + ) if cmd: cmd = cmd + " hba-port" rc, fcinfo_out, err = module.run_command(cmd) @@ -59,8 +62,14 @@ class FcWwnInitiatorFactCollector(BaseFactCollector): data = line.split(' ') fc_facts['fibre_channel_wwn'].append(data[-1].rstrip()) elif sys.platform.startswith('aix'): - cmd = module.get_bin_path('lsdev') - lscfg_cmd = module.get_bin_path('lscfg') + cmd = module.get_bin_path( + 'lsdev', + warning="skipping fibre wwn initiator facts" + ) + lscfg_cmd = module.get_bin_path( + 'lscfg', + warning="skipping fibre wwn initiator facts" + ) if cmd and lscfg_cmd: # get list of available fibre-channel devices (fcs) cmd = cmd + " -Cc adapter -l fcs*" @@ -81,8 +90,15 @@ class FcWwnInitiatorFactCollector(BaseFactCollector): data = line.split('.') fc_facts['fibre_channel_wwn'].append(data[-1].rstrip()) elif sys.platform.startswith('hp-ux'): - cmd = module.get_bin_path('ioscan') - fcmsu_cmd = module.get_bin_path('fcmsutil', opt_dirs=['/opt/fcms/bin']) + cmd = module.get_bin_path( + 'ioscan', + warning="skipping fibre wwn initiator facts" + ) + fcmsu_cmd = module.get_bin_path( + 'fcmsutil', + opt_dirs=['/opt/fcms/bin'], + warning="skipping fibre wwn initiator facts" + ) # go ahead if we have both commands available if cmd and fcmsu_cmd: # ioscan / get list of available fibre-channel devices (fcd) diff --git a/lib/ansible/module_utils/facts/network/generic_bsd.py b/lib/ansible/module_utils/facts/network/generic_bsd.py index 54188638c60..d211b29d8a3 100644 --- a/lib/ansible/module_utils/facts/network/generic_bsd.py +++ b/lib/ansible/module_utils/facts/network/generic_bsd.py @@ -34,12 +34,18 @@ class GenericBsdIfconfigNetwork(Network): def populate(self, collected_facts=None): network_facts = {} - ifconfig_path = self.module.get_bin_path('ifconfig') + ifconfig_path = self.module.get_bin_path( + 'ifconfig', + warning="skipping network facts" + ) if ifconfig_path is None: return network_facts - route_path = self.module.get_bin_path('route') + route_path = self.module.get_bin_path( + 'route', + warning="skipping network facts" + ) if route_path is None: return network_facts diff --git a/lib/ansible/module_utils/facts/network/hpux.py b/lib/ansible/module_utils/facts/network/hpux.py index 61e1bdc644f..06cd8ed7353 100644 --- a/lib/ansible/module_utils/facts/network/hpux.py +++ b/lib/ansible/module_utils/facts/network/hpux.py @@ -29,7 +29,11 @@ class HPUXNetwork(Network): def populate(self, collected_facts=None): network_facts = {} - netstat_path = self.module.get_bin_path('netstat') + netstat_path = self.module.get_bin_path( + 'netstat', + opt_dirs=['/usr/bin'], + warning="skipping network facts" + ) if netstat_path is None: return network_facts @@ -46,7 +50,15 @@ class HPUXNetwork(Network): def get_default_interfaces(self): default_interfaces = {} - rc, out, err = self.module.run_command("/usr/bin/netstat -nr") + netstat_path = self.module.get_bin_path( + 'netstat', + opt_dirs=['/usr/bin'], + warning="skipping default interface facts" + ) + + if netstat_path is None: + return default_interfaces + rc, out, err = self.module.run_command("%s -nr" % netstat_path) lines = out.splitlines() for line in lines: words = line.split() @@ -59,7 +71,15 @@ class HPUXNetwork(Network): def get_interfaces_info(self): interfaces = {} - rc, out, err = self.module.run_command("/usr/bin/netstat -niw") + netstat_path = self.module.get_bin_path( + 'netstat', + opt_dirs=['/usr/bin'], + warning="skipping default interface info facts" + ) + + if netstat_path is None: + return interfaces + rc, out, err = self.module.run_command("%s -niw" % netstat_path) lines = out.splitlines() for line in lines: words = line.split() diff --git a/lib/ansible/module_utils/facts/network/hurd.py b/lib/ansible/module_utils/facts/network/hurd.py index 05f23e5f445..e3941587f67 100644 --- a/lib/ansible/module_utils/facts/network/hurd.py +++ b/lib/ansible/module_utils/facts/network/hurd.py @@ -63,7 +63,10 @@ class HurdPfinetNetwork(Network): def populate(self, collected_facts=None): network_facts = {} - fsysopts_path = self.module.get_bin_path('fsysopts') + fsysopts_path = self.module.get_bin_path( + 'fsysopts', + warning="skipping network facts" + ) if fsysopts_path is None: return network_facts diff --git a/lib/ansible/module_utils/facts/network/iscsi.py b/lib/ansible/module_utils/facts/network/iscsi.py index 8f7a61590fb..dddb19ee22c 100644 --- a/lib/ansible/module_utils/facts/network/iscsi.py +++ b/lib/ansible/module_utils/facts/network/iscsi.py @@ -21,7 +21,6 @@ import sys import ansible.module_utils.compat.typing as t -from ansible.module_utils.common.process import get_bin_path from ansible.module_utils.facts.utils import get_file_content from ansible.module_utils.facts.network.base import NetworkCollector @@ -80,9 +79,11 @@ class IscsiInitiatorNetworkCollector(NetworkCollector): iscsi_facts['iscsi_iqn'] = line.split('=', 1)[1] break elif sys.platform.startswith('aix'): - try: - cmd = get_bin_path('lsattr') - except ValueError: + cmd = module.get_bin_path( + 'lsattr', + warning="skipping iscsi initiator facts" + ) + if cmd is None: return iscsi_facts cmd += " -E -l iscsi0" @@ -92,10 +93,12 @@ class IscsiInitiatorNetworkCollector(NetworkCollector): iscsi_facts['iscsi_iqn'] = line.split()[1].rstrip() elif sys.platform.startswith('hp-ux'): - # try to find it in the default PATH and opt_dirs - try: - cmd = get_bin_path('iscsiutil', opt_dirs=['/opt/iscsi/bin']) - except ValueError: + cmd = module.get_bin_path( + 'iscsiutil', + opt_dirs=['/opt/iscsi/bin'], + warning="skipping iscsi initiator facts" + ) + if cmd is None: return iscsi_facts cmd += " -l" diff --git a/lib/ansible/module_utils/facts/other/facter.py b/lib/ansible/module_utils/facts/other/facter.py index ec1771ecfac..e83f7355a94 100644 --- a/lib/ansible/module_utils/facts/other/facter.py +++ b/lib/ansible/module_utils/facts/other/facter.py @@ -22,8 +22,16 @@ class FacterFactCollector(BaseFactCollector): namespace=namespace) def find_facter(self, module): - facter_path = module.get_bin_path('facter', opt_dirs=['/opt/puppetlabs/bin']) - cfacter_path = module.get_bin_path('cfacter', opt_dirs=['/opt/puppetlabs/bin']) + facter_path = module.get_bin_path( + 'facter', + opt_dirs=['/opt/puppetlabs/bin'], + warning="falling back to cfacter to gather facter facts" + ) + cfacter_path = module.get_bin_path( + 'cfacter', + opt_dirs=['/opt/puppetlabs/bin'], + warning="skipping facter facts" + ) # Prefer to use cfacter if available if cfacter_path is not None: @@ -73,7 +81,6 @@ class FacterFactCollector(BaseFactCollector): try: facter_dict = json.loads(facter_output) except Exception: - # FIXME: maybe raise a FactCollectorError with some info attrs? - pass + module.warn("Failed to parse facter facts") return facter_dict diff --git a/lib/ansible/module_utils/facts/other/ohai.py b/lib/ansible/module_utils/facts/other/ohai.py index 75968ef29f1..7aae1b7c84c 100644 --- a/lib/ansible/module_utils/facts/other/ohai.py +++ b/lib/ansible/module_utils/facts/other/ohai.py @@ -36,10 +36,12 @@ class OhaiFactCollector(BaseFactCollector): namespace=namespace) def find_ohai(self, module): - ohai_path = module.get_bin_path('ohai') - return ohai_path + return module.get_bin_path( + 'ohai', + warning="skipping ohai facts" + ) - def run_ohai(self, module, ohai_path,): + def run_ohai(self, module, ohai_path): rc, out, err = module.run_command(ohai_path) return rc, out, err @@ -67,7 +69,6 @@ class OhaiFactCollector(BaseFactCollector): try: ohai_facts = json.loads(ohai_output) except Exception: - # FIXME: useful error, logging, something... - pass + module.warn("Failed to gather ohai facts") return ohai_facts diff --git a/lib/ansible/module_utils/facts/sysctl.py b/lib/ansible/module_utils/facts/sysctl.py index 1f94091200b..6cd065ae604 100644 --- a/lib/ansible/module_utils/facts/sysctl.py +++ b/lib/ansible/module_utils/facts/sysctl.py @@ -21,41 +21,43 @@ from ansible.module_utils.common.text.converters import to_text def get_sysctl(module, prefixes): - sysctl_cmd = module.get_bin_path('sysctl') - cmd = [sysctl_cmd] - cmd.extend(prefixes) sysctl = dict() - - try: - rc, out, err = module.run_command(cmd) - except (IOError, OSError) as e: - module.warn('Unable to read sysctl: %s' % to_text(e)) - rc = 1 - - if rc == 0: - key = '' - value = '' - for line in out.splitlines(): - if not line.strip(): - continue - - if line.startswith(' '): - # handle multiline values, they will not have a starting key - # Add the newline back in so people can split on it to parse - # lines if they need to. - value += '\n' + line - continue + sysctl_cmd = module.get_bin_path('sysctl', warning='skipping sysctl based facts') + if sysctl_cmd is not None: + + cmd = [sysctl_cmd] + cmd.extend(prefixes) + + try: + rc, out, err = module.run_command(cmd) + except (IOError, OSError) as e: + module.warn('Unable to read sysctl: %s' % to_text(e)) + rc = 1 + + if rc == 0: + key = '' + value = '' + for line in out.splitlines(): + if not line.strip(): + continue + + if line.startswith(' '): + # handle multiline values, they will not have a starting key + # Add the newline back in so people can split on it to parse + # lines if they need to. + value += '\n' + line + continue + + if key: + sysctl[key] = value.strip() + + try: + (key, value) = re.split(r'\s?=\s?|: ', line, maxsplit=1) + except Exception as e: + module.warn('Unable to split sysctl line (%s): %s' % (to_text(line), to_text(e))) if key: sysctl[key] = value.strip() - try: - (key, value) = re.split(r'\s?=\s?|: ', line, maxsplit=1) - except Exception as e: - module.warn('Unable to split sysctl line (%s): %s' % (to_text(line), to_text(e))) - - if key: - sysctl[key] = value.strip() - return sysctl diff --git a/test/units/module_utils/facts/network/test_fc_wwn.py b/test/units/module_utils/facts/network/test_fc_wwn.py index 3f91654fec4..3f4bd9a96b8 100644 --- a/test/units/module_utils/facts/network/test_fc_wwn.py +++ b/test/units/module_utils/facts/network/test_fc_wwn.py @@ -89,34 +89,28 @@ FCMSUTIL_OUT = """ """ -def mock_get_bin_path(cmd, required=False, opt_dirs=None): - result = None - if cmd == 'lsdev': - result = '/usr/sbin/lsdev' - elif cmd == 'lscfg': - result = '/usr/sbin/lscfg' - elif cmd == 'fcinfo': - result = '/usr/sbin/fcinfo' - elif cmd == 'ioscan': - result = '/usr/bin/ioscan' - elif cmd == 'fcmsutil': - result = '/opt/fcms/bin/fcmsutil' - return result +def mock_get_bin_path(cmd, required=False, opt_dirs=None, warning=None): + cmds = { + 'lsdev': '/usr/sbin/lsdev', + 'lscfg': '/usr/sbin/lscfg', + 'fcinfo': '/usr/sbin/fcinfo', + 'ioscan': '/usr/bin/ioscan', + 'fcmsutil': '/opt/fcms/bin/fcmsutil', + } + return cmds.get(cmd, None) def mock_run_command(cmd): rc = 0 - if 'lsdev' in cmd: - result = LSDEV_OUTPUT - elif 'lscfg' in cmd: - result = LSCFG_OUTPUT - elif 'fcinfo' in cmd: - result = FCINFO_OUTPUT - elif 'ioscan' in cmd: - result = IOSCAN_OUT - elif 'fcmsutil' in cmd: - result = FCMSUTIL_OUT - else: + COMMANDS = { + '/usr/sbin/lsdev': LSDEV_OUTPUT, + '/usr/sbin/lscfg': LSCFG_OUTPUT, + '/usr/sbin/fcinfo': FCINFO_OUTPUT, + '/usr/bin/ioscan': IOSCAN_OUT, + '/opt/fcms/bin/fcmsutil': FCMSUTIL_OUT, + } + result = COMMANDS.get(cmd.split()[0]) + if result is None: rc = 1 result = 'Error' return (rc, result, '') diff --git a/test/units/module_utils/facts/network/test_generic_bsd.py b/test/units/module_utils/facts/network/test_generic_bsd.py index 4e4061dbb74..ee104c38e5e 100644 --- a/test/units/module_utils/facts/network/test_generic_bsd.py +++ b/test/units/module_utils/facts/network/test_generic_bsd.py @@ -22,12 +22,12 @@ import unittest from ansible.module_utils.facts.network import generic_bsd -def get_bin_path(command): - if command == 'ifconfig': - return 'fake/ifconfig' - elif command == 'route': - return 'fake/route' - return None +def get_bin_path(command, warning=None): + cmds = { + 'ifconfig': 'fake/ifconfig', + 'route': 'fake/route', + } + return cmds.get(command, None) netbsd_ifconfig_a_out_7_1 = r''' diff --git a/test/units/module_utils/facts/network/test_iscsi_get_initiator.py b/test/units/module_utils/facts/network/test_iscsi_get_initiator.py index 48f97b38fad..16ec7d95171 100644 --- a/test/units/module_utils/facts/network/test_iscsi_get_initiator.py +++ b/test/units/module_utils/facts/network/test_iscsi_get_initiator.py @@ -41,13 +41,13 @@ def test_get_iscsi_info(mocker): inst = iscsi.IscsiInitiatorNetworkCollector() mocker.patch('sys.platform', 'aix6') - mocker.patch('ansible.module_utils.facts.network.iscsi.get_bin_path', return_value='/usr/sbin/lsattr') + mocker.patch.object(module, 'get_bin_path', return_value='/usr/sbin/lsattr') mocker.patch.object(module, 'run_command', return_value=(0, LSATTR_OUTPUT, '')) aix_iscsi_expected = {"iscsi_iqn": "iqn.localhost.hostid.7f000002"} assert aix_iscsi_expected == inst.collect(module=module) mocker.patch('sys.platform', 'hp-ux') - mocker.patch('ansible.module_utils.facts.network.iscsi.get_bin_path', return_value='/opt/iscsi/bin/iscsiutil') + mocker.patch.object(module, 'get_bin_path', return_value='/opt/iscsi/bin/iscsiutil') mocker.patch.object(module, 'run_command', return_value=(0, ISCSIUTIL_OUTPUT, '')) hpux_iscsi_expected = {"iscsi_iqn": " iqn.2001-04.com.hp.stor:svcio"} assert hpux_iscsi_expected == inst.collect(module=module)