junos_linkagg implementation and junos modules refactor (#26587)

* junos_linkagg implementation and junos modules refactor

*  junos_linkagg implementation
*  junos_linkagg integration test
*  net_linkagg integration test for junos
*  decouple `load_config` and `commit` operations,
   to allow single commit (in case on confirm commit) and
   to perform batch commit (multiple `load_config` followed by single
   `commit`)
*  Other related refactor

* Fix CI issues

* Fix unit test failure

lib/ansible/module_utils/junos.py

@@ -191,13 +191,11 @@ def get_diff(module):
         return to_text(output.text, encoding='latin1').strip()
 
 
-def load_config(module, candidate, warnings, action='merge', commit=False, format='xml',
-                comment=None, confirm=False, confirm_timeout=None):
+def load_config(module, candidate, warnings, action='merge', format='xml'):
 
     if not candidate:
         return
 
-    with locked_config(module):
     if isinstance(candidate, list):
         candidate = '\n'.join(candidate)
 
@@ -206,16 +204,8 @@ def load_config(module, candidate, warnings, action='merge', commit=False, forma
         warnings.extend(reply)
 
     validate(module)
-        diff = get_diff(module)
 
-        if diff:
-            if commit:
-                commit_configuration(module, confirm=confirm, comment=comment,
-                                     confirm_timeout=confirm_timeout)
-            else:
-                discard_changes(module)
-
-        return diff
+    return get_diff(module)
 
 
 def get_param(module, key):
@@ -293,20 +283,21 @@ def map_obj_to_ele(module, want, top, value_map=None):
         oper = 'inactive'
 
     # build xml subtree
-    for obj in want:
     if container.tag != top_ele[-1]:
         node = SubElement(container, top_ele[-1])
     else:
         node = container
 
-        for fxpath, attributes in obj.items():
+    for fxpath, attributes in want.items():
         for attr in attributes:
             tag_only = attr.get('tag_only', False)
             leaf_only = attr.get('leaf_only', False)
-                is_value = attr.get('value_req', False)
+            value_req = attr.get('value_req', False)
             is_key = attr.get('is_key', False)
+            parent_attrib = attr.get('parent_attrib', True)
             value = attr.get('value')
             field_top = attr.get('top')
+
             # operation 'delete' is added as element attribute
             # only if it is key or leaf only node
             if state == 'absent' and not (is_key or leaf_only):
@@ -366,7 +357,7 @@ def map_obj_to_ele(module, want, top, value_map=None):
                         # Add value of leaf node if required while deleting.
                         # in some cases if value is present while deleting, it
                         # can result in error, hence the check
-                            if is_value:
+                        if value_req:
                             ele.text = value
                         if is_key:
                             par = ele.getparent()
@@ -375,6 +366,7 @@ def map_obj_to_ele(module, want, top, value_map=None):
                     ele.text = value
                     par = ele.getparent()
 
+                    if parent_attrib:
                         if state == 'present':
                             # set replace attribute at parent node
                             if not par.attrib.get('replace'):

lib/ansible/modules/network/interface/net_linkagg.py

@@ -40,12 +40,16 @@ options:
     required: true
   mode:
     description:
-      - Mode of the link aggregation group.
+      - Mode of the link aggregation group. A value of C(on) will enable LACP.
+        C(active) configures the link to actively information about the state of the link,
+        or it can be configured in C(passive) mode ie. send link state information only when
+        received them from another link.
     default: on
     choices: ['on', 'active', 'passive']
   members:
     description:
-      - List of members of the link aggregation group.
+      - List of members interfaces of the link aggregation group. The value can be
+        single interface or list of interfaces.
     required: true
   min_links:
     description:

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

@@ -112,6 +112,7 @@ 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.junos import commit_configuration, discard_changes, locked_config
 
 USE_PERSISTENT_CONNECTION = True
 DEFAULT_COMMENT = 'configured by junos_template'
@@ -153,9 +154,15 @@ def main():
             module.fail_json(msg='unable to retrieve device configuration')
         result['__backup__'] = str(match.text).strip()
 
-    diff = load_config(module, src, warnings, action=action, commit=commit, format=fmt)
+    with locked_config(module):
+        diff = load_config(module, src, warnings, action=action, format=fmt)
         if diff:
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
             result['changed'] = True
+
             if module._diff:
                 result['diff'] = {'prepared': diff}
 

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

@@ -101,9 +101,9 @@ EXAMPLES = """
 """
 
 RETURN = """
-diff:
+diff.prepared:
   description: Configuration difference before and after applying change.
-  returned: when configuration is changed.
+  returned: when configuration is changed and diff option is enabled.
   type: string
   sample: >
           [edit system login]
@@ -111,9 +111,10 @@ diff:
 """
 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 junos_argument_spec, check_args
 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
 
 try:
     from lxml.etree import tostring
@@ -167,20 +168,22 @@ def main():
 
     validate_param_values(module, param_to_xpath_map)
 
-    want = list()
-    want.append(map_params_to_obj(module, param_to_xpath_map))
+    want = 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)
+    with locked_config(module):
+        diff = load_config(module, tostring(ele), warnings, action='replace')
 
+        commit = not module.check_mode
         if diff:
-        result.update({
-            'changed': True,
-            'diff': diff,
-        })
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
+            if module._diff:
+                result['diff'] = {'prepared': diff}
 
     module.exit_json(**result)
 

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

@@ -172,8 +172,8 @@ import time
 import re
 import shlex
 
-from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.junos import junos_argument_spec, check_args
 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

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

@@ -137,6 +137,14 @@ options:
     default: merge
     choices: ['merge', 'override', 'replace']
     version_added: "2.3"
+  confirm_commit:
+    description:
+      - This argument will execute commit operation on remote device.
+        It can be used to confirm a previous commit.
+    required: false
+    default: no
+    choices: ['yes', 'no']
+    version_added: "2.4"
 requirements:
   - ncclient (>=v0.5.2)
 notes:
@@ -173,6 +181,7 @@ EXAMPLES = """
 
 - name: confirm a previous commit
   junos_config:
+    confirm_commit: yes
     provider: "{{ netconf }}"
 """
 
@@ -185,10 +194,10 @@ backup_path:
 """
 import re
 import json
-import sys
 
 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 commit_configuration, discard_changes, locked_config
 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
@@ -266,20 +275,9 @@ def filter_delete_statements(module, candidate):
     return modified_candidate
 
 
-def configure_device(module, warnings):
-    candidate = module.params['lines'] or module.params['src']
-
-    kwargs = {
-        'comment': module.params['comment'],
-        'commit': not module.check_mode
-    }
-
-    if module.params['confirm'] > 0:
-        kwargs.update({
-            'confirm': True,
-            'confirm_timeout': module.params['confirm']
-        })
+def configure_device(module, warnings, candidate):
 
+    kwargs = {}
     config_format = None
 
     if module.params['src']:
@@ -319,6 +317,7 @@ def main():
 
         confirm=dict(default=0, type='int'),
         comment=dict(default=DEFAULT_COMMENT),
+        confirm_commit=dict(type='bool', default=False),
 
         # config operations
         backup=dict(type='bool', default=False),
@@ -338,6 +337,9 @@ def main():
     warnings = list()
     check_args(module, warnings)
 
+    candidate = module.params['lines'] or module.params['src']
+    commit = not module.check_mode
+
     result = {'changed': False, 'warnings': warnings}
 
     if module.params['backup']:
@@ -352,22 +354,45 @@ def main():
         result['__backup__'] = match.text.strip()
 
     if module.params['rollback']:
-        if not module.check_mode:
+        if commit:
             diff = rollback(module)
             if module._diff:
                 result['diff'] = {'prepared': diff}
         result['changed'] = True
 
     elif module.params['zeroize']:
-        if not module.check_mode:
+        if commit:
             zeroize(module)
         result['changed'] = True
 
     else:
-        diff = configure_device(module, warnings)
+        if candidate:
+            with locked_config(module):
+                diff = configure_device(module, warnings, candidate)
                 if diff:
+                    if commit:
+                        kwargs = {
+                            'comment': module.params['comment']
+                        }
+
+                        if module.params['confirm'] > 0:
+                            kwargs.update({
+                                'confirm': True,
+                                'confirm_timeout': module.params['confirm']
+                            })
+                        commit_configuration(module, **kwargs)
+                    else:
+                        discard_changes(module)
+                    result['changed'] = True
+
                     if module._diff:
                         result['diff'] = {'prepared': diff}
+
+        elif module.params['confirm_commit']:
+            with locked_config(module):
+                # confirm a previous commit
+                commit_configuration(module)
+
             result['changed'] = True
 
     module.exit_json(**result)

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

@@ -84,11 +84,11 @@ ansible_facts:
   type: dict
 """
 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 get_configuration
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.netconf import send_request
+from ansible.module_utils.six import iteritems
 
 try:
     from lxml.etree import Element, SubElement, tostring

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

@@ -129,9 +129,9 @@ EXAMPLES = """
 """
 
 RETURN = """
-diff:
+diff.prepared:
   description: Configuration difference before and after applying change.
-  returned: when configuration is changed.
+  returned: when configuration is changed and diff option is enabled.
   type: string
   sample: >
         [edit interfaces]
@@ -141,9 +141,10 @@ diff:
 """
 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 junos_argument_spec, check_args
 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
 
 try:
     from lxml.etree import tostring
@@ -225,21 +226,22 @@ def main():
 
     validate_param_values(module, param_to_xpath_map)
 
-    want = list()
-    want.append(map_params_to_obj(module, param_to_xpath_map))
-
+    want = map_params_to_obj(module, param_to_xpath_map)
     ele = map_obj_to_ele(module, want, top, choice_to_value_map)
 
-    kwargs = {'commit': not module.check_mode}
-    kwargs['action'] = 'replace'
-
-    diff = load_config(module, tostring(ele), warnings, **kwargs)
+    with locked_config(module):
+        diff = load_config(module, tostring(ele), warnings, action='replace')
 
+        commit = not module.check_mode
         if diff:
-        result.update({
-            'changed': True,
-            'diff': diff,
-        })
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
+            if module._diff:
+                result['diff'] = {'prepared': diff}
 
     module.exit_json(**result)
 

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

@@ -0,0 +1,339 @@
+#!/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_linkagg
+version_added: "2.4"
+author: "Ganesh Nalawade (@ganeshrn)"
+short_description: Manage link aggregation groups on Juniper JUNOS network devices
+description:
+  - This module provides declarative management of link aggregation groups
+    on Juniper JUNOS network devices.
+options:
+  name:
+    description:
+      - Name of the link aggregation group.
+    required: true
+  mode:
+    description:
+      - Mode of the link aggregation group. A value of C(on) will enable LACP in C(passive) mode.
+        C(active) configures the link to actively information about the state of the link,
+        or it can be configured in C(passive) mode ie. send link state information only when
+        received them from another link. A value of C(off) will disable LACP.
+    default: off
+    choices: ['on', 'off', 'active', 'passive']
+  members:
+    description:
+      - List of members interfaces of the link aggregation group. The value can be
+        single interface or list of interfaces.
+    required: true
+  min_links:
+    description:
+      - Minimum members that should be up
+        before bringing up the link aggregation group.
+  device_count:
+    description:
+      - Number of aggregated ethernet devices that can be configured.
+        Acceptable integer value is between 1 and 128.
+  description:
+    description:
+      - Description of Interface.
+  collection:
+    description: List of link aggregation definitions.
+  purge:
+    description:
+      - Purge link aggregation groups not defined in the collections parameter.
+    default: no
+  state:
+    description:
+      - State of the link aggregation group.
+    default: present
+    choices: ['present', 'absent', 'up', 'down']
+  active:
+    description:
+      - Specifies whether or not the configuration is active or deactivated
+    default: True
+    choices: [True, False]
+requirements:
+  - ncclient (>=v0.5.2)
+notes:
+  - This module requires the netconf system service be enabled on
+    the remote device being managed
+"""
+
+EXAMPLES = """
+- name: configure link aggregation
+  junos_linkagg:
+    name: ae11
+    members:
+      - ge-0/0/5
+      - ge-0/0/6
+      - ge-0/0/7
+    lacp: active
+    device_count: 4
+    state: present
+
+- name: delete link aggregation
+  junos_linkagg:
+    name: ae11
+    members:
+      - ge-0/0/5
+      - ge-0/0/6
+      - ge-0/0/7
+    lacp: active
+    device_count: 4
+    state: delete
+
+- name: deactivate link aggregation
+  junos_linkagg:
+    name: ae11
+    members:
+      - ge-0/0/5
+      - ge-0/0/6
+      - ge-0/0/7
+    lacp: active
+    device_count: 4
+    state: present
+    active: False
+
+- name: Activate link aggregation
+  junos_linkagg:
+    name: ae11
+    members:
+      - ge-0/0/5
+      - ge-0/0/6
+      - ge-0/0/7
+    lacp: active
+    device_count: 4
+    state: present
+    active: True
+
+- name: Disable link aggregation
+  junos_linkagg:
+    name: ae11
+    state: down
+
+- name: Enable link aggregation
+  junos_linkagg:
+    name: ae11
+    state: up
+"""
+
+RETURN = """
+diff:
+  description: Configuration difference before and after applying change.
+  returned: when configuration is changed and diff option is enabled.
+  type: string
+  sample: >
+            [edit interfaces]
+            +   ge-0/0/6 {
+            +       ether-options {
+            +           802.3ad ae0;
+            +       }
+            +   }
+            [edit interfaces ge-0/0/7]
+            +   ether-options {
+            +       802.3ad ae0;
+            +   }
+            [edit interfaces]
+            +   ae0 {
+            +       description "configured by junos_linkagg";
+            +       aggregated-ether-options {
+            +           lacp {
+            +               active;
+            +           }
+            +       }
+            +   }
+"""
+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 load_config, map_params_to_obj, map_obj_to_ele
+from ansible.module_utils.junos import commit_configuration, discard_changes, locked_config, get_configuration
+
+try:
+    from lxml.etree import tostring
+except ImportError:
+    from xml.etree.ElementTree import tostring
+
+USE_PERSISTENT_CONNECTION = True
+DEFAULT_COMMENT = 'configured by junos_linkagg'
+
+
+def validate_device_count(value, module):
+    if value and not 1 <= value <= 128:
+        module.fail_json(msg='device_count must be between 1 and 128')
+
+
+def validate_min_links(value, module):
+    if value and not 1 <= value <= 8:
+        module.fail_json(msg='min_links must be between 1 and 8')
+
+
+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 configure_lag_params(module, warnings):
+    top = 'interfaces/interface'
+    param_lag_to_xpath_map = collections.OrderedDict()
+    param_lag_to_xpath_map.update([
+        ('name', {'xpath': 'name', 'is_key': True}),
+        ('description', 'description'),
+        ('min_links', {'xpath': 'minimum-links', 'top': 'aggregated-ether-options'}),
+        ('disable', {'xpath': 'disable', 'tag_only': True}),
+        ('mode', {'xpath': module.params['mode'], 'tag_only': True, 'top': 'aggregated-ether-options/lacp'}),
+    ])
+
+    validate_param_values(module, param_lag_to_xpath_map)
+
+    want = map_params_to_obj(module, param_lag_to_xpath_map)
+    ele = map_obj_to_ele(module, want, top)
+
+    diff = load_config(module, tostring(ele), warnings, action='replace')
+    if module.params['device_count']:
+        top = 'chassis/aggregated-devices/ethernet'
+        device_count_to_xpath_map = {'device_count': {'xpath': 'device-count', 'leaf_only': True}}
+
+        validate_param_values(module, device_count_to_xpath_map)
+
+        want = map_params_to_obj(module, device_count_to_xpath_map)
+        ele = map_obj_to_ele(module, want, top)
+
+        diff = load_config(module, tostring(ele), warnings, action='replace')
+
+    return diff
+
+
+def configure_member_params(module, warnings, diff=None):
+    top = 'interfaces/interface'
+    members = module.params['members']
+
+    if members:
+        member_to_xpath_map = collections.OrderedDict()
+        member_to_xpath_map.update([
+            ('name', {'xpath': 'name', 'is_key': True, 'parent_attrib': False}),
+            ('bundle', {'xpath': 'bundle', 'leaf_only': True, 'top': 'ether-options/ieee-802.3ad', 'is_key': True}),
+        ])
+
+        # link aggregation bundle assigned to member
+        module.params['bundle'] = module.params['name']
+
+        for member in members:
+
+            if module.params['state'] == 'absent':
+                # if link aggregate bundle is not assigned to member, trying to
+                # delete it results in rpc-reply error, hence if is not assigned
+                # skip deleting it and continue to next member.
+                resp = get_configuration(module)
+                bundle = resp.xpath("configuration/interfaces/interface[name='%s']/ether-options/"
+                                    "ieee-802.3ad[bundle='%s']" % (member, module.params['bundle']))
+                if not bundle:
+                    continue
+            # Name of member to be assigned to link aggregation bundle
+            module.params['name'] = member
+
+            validate_param_values(module, member_to_xpath_map)
+
+            want = map_params_to_obj(module, member_to_xpath_map)
+            ele = map_obj_to_ele(module, want, top)
+            diff = load_config(module, tostring(ele), warnings)
+
+    return diff
+
+
+def main():
+    """ main entry point for module execution
+    """
+    argument_spec = dict(
+        name=dict(required=True),
+        mode=dict(default='on', type='str', choices=['on', 'off', 'active', 'passive']),
+        members=dict(type='list'),
+        min_links=dict(type='int'),
+        device_count=dict(type='int'),
+        description=dict(default=DEFAULT_COMMENT),
+        collection=dict(type='list'),
+        purge=dict(type='bool'),
+        state=dict(default='present', choices=['present', 'absent', 'up', 'down']),
+        active=dict(default=True, type='bool')
+    )
+
+    argument_spec.update(junos_argument_spec)
+    required_one_of = [['name', 'collection']]
+    mutually_exclusive = [['name', 'collection']]
+
+    module = AnsibleModule(argument_spec=argument_spec,
+                           required_one_of=required_one_of,
+                           mutually_exclusive=mutually_exclusive,
+                           supports_check_mode=True)
+
+    warnings = list()
+    check_args(module, warnings)
+
+    result = {'changed': False}
+
+    if warnings:
+        result['warnings'] = warnings
+
+    state = module.params.get('state')
+    module.params['disable'] = True if state == 'down' else False
+
+    if state in ('present', 'up', 'down'):
+        module.params['state'] = 'present'
+
+    else:
+        module.params['disable'] = True
+
+    if module.params.get('mode') == 'off':
+        module.params['mode'] = False
+    elif module.params.get('mode') == 'on':
+        module.params['mode'] = 'passive'
+
+    with locked_config(module):
+        diff = configure_lag_params(module, warnings)
+        diff = configure_member_params(module, warnings, diff)
+
+        commit = not module.check_mode
+        if diff:
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
+            if module._diff:
+                result['diff'] = {'prepared': diff}
+
+    module.exit_json(**result)
+
+if __name__ == "__main__":
+    main()

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

@@ -111,9 +111,9 @@ EXAMPLES = """
 """
 
 RETURN = """
-diff:
+diff.prepared:
   description: Configuration difference before and after applying change.
-  returned: when configuration is changed.
+  returned: when configuration is changed and diff option is enabled.
   type: string
   sample: >
           [edit system syslog]
@@ -125,9 +125,10 @@ diff:
 """
 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 junos_argument_spec, check_args
 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
 
 try:
     from lxml.etree import tostring
@@ -224,20 +225,22 @@ def main():
 
     validate_param_values(module, param_to_xpath_map)
 
-    want = list()
-    want.append(map_params_to_obj(module, param_to_xpath_map))
+    want = 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)
+    with locked_config(module):
+        diff = load_config(module, tostring(ele), warnings, action='replace')
 
+        commit = not module.check_mode
         if diff:
-        result.update({
-            'changed': True,
-            'diff': diff,
-        })
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
+            if module._diff:
+                result['diff'] = {'prepared': diff}
 
     module.exit_json(**result)
 

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

@@ -76,9 +76,10 @@ commands:
 """
 import re
 
-from ansible.module_utils.junos import junos_argument_spec, check_args
 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 commit_configuration, discard_changes
 from ansible.module_utils.network_common import to_list
 from ansible.module_utils.six import iteritems
 
@@ -195,11 +196,16 @@ def main():
 
     if commands:
         commit = not module.check_mode
-        diff = load_config(module, commands, commit=commit)
+        diff = load_config(module, commands)
         if diff:
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
             if module._diff:
                 result['diff'] = {'prepared': diff}
-            result['changed'] = True
 
     module.exit_json(**result)
 

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

@@ -99,8 +99,8 @@ EXAMPLES = """
     reboot: no
 """
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.junos import junos_argument_spec, get_param
+from ansible.module_utils.pycompat24 import get_exception
 
 try:
     from jnpr.junos import Device

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

@@ -89,8 +89,8 @@ output_lines:
   returned: always
   type: list
 """
-from ansible.module_utils.junos import junos_argument_spec, check_args
 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.six import iteritems
 

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

@@ -82,6 +82,7 @@ EXAMPLES = """
   junos_static_route:
     address: 192.168.2.0/24
     next_hop: 10.0.0.1
+    preference: 10
     qualified_next_hop: 10.0.0.2
     qualified_preference: 3
     state: present
@@ -95,6 +96,7 @@ EXAMPLES = """
   junos_static_route:
     address: 192.168.2.0/24
     next_hop: 10.0.0.1
+    preference: 10
     qualified_next_hop: 10.0.0.2
     qualified_preference: 3
     state: present
@@ -104,6 +106,7 @@ EXAMPLES = """
   junos_static_route:
     address: 192.168.2.0/24
     next_hop: 10.0.0.1
+    preference: 10
     qualified_next_hop: 10.0.0.2
     qualified_preference: 3
     state: present
@@ -111,9 +114,9 @@ EXAMPLES = """
 """
 
 RETURN = """
-diff:
+diff.prepared:
   description: Configuration difference before and after applying change.
-  returned: when configuration is changed.
+  returned: when configuration is changed and diff option is enabled.
   type: string
   sample: >
           [edit routing-options static]
@@ -128,9 +131,10 @@ diff:
 """
 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 junos_argument_spec, check_args
 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
 
 try:
     from lxml.etree import tostring
@@ -197,20 +201,22 @@ def main():
 
     validate_param_values(module, param_to_xpath_map)
 
-    want = list()
-    want.append(map_params_to_obj(module, param_to_xpath_map))
+    want = 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)
+    with locked_config(module):
+        diff = load_config(module, tostring(ele), warnings, action='replace')
 
+        commit = not module.check_mode
         if diff:
-        result.update({
-            'changed': True,
-            'diff': diff,
-        })
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
+            if module._diff:
+                result['diff'] = {'prepared': diff}
 
     module.exit_json(**result)
 

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

@@ -100,9 +100,9 @@ EXAMPLES = """
 """
 
 RETURN = """
-diff:
+diff.prepared:
   description: Configuration difference before and after applying change.
-  returned: when configuration is changed.
+  returned: when configuration is changed and diff option is enabled.
   type: string
   sample: >
           [edit system]
@@ -115,9 +115,10 @@ diff:
 """
 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 junos_argument_spec, check_args
 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
 
 try:
     from lxml.etree import tostring
@@ -179,20 +180,22 @@ def main():
 
     validate_param_values(module, param_to_xpath_map)
 
-    want = list()
-    want.append(map_params_to_obj(module, param_to_xpath_map))
+    want = 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)
+    with locked_config(module):
+        diff = load_config(module, tostring(ele), warnings, action='replace')
 
+        commit = not module.check_mode
         if diff:
-        result.update({
-            'changed': True,
-            'diff': diff,
-        })
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
+            if module._diff:
+                result['diff'] = {'prepared': diff}
 
     module.exit_json(**result)
 

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

@@ -124,9 +124,9 @@ EXAMPLES = """
 """
 
 RETURN = """
-diff:
+diff.prepared:
   description: Configuration difference before and after applying change.
-  returned: when configuration is changed.
+  returned: when configuration is changed and diff option is enabled.
   type: string
   sample: >
           [edit system login]
@@ -137,9 +137,10 @@ diff:
 """
 from functools import partial
 
-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.junos import junos_argument_spec, check_args
+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
 
 try:
@@ -279,17 +280,23 @@ def main():
     want = map_params_to_obj(module)
     ele = map_obj_to_ele(want)
 
-    kwargs = {'commit': not module.check_mode}
+    kwargs = {}
     if module.params['purge']:
         kwargs['action'] = 'replace'
 
+    with locked_config(module):
         diff = load_config(module, tostring(ele), warnings, **kwargs)
 
+        commit = not module.check_mode
         if diff:
-        result.update({
-            'changed': True,
-            'diff': diff
-        })
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
+            if module._diff:
+                result['diff'] = {'prepared': diff}
 
     module.exit_json(**result)
 

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

@@ -98,9 +98,9 @@ EXAMPLES = """
 """
 
 RETURN = """
-diff:
+diff.prepared:
   description: Configuration difference before and after applying change.
-  returned: when configuration is changed.
+  returned: when configuration is changed and diff option is enabled.
   type: string
   sample: >
          [edit vlans]
@@ -110,9 +110,10 @@ diff:
 """
 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 junos_argument_spec, check_args
 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
 
 try:
     from lxml.etree import tostring
@@ -173,20 +174,22 @@ def main():
 
     validate_param_values(module, param_to_xpath_map)
 
-    want = list()
-    want.append(map_params_to_obj(module, param_to_xpath_map))
+    want = 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)
+    with locked_config(module):
+        diff = load_config(module, tostring(ele), warnings, action='replace')
 
+        commit = not module.check_mode
         if diff:
-        result.update({
-            'changed': True,
-            'diff': diff,
-        })
+            if commit:
+                commit_configuration(module)
+            else:
+                discard_changes(module)
+            result['changed'] = True
+
+            if module._diff:
+                result['diff'] = {'prepared': diff}
 
     module.exit_json(**result)
 

test/integration/junos.yaml

@@ -106,6 +106,13 @@
       rescue:
         - set_fact: test_failed=true
 
+    - block:
+      - include_role:
+          name: junos_linkagg
+        when: "limit_to in ['*', 'junos_linkagg']"
+      rescue:
+        - set_fact: test_failed=true
+
 ###########
     - name: Has any previous test failed?
       fail:

test/integration/targets/junos_config/tests/netconf/single.yaml

@@ -31,6 +31,16 @@
     that:
       - "result.changed == false"
 
+- name: confirm previous commit
+  junos_config:
+    confirm_commit: yes
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+
 - name: teardown
   junos_config:
     lines:

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

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

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

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

test/integration/targets/junos_linkagg/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_linkagg/tests/netconf/basic.yaml

@@ -0,0 +1,252 @@
+---
+- debug: msg="START junos_linkagg netconf/basic.yaml"
+
+- name: setup - remove linkagg
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: absent
+    provider: "{{ netconf }}"
+
+- name: configure linkagg
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name>ae0</name>' in config.xml"
+      - "'<device-count>4</device-count>' in config.xml"
+      - "'<bundle>ae0</bundle>' in config.xml"
+      - "'<active/>' in config.xml"
+      - "'<description>configured by junos_linkagg</description>' in config.xml"
+
+- name: configure linkagg (idempotent)
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: configure lacp in passive
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: passive
+    device_count: 4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<passive/>' in config.xml"
+
+- name: delete lacp
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: off
+    device_count: 4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<lacp/>' not in config.xml"
+
+- name: Change device count
+  junos_linkagg:
+    name: ae0
+    device_count: 2
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<device-count>2</device-count>' in config.xml"
+
+- name: Disable linkagg interface
+  junos_linkagg:
+    name: ae0
+    state: down
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<disable/>' in config.xml"
+      - "'+   disable;' in result.diff.prepared"
+
+- name: Enable linkagg interface
+  junos_linkagg:
+    name: ae0
+    state: up
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<disable/>' not in config.xml"
+
+- name: Deactivate linkagg
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: present
+    active: False
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<bundle inactive=\"inactive\">ae0</bundle>' in config.xml"
+      - "'<device-count inactive=\"inactive\">4</device-count>' in config.xml"
+      - "'inactive: ae0' in result.diff.prepared"
+
+- name: Activate linkagg
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: present
+    active: True
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<active/>' in config.xml"
+      - "'<bundle>ae0</bundle>' in config.xml"
+      - "'active: device-count 4' in result.diff.prepared"
+      - "'active: ae0' in result.diff.prepared"
+
+- name: Delete linkagg
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<active/>' not in config.xml"
+      - "'<bundle>ae0</bundle>' not in config.xml"
+      - "'<device-count>4</device-count>' not in config.xml"
+      - "'<name>ae0</name>' not in config.xml"
+
+- name: Delete linkagg (idempotent)
+  junos_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"

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

@@ -71,7 +71,7 @@
     that:
       - "result.changed == true"
       - "'<route inactive=\"inactive\">' in config.xml"
-      - "'inactive: route 1.1.1.0/24' in result.diff"
+      - "'inactive: route 1.1.1.0/24' in result.diff.prepared"
 
 - name: Activate static route
   junos_static_route:

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

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

test/integration/targets/net_linkagg/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_linkagg/tests/junos/basic.yaml

@@ -0,0 +1,181 @@
+---
+- debug: msg="START net_linkagg junos/basic.yaml"
+
+- name: setup - remove linkagg
+  net_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: absent
+    provider: "{{ netconf }}"
+
+- name: configure linkagg
+  net_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<name>ae0</name>' in config.xml"
+      - "'<device-count>4</device-count>' in config.xml"
+      - "'<bundle>ae0</bundle>' in config.xml"
+      - "'<active/>' in config.xml"
+      - "'<description>configured by junos_linkagg</description>' in config.xml"
+
+- name: configure linkagg (idempotent)
+  net_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"
+
+- name: configure lacp in passive
+  net_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: passive
+    device_count: 4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<passive/>' in config.xml"
+
+- name: delete lacp
+  net_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: off
+    device_count: 4
+    state: present
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<lacp/>' not in config.xml"
+
+- name: Disable linkagg interface
+  net_linkagg:
+    name: ae0
+    state: down
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<disable/>' in config.xml"
+      - "'+   disable;' in result.diff.prepared"
+
+- name: Enable linkagg interface
+  net_linkagg:
+    name: ae0
+    state: up
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<disable/>' not in config.xml"
+
+- name: Delete linkagg
+  net_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- name: Get running configuration
+  junos_rpc:
+    rpc: get-configuration
+    provider: "{{ netconf }}"
+  register: config
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'<active/>' not in config.xml"
+      - "'<bundle>ae0</bundle>' not in config.xml"
+      - "'<device-count>4</device-count>' not in config.xml"
+      - "'<name>ae0</name>' not in config.xml"
+
+- name: Delete linkagg (idempotent)
+  net_linkagg:
+    name: ae0
+    members:
+      - ge-0/0/6
+      - ge-0/0/7
+    mode: active
+    device_count: 4
+    state: absent
+    provider: "{{ netconf }}"
+  register: result
+
+- assert:
+    that:
+      - "result.changed == false"

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

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

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

@@ -36,6 +36,15 @@ class TestJunosConfigModule(TestJunosModule):
         self.mock_load_config = patch('ansible.modules.network.junos.junos_config.load_config')
         self.load_config = self.mock_load_config.start()
 
+        self.mock_lock_configuration = patch('ansible.module_utils.junos.lock_configuration')
+        self.lock_configuration = self.mock_lock_configuration.start()
+
+        self.mock_unlock_configuration = patch('ansible.module_utils.junos.unlock_configuration')
+        self.unlock_configuration = self.mock_unlock_configuration.start()
+
+        self.mock_commit_configuration = patch('ansible.modules.network.junos.junos_config.commit_configuration')
+        self.commit_configuration = self.mock_commit_configuration.start()
+
         self.mock_get_diff = patch('ansible.modules.network.junos.junos_config.get_diff')
         self.get_diff = self.mock_get_diff.start()
 
@@ -45,6 +54,10 @@ class TestJunosConfigModule(TestJunosModule):
     def tearDown(self):
         self.mock_get_config.stop()
         self.mock_load_config.stop()
+        self.mock_lock_configuration.stop()
+        self.mock_unlock_configuration.stop()
+        self.mock_commit_configuration.stop()
+        self.mock_get_diff.stop()
         self.mock_send_request.stop()
 
     def load_fixtures(self, commands=None, format='text', changed=False):
@@ -83,8 +96,8 @@ class TestJunosConfigModule(TestJunosModule):
     def test_junos_config_confirm(self):
         src = load_fixture('junos_config.set', content='str')
         set_module_args(dict(src=src, confirm=40))
-        self.execute_module()
-        args, kwargs = self.load_config.call_args
+        self.execute_module(changed=True)
+        args, kwargs = self.commit_configuration.call_args
         self.assertEqual(kwargs['confirm_timeout'], 40)
 
     def test_junos_config_rollback(self):
@@ -144,3 +157,8 @@ class TestJunosConfigModule(TestJunosModule):
         self.execute_module()
         args, kwargs = self.load_config.call_args
         self.assertEqual(kwargs['format'], 'xml')
+
+    def test_junos_config_confirm_commit(self):
+        set_module_args(dict(confirm_commit=True))
+        self.execute_module(changed=True)
+        self.assertEqual(self.commit_configuration.call_count, 1)

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

@@ -34,8 +34,20 @@ class TestJunosCommandModule(TestJunosModule):
         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()
+
+        self.mock_unlock_configuration = patch('ansible.module_utils.junos.unlock_configuration')
+        self.unlock_configuration = self.mock_unlock_configuration.start()
+
+        self.mock_commit_configuration = patch('ansible.modules.network.junos.junos_netconf.commit_configuration')
+        self.commit_configuration = self.mock_commit_configuration.start()
+
     def tearDown(self):
         self.mock_exec_command.stop()
+        self.mock_lock_configuration.stop()
+        self.mock_unlock_configuration.stop()
+        self.mock_commit_configuration.stop()
 
     def test_junos_netconf_enable(self):
         self.exec_command.return_value = 0, '', None