nxos_nxapi fix (#25306)

* Add nxos_nxapi tests

* Simple changes to nxos_nxapi

* Move validation to check_args

* Don't mark protocol change unless change is requested

* Add different regex to handle HTTP{,S} ports on a different version of nxos

lib/ansible/modules/network/nxos/nxos_nxapi.py

@@ -124,13 +124,10 @@ updates:
 """
 import re
 
-from functools import partial
-
 from ansible.module_utils.nxos import run_commands, load_config
 from ansible.module_utils.nxos import nxos_argument_spec
 from ansible.module_utils.nxos import check_args as nxos_check_args
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.netcfg import NetworkConfig
 from ansible.module_utils.six import iteritems
 
 def check_args(module, warnings):
@@ -159,11 +156,15 @@ def check_args(module, warnings):
         if module.params[key]:
             warnings.append('argument %s is deprecated and will be ignored' % key)
 
+    for key in ['http_port', 'https_port']:
+        if module.params[key] is not None:
+            if not 1 <= module.params[key] <= 65535:
+                module.fail_json(msg='%s must be between 1 and 65535' % key)
+
     return warnings
 
-def map_obj_to_commands(updates, module):
+def map_obj_to_commands(want, have, module):
     commands = list()
-    want, have = updates
 
     needs_update = lambda x: want.get(x) is not None and (want.get(x) != have.get(x))
 
@@ -172,14 +173,14 @@ def map_obj_to_commands(updates, module):
             return ['no feature nxapi']
         commands.append('feature nxapi')
 
-    if any((needs_update('http'), needs_update('http_port'))):
+    if needs_update('http') or (have.get('http') and needs_update('http_port')):
         if want['http'] is True or (want['http'] is None and have['http'] is True):
             port = want['http_port'] or 80
             commands.append('nxapi http port %s' % port)
         elif want['http'] is False:
             commands.append('no nxapi http')
 
-    if any((needs_update('https'), needs_update('https_port'))):
+    if needs_update('https') or (have.get('https') and needs_update('https_port')):
         if want['https'] is True or (want['https'] is None and have['https'] is True):
             port = want['https_port'] or 443
             commands.append('nxapi https port %s' % port)
@@ -195,32 +196,42 @@ def map_obj_to_commands(updates, module):
     return commands
 
 def parse_http(data):
-    match = re.search('HTTP Port:\s+(\d+)', data, re.M)
+    http_res = [r'HTTP Port:\s+(\d+)', r'HTTP Listen on port (\d+)']
+    http_port = None
+
+    for regex in http_res:
+        match = re.search(regex, data, re.M)
         if match:
-        return {'http': True, 'http_port': int(match.group(1))}
+            http_port = int(match.group(1))
-    else:
+            break
-        return {'http': False, 'http_port': None}
+
+    return {'http': http_port is not None, 'http_port': http_port}
 
 def parse_https(data):
-    match = re.search('HTTPS Port:\s+(\d+)', data, re.M)
+    https_res = [r'HTTPS Port:\s+(\d+)', r'HTTPS Listen on port (\d+)']
+    https_port = None
+
+    for regex in https_res:
+        match = re.search(regex, data, re.M)
         if match:
-        return {'https': True, 'https_port': int(match.group(1))}
+            https_port = int(match.group(1))
-    else:
+            break
-        return {'https': False, 'https_port': None}
+
+    return {'https': https_port is not None, 'https_port': https_port}
 
 def parse_sandbox(data):
-    match = re.search('Sandbox:\s+(.+)$', data, re.M)
+    match = re.search(r'Sandbox:\s+(.+)$', data, re.M)
-    value = None
+    value = False
     if match:
         value = match.group(1) == 'Enabled'
     return {'sandbox': value}
 
 def map_config_to_obj(module):
-    out = run_commands(module, ['show nxapi'], check_rc=False)
+    out = run_commands(module, ['show nxapi'], check_rc=False)[0]
-    if out[0] == '':
+    if out == '':
         return {'state': 'absent'}
 
-    out = str(out[0]).strip()
+    out = str(out).strip()
 
     obj = {'state': 'present'}
     obj.update(parse_http(out))
@@ -229,14 +240,6 @@ def map_config_to_obj(module):
 
     return obj
 
-def validate_http_port(value, module):
-    if not 1 <= module.params['http_port'] <= 65535:
-        module.fail_json(msg='http_port must be between 1 and 65535')
-
-def validate_https_port(value, module):
-    if not 1 <= module.params['https_port'] <= 65535:
-        module.fail_json(msg='https_port must be between 1 and 65535')
-
 def map_params_to_obj(module):
     obj = {
         'http': module.params['http'],
@@ -247,12 +250,6 @@ def map_params_to_obj(module):
         'state': module.params['state']
     }
 
-    for key, value in iteritems(obj):
-        if value:
-            validator = globals().get('validate_%s' % key)
-            if validator:
-                validator(value, module)
-
     return obj
 
 def main():
@@ -279,16 +276,16 @@ def main():
                            supports_check_mode=True)
 
 
-    result = {'changed': False}
 
     warnings = list()
     check_args(module, warnings)
-    result['warnings'] = warnings
+
+    result = {'changed': False, 'warnings': warnings}
 
     want = map_params_to_obj(module)
     have = map_config_to_obj(module)
 
-    commands = map_obj_to_commands((want, have), module)
+    commands = map_obj_to_commands(want, have, module)
     result['commands'] = commands
 
     if commands:

test/units/modules/network/nxos/fixtures/nxos_nxapi/show_nxapi

@@ -0,0 +1,4 @@
+
+NX-API:       Enabled         Sandbox:      Disabled    
+HTTP Port:    80              HTTPS Port:   Disabled
+

test/units/modules/network/nxos/test_nxos_nxapi.py

@@ -0,0 +1,69 @@
+# (c) 2016 Red Hat Inc.
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+import os
+
+from ansible.compat.tests.mock import patch
+from ansible.modules.network.nxos import nxos_nxapi
+from .nxos_module import TestNxosModule, load_fixture, set_module_args
+
+
+class TestNxosNxapiModule(TestNxosModule):
+
+    module = nxos_nxapi
+
+    def setUp(self):
+        self.mock_run_commands = patch('ansible.modules.network.nxos.nxos_nxapi.run_commands')
+        self.run_commands = self.mock_run_commands.start()
+
+        self.mock_load_config = patch('ansible.modules.network.nxos.nxos_nxapi.load_config')
+        self.load_config = self.mock_load_config.start()
+
+    def tearDown(self):
+        self.mock_run_commands.stop()
+        self.mock_load_config.stop()
+
+    def load_fixtures(self, commands=None):
+        def load_from_file(*args, **kwargs):
+            module, commands = args
+            output = list()
+
+            for command in commands:
+                filename = str(command).replace(' ', '_')
+                filename = os.path.join('nxos_nxapi', filename)
+                output.append(load_fixture(filename))
+            return output
+
+        self.run_commands.side_effect = load_from_file
+        self.load_config.return_value = None
+
+    def test_nxos_nxapi_no_change(self):
+        set_module_args(dict(http=True, https=False, http_port=80, https_port=443, sandbox=False))
+        self.execute_module(changed=False, commands=[])
+
+    def test_nxos_nxapi_disable(self):
+        set_module_args(dict(state='absent'))
+        self.execute_module(changed=True, commands=['no feature nxapi'])
+
+    def test_nxos_nxapi_no_http(self):
+        set_module_args(dict(https=True, http=False, https_port=8443))
+        self.execute_module(changed=True, commands=['no nxapi http', 'nxapi https port 8443'])