Refactor junos modules to Use netconf and cliconf plugins (#32621)

* Fix junos integration test fixes as per connection refactor (#33050)

Refactor netconf connection plugin to work with netconf plugin

* Fix junos integration test fixes as per connection refactor (#33050)

Refactor netconf connection plugin to work with netconf plugin
Fix CI failure
Fix unit test failure
Fix review comments

lib/ansible/module_utils/connection.py

@@ -34,8 +34,7 @@ import traceback
 import uuid
 
 from functools import partial
-
-from ansible.module_utils._text import to_bytes, to_native, to_text
+from ansible.module_utils._text import to_bytes, to_text
 from ansible.module_utils.six import iteritems
 
 
@@ -77,7 +76,7 @@ def request_builder(method, *args, **kwargs):
     reqid = str(uuid.uuid4())
     req = {'jsonrpc': '2.0', 'method': method, 'id': reqid}
 
-    params = list(args) or kwargs or None
+    params = args or kwargs or None
     if params:
         req['params'] = params
 
@@ -92,7 +91,7 @@ class ConnectionError(Exception):
             setattr(self, k, v)
 
 
-class Connection:
+class Connection(object):
 
     def __init__(self, socket_path):
         if socket_path is None:
@@ -107,15 +106,8 @@ class Connection:
                 raise AttributeError("'%s' object has no attribute '%s'" % (self.__class__.__name__, name))
             return partial(self.__rpc__, name)
 
-    def __rpc__(self, name, *args, **kwargs):
-        """Executes the json-rpc and returns the output received
-           from remote device.
-           :name: rpc method to be executed over connection plugin that implements jsonrpc 2.0
-           :args: Ordered list of params passed as arguments to rpc method
-           :kwargs: Dict of valid key, value pairs passed as arguments to rpc method
+    def _exec_jsonrpc(self, name, *args, **kwargs):
 
-           For usage refer the respective connection plugin docs.
-        """
         req = request_builder(name, *args, **kwargs)
         reqid = req['id']
 
@@ -133,6 +125,20 @@ class Connection:
         if response['id'] != reqid:
             raise ConnectionError('invalid json-rpc id received')
 
+        return response
+
+    def __rpc__(self, name, *args, **kwargs):
+        """Executes the json-rpc and returns the output received
+           from remote device.
+           :name: rpc method to be executed over connection plugin that implements jsonrpc 2.0
+           :args: Ordered list of params passed as arguments to rpc method
+           :kwargs: Dict of valid key, value pairs passed as arguments to rpc method
+
+           For usage refer the respective connection plugin docs.
+        """
+
+        response = self._exec_jsonrpc(name, *args, **kwargs)
+
         if 'error' in response:
             err = response.get('error')
             msg = err.get('data') or err['message']

lib/ansible/module_utils/junos.py

@@ -17,13 +17,13 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 import collections
+import json
 from contextlib import contextmanager
 from copy import deepcopy
 
 from ansible.module_utils.basic import env_fallback, return_values
-from ansible.module_utils.netconf import send_request, children
-from ansible.module_utils.netconf import discard_changes, validate
-from ansible.module_utils.six import string_types
+from ansible.module_utils.connection import Connection
+from ansible.module_utils.netconf import NetconfConnection
 from ansible.module_utils._text import to_text
 
 try:
@@ -45,7 +45,7 @@ junos_provider_spec = {
     'password': dict(fallback=(env_fallback, ['ANSIBLE_NET_PASSWORD']), no_log=True),
     'ssh_keyfile': dict(fallback=(env_fallback, ['ANSIBLE_NET_SSH_KEYFILE']), type='path'),
     'timeout': dict(type='int'),
-    'transport': dict()
+    'transport': dict(default='netconf', choices=['cli', 'netconf'])
 }
 junos_argument_spec = {
     'provider': dict(type='dict', options=junos_provider_spec),
@@ -66,8 +66,29 @@ def get_provider_argspec():
     return junos_provider_spec
 
 
-def check_args(module, warnings):
-    pass
+def get_connection(module):
+    if hasattr(module, '_junos_connection'):
+        return module._junos_connection
+
+    capabilities = get_capabilities(module)
+    network_api = capabilities.get('network_api')
+    if network_api == 'cliconf':
+        module._junos_connection = Connection(module._socket_path)
+    elif network_api == 'netconf':
+        module._junos_connection = NetconfConnection(module._socket_path)
+    else:
+        module.fail_json(msg='Invalid connection type %s' % network_api)
+
+    return module._junos_connection
+
+
+def get_capabilities(module):
+    if hasattr(module, '_junos_capabilities'):
+        return module._junos_capabilities
+
+    capabilities = Connection(module._socket_path).get_capabilities()
+    module._junos_capabilities = json.loads(capabilities)
+    return module._junos_capabilities
 
 
 def _validate_rollback_id(module, value):
@@ -96,73 +117,58 @@ def load_configuration(module, candidate=None, action='merge', rollback=None, fo
     if action == 'set' and not format == 'text':
         module.fail_json(msg='format must be text when action is set')
 
+    conn = get_connection(module)
     if rollback is not None:
         _validate_rollback_id(module, rollback)
-        xattrs = {'rollback': str(rollback)}
+        obj = Element('load-configuration', {'rollback': str(rollback)})
+        conn.execute_rpc(tostring(obj))
     else:
-        xattrs = {'action': action, 'format': format}
-
-    obj = Element('load-configuration', xattrs)
-
-    if candidate is not None:
-        lookup = {'xml': 'configuration', 'text': 'configuration-text',
-                  'set': 'configuration-set', 'json': 'configuration-json'}
-
-        if action == 'set':
-            cfg = SubElement(obj, 'configuration-set')
-        else:
-            cfg = SubElement(obj, lookup[format])
-
-        if isinstance(candidate, string_types):
-            if format == 'xml':
-                cfg.append(fromstring(candidate))
-            else:
-                cfg.text = to_text(candidate, encoding='latin-1')
-        else:
-            cfg.append(candidate)
-    return send_request(module, obj)
+        return conn.load_configuration(config=candidate, action=action, format=format)
 
 
-def get_configuration(module, compare=False, format='xml', rollback='0'):
+def get_configuration(module, compare=False, format='xml', rollback='0', filter=None):
     if format not in CONFIG_FORMATS:
         module.fail_json(msg='invalid config format specified')
-    xattrs = {'format': format}
+
+    conn = get_connection(module)
     if compare:
+        xattrs = {'format': format}
         _validate_rollback_id(module, rollback)
         xattrs['compare'] = 'rollback'
         xattrs['rollback'] = str(rollback)
-    return send_request(module, Element('get-configuration', xattrs))
+        reply = conn.execute_rpc(tostring(Element('get-configuration', xattrs)))
+    else:
+        reply = conn.get_configuration(format=format, filter=filter)
+
+    return reply
 
 
-def commit_configuration(module, confirm=False, check=False, comment=None, confirm_timeout=None):
-    obj = Element('commit-configuration')
-    if confirm:
-        SubElement(obj, 'confirmed')
+def commit_configuration(module, confirm=False, check=False, comment=None, confirm_timeout=None, synchronize=False,
+                         at_time=None, exit=False):
+    conn = get_connection(module)
     if check:
-        SubElement(obj, 'check')
-    if comment:
-        subele = SubElement(obj, 'log')
-        subele.text = str(comment)
-    if confirm_timeout:
-        subele = SubElement(obj, 'confirm-timeout')
-        subele.text = str(confirm_timeout)
-    return send_request(module, obj)
-
-
-def command(module, command, format='text', rpc_only=False):
-    xattrs = {'format': format}
+        reply = conn.validate()
+    else:
+        reply = conn.commit(confirmed=confirm, timeout=confirm_timeout, comment=comment, synchronize=synchronize, at_time=at_time)
+
+    return reply
+
+
+def command(module, cmd, format='text', rpc_only=False):
+    conn = get_connection(module)
     if rpc_only:
-        command += ' | display xml rpc'
-        xattrs['format'] = 'text'
-    return send_request(module, Element('command', xattrs, text=command))
+        cmd += ' | display xml rpc'
+    return conn.command(command=cmd, format=format)
 
 
 def lock_configuration(x):
-    return send_request(x, Element('lock-configuration'))
+    conn = get_connection(x)
+    return conn.lock()
 
 
 def unlock_configuration(x):
-    return send_request(x, Element('unlock-configuration'))
+    conn = get_connection(x)
+    return conn.unlock()
 
 
 @contextmanager
@@ -174,8 +180,12 @@ def locked_config(module):
         unlock_configuration(module)
 
 
-def get_diff(module, rollback='0'):
+def discard_changes(module, exit=False):
+    conn = get_connection(module)
+    return conn.discard_changes(exit=exit)
+
 
+def get_diff(module, rollback='0'):
     reply = get_configuration(module, compare=True, format='text', rollback=rollback)
     # if warning is received from device diff is empty.
     if isinstance(reply, list):
@@ -187,7 +197,7 @@ def get_diff(module, rollback='0'):
 
 
 def load_config(module, candidate, warnings, action='merge', format='xml'):
-
+    get_connection(module)
     if not candidate:
         return
 
@@ -198,8 +208,7 @@ def load_config(module, candidate, warnings, action='merge', format='xml'):
     if isinstance(reply, list):
         warnings.extend(reply)
 
-    validate(module)
-
+    module._junos_connection.validate()
     return get_diff(module)
 
 

lib/ansible/module_utils/netconf.py

@@ -25,89 +25,63 @@
 # LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #
-from contextlib import contextmanager
-
-from ansible.module_utils._text import to_bytes, to_text
-from ansible.module_utils.connection import exec_command
+from ansible.module_utils._text import to_text, to_native
+from ansible.module_utils.connection import Connection, ConnectionError
 
 try:
-    from lxml.etree import Element, SubElement, fromstring, tostring
+    from lxml.etree import Element, fromstring
 except ImportError:
-    from xml.etree.ElementTree import Element, SubElement, fromstring, tostring
+    from xml.etree.ElementTree import Element, fromstring
 
 NS_MAP = {'nc': "urn:ietf:params:xml:ns:netconf:base:1.0"}
 
 
-def send_request(module, obj, check_rc=True, ignore_warning=True):
-    request = to_text(tostring(obj), errors='surrogate_or_strict')
-    rc, out, err = exec_command(module, request)
-    if rc != 0 and check_rc:
-        error_root = fromstring(err)
-        fake_parent = Element('root')
-        fake_parent.append(error_root)
-
-        error_list = fake_parent.findall('.//nc:rpc-error', NS_MAP)
-        if not error_list:
-            module.fail_json(msg=str(err))
-
-        warnings = []
-        for rpc_error in error_list:
-            message = rpc_error.find('./nc:error-message', NS_MAP).text
-            severity = rpc_error.find('./nc:error-severity', NS_MAP).text
-
-            if severity == 'warning' and ignore_warning:
-                warnings.append(message)
-            else:
-                module.fail_json(msg=str(err))
-        return warnings
-    return fromstring(to_bytes(out, errors='surrogate_or_strict'))
-
-
-def children(root, iterable):
-    for item in iterable:
-        try:
-            ele = SubElement(ele, item)
-        except NameError:
-            ele = SubElement(root, item)
-
-
-def lock(module, target='candidate'):
-    obj = Element('lock')
-    children(obj, ('target', target))
-    return send_request(module, obj)
-
+def exec_rpc(module, *args, **kwargs):
+    connection = NetconfConnection(module._socket_path)
+    return connection.execute_rpc(*args, **kwargs)
 
-def unlock(module, target='candidate'):
-    obj = Element('unlock')
-    children(obj, ('target', target))
-    return send_request(module, obj)
 
+class NetconfConnection(Connection):
 
-def commit(module):
-    return send_request(module, Element('commit'))
+    def __init__(self, socket_path):
+        super(NetconfConnection, self).__init__(socket_path)
 
+    def __rpc__(self, name, *args, **kwargs):
+        """Executes the json-rpc and returns the output received
+           from remote device.
+           :name: rpc method to be executed over connection plugin that implements jsonrpc 2.0
+           :args: Ordered list of params passed as arguments to rpc method
+           :kwargs: Dict of valid key, value pairs passed as arguments to rpc method
 
-def discard_changes(module):
-    return send_request(module, Element('discard-changes'))
+           For usage refer the respective connection plugin docs.
+        """
+        self.check_rc = kwargs.pop('check_rc', True)
+        self.ignore_warning = kwargs.pop('ignore_warning', True)
 
+        response = self._exec_jsonrpc(name, *args, **kwargs)
+        if 'error' in response:
+            rpc_error = response['error'].get('data')
+            return self.parse_rpc_error(to_native(rpc_error, errors='surrogate_then_replace'))
 
-def validate(module):
-    obj = Element('validate')
-    children(obj, ('source', 'candidate'))
-    return send_request(module, obj)
+        return fromstring(to_native(response['result'], errors='surrogate_then_replace'))
 
+    def parse_rpc_error(self, rpc_error):
+        if self.check_rc:
+            error_root = fromstring(rpc_error)
+            root = Element('root')
+            root.append(error_root)
 
-def get_config(module, source='running', filter=None):
-    obj = Element('get-config')
-    children(obj, ('source', source))
-    children(obj, ('filter', filter))
-    return send_request(module, obj)
+            error_list = root.findall('.//nc:rpc-error', NS_MAP)
+            if not error_list:
+                raise ConnectionError(to_text(rpc_error, errors='surrogate_then_replace'))
 
+            warnings = []
+            for error in error_list:
+                message = error.find('./nc:error-message', NS_MAP).text
+                severity = error.find('./nc:error-severity', NS_MAP).text
 
-@contextmanager
-def locked_config(module):
-    try:
-        lock(module)
-        yield
-    finally:
-        unlock(module)
+                if severity == 'warning' and self.ignore_warning:
+                    warnings.append(message)
+                else:
+                    raise ConnectionError(to_text(rpc_error, errors='surrogate_then_replace'))
+            return warnings

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

@@ -102,7 +102,7 @@ diff.prepared:
 import collections
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
 
@@ -141,8 +141,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -171,11 +171,11 @@ import re
 import shlex
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.junos import junos_argument_spec, check_args, get_configuration
+from ansible.module_utils.netconf import exec_rpc
+from ansible.module_utils.junos import junos_argument_spec, get_configuration, get_connection, get_capabilities
 from ansible.module_utils.netcli import Conditional, FailedConditionalError
-from ansible.module_utils.netconf import send_request
 from ansible.module_utils.six import string_types, iteritems
-from ansible.module_utils.connection import Connection
+
 
 try:
     from lxml.etree import Element, SubElement, tostring
@@ -203,7 +203,6 @@ def to_lines(stdout):
 def rpc(module, items):
 
     responses = list()
-
     for item in items:
         name = item['name']
         xattrs = item['xattrs']
@@ -241,7 +240,7 @@ def rpc(module, items):
         if fetch_config:
             reply = get_configuration(module, format=xattrs['format'])
         else:
-            reply = send_request(module, element, ignore_warning=False)
+            reply = exec_rpc(module, tostring(element), ignore_warning=False)
 
         if xattrs['format'] == 'text':
             if fetch_config:
@@ -365,16 +364,24 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
+    conn = get_connection(module)
+    capabilities = get_capabilities(module)
 
-    if module.params['provider'] and module.params['provider']['transport'] == 'cli':
+    if capabilities.get('network_api') == 'cliconf':
         if any((module.params['wait_for'], module.params['match'], module.params['rpcs'])):
             module.warn('arguments wait_for, match, rpcs are not supported when using transport=cli')
         commands = module.params['commands']
-        conn = Connection(module)
+
         output = list()
+        display = module.params['display']
         for cmd in commands:
-            output.append(conn.get(cmd))
+            # if display format is not mentioned in command, add the display format
+            # from the modules params
+            if ('display json' not in cmd) and ('display xml' not in cmd):
+                if display and display != 'text':
+                    cmd += ' | display {0}'.format(display)
+            output.append(conn.get(command=cmd))
+
         lines = [out.split('\n') for out in output]
         result = {'changed': False, 'stdout': output, 'stdout_lines': lines}
         module.exit_json(**result)

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

@@ -189,11 +189,10 @@ import re
 import json
 
 from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.netconf import exec_rpc
 from ansible.module_utils.junos import get_diff, load_config, get_configuration
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
-from ansible.module_utils.junos import junos_argument_spec, load_configuration
-from ansible.module_utils.junos import check_args as junos_check_args
-from ansible.module_utils.netconf import send_request
+from ansible.module_utils.junos import junos_argument_spec, load_configuration, get_connection, tostring
 from ansible.module_utils.six import string_types
 from ansible.module_utils._text import to_native
 
@@ -217,14 +216,12 @@ DEFAULT_COMMENT = 'configured by junos_config'
 
 
 def check_args(module, warnings):
-    junos_check_args(module, warnings)
-
     if module.params['replace'] is not None:
         module.fail_json(msg='argument replace is deprecated, use update')
 
 
-def zeroize(ele):
-    return send_request(ele, Element('request-system-zeroize'))
+def zeroize(module):
+    return exec_rpc(module, tostring(Element('request-system-zeroize')), ignore_warning=False)
 
 
 def rollback(ele, id='0'):

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

@@ -78,10 +78,10 @@ ansible_facts:
   type: dict
 """
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.junos import junos_argument_spec, check_args, get_param
-from ansible.module_utils.junos import get_configuration
+from ansible.module_utils.netconf import exec_rpc
+from ansible.module_utils.junos import junos_argument_spec, get_param
+from ansible.module_utils.junos import get_configuration, get_connection
 from ansible.module_utils.pycompat24 import get_exception
-from ansible.module_utils.netconf import send_request
 from ansible.module_utils.six import iteritems
 
 
@@ -117,7 +117,7 @@ class FactsBase(object):
         return str(output.text).strip()
 
     def rpc(self, rpc):
-        return send_request(self.module, Element(rpc))
+        return exec_rpc(self.module, tostring(Element(rpc)))
 
     def get_text(self, ele, tag):
         try:
@@ -222,7 +222,7 @@ class Interfaces(FactsBase):
     def populate(self):
         ele = Element('get-interface-information')
         SubElement(ele, 'detail')
-        reply = send_request(self.module, ele)
+        reply = exec_rpc(self.module, tostring(ele))
 
         interfaces = {}
 
@@ -309,9 +309,8 @@ def main():
     module = AnsibleModule(argument_spec=argument_spec,
                            supports_check_mode=True)
 
+    get_connection(module)
     warnings = list()
-    check_args(module, warnings)
-
     gather_subset = module.params['gather_subset']
     ofacts = False
 

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

@@ -185,10 +185,10 @@ from copy import deepcopy
 from time import sleep
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.netconf import send_request
+from ansible.module_utils.netconf import exec_rpc
 from ansible.module_utils.network_common import remove_default_spec
 from ansible.module_utils.network_common import conditional
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config, to_param_list
 
@@ -260,8 +260,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:
@@ -338,8 +336,7 @@ def main():
         if result['changed']:
             sleep(item.get('delay'))
 
-        reply = send_request(module, element, ignore_warning=False)
-
+        reply = exec_rpc(module, tostring(element), ignore_warning=False)
         if state in ('up', 'down'):
             admin_status = reply.xpath('interface-information/physical-interface/admin-status')
             if not admin_status or not conditional(state, admin_status[0].text.strip()):
@@ -361,7 +358,7 @@ def main():
                 intf_name = SubElement(element, 'interface-device')
                 intf_name.text = item.get('name')
 
-                reply = send_request(module, element, ignore_warning=False)
+                reply = exec_rpc(module, tostring(element), ignore_warning=False)
                 have_host = [item.text for item in reply.xpath('lldp-neighbors-information/lldp-neighbor-information/lldp-remote-system-name')]
                 have_port = [item.text for item in reply.xpath('lldp-neighbors-information/lldp-neighbor-information/lldp-remote-port-id')]
 

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

@@ -103,7 +103,7 @@ from copy import deepcopy
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network_common import remove_default_spec
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config, to_param_list
 
@@ -149,8 +149,6 @@ def main():
                            required_one_of=required_one_of)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -161,7 +161,7 @@ from copy import deepcopy
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network_common import remove_default_spec
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele, to_param_list
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config, get_configuration
 
@@ -290,8 +290,6 @@ def main():
                            mutually_exclusive=mutually_exclusive)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -104,7 +104,7 @@ diff.prepared:
 import collections
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
 
@@ -156,8 +156,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -93,7 +93,7 @@ diff.prepared:
 import collections
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
 
@@ -120,8 +120,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -139,7 +139,7 @@ from copy import deepcopy
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network_common import remove_default_spec
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele, to_param_list
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
 
@@ -214,8 +214,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -71,8 +71,7 @@ commands:
 import re
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.connection import exec_command
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec, get_connection
 from ansible.module_utils.junos import commit_configuration, discard_changes
 from ansible.module_utils.network_common import to_list
 from ansible.module_utils.six import iteritems
@@ -103,10 +102,10 @@ def parse_port(config):
 
 
 def map_config_to_obj(module):
-    cmd = 'show configuration system services netconf'
-    rc, out, err = exec_command(module, cmd)
-    if rc != 0:
-        module.fail_json(msg='unable to retrieve current config', stderr=err)
+    conn = get_connection(module)
+    out = conn.get(command='show configuration system services netconf')
+    if out is None:
+        module.fail_json(msg='unable to retrieve current config')
     config = str(out).strip()
 
     obj = {'state': 'absent'}
@@ -139,23 +138,16 @@ def map_params_to_obj(module):
 
 
 def load_config(module, config, commit=False):
+    conn = get_connection(module)
 
-    exec_command(module, 'configure')
-
-    for item in to_list(config):
-        rc, out, err = exec_command(module, item)
-        if rc != 0:
-            module.fail_json(msg=str(err))
-
-    exec_command(module, 'top')
-    rc, diff, err = exec_command(module, 'show | compare')
-
+    conn.edit_config(to_list(config) + ['top'])
+    diff = conn.compare_configuration()
     if diff:
         if commit:
-            exec_command(module, 'commit and-quit')
+            commit_configuration(module)
+
         else:
-            for cmd in ['rollback 0', 'exit']:
-                exec_command(module, cmd)
+            discard_changes(module)
 
     return str(diff).strip()
 
@@ -174,8 +166,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False, 'warnings': warnings}
 
     want = map_params_to_obj(module)

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

@@ -95,8 +95,8 @@ output_lines:
   type: list
 """
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.junos import junos_argument_spec, check_args
-from ansible.module_utils.netconf import send_request
+from ansible.module_utils.netconf import exec_rpc
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.six import iteritems
 
 USE_PERSISTENT_CONNECTION = True
@@ -123,8 +123,6 @@ def main():
                            supports_check_mode=False)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False, 'warnings': warnings}
 
     rpc = str(module.params['rpc']).replace('_', '-')
@@ -154,7 +152,7 @@ def main():
             if value is not True:
                 child.text = value
 
-    reply = send_request(module, element)
+    reply = exec_rpc(module, tostring(element), ignore_warning=False)
 
     result['xml'] = str(tostring(reply))
 

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

@@ -135,7 +135,7 @@ from copy import deepcopy
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network_common import remove_default_spec
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele, to_param_list
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
 
@@ -183,8 +183,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -106,7 +106,7 @@ diff.prepared:
 import collections
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
 
@@ -151,8 +151,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -147,8 +147,7 @@ from copy import deepcopy
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network_common import remove_default_spec
-from ansible.module_utils.netconf import send_request
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec, get_connection
 from ansible.module_utils.junos import commit_configuration, discard_changes
 from ansible.module_utils.junos import load_config, locked_config
 from ansible.module_utils.six import iteritems
@@ -167,7 +166,8 @@ def handle_purge(module, want):
     element = Element('system')
     login = SubElement(element, 'login')
 
-    reply = send_request(module, Element('get-configuration'), ignore_warning=False)
+    conn = get_connection(module)
+    reply = conn.execute_rpc(tostring(Element('get-configuration')), ignore_warning=False)
     users = reply.xpath('configuration/system/login/user/name')
     if users:
         for item in users:
@@ -310,8 +310,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False, 'warnings': warnings}
 
     want = map_params_to_obj(module)

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

@@ -112,7 +112,7 @@ from copy import deepcopy
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network_common import remove_default_spec
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele, to_param_list
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
 
@@ -173,8 +173,6 @@ def main():
                            supports_check_mode=True)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

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

@@ -168,7 +168,7 @@ from copy import deepcopy
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network_common import remove_default_spec
-from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele, to_param_list
 from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config
 
@@ -216,8 +216,6 @@ def main():
                            mutually_exclusive=mutually_exclusive)
 
     warnings = list()
-    check_args(module, warnings)
-
     result = {'changed': False}
 
     if warnings:

lib/ansible/plugins/action/junos.py

@@ -87,7 +87,7 @@ class ActionModule(_ActionModule):
 
             conn = Connection(socket_path)
             out = conn.get_prompt()
-            while to_text(out, errors='surrogate_then_replace').strip().endswith(')#'):
+            while to_text(out, errors='surrogate_then_replace').strip().endswith('#'):
                 display.vvvv('wrong context, sending exit to device', self._play_context.remote_addr)
                 conn.send_command('exit')
                 out = conn.get_prompt()

lib/ansible/plugins/cliconf/__init__.py

@@ -89,7 +89,9 @@ class CliconfBase(with_metaclass(ABCMeta, object)):
         self._connection = connection
 
     def _alarm_handler(self, signum, frame):
-        raise AnsibleConnectionFailure('timeout waiting for command to complete')
+        """Alarm handler raised in case of command timeout """
+        display.display('closing shell due to command timeout (%s seconds).' % self._connection._play_context.timeout, log_only=True)
+        self.close()
 
     def send_command(self, command, prompt=None, answer=None, sendonly=False):
         """Executes a cli command and returns the results
@@ -97,10 +99,9 @@ class CliconfBase(with_metaclass(ABCMeta, object)):
         the results to the caller.  The command output will be returned as a
         string
         """
-        timeout = self._connection._play_context.timeout or 30
-        signal.signal(signal.SIGALRM, self._alarm_handler)
-        signal.alarm(timeout)
-        display.display("command: %s" % command, log_only=True)
+        if not signal.getsignal(signal.SIGALRM):
+            signal.signal(signal.SIGALRM, self._alarm_handler)
+        signal.alarm(self._connection._play_context.timeout)
         resp = self._connection.send(command, prompt, answer, sendonly)
         signal.alarm(0)
         return resp

lib/ansible/plugins/cliconf/junos.py

@@ -19,11 +19,9 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-import re
 import json
-
+import re
 from itertools import chain
-from xml.etree.ElementTree import fromstring
 
 from ansible.module_utils._text import to_bytes, to_text
 from ansible.module_utils.network_common import to_list
@@ -39,49 +37,66 @@ class Cliconf(CliconfBase):
             pass
 
     def get_device_info(self):
-        device_info = {}
-
+        device_info = dict()
         device_info['network_os'] = 'junos'
-        reply = self.get(b'show version | display xml')
-        data = fromstring(to_text(reply, errors='surrogate_then_replace').strip())
 
-        sw_info = data.find('.//software-information')
+        reply = self.get(command='show version')
+        data = to_text(reply, errors='surrogate_or_strict').strip()
+
+        match = re.search(r'Junos: (\S+)', data)
+        if match:
+            device_info['network_os_version'] = match.group(1)
 
-        device_info['network_os_version'] = self.get_text(sw_info, 'junos-version')
-        device_info['network_os_hostname'] = self.get_text(sw_info, 'host-name')
-        device_info['network_os_model'] = self.get_text(sw_info, 'product-model')
+        match = re.search(r'Model: (\S+)', data, re.M)
+        if match:
+            device_info['network_os_model'] = match.group(1)
 
+        match = re.search(r'Hostname: (\S+)', data, re.M)
+        if match:
+            device_info['network_os_hostname'] = match.group(1)
         return device_info
 
     def get_config(self, source='running', format='text'):
         if source != 'running':
             return self.invalid_params("fetching configuration from %s is not supported" % source)
         if format == 'text':
-            cmd = b'show configuration'
+            cmd = 'show configuration'
         else:
-            cmd = b'show configuration | display %s' % format
-        return self.send_command(to_bytes(cmd, errors='surrogate_or_strict'))
+            cmd = 'show configuration | display %s' % format
+        return self.send_command(cmd)
 
     def edit_config(self, command):
-        for cmd in chain([b'configure'], to_list(command)):
+        for cmd in chain(['configure'], to_list(command)):
             self.send_command(cmd)
 
     def get(self, *args, **kwargs):
-        return self.send_command(*args, **kwargs)
+        command = kwargs.get('command')
+        return self.send_command(command)
 
-    def commit(self, comment=None):
+    def commit(self, *args, **kwargs):
+        comment = kwargs.get('comment', None)
+        command = b'commit'
         if comment:
-            command = b'commit comment {0}'.format(comment)
-        else:
-            command = b'commit'
-        self.send_command(command)
-
-    def discard_changes(self):
-        self.send_command(b'rollback')
+            command += b' comment {0}'.format(comment)
+        command += b' and-quit'
+        return self.send_command(command)
+
+    def discard_changes(self, rollback_id=None):
+        command = b'rollback'
+        if rollback_id is not None:
+            command += b' %s' % int(rollback_id)
+        for cmd in chain(to_list(command), b'exit'):
+            self.send_command(cmd)
 
     def get_capabilities(self):
-        result = {}
+        result = dict()
         result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes']
         result['network_api'] = 'cliconf'
         result['device_info'] = self.get_device_info()
         return json.dumps(result)
+
+    def compare_configuration(self, rollback_id=None):
+        command = b'show | compare'
+        if rollback_id is not None:
+            command += b' rollback %s' % int(rollback_id)
+        return self.send_command(command)

lib/ansible/plugins/connection/netconf.py

@@ -71,10 +71,11 @@ DOCUMENTATION = """
 
 import os
 import logging
+import json
 
 from ansible import constants as C
 from ansible.errors import AnsibleConnectionFailure, AnsibleError
-from ansible.module_utils._text import to_bytes, to_native
+from ansible.module_utils._text import to_bytes, to_native, to_text
 from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE
 from ansible.plugins.loader import netconf_loader
 from ansible.plugins.connection import ConnectionBase, ensure_connect
@@ -110,11 +111,20 @@ class Connection(ConnectionBase):
         self._network_os = self._play_context.network_os or 'default'
         display.display('network_os is set to %s' % self._network_os, log_only=True)
 
+        self._netconf = None
         self._manager = None
         self._connected = False
 
         self._local = LocalConnection(play_context, new_stdin, *args, **kwargs)
 
+    def __getattr__(self, name):
+        try:
+            return self.__dict__[name]
+        except KeyError:
+            if name.startswith('_'):
+                raise AttributeError("'%s' object has no attribute '%s'" % (self.__class__.__name__, name))
+            return getattr(self._netconf, name)
+
     def exec_command(self, request, in_data=None, sudoable=True):
         """Sends the request to the node and returns the reply
         The method accepts two forms of request.  The first form is as a byte
@@ -131,7 +141,8 @@ class Connection(ConnectionBase):
             try:
                 reply = self._manager.rpc(request)
             except RPCError as exc:
-                return to_xml(exc.xml)
+                error = self.internal_error(data=to_text(to_xml(exc.xml), errors='surrogate_or_strict'))
+                return json.dumps(error)
 
             return reply.data_xml
         else:

lib/ansible/plugins/netconf/__init__.py

@@ -22,8 +22,15 @@ __metaclass__ = type
 from abc import ABCMeta, abstractmethod
 from functools import wraps
 
+from ansible.errors import AnsibleError
 from ansible.module_utils.six import with_metaclass
 
+try:
+    from ncclient.operations import RPCError
+    from ncclient.xml_ import to_xml
+except ImportError:
+    raise AnsibleError("ncclient is not installed")
+
 
 def ensure_connected(func):
     @wraps(func)
@@ -115,7 +122,10 @@ class NetconfBase(with_metaclass(ABCMeta, object)):
             :error_option: if specified must be one of { `"stop-on-error"`, `"continue-on-error"`, `"rollback-on-error"` }
             The `"rollback-on-error"` *error_option* depends on the `:rollback-on-error` capability.
         """
-        return self.m.get_config(*args, **kwargs).data_xml
+        try:
+            return self.m.edit_config(*args, **kwargs).data_xml
+        except RPCError as exc:
+            raise Exception(to_xml(exc.xml))
 
     @ensure_connected
     def validate(self, *args, **kwargs):
@@ -146,7 +156,7 @@ class NetconfBase(with_metaclass(ABCMeta, object)):
         """Release a configuration lock, previously obtained with the lock operation.
         :target: is the name of the configuration datastore to unlock
         """
-        return self.m.lock(*args, **kwargs).data_xml
+        return self.m.unlock(*args, **kwargs).data_xml
 
     @ensure_connected
     def discard_changes(self, *args, **kwargs):
@@ -166,7 +176,16 @@ class NetconfBase(with_metaclass(ABCMeta, object)):
         :confirmed: whether this is a confirmed commit
         :timeout: specifies the confirm timeout in seconds
         """
-        return self.m.commit(*args, **kwargs).data_xml
+        try:
+            return self.m.commit(*args, **kwargs).data_xml
+        except RPCError as exc:
+            raise Exception(to_xml(exc.xml))
+
+    @ensure_connected
+    def validate(self, *args, **kwargs):
+        """Validate the contents of the specified configuration.
+           :source: name of configuration data store"""
+        return self.m.validate(*args, **kwargs).data_xml
 
     @abstractmethod
     def get_capabilities(self, commands):

lib/ansible/plugins/netconf/junos.py

@@ -22,10 +22,8 @@ __metaclass__ = type
 import json
 import re
 
-from xml.etree.ElementTree import fromstring
-
 from ansible import constants as C
-from ansible.module_utils._text import to_text
+from ansible.module_utils._text import to_text, to_bytes
 from ansible.errors import AnsibleConnectionFailure, AnsibleError
 from ansible.plugins.netconf import NetconfBase
 from ansible.plugins.netconf import ensure_connected
@@ -48,11 +46,11 @@ class Netconf(NetconfBase):
             pass
 
     def get_device_info(self):
-        device_info = {}
-
+        device_info = dict()
         device_info['network_os'] = 'junos'
-        data = self.execute_rpc('get-software-information')
-        reply = fromstring(data)
+        ele = new_ele('get-software-information')
+        data = self.execute_rpc(to_xml(ele))
+        reply = to_ele(to_bytes(data, errors='surrogate_or_strict'))
         sw_info = reply.find('.//software-information')
 
         device_info['network_os_version'] = self.get_text(sw_info, 'junos-version')
@@ -62,11 +60,14 @@ class Netconf(NetconfBase):
         return device_info
 
     @ensure_connected
-    def execute_rpc(self, rpc):
+    def execute_rpc(self, name):
         """RPC to be execute on remote device
-           :rpc: Name of rpc in string format"""
-        name = new_ele(rpc)
-        return self.m.rpc(name).data_xml
+           :name: Name of rpc in string format"""
+        try:
+            obj = to_ele(to_bytes(name, errors='surrogate_or_strict'))
+            return self.m.rpc(obj).data_xml
+        except RPCError as exc:
+            raise Exception(to_xml(exc.xml))
 
     @ensure_connected
     def load_configuration(self, *args, **kwargs):
@@ -75,11 +76,21 @@ class Netconf(NetconfBase):
         :action: Action to be performed (merge, replace, override, update)
         :target: is the name of the configuration datastore being edited
         :config: is the configuration in string format."""
-        return self.m.load_configuration(*args, **kwargs).data_xml
+        if kwargs.get('config'):
+            kwargs['config'] = to_bytes(kwargs['config'], errors='surrogate_or_strict')
+            if kwargs.get('format', 'xml') == 'xml':
+                kwargs['config'] = to_ele(kwargs['config'])
+
+        try:
+            return self.m.load_configuration(*args, **kwargs).data_xml
+        except RPCError as exc:
+            raise Exception(to_xml(exc.xml))
 
     def get_capabilities(self):
-        result = {}
-        result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes', 'validate', 'lock', 'unlock', 'copy_copy']
+        result = dict()
+        result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes', 'validate', 'lock', 'unlock', 'copy_copy',
+                                               'execute_rpc', 'load_configuration', 'get_configuration', 'command',
+                                               'reboot', 'halt']
         result['network_api'] = 'netconf'
         result['device_info'] = self.get_device_info()
         result['server_capabilities'] = [c for c in self.m.server_capabilities]
@@ -112,3 +123,32 @@ class Netconf(NetconfBase):
 
         m.close_session()
         return guessed_os
+
+    @ensure_connected
+    def get_configuration(self, *args, **kwargs):
+        """Retrieve all or part of a specified configuration.
+           :format: format in configuration should be retrieved
+           :filter: specifies the portion of the configuration to retrieve
+           (by default entire configuration is retrieved)"""
+        return self.m.get_configuration(*args, **kwargs).data_xml
+
+    @ensure_connected
+    def compare_configuration(self, *args, **kwargs):
+        """Compare configuration
+           :rollback: rollback id"""
+        return self.m.compare_configuration(*args, **kwargs).data_xml
+
+    @ensure_connected
+    def halt(self):
+        """reboot the device"""
+        return self.m.halt().data_xml
+
+    @ensure_connected
+    def reboot(self):
+        """reboot the device"""
+        return self.m.reboot().data_xml
+
+    @ensure_connected
+    def halt(self):
+        """reboot the device"""
+        return self.m.halt().data_xml

lib/ansible/utils/jsonrpc.py

@@ -7,7 +7,6 @@ __metaclass__ = type
 import json
 import traceback
 
-from ansible import constants as C
 from ansible.module_utils._text import to_text
 from ansible.module_utils.six import binary_type
 

test/integration/targets/junos_command/tests/netconf_json/output.yaml

@@ -29,4 +29,33 @@
       - "result.stdout is defined"
       - "result.stdout_lines is defined"
 
+- name: get output for single command with cli transport
+  junos_command:
+    commands: ['show version | display json']
+    provider:
+      transport: cli
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+      - "result.stdout is defined"
+      - "result.stdout_lines is defined"
+
+- name: get output for multiple commands with cli transport
+  junos_command:
+    commands:
+      - show version
+      - show route
+    format: json
+    provider:
+      transport: cli
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+      - "result.stdout is defined"
+      - "result.stdout_lines is defined"
+
 - debug: msg="END netconf_json/output.yaml"

test/integration/targets/junos_command/tests/netconf_text/output.yaml

@@ -29,4 +29,34 @@
       - "result.stdout is defined"
       - "result.stdout_lines is defined"
 
+- name: get output for single command with cli transport
+  junos_command:
+    commands: show version
+    display: text
+    provider:
+      transport: cli
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+      - "result.stdout is defined"
+      - "result.stdout_lines is defined"
+
+- name: get output for multiple commands with cli transport
+  junos_command:
+    commands:
+      - show version
+      - show route
+    display: text
+    provider:
+      transport: cli
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+      - "result.stdout is defined"
+      - "result.stdout_lines is defined"
+
 - debug: msg="END netconf_text/output.yaml"

test/integration/targets/junos_command/tests/netconf_xml/output.yaml

@@ -29,4 +29,33 @@
       - "result.stdout is defined"
       - "result.stdout_lines is defined"
 
+- name: get output for single command with cli transport
+  junos_command:
+    commands: show version | display xml
+    provider:
+      transport: cli
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+      - "result.stdout is defined"
+      - "result.stdout_lines is defined"
+
+- name: get output for multiple commands with cli transport
+  junos_command:
+    commands:
+      - show version
+      - show route
+    display: xml
+    provider:
+      transport: cli
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+      - "result.stdout is defined"
+      - "result.stdout_lines is defined"
+
 - debug: msg="END netconf_xml/output.yaml"

test/integration/targets/junos_facts/tests/netconf/facts.yaml

@@ -87,6 +87,13 @@
     that:
       - "result.changed == false"
       - "'{{ inventory_hostname_short }}' == '{{ result['ansible_facts']['ansible_net_config']['configuration'][0]['system'][0]['host-name'][0]['data'] }}' "
+  when: ansible_net_version == "15.1X49-D15.4"
+
+- assert:
+    that:
+      - "result.changed == false"
+      - "'{{ inventory_hostname_short }}' == '{{ result['ansible_facts']['ansible_net_config']['configuration']['system']['host-name'] }}' "
+  when: ansible_net_version == "17.3R1.10"
 
 - name: Collect config facts from device in text format
   junos_facts:

test/integration/targets/junos_interface/tests/netconf/intent.yaml

@@ -9,17 +9,22 @@
 
 - name: Define interface name for vSRX
   set_fact:
-    name: pp0
+    intf_name: pp0
   when: result['ansible_facts']['ansible_net_model'] | search("vSRX*")
 
+- name: Define interface name for vsrx
+  set_fact:
+    intf_name: pp0
+  when: result['ansible_facts']['ansible_net_model'] | search("vsrx")
+
 - name: Define interface name for vQFX
   set_fact:
-    name: gr-0/0/0
+    intf_name: gr-0/0/0
   when: result['ansible_facts']['ansible_net_model'] | search("vqfx*")
 
 - name: Check intent arguments
   junos_interface:
-    name: "{{ name }}"
+    name: "{{ intf_name }}"
     state: up
     tx_rate: ge(0)
     rx_rate: le(0)
@@ -32,7 +37,7 @@
 
 - name: Check intent arguments (failed condition)
   junos_interface:
-    name: "{{ name }}"
+    name: "{{ intf_name }}"
     state: down
     tx_rate: gt(0)
     rx_rate: lt(0)
@@ -49,7 +54,7 @@
 
 - name: Config + intent
   junos_interface:
-    name: "{{ name }}"
+    name: "{{ intf_name }}"
     enabled: False
     state: down
     provider: "{{ netconf }}"
@@ -62,7 +67,7 @@
 
 - name: Config + intent (fail)
   junos_interface:
-    name: "{{ name }}"
+    name: "{{ intf_name }}"
     enabled: False
     state: up
     provider: "{{ netconf }}"
@@ -77,7 +82,7 @@
 - name: Aggregate config + intent (pass)
   junos_interface:
     aggregate:
-    - name: "{{ name }}"
+    - name: "{{ intf_name }}"
       enabled: True
       state: up
     provider: "{{ netconf }}"

test/units/modules/network/junos/test_junos_command.py

@@ -19,6 +19,11 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+try:
+    from lxml.etree import fromstring
+except ImportError:
+    from xml.etree.ElementTree import fromstring
+
 from ansible.compat.tests.mock import patch
 from ansible.modules.network.junos import junos_command
 from units.modules.utils import set_module_args
@@ -36,18 +41,37 @@ class TestJunosCommandModule(TestJunosModule):
     def setUp(self):
         super(TestJunosCommandModule, self).setUp()
 
-        self.mock_send_request = patch('ansible.modules.network.junos.junos_command.send_request')
-        self.send_request = self.mock_send_request.start()
+        self.mock_conn = patch('ansible.module_utils.junos.Connection')
+        self.conn = self.mock_conn.start()
+
+        self.mock_netconf = patch('ansible.module_utils.junos.NetconfConnection')
+        self.netconf_conn = self.mock_netconf.start()
+
+        self.mock_exec_rpc = patch('ansible.modules.network.junos.junos_command.exec_rpc')
+        self.exec_rpc = self.mock_exec_rpc.start()
+
+        self.mock_netconf_rpc = patch('ansible.module_utils.netconf.NetconfConnection')
+        self.netconf_rpc = self.mock_netconf_rpc.start()
+
+        self.mock_get_connection = patch('ansible.modules.network.junos.junos_command.get_connection')
+        self.get_connection = self.mock_get_connection.start()
+
+        self.mock_get_capabilities = patch('ansible.modules.network.junos.junos_command.get_capabilities')
+        self.get_capabilities = self.mock_get_capabilities.start()
+        self.get_capabilities.return_value = {'network_api': 'netconf'}
 
     def tearDown(self):
         super(TestJunosCommandModule, self).tearDown()
-
-        self.mock_send_request.stop()
+        self.mock_conn.stop()
+        self.mock_netconf.stop()
+        self.mock_get_capabilities.stop()
+        self.mock_netconf_rpc.stop()
+        self.mock_exec_rpc.stop()
+        self.mock_get_connection.stop()
 
     def load_fixtures(self, commands=None, format='text', changed=False):
         def load_from_file(*args, **kwargs):
-            module, element = args
-
+            element = fromstring(args[1])
             if element.text:
                 path = str(element.text)
             else:
@@ -57,7 +81,7 @@ class TestJunosCommandModule(TestJunosModule):
             filename = '%s_%s.txt' % (filename, format)
             return load_fixture(filename)
 
-        self.send_request.side_effect = load_from_file
+        self.exec_rpc.side_effect = load_from_file
 
     def test_junos_command_simple(self):
         set_module_args(dict(commands=['show version']))
@@ -80,13 +104,13 @@ class TestJunosCommandModule(TestJunosModule):
         wait_for = 'result[0] contains "test string"'
         set_module_args(dict(commands=['show version'], wait_for=wait_for))
         self.execute_module(failed=True)
-        self.assertEqual(self.send_request.call_count, 10)
+        self.assertEqual(self.exec_rpc.call_count, 10)
 
     def test_junos_command_retries(self):
         wait_for = 'result[0] contains "test string"'
         set_module_args(dict(commands=['show version'], wait_for=wait_for, retries=2))
         self.execute_module(failed=True)
-        self.assertEqual(self.send_request.call_count, 2)
+        self.assertEqual(self.exec_rpc.call_count, 2)
 
     def test_junos_command_match_any(self):
         wait_for = ['result[0] contains "Junos:"',

test/units/modules/network/junos/test_junos_config.py

@@ -54,8 +54,17 @@ class TestJunosConfigModule(TestJunosModule):
         self.mock_get_diff = patch('ansible.modules.network.junos.junos_config.get_diff')
         self.get_diff = self.mock_get_diff.start()
 
-        self.mock_send_request = patch('ansible.modules.network.junos.junos_config.send_request')
-        self.send_request = self.mock_send_request.start()
+        self.mock_conn = patch('ansible.module_utils.connection.Connection')
+        self.conn = self.mock_conn.start()
+
+        self.mock_netconf = patch('ansible.module_utils.junos.NetconfConnection')
+        self.netconf_conn = self.mock_netconf.start()
+
+        self.mock_exec_rpc = patch('ansible.modules.network.junos.junos_config.exec_rpc')
+        self.exec_rpc = self.mock_exec_rpc.start()
+
+        self.mock_netconf_rpc = patch('ansible.module_utils.netconf.NetconfConnection')
+        self.netconf_rpc = self.mock_netconf_rpc.start()
 
     def tearDown(self):
         super(TestJunosConfigModule, self).tearDown()
@@ -65,8 +74,11 @@ class TestJunosConfigModule(TestJunosModule):
         self.mock_unlock_configuration.stop()
         self.mock_commit_configuration.stop()
         self.mock_get_diff.stop()
-        self.mock_send_request.stop()
         self.load_configuration.stop()
+        self.mock_conn.stop()
+        self.mock_netconf.stop()
+        self.mock_exec_rpc.stop()
+        self.mock_netconf_rpc.stop()
 
     def load_fixtures(self, commands=None, format='text', changed=False):
         self.get_config.return_value = load_fixture('get_configuration_rpc_reply.txt')
@@ -162,7 +174,7 @@ class TestJunosConfigModule(TestJunosModule):
         src = load_fixture('junos_config.json', content='str')
         set_module_args(dict(zeroize='yes'))
         self.execute_module(changed=True)
-        self.assertEqual(self.send_request.call_count, 1)
+        self.assertEqual(self.exec_rpc.call_count, 1)
 
     def test_junos_config_src_format_xml(self):
         src = load_fixture('junos_config.json', content='str')

test/units/modules/network/junos/test_junos_facts.py

@@ -19,6 +19,11 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+try:
+    from lxml.etree import fromstring
+except ImportError:
+    from xml.etree.ElementTree import fromstring
+
 from ansible.compat.tests.mock import patch
 from ansible.modules.network.junos import junos_facts
 from units.modules.utils import set_module_args
@@ -44,16 +49,33 @@ class TestJunosCommandModule(TestJunosModule):
         self.mock_get_config = patch('ansible.modules.network.junos.junos_facts.get_configuration')
         self.get_config = self.mock_get_config.start()
 
-        self.mock_send_request = patch('ansible.modules.network.junos.junos_facts.send_request')
-        self.send_request = self.mock_send_request.start()
+        self.mock_conn = patch('ansible.module_utils.connection.Connection')
+        self.conn = self.mock_conn.start()
+
+        self.mock_netconf = patch('ansible.module_utils.junos.NetconfConnection')
+        self.netconf_conn = self.mock_netconf.start()
+
+        self.mock_exec_rpc = patch('ansible.modules.network.junos.junos_facts.exec_rpc')
+        self.exec_rpc = self.mock_exec_rpc.start()
+
+        self.mock_netconf_rpc = patch('ansible.module_utils.netconf.NetconfConnection')
+        self.netconf_rpc = self.mock_netconf_rpc.start()
+
+        self.mock_get_capabilities = patch('ansible.module_utils.junos.get_capabilities')
+        self.get_capabilities = self.mock_get_capabilities.start()
+        self.get_capabilities.return_value = {'network_api': 'netconf'}
 
     def tearDown(self):
         super(TestJunosCommandModule, self).tearDown()
-        self.mock_send_request.stop()
+        self.mock_conn.stop()
+        self.mock_netconf.stop()
+        self.mock_exec_rpc.stop()
+        self.mock_netconf_rpc.stop()
+        self.mock_get_capabilities.stop()
 
     def load_fixtures(self, commands=None, format='text', changed=False):
         def load_from_file(*args, **kwargs):
-            module, element = args
+            element = fromstring(args[1])
 
             if element.text:
                 path = str(element.text)
@@ -64,7 +86,7 @@ class TestJunosCommandModule(TestJunosModule):
             filename = '%s_%s.txt' % (filename, format)
             return load_fixture(filename)
 
-        self.send_request.side_effect = load_from_file
+        self.exec_rpc.side_effect = load_from_file
 
     def test_junos_get_facts(self):
         set_module_args(dict())

test/units/modules/network/junos/test_junos_netconf.py

@@ -32,9 +32,6 @@ class TestJunosCommandModule(TestJunosModule):
     def setUp(self):
         super(TestJunosCommandModule, self).setUp()
 
-        self.mock_exec_command = patch('ansible.modules.network.junos.junos_netconf.exec_command')
-        self.exec_command = self.mock_exec_command.start()
-
         self.mock_lock_configuration = patch('ansible.module_utils.junos.lock_configuration')
         self.lock_configuration = self.mock_lock_configuration.start()
 
@@ -44,17 +41,33 @@ class TestJunosCommandModule(TestJunosModule):
         self.mock_commit_configuration = patch('ansible.modules.network.junos.junos_netconf.commit_configuration')
         self.commit_configuration = self.mock_commit_configuration.start()
 
+        self.mock_conn = patch('ansible.module_utils.connection.Connection')
+        self.conn = self.mock_conn.start()
+
+        self.mock_netconf = patch('ansible.module_utils.junos.NetconfConnection')
+        self.netconf_conn = self.mock_netconf.start()
+
+        self.mock_netconf_rpc = patch('ansible.module_utils.netconf.NetconfConnection')
+        self.netconf_rpc = self.mock_netconf_rpc.start()
+
+        self.mock_get_capabilities = patch('ansible.module_utils.junos.get_capabilities')
+        self.get_capabilities = self.mock_get_capabilities.start()
+        self.get_capabilities.return_value = {'network_api': 'netconf'}
+
     def tearDown(self):
         super(TestJunosCommandModule, self).tearDown()
-        self.mock_exec_command.stop()
         self.mock_lock_configuration.stop()
         self.mock_unlock_configuration.stop()
         self.mock_commit_configuration.stop()
+        self.mock_conn.stop()
+        self.mock_netconf.stop()
+        self.mock_netconf_rpc.stop()
+        self.mock_get_capabilities.stop()
 
     def test_junos_netconf_enable(self):
-        self.exec_command.return_value = 0, '', None
+        self.netconf_conn().get.return_value = ''
         set_module_args(dict(state='present'))
-        result = self.execute_module()
+        result = self.execute_module(changed=True)
         self.assertEqual(result['commands'], ['set system services netconf ssh port 830'])
 
     def test_junos_netconf_disable(self):
@@ -63,7 +76,7 @@ class TestJunosCommandModule(TestJunosModule):
                 port 830;
                 }
             '''
-        self.exec_command.return_value = 0, out, None
+        self.netconf_conn().get.return_value = out
         set_module_args(dict(state='absent'))
         result = self.execute_module(changed=True)
         self.assertEqual(result['commands'], ['delete system services netconf'])
@@ -74,7 +87,7 @@ class TestJunosCommandModule(TestJunosModule):
                 port 830;
                 }
             '''
-        self.exec_command.return_value = 0, out, None
+        self.netconf_conn().get.return_value = out
         set_module_args(dict(state='present', netconf_port=22))
         result = self.execute_module(changed=True)
         self.assertEqual(result['commands'], ['set system services netconf ssh port 22'])
@@ -85,13 +98,13 @@ class TestJunosCommandModule(TestJunosModule):
                 port 22;
                 }
             '''
-        self.exec_command.return_value = 0, out, None
+        self.netconf_conn().get.return_value = out
         set_module_args(dict(state='present', netconf_port=0))
         result = self.execute_module(changed=True, failed=True)
         self.assertEqual(result['msg'], 'netconf_port must be between 1 and 65535')
 
     def test_junos_netconf_config_error(self):
-        self.exec_command.return_value = 1, None, None
+        self.netconf_conn().get.return_value = None
         set_module_args(dict(state='present'))
         result = self.execute_module(failed=True)
         self.assertEqual(result['msg'], 'unable to retrieve current config')

test/units/modules/network/junos/test_junos_rpc.py

@@ -20,9 +20,9 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 try:
-    from lxml.etree import tostring
+    from lxml.etree import tostring, fromstring
 except ImportError:
-    from xml.etree.ElementTree import tostring
+    from xml.etree.ElementTree import tostring, fromstring
 
 from ansible.compat.tests.mock import patch
 from ansible.modules.network.junos import junos_rpc
@@ -46,16 +46,28 @@ class TestJunosCommandModule(TestJunosModule):
 
     def setUp(self):
         super(TestJunosCommandModule, self).setUp()
-        self.mock_send_request = patch('ansible.modules.network.junos.junos_rpc.send_request')
-        self.send_request = self.mock_send_request.start()
+        self.mock_conn = patch('ansible.module_utils.connection.Connection')
+        self.conn = self.mock_conn.start()
+
+        self.mock_netconf = patch('ansible.module_utils.junos.NetconfConnection')
+        self.netconf_conn = self.mock_netconf.start()
+
+        self.mock_netconf_rpc = patch('ansible.module_utils.netconf.NetconfConnection')
+        self.netconf_rpc = self.mock_netconf_rpc.start()
+
+        self.mock_exec_rpc = patch('ansible.modules.network.junos.junos_rpc.exec_rpc')
+        self.exec_rpc = self.mock_exec_rpc.start()
 
     def tearDown(self):
         super(TestJunosCommandModule, self).tearDown()
-        self.mock_send_request.stop()
+        self.mock_conn.stop()
+        self.mock_netconf.stop()
+        self.mock_netconf_rpc.stop()
+        self.mock_exec_rpc.stop()
 
     def load_fixtures(self, commands=None, format='text', changed=False):
         def load_from_file(*args, **kwargs):
-            module, element = args
+            element = fromstring(args[1])
             if element.text:
                 path = str(element.text)
             else:
@@ -69,7 +81,7 @@ class TestJunosCommandModule(TestJunosModule):
 
             return load_fixture(filename)
 
-        self.send_request.side_effect = load_from_file
+        self.exec_rpc.side_effect = load_from_file
 
     def test_junos_rpc_xml(self):
         set_module_args(dict(rpc='get-chassis-inventory'))
@@ -89,9 +101,9 @@ class TestJunosCommandModule(TestJunosModule):
     def test_junos_rpc_args(self):
         set_module_args(dict(rpc='get-software-information', args={'interface': 'em0', 'media': True}))
         result = self.execute_module(format='xml')
-        args, kwargs = self.send_request.call_args
-        reply = tostring(args[1]).decode()
-        self.assertTrue(reply.find('<interface>em0</interface><media /></get-software-information>'))
+        args, kwargs = self.exec_rpc.call_args
+        reply = args[1]
+        self.assertTrue(reply.find(b'<interface>em0</interface><media /></get-software-information>'))
 
     def test_junos_rpc_attrs(self):
         set_module_args(dict(rpc='load-configuration', output='xml', attrs={'url': '/var/tmp/config.conf'}))