Update junos cliconf plugin (ansible#43643)

* Update junos cliconf plugin

Fixes ansible#39056

Refactor junos cliconf plugin api and other
minor changes

* Fix CI issue

* Fix CI failure

lib/ansible/module_utils/network/junos/junos.py

@@ -101,6 +101,11 @@ def get_capabilities(module):
     return module._junos_capabilities
 
 
+def is_netconf(module):
+    capabilities = get_capabilities(module)
+    return True if capabilities.get('network_api') == 'netconf' else False
+
+
 def _validate_rollback_id(module, value):
     try:
         if not 0 <= int(value) <= 49:
@@ -158,14 +163,16 @@ def get_configuration(module, compare=False, format='xml', rollback='0', filter=
     return reply
 
 
-def commit_configuration(module, confirm=False, check=False, comment=None, confirm_timeout=None, synchronize=False,
-                         at_time=None, exit=False):
+def commit_configuration(module, confirm=False, check=False, comment=None, confirm_timeout=None, synchronize=False, at_time=None):
     conn = get_connection(module)
     try:
         if check:
             reply = conn.validate()
         else:
-            reply = conn.commit(confirmed=confirm, timeout=confirm_timeout, comment=comment, synchronize=synchronize, at_time=at_time)
+            if is_netconf(module):
+                reply = conn.commit(confirmed=confirm, timeout=confirm_timeout, comment=comment, synchronize=synchronize, at_time=at_time)
+            else:
+                reply = conn.commit(comment=comment, confirmed=confirm, at_time=at_time, synchronize=synchronize)
     except ConnectionError as exc:
         module.fail_json(msg=to_text(exc, errors='surrogate_then_replace'))
     return reply

lib/ansible/modules/network/junos/junos_netconf.py

@@ -149,11 +149,11 @@ def map_params_to_obj(module):
 def load_config(module, config, commit=False):
     conn = get_connection(module)
     try:
-        conn.edit_config(to_list(config) + ['top'])
-        diff = conn.compare_configuration()
+        resp = conn.edit_config(to_list(config) + ['top'])
     except ConnectionError as exc:
         module.fail_json(msg=to_text(exc, errors='surrogate_then_replace'))
 
+    diff = resp.get('diff', '')
     if diff:
         if commit:
             commit_configuration(module)

lib/ansible/plugins/cliconf/eos.py

@@ -82,7 +82,7 @@ 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)
+            raise ValueError("fetching configuration from %s is not supported" % source)
 
         cmd = 'show %s ' % lookup[source]
         if format and format is not 'text':

lib/ansible/plugins/cliconf/ios.py

@@ -39,7 +39,7 @@ class Cliconf(CliconfBase):
     @enable_mode
     def get_config(self, source='running', flags=None, format=None):
         if source not in ('running', 'startup'):
-            return self.invalid_params("fetching configuration from %s is not supported" % source)
+            raise ValueError("fetching configuration from %s is not supported" % source)
 
         if format:
             raise ValueError("'format' value %s is not supported for get_config" % format)

lib/ansible/plugins/cliconf/junos.py

@@ -19,15 +19,28 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import collections
 import json
 import re
+
 from itertools import chain
+from functools import wraps
 
 from ansible.module_utils._text import to_text
 from ansible.module_utils.network.common.utils import to_list
 from ansible.plugins.cliconf import CliconfBase
 
 
+def configure(func):
+    @wraps(func)
+    def wrapped(self, *args, **kwargs):
+        prompt = self._connection.get_prompt()
+        if not to_text(prompt, errors='surrogate_or_strict').strip().endswith('#'):
+            self.send_command('configure')
+        return func(self, *args, **kwargs)
+    return wrapped
+
+
 class Cliconf(CliconfBase):
 
     def get_text(self, ele, tag):
@@ -56,48 +69,148 @@ def get_device_info(self):
             device_info['network_os_hostname'] = match.group(1)
         return device_info
 
-    def get_config(self, source='running', format='text'):
+    def get_config(self, source='running', format='text', flags=None):
         if source != 'running':
-            return self.invalid_params("fetching configuration from %s is not supported" % source)
+            raise ValueError("fetching configuration from %s is not supported" % source)
+
+        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'])))
+
         if format == 'text':
             cmd = 'show configuration'
         else:
             cmd = 'show configuration | display %s' % format
+
+        cmd += ' '.join(to_list(flags))
+        cmd = cmd.strip()
         return self.send_command(cmd)
 
-    def edit_config(self, command):
-        for cmd in chain(['configure'], to_list(command)):
-            self.send_command(cmd)
+    @configure
+    def edit_config(self, candidate=None, commit=True, replace=None, comment=None):
+
+        operations = self.get_device_operations()
+        self.check_edit_config_capabiltiy(operations, candidate, commit, replace, comment)
+
+        resp = {}
+        results = []
+        requests = []
+
+        if replace:
+            candidate = 'load replace {0}'.format(replace)
+
+        for line in to_list(candidate):
+            if not isinstance(line, collections.Mapping):
+                line = {'command': line}
+            cmd = line['command']
+            results.append(self.send_command(**line))
+            requests.append(cmd)
+
+        diff = self.compare_configuration()
+        if diff:
+            resp['diff'] = diff
+
+        if commit:
+            self.commit(comment=comment)
+        else:
+            self.discard_changes()
+
+        resp['request'] = requests
+        resp['response'] = results
+        return resp
 
-    def get(self, command, prompt=None, answer=None, sendonly=False):
+    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, *args, **kwargs):
-        """Execute commit command on remote device.
-        :kwargs:
-            comment: Optional commit description.
+    @configure
+    def commit(self, comment=None, confirmed=False, at_time=None, synchronize=False):
+        """
+        Execute commit command on remote device.
+        :param comment: Comment to be associated with commit
+        :param confirmed: Boolean flag to indicate if the previous commit should confirmed
+        :param at_time: Time at which to activate configuration changes
+        :param synchronize: Boolean flag to indicate if commit should synchronize on remote peers
+        :return: Command response received from device
         """
-        comment = kwargs.get('comment', None)
         command = 'commit'
         if comment:
             command += ' comment {0}'.format(comment)
+        if confirmed:
+            command += ' confirmed'
+        if at_time:
+            command += ' {0}'.format(at_time)
+        if synchronize:
+            command += ' peers-synchronize'
+
         command += ' and-quit'
         return self.send_command(command)
 
+    @configure
     def discard_changes(self):
         command = 'rollback 0'
         for cmd in chain(to_list(command), 'exit'):
             self.send_command(cmd)
 
+    @configure
+    def validate(self):
+        return self.send_command('commit check')
+
+    @configure
+    def compare_configuration(self, rollback_id=None):
+        command = 'show | compare'
+        if rollback_id is not None:
+            command += ' rollback %s' % int(rollback_id)
+        resp = self.send_command(command)
+        return resp
+
+    def get_diff(self, rollback_id=None):
+        return self.compare_configuration(rollback_id=rollback_id)
+
+    def get_device_operations(self):
+        return {
+            'supports_diff_replace': False,
+            'supports_commit': True,
+            'supports_rollback': True,
+            'supports_defaults': False,
+            'supports_onbox_diff': True,
+            'supports_commit_comment': True,
+            'supports_multiline_delimiter': False,
+            'supports_diff_match': False,
+            'supports_diff_ignore_lines': False,
+            'supports_generate_diff': False,
+            'supports_replace': True
+        }
+
+    def get_option_values(self):
+        return {
+            'format': ['text', 'set', 'xml', 'json'],
+            'diff_match': [],
+            'diff_replace': [],
+            'output': ['text', 'set', 'xml', 'json']
+        }
+
     def get_capabilities(self):
         result = dict()
-        result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes']
+        result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes', 'run_commands', 'compare_configuration', 'validate', 'get_diff']
         result['network_api'] = 'cliconf'
         result['device_info'] = self.get_device_info()
+        result['device_operations'] = self.get_device_operations()
+        result.update(self.get_option_values())
         return json.dumps(result)
 
-    def compare_configuration(self, rollback_id=None):
-        command = 'show | compare'
-        if rollback_id is not None:
-            command += ' rollback %s' % int(rollback_id)
-        return self.send_command(command)
+    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('| display json'):
+            cmd = '%s | display json' % command
+        elif output == 'xml' and not command.endswith('| display xml'):
+            cmd = '%s | display xml' % command
+        elif output == 'text' and (command.endswith('| display json') or command.endswith('| display xml')):
+            cmd = command.rsplit('|', 1)[0]
+        else:
+            cmd = command
+        return cmd

lib/ansible/plugins/cliconf/nxos.py

@@ -23,8 +23,6 @@
 import json
 import re
 
-from itertools import chain
-
 from ansible.errors import AnsibleConnectionFailure
 from ansible.module_utils._text import to_bytes, to_text
 from ansible.module_utils.connection import ConnectionError
@@ -138,7 +136,7 @@ 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)
+            raise ValueError("fetching configuration from %s is not supported" % source)
 
         cmd = 'show {0} '.format(lookup[source])
         if format and format is not 'text':