Add junos_system declartive module and other related change (#25859)

* Add junos_system declartive module and other related change

*  junos_system declartive module
*  integration test for junos_system
*  integration test for net_system (junos platform)
*  pep8 fixes for junos modules
*  move to lxml from elementree for xml parsing as it support
   complete set of xpath api's
*  other minor changes

* Fix CI and doc changes

* Fix unit test failures

* Fix typo in import

* Fix import issue for py2.6

* Add missed Element in import

lib/ansible/module_utils/junos.py

@@ -17,9 +17,8 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 import collections
-
 from contextlib import contextmanager
-from xml.etree.ElementTree import Element, SubElement, fromstring
+from copy import deepcopy
 
 from ansible.module_utils.basic import env_fallback, return_values
 from ansible.module_utils.netconf import send_request, children
@@ -27,6 +26,13 @@ from ansible.module_utils.netconf import discard_changes, validate
 from ansible.module_utils.six import string_types
 from ansible.module_utils._text import to_text
 
+try:
+    from lxml.etree import Element, SubElement, fromstring, tostring
+    HAS_LXML = True
+except ImportError:
+    from xml.etree.ElementTree import Element, SubElement, fromstring, tostring
+    HAS_LXML = False
+
 ACTIONS = frozenset(['merge', 'override', 'replace', 'update', 'set'])
 JSON_ACTIONS = frozenset(['merge', 'override', 'update'])
 FORMATS = frozenset(['xml', 'text', 'json'])
@@ -225,17 +231,18 @@ def get_param(module, key):
 def map_params_to_obj(module, param_to_xpath_map):
     """
     Creates a new dictionary with key as xpath corresponding
-    to param and value is a dict with metadata and value for
+    to param and value is a list of dict with metadata and values for
     the xpath.
     Acceptable metadata keys:
-        'xpath': Relative xpath corresponding to module param.
         'value': Value of param.
         'tag_only': Value is indicated by tag only in xml hierarchy.
         'leaf_only': If operation is to be added at leaf node only.
+        'value_req': If value(text) is requried for leaf node.
+        'is_key': If the field is key or not.
     eg: Output
     {
-        'name': {'xpath': 'name', 'value': 'ge-0/0/1'}
+        'name': [{'value': 'ge-0/0/1'}]
-        'disable': {'xpath': 'disable', 'tag_only': True}
+        'disable': [{'value': True, tag_only': True}]
     }
 
     :param module:
@@ -243,17 +250,32 @@ def map_params_to_obj(module, param_to_xpath_map):
     :return: obj
     """
     obj = collections.OrderedDict()
-    for key, attrib in param_to_xpath_map.items():
+    for key, attribute in param_to_xpath_map.items():
         if key in module.params:
-            if isinstance(attrib, dict):
+            is_attribute_dict = False
-                xpath = attrib.get('xpath')
-                del attrib['xpath']
 
-                attrib.update({'value': module.params[key]})
+            value = module.params[key]
-                obj.update({xpath: attrib})
+            if not isinstance(value, (list, tuple)):
+                value = [value]
+
+            if isinstance(attribute, dict):
+                xpath = attribute.get('xpath')
+                is_attribute_dict = True
             else:
-                xpath = attrib
+                xpath = attribute
-                obj.update({xpath: {'value': module.params[key]}})
+
+            if not obj.get(xpath):
+                obj[xpath] = list()
+
+            for val in value:
+                if is_attribute_dict:
+                    attr = deepcopy(attribute)
+                    del attr['xpath']
+
+                    attr.update({'value': val})
+                    obj[xpath].append(attr)
+                else:
+                    obj[xpath].append({'value': val})
     return obj
 
 
@@ -261,7 +283,7 @@ def map_obj_to_ele(module, want, top, value_map=None):
     top_ele = top.split('/')
     root = Element(top_ele[0])
     ele = root
-    oper = None
+
     if len(top_ele) > 1:
         for item in top_ele[1:-1]:
             ele = SubElement(ele, item)
@@ -270,41 +292,58 @@ def map_obj_to_ele(module, want, top, value_map=None):
 
     # build xml subtree
     for obj in want:
-        node = SubElement(container, top_ele[-1])
+        oper = None
+        if container.tag != top_ele[-1]:
+            node = SubElement(container, top_ele[-1])
+        else:
+            node = container
+
         if state and state != 'present':
             oper = OPERATION_LOOK_UP.get(state)
-            node.set(oper, oper)
 
-        for xpath, attrib in obj.items():
+        for xpath, attributes in obj.items():
-            tag_only = attrib.get('tag_only', False)
+            for attr in attributes:
-            leaf_only = attrib.get('leaf_only', False)
+                tag_only = attr.get('tag_only', False)
-            value = attrib.get('value')
+                leaf_only = attr.get('leaf_only', False)
+                is_value = attr.get('value_req', False)
+                is_key = attr.get('is_key', False)
+                value = attr.get('value')
 
-            # convert param value to device specific value
+                # operation (delete/active/inactive) is added as element attribute
-            if value_map and xpath in value_map:
+                # only if it is key or tag only or leaf only node
-                value = value_map[xpath].get(value)
+                if oper and not (is_key or tag_only or leaf_only):
+                    continue
 
-            # for leaf only fields operation attributes should be at leaf level
+                # convert param value to device specific value
-            # and not at node level.
+                if value_map and xpath in value_map:
-            if leaf_only and node.attrib.get(oper):
+                    value = value_map[xpath].get(value)
-                node.attrib.pop(oper)
 
-            if value or tag_only or leaf_only:
+                if value or tag_only or (leaf_only and value):
-                ele = node
+                    ele = node
-                tags = xpath.split('/')
+                    tags = xpath.split('/')
+                    if value:
+                        value = to_text(value, errors='surrogate_then_replace')
 
-                for item in tags:
+                    for item in tags:
-                    ele = SubElement(ele, item)
+                        ele = SubElement(ele, item)
 
-                if tag_only:
+                    if tag_only:
-                    if not value:
+                        if not value:
-                        ele.set('delete', 'delete')
+                            ele.set('delete', 'delete')
-                elif leaf_only and oper:
+                    elif leaf_only:
-                    ele.set(oper, oper)
+                        if oper:
-                else:
+                            ele.set(oper, oper)
-                    ele.text = to_text(value, errors='surrogate_then_replace')
+                            if is_value:
-
+                                ele.text = value
-                if state != 'present':
+                        else:
-                    break
+                            ele.text = value
+                    else:
+                        ele.text = value
+                        if HAS_LXML:
+                            par = ele.getparent()
+                        else:
+                            module.fail_json(msg='lxml is not installed.')
+                        if is_key and oper and not par.attrib.get(oper):
+                            par.set(oper, oper)
 
     return root

lib/ansible/module_utils/netconf.py

@@ -26,10 +26,14 @@
 # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #
 from contextlib import contextmanager
-from xml.etree.ElementTree import Element, SubElement, fromstring, tostring
 
 from ansible.module_utils.connection import exec_command
 
+try:
+    from lxml.etree import Element, SubElement, fromstring, tostring
+except ImportError:
+    from xml.etree.ElementTree import Element, SubElement, fromstring, tostring
+
 NS_MAP = {'nc': "urn:ietf:params:xml:ns:netconf:base:1.0"}
 
 

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

@@ -21,7 +21,6 @@ ANSIBLE_METADATA = {'metadata_version': '1.0',
                     'supported_by': 'community'}
 
 
-
 DOCUMENTATION = """
 ---
 module: junos_template
@@ -87,6 +86,8 @@ options:
     required: false
     default: null
     choices: ['text', 'xml', 'set']
+requirements:
+  - ncclient (>=v0.5.2)
 notes:
   - This module requires the netconf system service be enabled on
     the remote device being managed
@@ -111,11 +112,11 @@ EXAMPLES = """
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.junos import check_args, junos_argument_spec
 from ansible.module_utils.junos import get_configuration, load_config
-from ansible.module_utils.six import text_type
 
 USE_PERSISTENT_CONNECTION = True
 DEFAULT_COMMENT = 'configured by junos_template'
 
+
 def main():
 
     argument_spec = dict(
@@ -137,16 +138,13 @@ def main():
 
     result = {'changed': False, 'warnings': warnings}
 
-    comment = module.params['comment']
-    confirm = module.params['confirm']
     commit = not module.check_mode
     action = module.params['action']
     src = module.params['src']
     fmt = module.params['config_format']
 
     if action == 'overwrite' and fmt == 'set':
-        module.fail_json(msg="overwrite cannot be used when format is "
+        module.fail_json(msg="overwrite cannot be used when format is set per junos-pyez documentation")
-            "set per junos-pyez documentation")
 
     if module.params['backup']:
         reply = get_configuration(module, format='set')

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

@@ -54,6 +54,11 @@ options:
         present in the current devices active running configuration.
     default: present
     choices: ['present', 'absent', 'active', 'suspend']
+requirements:
+  - ncclient (>=v0.5.2)
+notes:
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
 """
 
 EXAMPLES = """
@@ -102,12 +107,15 @@ rpc:
 """
 import collections
 
-from xml.etree.ElementTree import tostring
-
 from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 
+try:
+    from lxml.etree import tostring
+except ImportError:
+    from xml.etree.ElementTree import tostring
+
 USE_PERSISTENT_CONNECTION = True
 
 

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

@@ -104,6 +104,10 @@ options:
     version_added: "2.3"
 requirements:
   - jxmlease
+  - ncclient (>=v0.5.2)
+notes:
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
 """
 
 EXAMPLES = """
@@ -163,17 +167,17 @@ import time
 import re
 import shlex
 
-from functools import partial
-from xml.etree import ElementTree as etree
-from xml.etree.ElementTree import Element, SubElement, tostring
-
 from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.netcli import Conditional, FailedConditionalError
 from ansible.module_utils.netconf import send_request
-from ansible.module_utils.network_common import ComplexList, to_list
 from ansible.module_utils.six import string_types, iteritems
 
+try:
+    from lxml.etree import Element, SubElement, tostring
+except ImportError:
+    from xml.etree.ElementTree import Element, SubElement, tostring
+
 try:
     import jxmlease
     HAS_JXMLEASE = True
@@ -182,6 +186,7 @@ except ImportError:
 
 USE_PERSISTENT_CONNECTION = True
 
+
 def to_lines(stdout):
     lines = list()
     for item in stdout:
@@ -190,6 +195,7 @@ def to_lines(stdout):
         lines.append(item)
     return lines
 
+
 def rpc(module, items):
 
     responses = list()
@@ -238,6 +244,7 @@ def rpc(module, items):
 
     return responses
 
+
 def split(value):
     lex = shlex.shlex(value)
     lex.quotes = '"'
@@ -245,6 +252,7 @@ def split(value):
     lex.commenters = ''
     return list(lex)
 
+
 def parse_rpcs(module):
     items = list()
 
@@ -270,6 +278,7 @@ def parse_rpcs(module):
 
     return items
 
+
 def parse_commands(module, warnings):
     items = list()
 
@@ -329,7 +338,6 @@ def main():
     items.extend(parse_rpcs(module))
 
     wait_for = module.params['wait_for'] or list()
-    display = module.params['display']
     conditionals = [Conditional(c) for c in wait_for]
 
     retries = module.params['retries']
@@ -344,8 +352,8 @@ def main():
         for item, resp in zip(items, responses):
             if item['xattrs']['format'] == 'xml':
                 if not HAS_JXMLEASE:
-                    module.fail_json(msg='jxmlease is required but does not appear to '
+                    module.fail_json(msg='jxmlease is required but does not appear to be installed. '
-                        'be installed.  It can be installed using `pip install jxmlease`')
+                                         'It can be installed using `pip install jxmlease`')
 
                 try:
                     transformed.append(jxmlease.parse(resp))
@@ -382,9 +390,7 @@ def main():
         'stdout_lines': to_lines(responses)
     }
 
-
     module.exit_json(**result)
 
-
 if __name__ == '__main__':
     main()

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

@@ -137,6 +137,8 @@ options:
     default: merge
     choices: ['merge', 'override', 'replace']
     version_added: "2.3"
+requirements:
+  - ncclient (>=v0.5.2)
 notes:
   - This module requires the netconf system service be enabled on
     the remote device being managed.
@@ -185,33 +187,47 @@ import re
 import json
 import sys
 
-from xml.etree import ElementTree
-
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.junos import get_diff, load_config, get_configuration
 from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import check_args as junos_check_args
 from ansible.module_utils.netconf import send_request
 from ansible.module_utils.six import string_types
-from ansible.module_utils._text import to_text, to_native
+from ansible.module_utils._text import to_native
 
-if sys.version_info < (2, 7):
+try:
-    from xml.parsers.expat import ExpatError
+    from lxml.etree import Element, fromstring
-    ParseError = ExpatError
+except ImportError:
-else:
+    from xml.etree.ElementTree import Element, fromstring
-    ParseError = ElementTree.ParseError
+
+try:
+    from lxml.etree import ParseError
+except ImportError:
+    try:
+        from xml.etree.ElementTree import ParseError
+    except ImportError:
+        # for Python < 2.7
+        from xml.parsers.expat import ExpatError
+        ParseError = ExpatError
 
 USE_PERSISTENT_CONNECTION = True
 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')
 
-zeroize = lambda x: send_request(x, ElementTree.Element('request-system-zeroize'))
+
-rollback = lambda x: get_diff(x)
+def zeroize(ele):
+    return send_request(ele, Element('request-system-zeroize'))
+
+
+def rollback(ele):
+    return get_diff(ele)
+
 
 def guess_format(config):
     try:
@@ -221,7 +237,7 @@ def guess_format(config):
         pass
 
     try:
-        ElementTree.fromstring(config)
+        fromstring(config)
         return 'xml'
     except ParseError:
         pass
@@ -231,6 +247,7 @@ def guess_format(config):
 
     return 'text'
 
+
 def filter_delete_statements(module, candidate):
     reply = get_configuration(module, format='set')
     match = reply.find('.//configuration-set')
@@ -248,6 +265,7 @@ def filter_delete_statements(module, candidate):
 
     return modified_candidate
 
+
 def configure_device(module, warnings):
     candidate = module.params['lines'] or module.params['src']
 
@@ -283,6 +301,7 @@ def configure_device(module, warnings):
 
     return load_config(module, candidate, warnings, **kwargs)
 
+
 def main():
     """ main entry point for module execution
     """

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

@@ -59,9 +59,13 @@ options:
     default: text
     choices: ['xml', 'set', 'text', 'json']
     version_added: "2.3"
+requirements:
+  - ncclient (>=v0.5.2)
 notes:
   - Ensure I(config_format) used to retrieve configuration from device
     is supported by junos version running on device.
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
 """
 
 EXAMPLES = """
@@ -79,16 +83,18 @@ ansible_facts:
   returned: always
   type: dict
 """
-
-from xml.etree.ElementTree import Element, SubElement, tostring
-
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.six import iteritems
 from ansible.module_utils.junos import junos_argument_spec, check_args, get_param
-from ansible.module_utils.junos import command, get_configuration
+from ansible.module_utils.junos import get_configuration
 from ansible.module_utils.netconf import send_request
 
+try:
+    from lxml.etree import Element, SubElement, tostring
+except ImportError:
+    from xml.etree.ElementTree import Element, SubElement, tostring
+
 try:
     from jnpr.junos import Device
     from jnpr.junos.exception import ConnectError

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

@@ -76,6 +76,11 @@ options:
       - State of the Interface configuration.
     default: present
     choices: ['present', 'absent', 'active', 'suspend']
+requirements:
+  - ncclient (>=v0.5.2)
+notes:
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
 """
 
 EXAMPLES = """
@@ -136,12 +141,15 @@ rpc:
 """
 import collections
 
-from xml.etree.ElementTree import tostring
-
 from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 
+try:
+    from lxml.etree import tostring
+except ImportError:
+    from xml.etree.ElementTree import tostring
+
 USE_PERSISTENT_CONNECTION = True
 
 
@@ -193,7 +201,7 @@ def main():
 
     param_to_xpath_map = collections.OrderedDict()
     param_to_xpath_map.update({
-        'name': 'name',
+        'name': {'xpath': 'name', 'is_key': True},
         'description': 'description',
         'speed': 'speed',
         'mtu': 'mtu',

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

@@ -105,11 +105,13 @@ def map_obj_to_commands(updates, module):
 
     return commands
 
+
 def parse_port(config):
     match = re.search(r'port (\d+)', config)
     if match:
         return int(match.group(1))
 
+
 def map_config_to_obj(module):
     cmd = 'show configuration system services netconf'
     rc, out, err = exec_command(module, cmd)
@@ -130,6 +132,7 @@ def validate_netconf_port(value, module):
     if not 1 <= value <= 65535:
         module.fail_json(msg='netconf_port must be between 1 and 65535')
 
+
 def map_params_to_obj(module):
     obj = {
         'netconf_port': module.params['netconf_port'],
@@ -144,6 +147,7 @@ def map_params_to_obj(module):
 
     return obj
 
+
 def load_config(module, config, commit=False):
 
     exec_command(module, 'configure')
@@ -164,6 +168,7 @@ def load_config(module, config, commit=False):
 
     return str(diff).strip()
 
+
 def main():
     """main entry point for module execution
     """

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

@@ -79,6 +79,7 @@ options:
     choices: ['true', 'false']
 requirements:
   - junos-eznc
+  - ncclient (>=v0.5.2)
 notes:
   - This module requires the netconf system service be enabled on
     the remote device being managed
@@ -142,7 +143,8 @@ def install_package(module, device):
     package = module.params['src']
     no_copy = module.params['no_copy']
 
-    progress_log = lambda x, y: module.log(y)
+    def progress_log(dev, report):
+        module.log(report)
 
     module.log('installing package')
     result = junos.install(package, progress=progress_log, no_copy=no_copy)

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

@@ -55,6 +55,11 @@ options:
         version of software that supports native JSON output.
     required: false
     default: xml
+requirements:
+  - ncclient (>=v0.5.2)
+notes:
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
 """
 
 EXAMPLES = """
@@ -84,8 +89,6 @@ output_lines:
   returned: always
   type: list
 """
-from xml.etree.ElementTree import Element, SubElement, tostring
-
 from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.netconf import send_request
@@ -93,6 +96,11 @@ from ansible.module_utils.six import iteritems
 
 USE_PERSISTENT_CONNECTION = True
 
+try:
+    from lxml.etree import Element, SubElement, tostring
+except ImportError:
+    from xml.etree.ElementTree import Element, SubElement, tostring
+
 
 def main():
     """main entry point for Ansible module

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

@@ -0,0 +1,195 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# (c) 2017, Ansible by Red Hat, inc
+#
+# This file is part of Ansible by Red Hat
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+ANSIBLE_METADATA = {'metadata_version': '1.0',
+                    'status': ['preview'],
+                    'supported_by': 'core'}
+
+
+DOCUMENTATION = """
+---
+module: junos_system
+version_added: "2.4"
+author: "Ganesh Nalawade (@ganeshrn)"
+short_description: Manage the system attributes on Juniper JUNOS devices
+description:
+  - This module provides declarative management of node system attributes
+    on Juniper JUNOS devices.  It provides an option to configure host system
+    parameters or remove those parameters from the device active
+    configuration.
+options:
+  hostname:
+    description:
+      - Configure the device hostname parameter. This option takes an ASCII string value.
+  domain_name:
+    description:
+      - Configure the IP domain name
+        on the remote device to the provided value. Value
+        should be in the dotted name form and will be
+        appended to the C(hostname) to create a fully-qualified
+        domain name.
+  domain_search:
+    description:
+      - Provides the list of domain suffixes to
+        append to the hostname for the purpose of doing name resolution.
+        This argument accepts a list of names and will be reconciled
+        with the current active configuration on the running node.
+  name_servers:
+    description:
+      - List of DNS name servers by IP address to use to perform name resolution
+        lookups.  This argument accepts either a list of DNS servers See
+        examples.
+  state:
+    description:
+      - State of the configuration
+        values in the device's current active configuration.  When set
+        to I(present), the values should be configured in the device active
+        configuration and when set to I(absent) the values should not be
+        in the device active configuration
+    default: present
+    choices: ['present', 'absent', 'active', 'suspend']
+requirements:
+  - ncclient (>=v0.5.2)
+notes:
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
+"""
+
+EXAMPLES = """
+- name: configure hostname and domain name
+  junos_system:
+    hostname: junos01
+    domain_name: test.example.com
+    domain-search:
+      - ansible.com
+      - redhat.com
+      - juniper.com
+
+- name: remove configuration
+  junos_system:
+    state: absent
+
+- name: configure name servers
+  junos_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+"""
+
+RETURN = """
+rpc:
+  description: load-configuration RPC send to the device
+  returned: when configuration is changed on device
+  type: string
+  sample: >
+            <interfaces>
+                <interface>
+                    <name>ge-0/0/0</name>
+                    <description>test interface</description>
+                </interface>
+            </interfaces>
+"""
+import collections
+
+from ansible.module_utils.junos import junos_argument_spec, check_args
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
+
+try:
+    from lxml.etree import tostring
+except ImportError:
+    from xml.etree.ElementTree import tostring
+
+USE_PERSISTENT_CONNECTION = True
+
+
+def validate_param_values(module, obj):
+    for key in obj:
+        # validate the param value (if validator func exists)
+        validator = globals().get('validate_%s' % key)
+        if callable(validator):
+            validator(module.params.get(key), module)
+
+
+def main():
+    """ main entry point for module execution
+    """
+    argument_spec = dict(
+        hostname=dict(),
+        domain_name=dict(),
+        domain_search=dict(type='list'),
+        name_servers=dict(type='list'),
+
+        state=dict(choices=['present', 'absent', 'active', 'suspend'], default='present')
+    )
+
+    argument_spec.update(junos_argument_spec)
+
+    params = ['hostname', 'domain_name', 'domain_search', 'name_servers']
+    required_if = [('state', 'present', params, True),
+                   ('state', 'absent', params, True),
+                   ('state', 'active', params, True),
+                   ('state', 'suspend', params, True)]
+
+    module = AnsibleModule(argument_spec=argument_spec,
+                           required_if=required_if,
+                           supports_check_mode=True)
+
+    warnings = list()
+    check_args(module, warnings)
+
+    result = {'changed': False}
+
+    if warnings:
+        result['warnings'] = warnings
+
+    top = 'system'
+
+    param_to_xpath_map = collections.OrderedDict()
+    param_to_xpath_map.update({
+        'hostname': {'xpath': 'host-name', 'leaf_only': True},
+        'domain_name': {'xpath': 'domain-name', 'leaf_only': True},
+        'domain_search': {'xpath': 'domain-search', 'leaf_only': True, 'value_req': True},
+        'name_servers': {'xpath': 'name-server/name', 'is_key': True}
+    })
+
+    validate_param_values(module, param_to_xpath_map)
+
+    want = list()
+    want.append(map_params_to_obj(module, param_to_xpath_map))
+    ele = map_obj_to_ele(module, want, top)
+
+    kwargs = {'commit': not module.check_mode}
+    kwargs['action'] = 'replace'
+
+    diff = load_config(module, tostring(ele), warnings, **kwargs)
+
+    if diff:
+        result.update({
+            'changed': True,
+            'diff': {'prepared': diff},
+            'rpc': tostring(ele)
+        })
+
+    module.exit_json(**result)
+
+if __name__ == "__main__":
+    main()

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

@@ -91,6 +91,11 @@ options:
     required: false
     default: present
     choices: ['present', 'absent']
+requirements:
+  - ncclient (>=v0.5.2)
+notes:
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
 """
 
 EXAMPLES = """
@@ -116,13 +121,16 @@ RETURN = """
 """
 from functools import partial
 
-from xml.etree.ElementTree import Element, SubElement, tostring
-
 from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.junos import load_config
 from ansible.module_utils.six import iteritems
 
+try:
+    from lxml.etree import Element, SubElement, tostring
+except ImportError:
+    from xml.etree.ElementTree import Element, SubElement, tostring
+
 ROLES = ['operator', 'read-only', 'super-user', 'unauthorized']
 USE_PERSISTENT_CONNECTION = True
 

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

@@ -60,6 +60,11 @@ options:
       - State of the VLAN configuration.
     default: present
     choices: ['present', 'absent', 'active', 'suspend']
+requirements:
+  - ncclient (>=v0.5.2)
+notes:
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
 """
 
 EXAMPLES = """
@@ -94,12 +99,15 @@ rpc:
 """
 import collections
 
-from xml.etree.ElementTree import tostring
-
 from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.junos import load_config, map_params_to_obj, map_obj_to_ele
 
+try:
+    from lxml.etree import tostring
+except ImportError:
+    from xml.etree.ElementTree import tostring
+
 USE_PERSISTENT_CONNECTION = True
 
 
@@ -147,7 +155,7 @@ def main():
 
     param_to_xpath_map = collections.OrderedDict()
     param_to_xpath_map.update({
-        'name': 'name',
+        'name': {'xpath': 'name', 'is_key': True},
         'vlan_id': 'vlan-id',
         'description': 'description'
     })

lib/ansible/modules/network/net_interface.py

@@ -112,6 +112,7 @@ commands:
   sample:
     - interface 20
     - name test-interface
+
 rpc:
   description: load-configuration RPC send to the device
   returned: C(rpc) is returned only for junos device
@@ -124,5 +125,4 @@ rpc:
                     <description>test interface</description>
                 </interface>
             </interfaces>
-
 """

lib/ansible/modules/network/net_system.py

@@ -107,4 +107,17 @@ commands:
   sample:
     - hostname ios01
     - ip domain name test.example.com
+
+rpc:
+  description: load-configuration RPC send to the device
+  returned: C(rpc) is returned only for junos device
+            when configuration is changed on device
+  type: string
+  sample: >
+            <interfaces>
+                <interface>
+                    <name>ge-0/0/0</name>
+                    <description>test interface</description>
+                </interface>
+            </interfaces>
 """

lib/ansible/modules/network/net_vlan.py

@@ -81,6 +81,7 @@ commands:
   sample:
     - vlan 20
     - name test-vlan
+
 rpc:
   description: load-configuration RPC send to the device
   returned: C(rpc) is returned only for junos device

lib/ansible/utils/jsonrpc.py

@@ -65,7 +65,6 @@ class Rpc:
         else:
             try:
                 result = rpc_method(*args, **kwargs)
-                display.display(" -- result -- %s" % result, log_only=True)
             except Exception as exc:
                 display.display(traceback.format_exc(), log_only=True)
                 error = self.internal_error(data=to_text(exc, errors='surrogate_then_replace'))
@@ -78,7 +77,6 @@ class Rpc:
 
                 response = json.dumps(response)
 
-        display.display(" -- response -- %s" % response, log_only=True)
         delattr(self, '_identifier')
         return response
 

test/integration/junos.yaml

@@ -17,3 +17,4 @@
     - { role: junos_vlan, when: "limit_to in ['*', 'junos_vlan']" }
     - { role: junos_interface, when: "limit_to in ['*', 'junos_interface']" }
     - { role: junos_banner, when: "limit_to in ['*', 'junos_banner']" }
+    - { role: junos_system, when: "limit_to in ['*', 'junos_system']" }

test/integration/targets/junos_system/aliases

@@ -0,0 +1 @@
+network/ci

test/integration/targets/junos_system/defaults/main.yaml

@@ -0,0 +1,2 @@
+---
+testcase: "*"

test/integration/targets/junos_system/tasks/main.yaml

@@ -0,0 +1,2 @@
+---
+- { include: netconf.yaml, tags: ['netconf'] }

test/integration/targets/junos_system/tasks/netconf.yaml

@@ -0,0 +1,16 @@
+---
+- name: collect all netconf test cases
+  find:
+    paths: "{{ role_path }}/tests/netconf"
+    patterns: "{{ testcase }}.yaml"
+  register: test_cases
+  delegate_to: localhost
+
+- name: set test_items
+  set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}"
+
+- name: run test case
+  include: "{{ test_case_to_run }}"
+  with_items: "{{ test_items }}"
+  loop_control:
+    loop_var: test_case_to_run

test/integration/targets/junos_system/tests/netconf/basic.yaml

@@ -0,0 +1,308 @@
+---
+- debug: msg="START junos_system netconf/basic.yaml"
+
+- name: setup - remove hostname
+  junos_system:
+    hostname: vsrx01
+    state: absent
+    provider: "{{ netconf }}"
+
+- name: Set hostname
+  junos_system:
+    hostname: vsrx01
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<host-name>vsrx01</host-name>' in result.rpc"
+
+- name: Set hostname (idempotent)
+  junos_system:
+    hostname: vsrx01
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: Deactivate hostname configuration
+  junos_system:
+    hostname: vsrx01
+    state: suspend
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<host-name inactive=\"inactive\" />' in result.rpc"
+
+- name: Activate hostname configuration
+  junos_system:
+    hostname: vsrx01
+    state: active
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<host-name active=\"active\" />' in result.rpc"
+
+- name: Delete hostname configuration
+  junos_system:
+    hostname: vsrx01
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<host-name delete=\"delete\" />' in result.rpc"
+
+- name: Teardown - set hostname
+  junos_system:
+    hostname: vsrx01
+    state: present
+    provider: "{{ netconf }}"
+
+- name: setup - remove domain name
+  junos_system:
+    domain_name: ansible.com
+    state: absent
+    provider: "{{ netconf }}"
+
+- name: Set domain name
+  junos_system:
+    domain_name: ansible.com
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-name>ansible.com</domain-name>' in result.rpc"
+
+- name: Set domain name (idempotent)
+  junos_system:
+    domain_name: ansible.com
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: Deactivate domain name
+  junos_system:
+    domain_name: ansible.com
+    state: suspend
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-name inactive=\"inactive\" />' in result.rpc"
+
+- name: Activate domain name
+  junos_system:
+    domain_name: ansible.com
+    state: active
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-name active=\"active\" />' in result.rpc"
+
+- name: Delete domain name
+  junos_system:
+    domain_name: ansible.com
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-name delete=\"delete\" />' in result.rpc"
+
+- name: Teardown - set domain name
+  junos_system:
+    domain_name: ansible.com
+    state: present
+    provider: "{{ netconf }}"
+
+- name: Setup - delete domain search
+  junos_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Set domain search
+  junos_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-search>test.com</domain-search>' in result.rpc"
+      - "'<domain-search>sample.com</domain-search>' in result.rpc"
+
+- name: Set domain search
+  junos_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: Deactivate domain search
+  junos_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: suspend
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-search inactive=\"inactive\">test.com</domain-search>' in result.rpc"
+      - "'<domain-search inactive=\"inactive\">sample.com</domain-search>' in result.rpc"
+
+- name: Activate domain search
+  junos_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: active
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-search active=\"active\">test.com</domain-search>' in result.rpc"
+      - "'<domain-search active=\"active\">sample.com</domain-search>' in result.rpc"
+
+- name: Delete domain search
+  junos_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-search delete=\"delete\">test.com</domain-search>' in result.rpc"
+      - "'<domain-search delete=\"delete\">sample.com</domain-search>' in result.rpc"
+
+- name: Setup - delete name servers
+  junos_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Set name servers
+  junos_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name-server><name>8.8.8.8</name></name-server>' in result.rpc"
+      - "'<name-server><name>8.8.4.4</name></name-server>' in result.rpc"
+
+- name: Set name servers (idempotent)
+  junos_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: Deactivate name servers
+  junos_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: suspend
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name-server inactive=\"inactive\"><name>8.8.8.8</name></name-server>' in result.rpc"
+      - "'<name-server inactive=\"inactive\"><name>8.8.4.4</name></name-server>' in result.rpc"
+
+- name: Activate name servers
+  junos_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: active
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name-server active=\"active\"><name>8.8.8.8</name></name-server>' in result.rpc"
+      - "'<name-server active=\"active\"><name>8.8.4.4</name></name-server>' in result.rpc"
+
+- name: Delete name servers
+  junos_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name-server delete=\"delete\"><name>8.8.8.8</name></name-server>' in result.rpc"
+      - "'<name-server delete=\"delete\"><name>8.8.4.4</name></name-server>' in result.rpc"

test/integration/targets/net_system/tasks/main.yaml

@@ -1,2 +1,3 @@
 ---
 - { include: cli.yaml, tags: ['cli'] }
+- { include: netconf.yaml, tags: ['netconf'] }

test/integration/targets/net_system/tasks/netconf.yaml

@@ -0,0 +1,16 @@
+---
+- name: collect all netconf test cases
+  find:
+    paths: "{{ role_path }}/tests/netconf"
+    patterns: "{{ testcase }}.yaml"
+  register: test_cases
+  delegate_to: localhost
+
+- name: set test_items
+  set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}"
+
+- name: run test case
+  include: "{{ test_case_to_run }}"
+  with_items: "{{ test_items }}"
+  loop_control:
+    loop_var: test_case_to_run

test/integration/targets/net_system/tests/junos/basic.yaml

@@ -0,0 +1,308 @@
+---
+- debug: msg="START net_system junos/basic.yaml"
+
+- name: setup - remove hostname
+  net_system:
+    hostname: vsrx01
+    state: absent
+    provider: "{{ netconf }}"
+
+- name: Set hostname
+  net_system:
+    hostname: vsrx01
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<host-name>vsrx01</host-name>' in result.rpc"
+
+- name: Set hostname (idempotent)
+  net_system:
+    hostname: vsrx01
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: Deactivate hostname configuration
+  net_system:
+    hostname: vsrx01
+    state: suspend
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<host-name inactive=\"inactive\" />' in result.rpc"
+
+- name: Activate hostname configuration
+  net_system:
+    hostname: vsrx01
+    state: active
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<host-name active=\"active\" />' in result.rpc"
+
+- name: Delete hostname configuration
+  net_system:
+    hostname: vsrx01
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<host-name delete=\"delete\" />' in result.rpc"
+
+- name: Teardown - set hostname
+  net_system:
+    hostname: vsrx01
+    state: present
+    provider: "{{ netconf }}"
+
+- name: setup - remove domain name
+  net_system:
+    domain_name: ansible.com
+    state: absent
+    provider: "{{ netconf }}"
+
+- name: Set domain name
+  net_system:
+    domain_name: ansible.com
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-name>ansible.com</domain-name>' in result.rpc"
+
+- name: Set domain name (idempotent)
+  net_system:
+    domain_name: ansible.com
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: Deactivate domain name
+  net_system:
+    domain_name: ansible.com
+    state: suspend
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-name inactive=\"inactive\" />' in result.rpc"
+
+- name: Activate domain name
+  net_system:
+    domain_name: ansible.com
+    state: active
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-name active=\"active\" />' in result.rpc"
+
+- name: Delete domain name
+  net_system:
+    domain_name: ansible.com
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-name delete=\"delete\" />' in result.rpc"
+
+- name: Teardown - set domain name
+  net_system:
+    domain_name: ansible.com
+    state: present
+    provider: "{{ netconf }}"
+
+- name: Setup - delete domain search
+  net_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Set domain search
+  net_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-search>test.com</domain-search>' in result.rpc"
+      - "'<domain-search>sample.com</domain-search>' in result.rpc"
+
+- name: Set domain search
+  net_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: Deactivate domain search
+  net_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: suspend
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-search inactive=\"inactive\">test.com</domain-search>' in result.rpc"
+      - "'<domain-search inactive=\"inactive\">sample.com</domain-search>' in result.rpc"
+
+- name: Activate domain search
+  net_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: active
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-search active=\"active\">test.com</domain-search>' in result.rpc"
+      - "'<domain-search active=\"active\">sample.com</domain-search>' in result.rpc"
+
+- name: Delete domain search
+  net_system:
+    domain_search:
+      - test.com
+      - sample.com
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<domain-search delete=\"delete\">test.com</domain-search>' in result.rpc"
+      - "'<domain-search delete=\"delete\">sample.com</domain-search>' in result.rpc"
+
+- name: Setup - delete name servers
+  net_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Set name servers
+  net_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name-server><name>8.8.8.8</name></name-server>' in result.rpc"
+      - "'<name-server><name>8.8.4.4</name></name-server>' in result.rpc"
+
+- name: Set name servers (idempotent)
+  net_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: Deactivate name servers
+  net_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: suspend
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name-server inactive=\"inactive\"><name>8.8.8.8</name></name-server>' in result.rpc"
+      - "'<name-server inactive=\"inactive\"><name>8.8.4.4</name></name-server>' in result.rpc"
+
+- name: Activate name servers
+  net_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: active
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name-server active=\"active\"><name>8.8.8.8</name></name-server>' in result.rpc"
+      - "'<name-server active=\"active\"><name>8.8.4.4</name></name-server>' in result.rpc"
+
+- name: Delete name servers
+  net_system:
+    name_servers:
+      - 8.8.8.8
+      - 8.8.4.4
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name-server delete=\"delete\"><name>8.8.8.8</name></name-server>' in result.rpc"
+      - "'<name-server delete=\"delete\"><name>8.8.4.4</name></name-server>' in result.rpc"

test/integration/targets/net_system/tests/netconf/basic.yaml

@@ -0,0 +1,3 @@
+---
+- include: "{{ role_path }}/tests/junos/basic.yaml"
+  when: hostvars[inventory_hostname]['ansible_network_os'] == 'junos'

test/sanity/pep8/legacy-files.txt

@@ -340,11 +340,6 @@ lib/ansible/modules/network/iosxr/iosxr_facts.py
 lib/ansible/modules/network/iosxr/iosxr_system.py
 lib/ansible/modules/net_tools/ipify_facts.py
 lib/ansible/modules/net_tools/ipinfoio_facts.py
-lib/ansible/modules/network/junos/_junos_template.py
-lib/ansible/modules/network/junos/junos_command.py
-lib/ansible/modules/network/junos/junos_config.py
-lib/ansible/modules/network/junos/junos_netconf.py
-lib/ansible/modules/network/junos/junos_package.py
 lib/ansible/modules/network/lenovo/cnos_conditional_template.py
 lib/ansible/modules/network/lenovo/cnos_template.py
 lib/ansible/modules/network/lenovo/cnos_vlan.py