Update eos, ios, vyos cliconf plugin (#42300)

* Update eos cliconf plugin methods

*  Refactor eos cliconf plugin
*  Changes in eos module_utils as per cliconf plugin refactor

* Fix unit test and sanity failures

* Fix review comment

lib/ansible/plugins/cliconf/__init__.py

@@ -19,12 +19,13 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-from abc import ABCMeta, abstractmethod
+from abc import abstractmethod
 from functools import wraps
 
+from ansible.plugins import AnsiblePlugin
 from ansible.errors import AnsibleError, AnsibleConnectionFailure
 from ansible.module_utils._text import to_bytes, to_text
-from ansible.module_utils.six import with_metaclass
+
 
 try:
     from scp import SCPClient
@@ -49,7 +50,7 @@ def enable_mode(func):
     return wrapped
 
 
-class CliconfBase(with_metaclass(ABCMeta, object)):
+class CliconfBase(AnsiblePlugin):
     """
     A base class for implementing cli connections
 
@@ -84,6 +85,7 @@ class CliconfBase(with_metaclass(ABCMeta, object)):
     __rpc__ = ['get_config', 'edit_config', 'get_capabilities', 'get', 'enable_response_logging', 'disable_response_logging']
 
     def __init__(self, connection):
+        super(CliconfBase, self).__init__()
         self._connection = connection
         self.history = list()
         self.response_logging = False
@@ -375,7 +377,7 @@ class CliconfBase(with_metaclass(ABCMeta, object)):
         """
         pass
 
-    def run_commands(self, commands):
+    def run_commands(self, commands=None, check_rc=True):
         """
         Execute a list of commands on remote host and return the list of response
         :param commands: The list of command that needs to be executed on remote host.
@@ -385,10 +387,12 @@ class CliconfBase(with_metaclass(ABCMeta, object)):
                     'command': <command to be executed>
                     'prompt': <expected prompt on executing the command>,
                     'answer': <answer for the prompt>,
-                    'output': <the format in which command output should be rendered eg: 'json', 'text', if supported by platform>,
+                    'output': <the format in which command output should be rendered eg: 'json', 'text'>,
                     'sendonly': <Boolean flag to indicate if it command execution response should be ignored or not>
                 }
-
+        :param check_rc: Boolean flag to check if returned response should be checked for error or not.
+                         If check_rc is False the error output is appended in return response list, else if the
+                         value is True an exception is raised.
         :return: List of returned response
         """
         pass

lib/ansible/plugins/cliconf/eos.py

@@ -19,39 +19,259 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+DOCUMENTATION = """
+---
+author: Ansible Networking Team
+cliconf: eos
+short_description: Use eos cliconf to run command on eos platform
+description:
+  - This eos plugin provides low level abstraction api's for
+    sending and receiving CLI commands from eos network devices.
+version_added: "2.7"
+options:
+  eos_use_sessions:
+    type: int
+    default: 1
+    description:
+      - Specifies if sessions should be used on remote host or not
+    env:
+      - name: ANSIBLE_EOS_USE_SESSIONS
+    vars:
+      - name: ansible_eos_use_sessions
+        version_added: '2.7'
+"""
+
+import collections
 import json
 import time
 
-from itertools import chain
-
 from ansible.errors import AnsibleConnectionFailure
-from ansible.module_utils._text import to_bytes
+from ansible.module_utils._text import to_text
 from ansible.module_utils.network.common.utils import to_list
+from ansible.module_utils.network.common.config import NetworkConfig, dumps
 from ansible.plugins.cliconf import CliconfBase, enable_mode
 from ansible.plugins.connection.network_cli import Connection as NetworkCli
+from ansible.plugins.connection.httpapi import Connection as HttpApi
 
 
 class Cliconf(CliconfBase):
 
-    def send_command(self, command, prompt=None, answer=None, sendonly=False, newline=True, prompt_retry_check=False):
+    def __init__(self, *args, **kwargs):
+        super(Cliconf, self).__init__(*args, **kwargs)
+        self._session_support = None
+        if isinstance(self._connection, NetworkCli):
+            self.network_api = 'network_cli'
+        elif isinstance(self._connection, HttpApi):
+            self.network_api = 'eapi'
+        else:
+            raise ValueError("Invalid connection type")
+
+    def _get_command_with_output(self, command, output):
+        options_values = self.get_option_values()
+        if output not in options_values['output']:
+            raise ValueError("'output' value %s is invalid. Valid values are %s" % (output, ','.join(options_values['output'])))
+
+        if output == 'json' and not command.endswith('| json'):
+            cmd = '%s | json' % command
+        else:
+            cmd = command
+        return cmd
+
+    def send_command(self, command, **kwargs):
         """Executes a cli command and returns the results
         This method will execute the CLI command on the connection and return
         the results to the caller.  The command output will be returned as a
         string
         """
-        kwargs = {'command': to_bytes(command), 'sendonly': sendonly,
-                  'newline': newline, 'prompt_retry_check': prompt_retry_check}
-        if prompt is not None:
-            kwargs['prompt'] = to_bytes(prompt)
-        if answer is not None:
-            kwargs['answer'] = to_bytes(answer)
-
-        if isinstance(self._connection, NetworkCli):
-            resp = self._connection.send(**kwargs)
+        if self.network_api == 'network_cli':
+            resp = super(Cliconf, self).send_command(command, **kwargs)
         else:
             resp = self._connection.send_request(command, **kwargs)
         return resp
 
+    @enable_mode
+    def get_config(self, source='running', format='text', filter=None):
+        options_values = self.get_option_values()
+        if format not in options_values['format']:
+            raise ValueError("'format' value %s is invalid. Valid values are %s" % (format, ','.join(options_values['format'])))
+
+        lookup = {'running': 'running-config', 'startup': 'startup-config'}
+        if source not in lookup:
+            return self.invalid_params("fetching configuration from %s is not supported" % source)
+
+        cmd = 'show %s ' % lookup[source]
+        if format and format is not 'text':
+            cmd += '| %s ' % format
+
+        cmd += ' '.join(to_list(filter))
+        cmd = cmd.strip()
+        return self.send_command(cmd)
+
+    @enable_mode
+    def edit_config(self, candidate=None, commit=True, replace=False, comment=None):
+
+        if not candidate:
+            raise ValueError("must provide a candidate config to load")
+
+        if commit not in (True, False):
+            raise ValueError("'commit' must be a bool, got %s" % commit)
+
+        operations = self.get_device_operations()
+        if replace not in (True, False):
+            raise ValueError("'replace' must be a bool, got %s" % replace)
+
+        if replace and not operations['supports_replace']:
+            raise ValueError("configuration replace is supported only with configuration session")
+
+        if comment and not operations['supports_commit_comment']:
+            raise ValueError("commit comment is not supported")
+
+        if (commit is False) and (not self.supports_sessions):
+            raise ValueError('check mode is not supported without configuration session')
+
+        response = {}
+        session = None
+        if self.supports_sessions:
+            session = 'ansible_%s' % int(time.time())
+            response.update({'session': session})
+            self.send_command('configure session %s' % session)
+            if replace:
+                self.send_command('rollback clean-config')
+        else:
+            self.send_command('configure')
+
+        results = []
+        multiline = False
+        for line in to_list(candidate):
+            if not isinstance(line, collections.Mapping):
+                line = {'command': line}
+
+            cmd = line['command']
+            if cmd == 'end':
+                continue
+            elif cmd.startswith('banner') or multiline:
+                multiline = True
+            elif cmd == 'EOF' and multiline:
+                multiline = False
+
+            if multiline:
+                line['sendonly'] = True
+
+            if cmd != 'end' and cmd[0] != '!':
+                try:
+                    results.append(self.send_command(**line))
+                except AnsibleConnectionFailure as e:
+                    self.discard_changes(session)
+                    raise AnsibleConnectionFailure(e.message)
+
+        response['response'] = results
+        if self.supports_sessions:
+            out = self.send_command('show session-config diffs')
+            if out:
+                response['diff'] = out.strip()
+
+            if commit:
+                self.commit()
+            else:
+                self.discard_changes(session)
+        else:
+            self.send_command('end')
+        return response
+
+    def get(self, command, prompt=None, answer=None, sendonly=False, output=None):
+        if output:
+            command = self._get_command_with_output(command, output)
+        return self.send_command(command, prompt=prompt, answer=answer, sendonly=sendonly)
+
+    def commit(self):
+        self.send_command('commit')
+
+    def discard_changes(self, session=None):
+        commands = ['end']
+        if self.supports_sessions:
+            # to close session gracefully execute abort in top level session prompt.
+            commands.extend(['configure session %s' % session, 'abort'])
+
+        for cmd in commands:
+            self.send_command(cmd)
+
+    def run_commands(self, commands=None, check_rc=True):
+        if commands is None:
+            raise ValueError("'commands' value is required")
+        responses = list()
+        for cmd in to_list(commands):
+            if not isinstance(cmd, collections.Mapping):
+                cmd = {'command': cmd}
+
+            output = cmd.pop('output', None)
+            if output:
+                cmd['command'] = self._get_command_with_output(cmd['command'], output)
+
+            try:
+                out = self.send_command(**cmd)
+            except AnsibleConnectionFailure as e:
+                if check_rc:
+                    raise
+                out = getattr(e, 'err', e)
+
+            if out is not None:
+                try:
+                    out = json.loads(out)
+                except ValueError:
+                    out = to_text(out, errors='surrogate_or_strict').strip()
+
+                responses.append(out)
+        return responses
+
+    def get_diff(self, candidate=None, running=None, match='line', diff_ignore_lines=None, path=None, replace='line'):
+        diff = {}
+        device_operations = self.get_device_operations()
+        option_values = self.get_option_values()
+
+        if candidate is None and device_operations['supports_generate_diff']:
+            raise ValueError("candidate configuration is required to generate diff")
+
+        if match not in option_values['diff_match']:
+            raise ValueError("'match' value %s in invalid, valid values are %s" % (match, ', '.join(option_values['diff_match'])))
+
+        if replace not in option_values['diff_replace']:
+            raise ValueError("'replace' value %s in invalid, valid values are %s" % (replace, ', '.join(option_values['diff_replace'])))
+
+        # prepare candidate configuration
+        candidate_obj = NetworkConfig(indent=3)
+        candidate_obj.load(candidate)
+
+        if running and match != 'none' and replace != 'config':
+            # running configuration
+            running_obj = NetworkConfig(indent=3, contents=running, ignore_lines=diff_ignore_lines)
+            configdiffobjs = candidate_obj.difference(running_obj, path=path, match=match, replace=replace)
+
+        else:
+            configdiffobjs = candidate_obj.items
+
+        configdiff = dumps(configdiffobjs, 'commands') if configdiffobjs else ''
+        diff['config_diff'] = configdiff if configdiffobjs else {}
+        return diff
+
+    @property
+    def supports_sessions(self):
+        use_session = self.get_option('eos_use_sessions')
+        try:
+            use_session = int(use_session)
+        except ValueError:
+            pass
+
+        if not bool(use_session):
+            self._session_support = False
+        else:
+            if self._session_support:
+                return self._session_support
+
+            response = self.get('show configuration sessions')
+            self._session_support = 'error' not in response
+
+        return self._session_support
+
     def get_device_info(self):
         device_info = {}
 
@@ -69,108 +289,35 @@ class Cliconf(CliconfBase):
 
         return device_info
 
-    @enable_mode
-    def get_config(self, source='running', format='text', flags=None):
-        lookup = {'running': 'running-config', 'startup': 'startup-config'}
-        if source not in lookup:
-            return self.invalid_params("fetching configuration from %s is not supported" % source)
+    def get_device_operations(self):
+        return {
+            'supports_diff_replace': True,
+            'supports_commit': True if self.supports_sessions else False,
+            'supports_rollback': True if self.supports_sessions else False,
+            'supports_defaults': False,
+            'supports_onbox_diff': True if self.supports_sessions else False,
+            'supports_commit_comment': False,
+            'supports_multiline_delimiter': False,
+            'support_diff_match': True,
+            'support_diff_ignore_lines': True,
+            'supports_generate_diff': True,
+            'supports_replace': True if self.supports_sessions else False
+        }
 
-        cmd = 'show %s ' % lookup[source]
-        if format and format is not 'text':
-            cmd += '| %s ' % format
-
-        cmd += ' '.join(to_list(flags))
-        cmd = cmd.strip()
-        return self.send_command(cmd)
-
-    @enable_mode
-    def edit_config(self, command):
-        for cmd in chain(['configure'], to_list(command), ['end']):
-            self.send_command(cmd)
-
-    def get(self, command, prompt=None, answer=None, sendonly=False):
-        return self.send_command(command, prompt=prompt, answer=answer, sendonly=sendonly)
+    def get_option_values(self):
+        return {
+            'format': ['text', 'json'],
+            'diff_match': ['line', 'strict', 'exact', 'none'],
+            'diff_replace': ['line', 'block', 'config'],
+            'output': ['text', 'json']
+        }
 
     def get_capabilities(self):
         result = {}
         result['rpc'] = self.get_base_rpc()
         result['device_info'] = self.get_device_info()
-        if isinstance(self._connection, NetworkCli):
-            result['network_api'] = 'cliconf'
-        else:
-            result['network_api'] = 'eapi'
+        result['network_api'] = self.network_api
+        result['device_info'] = self.get_device_info()
+        result['device_operations'] = self.get_device_operations()
+        result.update(self.get_option_values())
         return json.dumps(result)
-
-    # Imported from module_utils
-    def close_session(self, session):
-        # to close session gracefully execute abort in top level session prompt.
-        self.get('end')
-        self.get('configure session %s' % session)
-        self.get('abort')
-
-    def run_commands(self, commands, check_rc=True):
-        """Run list of commands on remote device and return results
-        """
-        responses = list()
-        multiline = False
-
-        for cmd in to_list(commands):
-            if isinstance(cmd, dict):
-                command = cmd['command']
-                prompt = cmd['prompt']
-                answer = cmd['answer']
-            else:
-                command = cmd
-                prompt = None
-                answer = None
-
-            if command == 'end':
-                continue
-            elif command.startswith('banner') or multiline:
-                multiline = True
-            elif command == 'EOF' and multiline:
-                multiline = False
-
-            try:
-                out = self.get(command, prompt, answer, multiline)
-            except AnsibleConnectionFailure as e:
-                if check_rc:
-                    raise
-                out = getattr(e, 'err', e)
-
-            if out is not None:
-                try:
-                    out = json.loads(out)
-                except ValueError:
-                    out = str(out).strip()
-
-                responses.append(out)
-
-        return responses
-
-    def load_config(self, commands, commit=False, replace=False):
-        """Loads the config commands onto the remote device
-        """
-        session = 'ansible_%s' % int(time.time())
-        result = {'session': session}
-
-        self.get('configure session %s' % session)
-        if replace:
-            self.get('rollback clean-config')
-
-        try:
-            self.run_commands(commands)
-        except AnsibleConnectionFailure:
-            self.close_session(session)
-            raise
-
-        out = self.get('show session-config diffs')
-        if out:
-            result['diff'] = out.strip()
-
-        if commit:
-            self.get('commit')
-        else:
-            self.close_session(session)
-
-        return result

lib/ansible/plugins/cliconf/ios.py

@@ -26,6 +26,7 @@ import json
 
 from itertools import chain
 
+from ansible.errors import AnsibleConnectionFailure
 from ansible.module_utils._text import to_text
 from ansible.module_utils.six import iteritems
 from ansible.module_utils.network.common.config import NetworkConfig, dumps
@@ -41,7 +42,7 @@ class Cliconf(CliconfBase):
             return self.invalid_params("fetching configuration from %s is not supported" % source)
 
         if format:
-            raise ValueError("'format' value %s is not supported on ios" % format)
+            raise ValueError("'format' value %s is not supported for get_config" % format)
 
         if not filter:
             filter = []
@@ -94,14 +95,14 @@ class Cliconf(CliconfBase):
         device_operations = self.get_device_operations()
         option_values = self.get_option_values()
 
-        if candidate is None and not device_operations['supports_onbox_diff']:
+        if candidate is None and device_operations['supports_generate_diff']:
             raise ValueError("candidate configuration is required to generate diff")
 
         if match not in option_values['diff_match']:
-            raise ValueError("'match' value %s in invalid, valid values are %s" % (match, option_values['diff_match']))
+            raise ValueError("'match' value %s in invalid, valid values are %s" % (match, ', '.join(option_values['diff_match'])))
 
         if replace not in option_values['diff_replace']:
-            raise ValueError("'replace' value %s in invalid, valid values are %s" % (replace, option_values['diff_replace']))
+            raise ValueError("'replace' value %s in invalid, valid values are %s" % (replace, ', '.join(option_values['diff_replace'])))
 
         # prepare candidate configuration
         candidate_obj = NetworkConfig(indent=1)
@@ -124,11 +125,13 @@ class Cliconf(CliconfBase):
         banners = self._diff_banners(want_banners, have_banners)
 
         diff['banner_diff'] = banners if banners else {}
-        return json.dumps(diff)
+        return diff
 
     @enable_mode
     def edit_config(self, candidate=None, commit=True, replace=False, comment=None):
         resp = {}
+        operations = self.get_device_operations()
+
         if not candidate:
             raise ValueError("must provide a candidate config to load")
 
@@ -138,9 +141,12 @@ class Cliconf(CliconfBase):
         if replace not in (True, False):
             raise ValueError("'replace' must be a bool, got %s" % replace)
 
+        if comment and not operations['supports_commit_comment']:
+            raise ValueError("commit comment is not supported")
+
         operations = self.get_device_operations()
         if replace and not operations['supports_replace']:
-            raise ValueError("configuration replace is not supported on ios")
+            raise ValueError("configuration replace is not supported")
 
         results = []
         if commit:
@@ -153,6 +159,8 @@ class Cliconf(CliconfBase):
                     results.append(self.send_command(**line))
 
             results.append(self.send_command('end'))
+        else:
+            raise ValueError('check mode is not supported')
 
         resp['response'] = results[1:-1]
         return resp
@@ -161,7 +169,7 @@ class Cliconf(CliconfBase):
         if not command:
             raise ValueError('must provide value of command to execute')
         if output:
-            raise ValueError("'output' value %s is not supported on ios" % output)
+            raise ValueError("'output' value %s is not supported for get" % output)
 
         return self.send_command(command=command, prompt=prompt, answer=answer, sendonly=sendonly)
 
@@ -247,7 +255,10 @@ class Cliconf(CliconfBase):
 
         return resp
 
-    def run_commands(self, commands):
+    def run_commands(self, commands=None, check_rc=True):
+        if commands is None:
+            raise ValueError("'commands' value is required")
+
         responses = list()
         for cmd in to_list(commands):
             if not isinstance(cmd, collections.Mapping):
@@ -255,9 +266,17 @@ class Cliconf(CliconfBase):
 
             output = cmd.pop('output', None)
             if output:
-                raise ValueError("'output' value %s is not supported on ios" % output)
+                raise ValueError("'output' value %s is not supported for run_commands" % output)
+
+            try:
+                out = self.send_command(**cmd)
+            except AnsibleConnectionFailure as e:
+                if check_rc:
+                    raise
+                out = getattr(e, 'err', e)
+
+            responses.append(out)
 
-            responses.append(self.send_command(**cmd))
         return responses
 
     def _extract_banners(self, config):

lib/ansible/plugins/cliconf/vyos.py

@@ -58,7 +58,7 @@ class Cliconf(CliconfBase):
         if format:
             option_values = self.get_option_values()
             if format not in option_values['format']:
-                raise ValueError("'format' value %s is invalid. Valid values of format are %s" % (format, ','.join(option_values['format'])))
+                raise ValueError("'format' value %s is invalid. Valid values of format are %s" % (format, ', '.join(option_values['format'])))
 
         if format == 'text':
             out = self.send_command('show configuration')
@@ -79,7 +79,7 @@ class Cliconf(CliconfBase):
 
         operations = self.get_device_operations()
         if replace and not operations['supports_replace']:
-            raise ValueError("configuration replace is not supported on vyos")
+            raise ValueError("configuration replace is not supported")
 
         results = []
 
@@ -110,14 +110,14 @@ class Cliconf(CliconfBase):
 
         resp['diff'] = diff_config
         resp['response'] = results[1:-1]
-        return json.dumps(resp)
+        return resp
 
     def get(self, command=None, prompt=None, answer=None, sendonly=False, output=None):
         if not command:
             raise ValueError('must provide value of command to execute')
 
         if output:
-            raise ValueError("'output' value %s is not supported on vyos" % output)
+            raise ValueError("'output' value %s is not supported for get" % output)
 
         return self.send_command(command, prompt=prompt, answer=answer, sendonly=sendonly)
 
@@ -136,20 +136,20 @@ class Cliconf(CliconfBase):
         device_operations = self.get_device_operations()
         option_values = self.get_option_values()
 
-        if candidate is None and not device_operations['supports_onbox_diff']:
+        if candidate is None and device_operations['supports_generate_diff']:
             raise ValueError("candidate configuration is required to generate diff")
 
         if match not in option_values['diff_match']:
-            raise ValueError("'match' value %s in invalid, valid values are %s" % (match, option_values['diff_match']))
+            raise ValueError("'match' value %s in invalid, valid values are %s" % (match, ', '.join(option_values['diff_match'])))
 
         if replace:
-            raise ValueError("'replace' in diff is not supported on vyos")
+            raise ValueError("'replace' in diff is not supported")
 
         if diff_ignore_lines:
-            raise ValueError("'diff_ignore_lines' in diff is not supported on vyos")
+            raise ValueError("'diff_ignore_lines' in diff is not supported")
 
         if path:
-            raise ValueError("'path' in diff is not supported on vyos")
+            raise ValueError("'path' in diff is not supported")
 
         set_format = candidate.startswith('set') or candidate.startswith('delete')
         candidate_obj = NetworkConfig(indent=4, contents=candidate)
@@ -171,7 +171,7 @@ class Cliconf(CliconfBase):
 
         if match == 'none':
             diff['config_diff'] = list(candidate_commands)
-            return json.dumps(diff)
+            return diff
 
         running_commands = [str(c).replace("'", '') for c in running.splitlines()]
 
@@ -198,9 +198,12 @@ class Cliconf(CliconfBase):
                             visited.add(line)
 
         diff['config_diff'] = list(updates)
-        return json.dumps(diff)
+        return diff
+
+    def run_commands(self, commands=None, check_rc=True):
+        if commands is None:
+            raise ValueError("'commands' value is required")
 
-    def run_commands(self, commands):
         responses = list()
         for cmd in to_list(commands):
             if not isinstance(cmd, collections.Mapping):
@@ -208,9 +211,17 @@ class Cliconf(CliconfBase):
 
             output = cmd.pop('output', None)
             if output:
-                raise ValueError("'output' value %s is not supported on vyos" % output)
+                raise ValueError("'output' value %s is not supported for run_commands" % output)
+
+            try:
+                out = self.send_command(**cmd)
+            except AnsibleConnectionFailure as e:
+                if check_rc:
+                    raise
+                out = getattr(e, 'err', e)
+
+            responses.append(out)
 
-            responses.append(self.send_command(**cmd))
         return responses
 
     def get_device_operations(self):
@@ -219,7 +230,7 @@ class Cliconf(CliconfBase):
             'supports_commit': True,
             'supports_rollback': True,
             'supports_defaults': False,
-            'supports_onbox_diff': False,
+            'supports_onbox_diff': True,
             'supports_commit_comment': True,
             'supports_multiline_delimiter': False,
             'support_diff_match': True,

lib/ansible/plugins/connection/network_cli.py

@@ -198,6 +198,7 @@ class Connection(NetworkConnectionBase):
         self._history = list()
 
         self._terminal = None
+        self.cliconf = None
         self.paramiko_conn = None
 
         if self._play_context.verbosity > 3:
@@ -258,7 +259,6 @@ class Connection(NetworkConnectionBase):
 
         self.reset_history()
         self.disable_response_logging()
-
         return messages
 
     def _connect(self):
@@ -291,10 +291,11 @@ class Connection(NetworkConnectionBase):
 
             display.vvvv('loaded terminal plugin for network_os %s' % self._network_os, host=host)
 
-            cliconf = cliconf_loader.get(self._network_os, self)
-            if cliconf:
+            self.cliconf = cliconf_loader.get(self._network_os, self)
+            if self.cliconf:
                 display.vvvv('loaded cliconf plugin for network_os %s' % self._network_os, host=host)
-                self._implementation_plugins.append(cliconf)
+                self._implementation_plugins.append(self.cliconf)
+                self.cliconf.set_options()
             else:
                 display.vvvv('unable to load cliconf for network_os %s' % self._network_os)