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 <matorola@gmail.com>
* Apply suggestions from code review #2
Co-Authored-By: Sam Doran <sdoran@redhat.com>
* 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

3 files changed, 77 insertions, 18 deletions

diff --git a/changelogs/fragments/55643-network-facts-iscsi.yaml b/changelogs/fragments/55643-network-facts-iscsi.yaml
new file mode 100644
index 0000000000..64d50b5604
--- /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 3db94d2d7b..98e62a2aa6 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 0000000000..9b668022cb
--- /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)