Clarify exception handling in net modules (ansible#15507)

* Clarify exception handling in EOS

Also modify to EOS to standardize modules. It makes vimdiff a lot less angry

* Move IOS exception handling into Cli

* Move IOS-XR exception handling into Cli

* Move JUNOS exception handling into Cli

* Move NXOS exception handling into Cli

And reorganize to make it match the other modules

* Move OpenSwitch exception handling into Cli

More speculative restructuring here

lib/ansible/module_utils/eos.py

@@ -16,12 +16,11 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
-import os
 
 import re
 
-from ansible.module_utils.basic import AnsibleModule, env_fallback
-from ansible.module_utils.shell import Shell, Command, HAS_PARAMIKO
+from ansible.module_utils.basic import AnsibleModule, env_fallback, get_exception
+from ansible.module_utils.shell import Shell, ShellError, Command, HAS_PARAMIKO
 from ansible.module_utils.netcfg import parse
 from ansible.module_utils.urls import fetch_url
 
@@ -57,6 +56,7 @@
     re.compile(r"[^\r\n]\/bin\/(?:ba)?sh")
 ]
 
+
 def to_list(val):
     if isinstance(val, (list, tuple)):
         return list(val)
@@ -151,20 +151,23 @@ def connect(self, **kwargs):
         key_filename = self.module.params['ssh_keyfile']
 
         try:
-            self.shell = Shell(prompts_re=CLI_PROMPTS_RE,
-                    errors_re=CLI_ERRORS_RE)
-            self.shell.open(host, port=port, username=username,
-                    password=password, key_filename=key_filename)
-        except Exception, exc:
-            msg = 'failed to connect to %s:%s - %s' % (host, port, str(exc))
+            self.shell = Shell(prompts_re=CLI_PROMPTS_RE, errors_re=CLI_ERRORS_RE)
+            self.shell.open(host, port=port, username=username, password=password, key_filename=key_filename)
+        except ShellError:
+            e = get_exception()
+            msg = 'failed to connect to %s:%s - %s' % (host, port, str(e))
             self.module.fail_json(msg=msg)
 
     def authorize(self):
         passwd = self.module.params['auth_pass']
         self.send(Command('enable', prompt=NET_PASSWD_RE, response=passwd))
 
     def send(self, commands):
-        return self.shell.send(commands)
+        try:
+            return self.shell.send(commands)
+        except ShellError:
+            e = get_exception()
+            self.module.fail_json(msg=e.message, commands=commands)
 
 
 class NetworkModule(AnsibleModule):
@@ -194,19 +197,18 @@ def _load_params(self):
                     self.params[key] = value
 
     def connect(self):
+        cls = globals().get(str(self.params['transport']).capitalize())
         try:
-            cls = globals().get(str(self.params['transport']).capitalize())
             self.connection = cls(self)
+        except TypeError:
+            e = get_exception()
+            self.fail_json(msg=e.message)
 
-            self.connection.connect()
-            self.connection.send('terminal length 0')
+        self.connection.connect()
+        self.connection.send('terminal length 0')
 
-            if self.params['authorize']:
-                self.connection.authorize()
-        except AttributeError, exc:
-            self.fail_json(msg=exc.message)
-        except Exception, exc:
-            self.fail_json(msg=exc.message)
+        if self.params['authorize']:
+            self.connection.authorize()
 
         self._connected = True
 
@@ -233,15 +235,13 @@ def config_replace(self, commands):
         return self.execute(command)
 
     def execute(self, commands, **kwargs):
-        try:
-            if not self.connected:
-                self.connect()
-            return self.connection.send(commands, **kwargs)
-        except Exception, exc:
-            self.fail_json(msg=exc.message, commands=commands)
+        if not self.connected:
+            self.connect()
+        return self.connection.send(commands, **kwargs)
 
     def disconnect(self):
         self.connection.close()
+        self._connected = False
 
     def parse_config(self, cfg):
         return parse(cfg, indent=3)
@@ -267,9 +267,7 @@ def get_module(**kwargs):
 
     module = NetworkModule(**kwargs)
 
-    # HAS_PARAMIKO is set by module_utils/shell.py
     if module.params['transport'] == 'cli' and not HAS_PARAMIKO:
         module.fail_json(msg='paramiko is required but does not appear to be installed')
 
     return module
-

lib/ansible/module_utils/ios.py

@@ -19,7 +19,7 @@
 
 import re
 
-from ansible.module_utils.basic import AnsibleModule, env_fallback
+from ansible.module_utils.basic import AnsibleModule, env_fallback, get_exception
 from ansible.module_utils.shell import Shell, ShellError, Command, HAS_PARAMIKO
 from ansible.module_utils.netcfg import parse
 
@@ -78,19 +78,23 @@ def connect(self, **kwargs):
         timeout = self.module.params['timeout']
 
         try:
-            self.shell = Shell(kickstart=False, prompts_re=CLI_PROMPTS_RE,
-                    errors_re=CLI_ERRORS_RE)
+            self.shell = Shell(kickstart=False, prompts_re=CLI_PROMPTS_RE, errors_re=CLI_ERRORS_RE)
             self.shell.open(host, port=port, username=username, password=password, key_filename=key_filename, timeout=timeout)
-        except Exception, exc:
-            msg = 'failed to connect to %s:%s - %s' % (host, port, str(exc))
+        except ShellError:
+            e = get_exception()
+            msg = 'failed to connect to %s:%s - %s' % (host, port, str(e))
             self.module.fail_json(msg=msg)
 
     def authorize(self):
         passwd = self.module.params['auth_pass']
         self.send(Command('enable', prompt=NET_PASSWD_RE, response=passwd))
 
     def send(self, commands):
-        return self.shell.send(commands)
+        try:
+            return self.shell.send(commands)
+        except ShellError:
+            e = get_exception()
+            self.module.fail_json(msg=e.message, commands=commands)
 
 
 class NetworkModule(AnsibleModule):
@@ -115,21 +119,20 @@ def _load_params(self):
         super(NetworkModule, self)._load_params()
         provider = self.params.get('provider') or dict()
         for key, value in provider.items():
-            if key in NET_COMMON_ARGS.keys():
+            if key in NET_COMMON_ARGS:
                 if self.params.get(key) is None and value is not None:
                     self.params[key] = value
 
     def connect(self):
-        try:
-            self.connection = Cli(self)
-            self.connection.connect()
-            self.connection.send('terminal length 0')
-            if self.params['authorize']:
-                self.connection.authorize()
-            self._connected = True
+        self.connection = Cli(self)
+
+        self.connection.connect()
+        self.connection.send('terminal length 0')
 
-        except Exception, exc:
-            self.fail_json(msg=exc.message)
+        if self.params['authorize']:
+            self.connection.authorize()
+
+        self._connected = True
 
     def configure(self, commands):
         commands = to_list(commands)
@@ -139,14 +142,9 @@ def configure(self, commands):
         return responses
 
     def execute(self, commands, **kwargs):
-        try:
-            if not self.connected:
-                self.connect()
-            return self.connection.send(commands, **kwargs)
-        except ShellError, exc:
-            self.fail_json(msg=exc.message, command=exc.command)
-        except Exception, exc:
-            self.fail_json(msg=exc.message, commands=commands)
+        if not self.connected:
+            self.connect()
+        return self.connection.send(commands, **kwargs)
 
     def disconnect(self):
         self.connection.close()
@@ -172,7 +170,6 @@ def get_module(**kwargs):
 
     module = NetworkModule(**kwargs)
 
-    # HAS_PARAMIKO is set by module_utils/shell.py
     if not HAS_PARAMIKO:
         module.fail_json(msg='paramiko is required but does not appear to be installed')
 

lib/ansible/module_utils/iosxr.py

@@ -19,8 +19,8 @@
 
 import re
 
-from ansible.module_utils.basic import AnsibleModule, env_fallback
-from ansible.module_utils.shell import Shell, HAS_PARAMIKO
+from ansible.module_utils.basic import AnsibleModule, env_fallback, get_exception
+from ansible.module_utils.shell import Shell, ShellError, HAS_PARAMIKO
 from ansible.module_utils.netcfg import parse
 
 NET_PASSWD_RE = re.compile(r"[\r\n]?password: $", re.I)
@@ -49,6 +49,7 @@
     re.compile(r"'[^']' +returned error code: ?\d+"),
 ]
 
+
 def to_list(val):
     if isinstance(val, (list, tuple)):
         return list(val)
@@ -57,6 +58,7 @@ def to_list(val):
     else:
         return list()
 
+
 class Cli(object):
 
     def __init__(self, module):
@@ -74,12 +76,18 @@ def connect(self, **kwargs):
         try:
             self.shell = Shell(kickstart=False, prompts_re=CLI_PROMPTS_RE, errors_re=CLI_ERRORS_RE)
             self.shell.open(host, port=port, username=username, password=password, key_filename=key_filename)
-        except Exception, exc:
-            msg = 'failed to connecto to %s:%s - %s' % (host, port, str(exc))
+        except ShellError:
+            e = get_exception()
+            msg = 'failed to connect to %s:%s - %s' % (host, port, str(e))
             self.module.fail_json(msg=msg)
 
     def send(self, commands):
-        return self.shell.send(commands)
+        try:
+            return self.shell.send(commands)
+        except ShellError:
+            e = get_exception()
+            self.module.fail_json(msg=e.message, commands=commands)
+
 
 class NetworkModule(AnsibleModule):
 
@@ -103,18 +111,16 @@ def _load_params(self):
         super(NetworkModule, self)._load_params()
         provider = self.params.get('provider') or dict()
         for key, value in provider.items():
-            if key in NET_COMMON_ARGS.keys():
+            if key in NET_COMMON_ARGS:
                 if self.params.get(key) is None and value is not None:
                     self.params[key] = value
 
     def connect(self):
-        try:
-            self.connection = Cli(self)
-            self.connection.connect()
-            self.connection.send('terminal length 0')
-            self._connected = True
-        except Exception, exc:
-            self.fail_json(msg=exc.message)
+        self.connection = Cli(self)
+
+        self.connection.connect()
+        self.connection.send('terminal length 0')
+        self._connected = True
 
     def configure(self, commands):
         commands = to_list(commands)
@@ -126,12 +132,9 @@ def configure(self, commands):
         return responses
 
     def execute(self, commands, **kwargs):
-        try:
-            if not self.connected:
-                self.connect()
-            return self.connection.send(commands)
-        except ShellError, exc:
-            self.fail_json(msg=exc.message, command=exc.command)
+        if not self.connected:
+            self.connect()
+        return self.connection.send(commands, **kwargs)
 
     def disconnect(self):
         self.connection.close()
@@ -157,4 +160,3 @@ def get_module(**kwargs):
         module.fail_json(msg='paramiko is required but does not appear to be installed')
 
     return module
-

lib/ansible/module_utils/junos.py

@@ -16,10 +16,11 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
+
 from distutils.version import LooseVersion
 
-from ansible.module_utils.basic import AnsibleModule, env_fallback
-from ansible.module_utils.shell import Shell, HAS_PARAMIKO
+from ansible.module_utils.basic import AnsibleModule, env_fallback, get_exception
+from ansible.module_utils.shell import Shell, ShellError, HAS_PARAMIKO
 from ansible.module_utils.netcfg import parse
 
 try:
@@ -67,6 +68,7 @@ def to_list(val):
     else:
         return list()
 
+
 def xml_to_json(val):
     if isinstance(val, basestring):
         return jxmlease.parse(val)
@@ -91,21 +93,24 @@ def connect(self, **kwargs):
         password = self.module.params['password']
         key_filename = self.module.params['ssh_keyfile']
 
-        self.shell = Shell()
-
         try:
-            self.shell.open(host, port=port, username=username,
-                    password=password, key_filename=key_filename)
-        except Exception, exc:
-            msg = 'failed to connect to %s:%s - %s' % (host, port, str(exc))
+            self.shell = Shell()
+            self.shell.open(host, port=port, username=username, password=password, key_filename=key_filename)
+        except ShellError:
+            e = get_exception()
+            msg = 'failed to connect to %s:%s - %s' % (host, port, str(e))
             self.module.fail_json(msg=msg)
 
         if self.shell._matched_prompt.strip().endswith('%'):
             self.shell.send('cli')
         self.shell.send('set cli screen-length 0')
 
     def run_commands(self, commands, **kwargs):
-        return self.shell.send(commands)
+        try:
+            return self.shell.send(commands)
+        except ShellError:
+            e = get_exception()
+            self.module.fail_json(msg=e.message, commands=commands)
 
     def configure(self, commands, **kwargs):
         commands = to_list(commands)
@@ -281,13 +286,18 @@ def _load_params(self):
         super(NetworkModule, self)._load_params()
         provider = self.params.get('provider') or dict()
         for key, value in provider.items():
-            if key in NET_COMMON_ARGS.keys():
+            if key in NET_COMMON_ARGS:
                 if self.params.get(key) is None and value is not None:
                     self.params[key] = value
 
     def connect(self):
         cls = globals().get(str(self.params['transport']).capitalize())
-        self.connection = cls(self)
+        try:
+            self.connection = cls(self)
+        except TypeError:
+            e = get_exception()
+            self.fail_json(msg=e.message)
+
         self.connection.connect()
 
         msg = 'connecting to host: {username}@{host}:{port}'.format(**self.params)
@@ -336,7 +346,6 @@ def get_module(**kwargs):
 
     module = NetworkModule(**kwargs)
 
-    # HAS_PARAMIKO is set by module_utils/shell.py
     if module.params['transport'] == 'cli' and not HAS_PARAMIKO:
         module.fail_json(msg='paramiko is required but does not appear to be installed')
     elif module.params['transport'] == 'netconf' and not HAS_PYEZ: