Update netconf plugins for junos and iosxr (#44157)

* Update netconf plugins for junos and iosxr

Fixes #39160

*  Update api signature for netcon api's
   from variable arguments to named arguments

* Udpate get default rpc supported

* Fix CI issue

lib/ansible/plugins/netconf/__init__.py

@@ -98,12 +98,19 @@ 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']
+
     def __init__(self, connection):
         self._connection = connection
         self.m = self._connection._manager
 
     @ensure_connected
     def rpc(self, name):
+        """
+        RPC to be execute on remote device
+        :param name: Name of rpc in string format
+        :return: Received rpc response from remote host
+        """
         """RPC to be execute on remote device
            :name: Name of rpc in string format"""
         try:
@@ -116,9 +123,9 @@ class NetconfBase(AnsiblePlugin):
 
     @ensure_connected
     def get_config(self, source=None, filter=None):
-        """Retrieve all or part of a specified configuration
+        """
+        Retrieve all or part of a specified configuration
         (by default entire configuration is retrieved).
-
         :param source: Name of the configuration datastore being queried, defaults to running datastore
         :param filter: This argument specifies the portion of the configuration data to retrieve
         :return: Returns xml string containing the RPC response received from remote host
@@ -132,88 +139,97 @@ class NetconfBase(AnsiblePlugin):
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def get(self, filter=None):
+    def get(self, filter=None, with_defaults=None):
-        """Retrieve device configuration and state information.
+        """
-
+        Retrieve device configuration and state information.
         :param filter: This argument specifies the portion of the state data to retrieve
                        (by default entire state data is retrieved)
+        :param with_defaults: defines an explicit method of retrieving default values
+                              from the configuration
         :return: Returns xml string containing the RPC response received from remote host
         """
         if isinstance(filter, list):
             filter = tuple(filter)
-        resp = self.m.get(filter=filter)
+        resp = self.m.get(filter=filter, with_defaults=with_defaults)
         response = resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
         return response
 
     @ensure_connected
-    def edit_config(self, *args, **kwargs):
+    def edit_config(self, config, format='xml', target='candidate', default_operation=None, test_option=None, error_option=None):
-        """Loads all or part of the specified *config* to the *target* configuration datastore.
+        """
-
+        Loads all or part of the specified *config* to the *target* configuration datastore.
-            :target: is the name of the configuration datastore being edited
+        :param config: Is the configuration, which must be rooted in the `config` element.
-            :config: is the configuration, which must be rooted in the `config` element.
                        It can be specified either as a string or an :class:`~xml.etree.ElementTree.Element`.
-            :default_operation: if specified must be one of { `"merge"`, `"replace"`, or `"none"` }
+        :param format: The format of configuration eg. xml, text
-            :test_option: if specified must be one of { `"test_then_set"`, `"set"` }
+        :param target: Is the name of the configuration datastore being edited
-            :error_option: if specified must be one of { `"stop-on-error"`, `"continue-on-error"`, `"rollback-on-error"` }
+        :param default_operation: If specified must be one of { `"merge"`, `"replace"`, or `"none"` }
+        :param test_option: If specified must be one of { `"test_then_set"`, `"set"` }
+        :param error_option: If specified must be one of { `"stop-on-error"`, `"continue-on-error"`, `"rollback-on-error"` }
                              The `"rollback-on-error"` *error_option* depends on the `:rollback-on-error` capability.
+        :return: Returns xml string containing the RPC response received from remote host
         """
-        resp = self.m.edit_config(*args, **kwargs)
+        resp = self.m.edit_config(config, format=format, target=target, default_operation=default_operation, test_option=test_option,
+                                  error_option=error_option)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def validate(self, *args, **kwargs):
+    def validate(self, source='candidate'):
-        """Validate the contents of the specified configuration.
-        :source: is the name of the configuration datastore being validated or `config`
-        element containing the configuration subtree to be validated
         """
-        resp = self.m.validate(*args, **kwargs)
+        Validate the contents of the specified configuration.
+        :param source: Is the name of the configuration datastore being validated or `config` element
+                       containing the configuration subtree to be validated
+        :return: Returns xml string containing the RPC response received from remote host
+        """
+        resp = self.m.validate(source=source)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def copy_config(self, *args, **kwargs):
+    def copy_config(self, source, target):
-        """Create or replace an entire configuration datastore with the contents of another complete
+        """
-        configuration datastore.
+        Create or replace an entire configuration datastore with the contents of another complete configuration datastore.
-        :source: is the name of the configuration datastore to use as the source of the
+        :param source: Is the name of the configuration datastore to use as the source of the copy operation or `config`
-                 copy operation or `config` element containing the configuration subtree to copy
+                       element containing the configuration subtree to copy
-        :target: is the name of the configuration datastore to use as the destination of the copy operation"""
+        :param target: Is the name of the configuration datastore to use as the destination of the copy operation
-        resp = self.m.copy_config(*args, **kwargs)
+        :return: Returns xml string containing the RPC response received from remote host
+        """
+        resp = self.m.copy_config(source, target)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def dispatch(self, request):
+    def dispatch(self, rpc_command, source=None, filter=None):
+        """
+        Execute rpc on the remote device eg. dispatch('clear-arp-table')
+        :param rpc_command: specifies rpc command to be dispatched either in plain text or in xml element format (depending on command)
+        :param source: name of the configuration datastore being queried
+        :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
+        """
         """Execute operation on the remote device
         :request: is the rpc request including attributes as XML string
         """
-        req = fromstring(request)
+        req = fromstring(rpc_command)
-        resp = self.m.dispatch(req)
+        resp = self.m.dispatch(req, source=source, filter=filter)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def lock(self, target=None):
+    def lock(self, target="candidate"):
         """
         Allows the client to lock the configuration system of a device.
         :param target: is the name of the configuration datastore to lock,
                         defaults to candidate datastore
         :return: Returns xml string containing the RPC response received from remote host
         """
-        if not target:
-            target = 'candidate'
         resp = self.m.lock(target=target)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def unlock(self, target=None):
+    def unlock(self, target="candidate"):
         """
         Release a configuration lock, previously obtained with the lock operation.
         :param target: is the name of the configuration datastore to unlock,
                        defaults to candidate datastore
         :return: Returns xml string containing the RPC response received from remote host
         """
-        """Release a configuration lock, previously obtained with the lock operation.
-        :target: is the name of the configuration datastore to unlock
-        """
-        if not target:
-            target = 'candidate'
         resp = self.m.unlock(target=target)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
@@ -228,60 +244,95 @@ class NetconfBase(AnsiblePlugin):
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def commit(self, *args, **kwargs):
+    def commit(self, confirmed=False, timeout=None, persist=None):
-        """Commit the candidate configuration as the device's new current configuration.
+        """
+        Commit the candidate configuration as the device's new current configuration.
         Depends on the `:candidate` capability.
         A confirmed commit (i.e. if *confirmed* is `True`) is reverted if there is no
         followup commit within the *timeout* interval. If no timeout is specified the
         confirm timeout defaults to 600 seconds (10 minutes).
         A confirming commit may have the *confirmed* parameter but this is not required.
         Depends on the `:confirmed-commit` capability.
-        :confirmed: whether this is a confirmed commit
+        :param confirmed: whether this is a confirmed commit
-        :timeout: specifies the confirm timeout in seconds
+        :param timeout: specifies the confirm timeout in seconds
+        :param persist: make the confirmed commit survive a session termination,
+                        and set a token on the ongoing confirmed commit
+        :return: Returns xml string containing the RPC response received from remote host
         """
-        resp = self.m.commit(*args, **kwargs)
+        resp = self.m.commit(confirmed=confirmed, timeout=timeout, persist=persist)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def get_schema(self, *args, **kwargs):
+    def get_schema(self, identifier, version=None, format=None):
-        """Retrieves the required schema from the device
         """
-        resp = self.m.get_schema(*args, **kwargs)
+        Retrieve a named schema, with optional revision and type.
+        :param identifier: name of the schema to be retrieved
+        :param version: version of schema to get
+        :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
+        """
+        resp = self.m.get_schema(identifier, version=version, format=format)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
     @ensure_connected
-    def locked(self, *args, **kwargs):
+    def locked(self, target):
-        resp = self.m.locked(*args, **kwargs)
+        """
-        return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
+        Returns a context manager for a lock on a datastore
+        :param target: Name of the configuration datastore to lock
+        :return: Locked context object
+        """
+        return self.m.locked(target)
 
     @abstractmethod
     def get_capabilities(self):
-        """Retrieves device information and supported
+        """
+        Retrieves device information and supported
         rpc methods by device platform and return result
         as a string
+        :return: Netconf session capability
         """
         pass
 
     @staticmethod
     def guess_network_os(obj):
-        """Identifies the operating system of
+        """
-            network device.
+        Identifies the operating system of network device.
+        :param obj: ncclient manager connection instance
+        :return: The name of network operating system.
         """
         pass
 
     def get_base_rpc(self):
-        """Returns list of base rpc method supported by remote device"""
+        """
-        return ['get_config', 'edit_config', 'get_capabilities', 'get']
+        Returns list of base rpc method supported by remote device
+        :return: List of RPC supported
+        """
+        return self.__rpc__
 
     def put_file(self, source, destination):
-        """Copies file over scp to remote device"""
+        """
+        Copies file to remote host
+        :param source: Source location of file
+        :param destination: Destination file path
+        :return: Returns xml string containing the RPC response received from remote host
+        """
         pass
 
     def fetch_file(self, source, destination):
-        """Fetch file over scp from remote device"""
+        """
+        Fetch file from remote host
+        :param source: Source location of file
+        :param destination: Source location of file
+        :return: Returns xml string containing the RPC response received from remote host
+        """
         pass
 
     def get_device_operations(self, server_capabilities):
+        """
+        Retrieve remote host capability from Netconf server hello message.
+        :param server_capabilities: Server capabilities received during Netconf session initialization
+        :return: Remote host capabilities in dictionary format
+        """
         operations = {}
         capabilities = '\n'.join(server_capabilities)
         operations['supports_commit'] = ':candidate' in capabilities

lib/ansible/plugins/netconf/iosxr.py

@@ -93,7 +93,11 @@ class Netconf(NetconfBase):
 
     @staticmethod
     def guess_network_os(obj):
-
+        """
+        Guess the remote network os name
+        :param obj: Netconf connection class object
+        :return: Network OS name
+        """
         try:
             m = manager.connect(
                 host=obj._play_context.remote_addr,
@@ -140,25 +144,26 @@ class Netconf(NetconfBase):
             raise Exception(to_xml(exc.xml))
 
     @ensure_connected
-    def edit_config(self, *args, **kwargs):
+    def edit_config(self, config, format='xml', target='candidate', default_operation=None, test_option=None, error_option=None):
         try:
-            response = self.m.edit_config(*args, **kwargs)
+            response = self.m.edit_config(config, format=format, target=target, default_operation=default_operation, test_option=test_option,
+                                          error_option=error_option)
             return remove_namespaces(response)
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
     @ensure_connected
-    def commit(self, *args, **kwargs):
+    def commit(self, confirmed=False, timeout=None, persist=None):
         try:
-            response = self.m.commit(*args, **kwargs)
+            response = self.m.commit(confirmed=confirmed, timeout=timeout, persist=persist)
             return remove_namespaces(response)
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
     @ensure_connected
-    def validate(self, *args, **kwargs):
+    def validate(self, source="candidate"):
         try:
-            response = self.m.validate(*args, **kwargs)
+            response = self.m.validate(source=source)
             return remove_namespaces(response)
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))

lib/ansible/plugins/netconf/junos.py

@@ -61,23 +61,29 @@ class Netconf(NetconfBase):
 
     @ensure_connected
     def execute_rpc(self, name):
-        """RPC to be execute on remote device
+        """
-           :name: Name of rpc in string format"""
+        RPC to be execute on remote device
+        :param name: Name of rpc in string format
+        :return: Received rpc response from remote host
+        """
         return self.rpc(name)
 
     @ensure_connected
-    def load_configuration(self, *args, **kwargs):
+    def load_configuration(self, format='xml', action='merge', target='candidate', config=None):
-        """Loads given configuration on device
+        """
-        :format: Format of configuration (xml, text, set)
+        Load given configuration on device
-        :action: Action to be performed (merge, replace, override, update)
+        :param format: Format of configuration (xml, text, set)
-        :target: is the name of the configuration datastore being edited
+        :param action: Action to be performed (merge, replace, override, update)
-        :config: is the configuration in string format."""
+        :param target: The name of the configuration datastore being edited
-        if kwargs.get('config'):
+        :param config: The configuration to be loaded on remote host in string format
-            if kwargs.get('format', 'xml') == 'xml':
+        :return: Received rpc response from remote host in string format
-                kwargs['config'] = to_ele(kwargs['config'])
+        """
+        if config:
+            if format == 'xml':
+                config = to_ele(config)
 
         try:
-            return self.m.load_configuration(*args, **kwargs).data_xml
+            return self.m.load_configuration(format=format, action=action, target=target, config=config).data_xml
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
@@ -96,7 +102,11 @@ class Netconf(NetconfBase):
 
     @staticmethod
     def guess_network_os(obj):
-
+        """
+        Guess the remote network os name
+        :param obj: Netconf connection class object
+        :return: Network OS name
+        """
         try:
             m = manager.connect(
                 host=obj._play_context.remote_addr,
@@ -121,18 +131,25 @@ class Netconf(NetconfBase):
         return guessed_os
 
     @ensure_connected
-    def get_configuration(self, *args, **kwargs):
+    def get_configuration(self, format='xml', filter=None):
-        """Retrieve all or part of a specified configuration.
+        """
-           :format: format in configuration should be retrieved
+        Retrieve all or part of a specified configuration.
-           :filter: specifies the portion of the configuration to retrieve
+        :param format: format in which configuration should be retrieved
-           (by default entire configuration is retrieved)"""
+        :param filter: specifies the portion of the configuration to retrieve
-        return self.m.get_configuration(*args, **kwargs).data_xml
+        :return: Received rpc response from remote host in string format
+        """
+        return self.m.get_configuration(format=format, filter=filter).data_xml
 
     @ensure_connected
-    def compare_configuration(self, *args, **kwargs):
+    def compare_configuration(self, rollback=0):
-        """Compare configuration
+        """
-           :rollback: rollback id"""
+        Compare the candidate configuration with running configuration
-        return self.m.compare_configuration(*args, **kwargs).data_xml
+        by default. The candidate configuration can be compared with older
+        committed configuration by providing rollback id.
+        :param rollback: Rollback id of previously commited configuration
+        :return: Received rpc response from remote host in string format
+        """
+        return self.m.compare_configuration(rollback=rollback).data_xml
 
     @ensure_connected
     def halt(self):
@@ -150,15 +167,21 @@ class Netconf(NetconfBase):
     # Remove below method after the issue in ncclient is fixed.
     @ensure_connected
     def commit(self, confirmed=False, check=False, timeout=None, comment=None, synchronize=False, at_time=None):
-        """Commit the candidate configuration as the device's new current configuration.
+        """
+        Commit the candidate configuration as the device's new current configuration.
         Depends on the `:candidate` capability.
         A confirmed commit (i.e. if *confirmed* is `True`) is reverted if there is no
         followup commit within the *timeout* interval. If no timeout is specified the
         confirm timeout defaults to 600 seconds (10 minutes).
         A confirming commit may have the *confirmed* parameter but this is not required.
         Depends on the `:confirmed-commit` capability.
-        :confirmed: whether this is a confirmed commit
+        :param confirmed: whether this is a confirmed commit
-        :timeout: specifies the confirm timeout in seconds
+        :param check: Check correctness of syntax
+        :param timeout: specifies the confirm timeout in seconds
+        :param comment: Message to write to commit log
+        :param synchronize: Synchronize commit on remote peers
+        :param at_time: Time at which to activate configuration changes
+        :return: Received rpc response from remote host
         """
         obj = new_ele('commit-configuration')
         if confirmed: