From 455dfbe73249bc07bcfd6235edcb85e817b603c5 Mon Sep 17 00:00:00 2001
From: Ganesh Nalawade <ganesh634@gmail.com>
Date: Tue, 4 Sep 2018 13:23:18 +0530
Subject: [PATCH] Fix netconf module and plugin issues (#45140)

* Fix netconf netconf issues

*  Identifier is optional for get_schema api
*  Fix dispatch api mandatory argument check
*  Add save option handling to copy config from target datastore
   to startup datastore if supported
*  Validate config in check-mode or if validate option set to true

* Copy config if check-mode is not enabled
---
 .../modules/network/netconf/netconf_config.py | 19 +++++++++++--------
 lib/ansible/plugins/netconf/__init__.py       |  4 +---
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/lib/ansible/modules/network/netconf/netconf_config.py b/lib/ansible/modules/network/netconf/netconf_config.py
index a45512f59b..2c9f5b4564 100644
--- a/lib/ansible/modules/network/netconf/netconf_config.py
+++ b/lib/ansible/modules/network/netconf/netconf_config.py
@@ -100,7 +100,8 @@ options:
     version_added: "2.7"
   save:
     description:
-      - The C(save) argument instructs the module to save the running-config to the startup-config if changed.
+      - The C(save) argument instructs the module to save the configuration in C(target) datastore to the
+        startup-config if changed and if :startup capability is supported by Netconf server.
     default: false
     version_added: "2.4"
   backup:
@@ -275,6 +276,7 @@ def main():
     confirm_commit = module.params['confirm_commit']
     confirm = module.params['confirm']
     validate = module.params['validate']
+    save = module.params['save']
 
     conn = Connection(module._socket_path)
     capabilities = get_capabilities(module)
@@ -298,10 +300,10 @@ def main():
             module.fail_json(msg='neither :candidate nor :writable-running are supported by this netconf server')
 
     # Netconf server capability validation against input options
-    if module.params['save'] and not supports_startup:
-        module.fail_json(msg='cannot copy <running/> to <startup/>, while :startup is not supported')
+    if save and not supports_startup:
+        module.fail_json(msg='cannot copy <%s/> to <startup/>, while :startup is not supported' % target)
 
-    if module.params['confirm_commit'] and not operations.get('supports_confirm_commit', False):
+    if confirm_commit and not operations.get('supports_confirm_commit', False):
         module.fail_json(msg='confirm commit is not supported by Netconf server')
 
     if confirm_commit or (confirm > 0) and not operations.get('supports_confirm_commit', False):
@@ -329,8 +331,7 @@ def main():
             before = to_text(response, errors='surrogate_then_replace').strip()
             result['__backup__'] = before.strip()
         if validate:
-            if not module.check_mode:
-                conn.validate(target)
+            conn.validate(target)
         if source:
             if not module.check_mode:
                 conn.copy(source, target)
@@ -377,8 +378,10 @@ def main():
             if sanitized_before != sanitized_after:
                 result['changed'] = True
 
-            if module._diff:
-                if result['changed']:
+            if result['changed']:
+                if save and not module.check_mode:
+                    conn.copy_config(target, 'startup')
+                if module._diff:
                     result['diff'] = {'before': sanitized_before, 'after': sanitized_after}
 
     except ConnectionError as e:
diff --git a/lib/ansible/plugins/netconf/__init__.py b/lib/ansible/plugins/netconf/__init__.py
index 0c27cea39a..1ccc767da1 100644
--- a/lib/ansible/plugins/netconf/__init__.py
+++ b/lib/ansible/plugins/netconf/__init__.py
@@ -204,7 +204,7 @@ class NetconfBase(AnsiblePlugin):
         :param filter: specifies the portion of the configuration to retrieve (by default entire configuration is retrieved)
         :return: Returns xml string containing the RPC response received from remote host
         """
-        if rpc_command:
+        if rpc_command is None:
             raise ValueError('rpc_command value must be provided')
         req = fromstring(rpc_command)
         resp = self.m.dispatch(req, source=source, filter=filter)
@@ -270,8 +270,6 @@ class NetconfBase(AnsiblePlugin):
         :param format: format of the schema to be retrieved, yang is the default
         :return: Returns xml string containing the RPC response received from remote host
         """
-        if identifier:
-            raise ValueError('identifier value must be provided')
         resp = self.m.get_schema(identifier, version=version, format=format)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml