Repository: ansible/ansible
Pull request: Fix netconf plugin related to collections


##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Fixes #65655 (partly)

*  Make netconf plugins configurable so that the
   information of ncclient device handler
   for give platform resides in the platform
   specific netconf plugin.
*  If the device handler value in ncclient is
   different from the ansible_network_os value
   the right value of `ncclient_device_handler`
   should be set in the plugin documentation.
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request
- Doc Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
constants.py
plugins/connection/netconf.py
plugins/netconf/__init__.py
plugins/netconf/ce.py
plugins/netconf/default.py
plugins/netconf/iosxr.py
plugins/netconf/junos.py
plugins/netconf/sros.py
docs/docsite/rst/plugins/netconf.rst

##### ADDITIONAL INFORMATION
<!--- Include additional information to help people understand the change here -->
<!--- A step-by-step reproduction of the problem is helpful if there is no related issue -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```paste below

```


Problem statement:
NETCONF connections/plugins require rework to support collections
##### SUMMARY
Ansible platform contains some hard coded behaviour that blocks making NETCONF plugins pluggable using Collections (Galaxy).

Detailed issues:
1. NETCONF connection plugin contains a dictionary called `NETWORK_OS_DEVICE_PARAM_MAP` that translates from `ansible_network_os` to name of the ncclient device_handler. If adding a new networking OS using collections, it is rather unclear how this list can be extended. In the particular case, I am using network OS `nokia.sros.sros` - which obviously is not a known device-handler supported by ncclient.
As the number of native device handlers supported by ncclient is very limited, the alternative option would be just to provide mappings to known device-handlers - while for all other platforms the ncclient "default" device handler would be used.
Another alternative could be, to use just the strip the collection name and namespace - so in the case above, it would translate to just "sros". However, the vendor/contributor must ensure that ncclient has a device handler with that name - which in case of "sros" would not be the case today (aka default works fine).

2. The method `get_device_info` returns a dictionary that has the key `network_os`. It is rather unclear, if this value is related to the `ansible_network_os` that is defined in the host inventory. In this particular case, it is assumed to return `sros` or `nokia.sros.sros`?

3. The method `guess_network_os` returns the guessed network OS. Same as (2), is there any expectation, that it returns simply `sros` or the full string `nokia.sros.sros`.

General comments about device family auto discovery:
- The implementation of `ansible_network_os = auto` is rather inefficient. For every known vendor Ansible connects via NETCONF and disconnects. It would be rather preferred, to simply connect via NETCONF once and keep the connection open. The server capabilities exchanged could be passed to the different plugins, to do an initial assessment based on the existence of certain capability values. The plugin might run a few more additional netconf-get requests, if the hello exchange does not contain enough information to take a decision.
- Device auto-negotiation should have a priority/preference values. That way, people could steer the order of detection to optimise performance but also to provide a more predictable result. In the particular case, there is Ansible built-in support for NETCONF on Nokia SROS devices. However, to remove vendor specific code from Ansible core, we've started to move the corresponding code to the collection "nokia.sros.sros". How can a developer or operator influence Ansible, so the auto-detection method from the collection is preferred against the built-on method (which likely will become deprecated with the next Ansible release)?
- It's rather unclear, how auto-discovery works across different protocols. Let's say, we want to do automation against Nokia SROS devices, while setting the `ansible_network_os` to `auto`. In that case, the expectation would be, that the network_os detection is only executed once - but can be used for various connection types: NETCONF, CLI, ....

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
lib/ansible/plugins/connection/netconf.py

##### ANSIBLE VERSION
```
ansible 2.9.2
  config file = None
  configured module search path = ['/Users/wiso/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.7/site-packages/ansible
  executable location = /usr/local/bin/unusable
  python version = 3.7.5 (default, Nov  1 2019, 02:16:32) [Clang 11.0.0 (clang-1100.0.33.8)]
```

##### STEPS TO REPRODUCE
Check with collection `nokia.sros`

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/plugins/connection/netconf.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/connection/netconf.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @NilashishC @Qalthos @danielmellado @ganeshrn @justjais @trishnaguha
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
@ganeshrn, please have a look into the above. It would be good to understand how this is supposed to work. If you have a collection that contains a NETCONF plugin this would be great thing to share - however, I've checked on Galaxy and did not found any.
You might use the code from collection `nokia.sros.sros` as a reference. Please let me know, if this is something that would be fixed in a Ansible 2.9.x maintenance release, or if we need to wait until Ansible 2.10.

Commit message: Fix netconf plugin related to collections

Fixes #65655 (partly)

*  Make netconf plugins configurable so that the
   information of ncclient device handler
   for give platform resides in the platform
   specific netconf plugin.
*  If the device handler value in ncclient is
   different from the ansible_network_os value
   the right value of `ncclient_device_handler`
   should be set in the plugin documentation.

Patch to review:
diff --git a/docs/docsite/rst/plugins/netconf.rst b/docs/docsite/rst/plugins/netconf.rst
new file mode 100644
index 00000000000000..a884ed86ca4ff5
--- /dev/null
+++ b/docs/docsite/rst/plugins/netconf.rst
@@ -0,0 +1,62 @@
+.. _netconf_plugins:
+
+Netconf Plugins
+===============
+
+.. contents::
+   :local:
+   :depth: 2
+
+Netconf plugins are abstactions over the Netconf interface to network devices. They provide a standard interface
+for Ansible to execute tasks on those network devices.
+
+These plugins generally correspond one-to-one to network device platforms. The appropriate netconf plugin will
+thus be automatically loaded based on the ``ansible_network_os`` variable. If the platform supports standard
+Netconf implementation as defined in the Netconf RFC specification the ``default`` netconf plugin will be used.
+In case if the platform supports propriety Netconf RPC's in that case the interface can be defined in platform
+specific netconf plugin.
+
+.. _enabling_netconf:
+
+Adding netconf plugins
+-------------------------
+
+You can extend Ansible to support other network devices by dropping a custom plugin into the ``netconf_plugins`` directory.
+
+.. _using_netconf:
+
+Using netconf plugins
+------------------------
+
+The netconf plugin to use is determined automatically from the ``ansible_network_os`` variable. There should be no reason to override this functionality.
+
+Most netconf plugins can operate without configuration. A few have additional options that can be set to impact how
+tasks are translated into netconf commands. A ncclient device specific handler name can be set in the netconf plugin
+or else the value of ``default`` is used as per ncclient device handler.
+
+
+Plugins are self-documenting. Each plugin should document its configuration options.
+
+.. _netconf_plugin_list:
+
+Plugin list
+-----------
+
+You can use ``ansible-doc -t netconf -l`` to see the list of available plugins.
+Use ``ansible-doc -t netconf <plugin name>`` to see detailed documentation and examples.
+
+
+.. toctree:: :maxdepth: 1
+    :glob:
+
+    netconf/*
+
+
+.. seealso::
+
+   :ref:`Ansible for Network Automation<network_guide>`
+       An overview of using Ansible to automate networking devices.
+   `User Mailing List <https://groups.google.com/group/ansible-devel>`_
+       Have a question?  Stop by the google group!
+   `irc.freenode.net <http://irc.freenode.net>`_
+       #ansible-network IRC chat channel
diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py
index 34446691b4d194..613c94ef6be0e4 100644
--- a/lib/ansible/constants.py
+++ b/lib/ansible/constants.py
@@ -102,7 +102,7 @@ def __getitem__(self, y):
 DEFAULT_REMOTE_PASS = None
 DEFAULT_SUBSET = None
 # FIXME: expand to other plugins, but never doc fragments
-CONFIGURABLE_PLUGINS = ('become', 'cache', 'callback', 'cliconf', 'connection', 'httpapi', 'inventory', 'lookup', 'shell', 'vars')
+CONFIGURABLE_PLUGINS = ('become', 'cache', 'callback', 'cliconf', 'connection', 'httpapi', 'inventory', 'lookup', 'netconf', 'shell', 'vars')
 # NOTE: always update the docs/docsite/Makefile to match
 DOCUMENTABLE_PLUGINS = CONFIGURABLE_PLUGINS + ('module', 'strategy')
 IGNORE_FILES = ("COPYING", "CONTRIBUTING", "LICENSE", "README", "VERSION", "GUIDELINES")  # ignore during module search
diff --git a/lib/ansible/plugins/connection/netconf.py b/lib/ansible/plugins/connection/netconf.py
index 47d5e12ee2b98c..fd21090783acb7 100644
--- a/lib/ansible/plugins/connection/netconf.py
+++ b/lib/ansible/plugins/connection/netconf.py
@@ -204,14 +204,6 @@
 
 logging.getLogger('ncclient').setLevel(logging.INFO)
 
-NETWORK_OS_DEVICE_PARAM_MAP = {
-    "nxos": "nexus",
-    "ios": "default",
-    "dellos10": "default",
-    "sros": "alu",
-    "ce": "huawei"
-}
-
 
 class Connection(NetworkConnectionBase):
     """NetConf connections"""
@@ -226,17 +218,17 @@ def __init__(self, play_context, new_stdin, *args, **kwargs):
         # This will be used to trigger the the use of guess_network_os when connecting.
         self._network_os = self._network_os or 'auto'
 
-        netconf = netconf_loader.get(self._network_os, self)
-        if netconf:
-            self._sub_plugin = {'type': 'netconf', 'name': netconf._load_name, 'obj': netconf}
+        self.netconf = netconf_loader.get(self._network_os, self)
+        if self.netconf :
+            self._sub_plugin = {'type': 'netconf', 'name': self.netconf._load_name, 'obj': self.netconf}
             self.queue_message('vvvv', 'loaded netconf plugin %s from path %s for network_os %s' %
-                               (netconf._load_name, netconf._original_path, self._network_os))
+                               (self.netconf._load_name, self.netconf._original_path, self._network_os))
         else:
-            netconf = netconf_loader.get("default", self)
-            self._sub_plugin = {'type': 'netconf', 'name': 'default', 'obj': netconf}
+            self.netconf = netconf_loader.get("default", self)
+            self._sub_plugin = {'type': 'netconf', 'name': 'default', 'obj': self.netconf}
             self.queue_message('display', 'unable to load netconf plugin for network_os %s, falling back to default plugin' % self._network_os)
-        self.queue_message('log', 'network_os is set to %s' % self._network_os)
 
+        self.queue_message('log', 'network_os is set to %s' % self._network_os)
         self._manager = None
         self.key_filename = None
         self._ssh_config = None
@@ -305,8 +297,12 @@ def _connect(self):
             # Network os not discovered. Set it to default
             self.queue_message('vvv', 'Unable to discover network_os. Falling back to default.')
             self._network_os = 'default'
-
-        device_params = {'name': NETWORK_OS_DEVICE_PARAM_MAP.get(self._network_os) or self._network_os}
+        try:
+            ncclient_device_handler = self.netconf.get_option('ncclient_device_handler')
+        except KeyError:
+            ncclient_device_handler = 'default'
+        self.queue_message('vvv', 'identified ncclient device handler: %s.' % ncclient_device_handler)
+        device_params = {'name':  ncclient_device_handler}
 
         try:
             port = self._play_context.port or 830
diff --git a/lib/ansible/plugins/netconf/__init__.py b/lib/ansible/plugins/netconf/__init__.py
index 2997d338f992af..b181f37696ad1c 100644
--- a/lib/ansible/plugins/netconf/__init__.py
+++ b/lib/ansible/plugins/netconf/__init__.py
@@ -102,9 +102,11 @@ class NetconfBase(AnsiblePlugin):
             conn.load_configuration(config=[''set system ntp server 1.1.1.1''], action='set', format='text')
     """
 
-    __rpc__ = ['get_config', 'edit_config', 'get_capabilities', 'get']
+    __rpc__ = ['rpc', 'get_config', 'get', 'edit_config', 'validate', 'copy_config', 'dispatch', 'lock', 'unlock',
+               'discard_changes', 'commit', 'get_schema', 'delete_config', 'get_device_operations']
 
     def __init__(self, connection):
+        super(NetconfBase, self).__init__()
         self._connection = connection
 
     @property
diff --git a/lib/ansible/plugins/netconf/ce.py b/lib/ansible/plugins/netconf/ce.py
index 46b3bc2725dac4..23000f89896b52 100644
--- a/lib/ansible/plugins/netconf/ce.py
+++ b/lib/ansible/plugins/netconf/ce.py
@@ -19,6 +19,23 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+DOCUMENTATION = """
+---
+netconf: ce
+short_description: Use ce netconf plugin to run netconf commands on Huawei cloudengine platform
+description:
+  - This ce plugin provides low level abstraction apis for
+    sending and receiving netconf commands from Juniper JUNOS network devices.
+version_added: "2.10"
+options:
+  ncclient_device_handler:
+    type: str
+    default: huawei
+    description:
+      - Specifies the ncclient device handler name for Huawei cloudengine.
+        To identify the ncclient device handler name refer ncclient library documentation.
+"""
+
 import json
 import re
 
@@ -101,9 +118,8 @@ def load_configuration(self, *args, **kwargs):
 
     def get_capabilities(self):
         result = dict()
-        result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes', 'validate', 'lock', 'unlock', 'copy_copy',
-                                               'execute_rpc', 'load_configuration', 'get_configuration', 'command',
-                                               'reboot', 'halt']
+        result['rpc'] = self.get_base_rpc() + ['execute_rpc', 'load_configuration', 'get_configuration', 'compare_configuration',
+                                               'execute_action', 'halt', 'reboot', 'execute_nc_cli', 'dispatch_rpc']
         result['network_api'] = 'netconf'
         result['device_info'] = self.get_device_info()
         result['server_capabilities'] = [c for c in self.m.server_capabilities]
diff --git a/lib/ansible/plugins/netconf/default.py b/lib/ansible/plugins/netconf/default.py
index dabebaaf06b472..2e85a325a0dfe3 100644
--- a/lib/ansible/plugins/netconf/default.py
+++ b/lib/ansible/plugins/netconf/default.py
@@ -19,6 +19,24 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+DOCUMENTATION = """
+---
+author: Ansible Networking Team
+netconf: default
+short_description: Use default netconf plugin to run standard netconf commands as per RFC
+description:
+  - This default plugin provides low level abstraction apis for
+    sending and receiving netconf commands as per Netconf RFC specification.
+version_added: "2.10"
+options:
+  ncclient_device_handler:
+    type: str
+    default: default
+    description:
+      - Specifies the ncclient device handler name for network os that support default netconf
+        implementation as per Netconf RFC specification. To identify the ncclient device handler
+        name refer ncclient library documentation.
+"""
 import json
 
 from ansible.module_utils._text import to_text, to_bytes
@@ -40,9 +58,7 @@ def get_device_info(self):
 
     def get_capabilities(self):
         result = dict()
-        result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes', 'validate', 'lock', 'unlock', 'copy_copy',
-                                               'execute_rpc', 'load_configuration', 'get_configuration', 'command',
-                                               'reboot', 'halt']
+        result['rpc'] = self.get_base_rpc()
         result['network_api'] = 'netconf'
         result['device_info'] = self.get_device_info()
         result['server_capabilities'] = [c for c in self.m.server_capabilities]
diff --git a/lib/ansible/plugins/netconf/iosxr.py b/lib/ansible/plugins/netconf/iosxr.py
index c80801d0e1e9a6..cce281f31b1461 100644
--- a/lib/ansible/plugins/netconf/iosxr.py
+++ b/lib/ansible/plugins/netconf/iosxr.py
@@ -20,6 +20,24 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+DOCUMENTATION = """
+---
+author: Ansible Networking Team
+netconf: iosxr
+short_description: Use iosxr netconf plugin to run netconf commands on Cisco IOSXR platform
+description:
+  - This iosxr plugin provides low level abstraction apis for
+    sending and receiving netconf commands from Cisco iosxr network devices.
+version_added: "2.10"
+options:
+  ncclient_device_handler:
+    type: str
+    default: iosxr
+    description:
+      - Specifies the ncclient device handler name for Cisco iosxr network os. To
+        identify the ncclient device handler name refer ncclient library documentation.
+"""
+
 import json
 import re
 import collections
@@ -77,7 +95,7 @@ def get_device_info(self):
 
     def get_capabilities(self):
         result = dict()
-        result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes', 'validate', 'lock', 'unlock', 'get-schema']
+        result['rpc'] = self.get_base_rpc()
         result['network_api'] = 'netconf'
         result['device_info'] = self.get_device_info()
         result['server_capabilities'] = [c for c in self.m.server_capabilities]
diff --git a/lib/ansible/plugins/netconf/junos.py b/lib/ansible/plugins/netconf/junos.py
index cbe42d2dcb0014..c2eac0c5fa7023 100644
--- a/lib/ansible/plugins/netconf/junos.py
+++ b/lib/ansible/plugins/netconf/junos.py
@@ -19,6 +19,24 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+DOCUMENTATION = """
+---
+author: Ansible Networking Team
+netconf: junos
+short_description: Use junos netconf plugin to run netconf commands on Juniper JUNOS platform
+description:
+  - This junos plugin provides low level abstraction apis for
+    sending and receiving netconf commands from Juniper JUNOS network devices.
+version_added: "2.10"
+options:
+  ncclient_device_handler:
+    type: str
+    default: junos
+    description:
+      - Specifies the ncclient device handler name for Juniper junos network os. To
+        identify the ncclient device handler name refer ncclient library documentation.
+"""
+
 import json
 import re
 
diff --git a/lib/ansible/plugins/netconf/sros.py b/lib/ansible/plugins/netconf/sros.py
index de9c6c8ccf2982..406dd91a01ba2b 100644
--- a/lib/ansible/plugins/netconf/sros.py
+++ b/lib/ansible/plugins/netconf/sros.py
@@ -19,6 +19,23 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+DOCUMENTATION = """
+---
+netconf: sros
+short_description: Use Nokia SROS netconf plugin to run netconf commands on Nokia SROS platform
+description:
+  - This sros plugin provides low level abstraction apis for
+    sending and receiving netconf commands from Nokia sros network devices.
+version_added: "2.10"
+options:
+  ncclient_device_handler:
+    type: str
+    default: alu
+    description:
+      - Specifies the ncclient device handler name for Nokia sros network os. To
+        identify the ncclient device handler name refer ncclient library documentation.
+"""
+
 import json
 import re
 
@@ -60,7 +77,7 @@ def get_device_info(self):
 
     def get_capabilities(self):
         result = dict()
-        result['rpc'] = self.get_base_rpc() + ['commit', 'discard_changes', 'validate', 'lock', 'unlock']
+        result['rpc'] = self.get_base_rpc()
         result['network_api'] = 'netconf'
         result['device_info'] = self.get_device_info()
         result['server_capabilities'] = [c for c in self.m.server_capabilities]


Code review comments:
diff_hunk: @@ -19,6 +19,23 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+DOCUMENTATION = """
+---
+netconf: sros
+short_description: Use Nokia SROS netconf plugin to run netconf commands on Nokia SROS platform
+description:
+  - This sros plugin provides low level abstraction apis for
+    sending and receiving netconf commands from Nokia sros network devices.
+version_added: "2.10"
+options:
+  ncclient_device_handler:
+    type: str
+    default: alu
path: lib/ansible/plugins/netconf/sros.py
text: @user1:
The ncclient device-handler for Nokia SR OS called "alu" was built years ago. From latest testing the usage of the "default" handler provides actually better results. Decision needs to be made, to either provide an updated version of the SROS handler in ncclient - or simply linking to the default handler from Ansible.