From 6e7315024421d9603ded041488cce103b1e0f2eb Mon Sep 17 00:00:00 2001 From: Anatoly Pugachev Date: Tue, 7 May 2019 18:26:20 +0200 Subject: [PATCH] refactor iscsi network facts module, remove external grep call, add unit test (#55643) * remove external grep call and parse with python * use function for repeated code * use module.get_bin_path() for iscsiutil on HPUX * some code opt for HPUX * clean up non-module code, module being defined is a requirement for this code * import get_bin_path() directly and use without module prefix * Add integration tests for AIX and HP-UX * add changelog fragment * Apply suggestions from code review Co-Authored-By: mator * Apply suggestions from code review #2 Co-Authored-By: Sam Doran * Remove strict requirement on executable to exist for get_bin_path() as it will allow facts gathering to continue without an error. Almost all other files under facts do not have "required=True" (except 2 files, which should be probably fixed). And check return value for get_bin_path() , before run attempt. * add check for AIX lsattr run_command return code --- .../fragments/55643-network-facts-iscsi.yaml | 2 + .../module_utils/facts/network/iscsi.py | 39 +++++++------- .../facts/network/test_iscsi_get_initiator.py | 54 +++++++++++++++++++ 3 files changed, 77 insertions(+), 18 deletions(-) create mode 100644 changelogs/fragments/55643-network-facts-iscsi.yaml create mode 100644 test/units/module_utils/facts/network/test_iscsi_get_initiator.py diff --git a/changelogs/fragments/55643-network-facts-iscsi.yaml b/changelogs/fragments/55643-network-facts-iscsi.yaml new file mode 100644 index 00000000000..64d50b5604e --- /dev/null +++ b/changelogs/fragments/55643-network-facts-iscsi.yaml @@ -0,0 +1,2 @@ +minor_changes: + - refactor iSCSI network facts for AIX and HP-UX, add unit test, remove external grep call diff --git a/lib/ansible/module_utils/facts/network/iscsi.py b/lib/ansible/module_utils/facts/network/iscsi.py index 3db94d2d7b3..98e62a2aa6d 100644 --- a/lib/ansible/module_utils/facts/network/iscsi.py +++ b/lib/ansible/module_utils/facts/network/iscsi.py @@ -21,6 +21,7 @@ __metaclass__ = type import sys import subprocess +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 @@ -79,24 +80,26 @@ class IscsiInitiatorNetworkCollector(NetworkCollector): iscsi_facts['iscsi_iqn'] = line.split('=', 1)[1] break elif sys.platform.startswith('aix'): - if module is not None: - rc, out, err = module.run_command('/usr/sbin/lsattr -E -l iscsi0 | grep initiator_name') - if out: - iscsi_facts['iscsi_iqn'] = out.split()[1].rstrip() - else: - aixcmd = '/usr/sbin/lsattr -E -l iscsi0 | grep initiator_name' - aixret = subprocess.check_output(aixcmd, shell=True) - if aixret[0].isalpha(): - iscsi_facts['iscsi_iqn'] = aixret.split()[1].rstrip() + cmd = get_bin_path('lsattr') + if cmd: + cmd += " -E -l iscsi0" + rc, out, err = module.run_command(cmd) + if rc == 0 and out: + line = self.findstr(out, 'initiator_name') + iscsi_facts['iscsi_iqn'] = line.split()[1].rstrip() elif sys.platform.startswith('hp-ux'): - if module is not None: - rc, out, err = module.run_command("/opt/iscsi/bin/iscsiutil -l | grep 'Initiator Name'", - use_unsafe_shell=True) + # try to find it in the default PATH and opt_dirs + cmd = get_bin_path('iscsiutil', opt_dirs=['/opt/iscsi/bin']) + if cmd: + cmd += " -l" + rc, out, err = module.run_command(cmd) if out: - iscsi_facts['iscsi_iqn'] = out.split(":", 1)[1].rstrip() - else: - hpuxcmd = "/opt/iscsi/bin/iscsiutil -l | grep 'Initiator Name'" - hpuxret = subprocess.check_output(hpuxcmd, shell=True) - if hpuxret[0].isalpha(): - iscsi_facts['iscsi_iqn'] = hpuxret.split(":", 1)[1].rstrip() + line = self.findstr(out, 'Initiator Name') + iscsi_facts['iscsi_iqn'] = line.split(":", 1)[1].rstrip() return iscsi_facts + + def findstr(self, text, match): + for line in text.splitlines(): + if match in line: + found = line + return found 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 new file mode 100644 index 00000000000..9b668022cb2 --- /dev/null +++ b/test/units/module_utils/facts/network/test_iscsi_get_initiator.py @@ -0,0 +1,54 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2019 Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +from ansible.module_utils.facts.network import iscsi +from units.compat.mock import Mock, patch + + +# AIX # lsattr -E -l iscsi0 +LSATTR_OUTPUT = """ +disc_filename /etc/iscsi/targets Configuration file False +disc_policy file Discovery Policy True +initiator_name iqn.localhost.hostid.7f000002 iSCSI Initiator Name True +isns_srvnames auto iSNS Servers IP Addresses True +isns_srvports iSNS Servers Port Numbers True +max_targets 16 Maximum Targets Allowed True +num_cmd_elems 200 Maximum number of commands to queue to driver True +""" + +# HP-UX # iscsiutil -l +ISCSIUTIL_OUTPUT = """ +Initiator Name : iqn.2001-04.com.hp.stor:svcio +Initiator Alias : +Authentication Method : None +CHAP Method : CHAP_UNI +Initiator CHAP Name : +CHAP Secret : +NAS Hostname : +NAS Secret : +Radius Server Hostname : +Header Digest : None,CRC32C (default) +Data Digest : None,CRC32C (default) +SLP Scope list for iSLPD : +""" + + +def test_get_iscsi_info(mocker): + module = Mock() + 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, '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, '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)