Fix netconf post "delay persistent connections" (#63463) (#65137)

* ensure_connect on manager use

* Remove ensure_connected from individual netconf plugins

(cherry picked from commit f5e0995cae)

changelogs/fragments/netconf_persistent_fix.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- netconf_rpc module does not work with nxos (https://github.com/ansible/ansible/issues/63255).

lib/ansible/plugins/connection/netconf.py

@@ -186,9 +186,10 @@ import json
 
 from ansible.errors import AnsibleConnectionFailure, AnsibleError
 from ansible.module_utils._text import to_bytes, to_native, to_text
+from ansible.module_utils.basic import missing_required_lib
 from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE, BOOLEANS_FALSE
 from ansible.plugins.loader import netconf_loader
-from ansible.plugins.connection import NetworkConnectionBase
+from ansible.plugins.connection import NetworkConnectionBase, ensure_connect
 
 try:
     from ncclient import manager
@@ -263,12 +264,14 @@ class Connection(NetworkConnectionBase):
         else:
             return super(Connection, self).exec_command(cmd, in_data, sudoable)
 
+    @property
+    @ensure_connect
+    def manager(self):
+        return self._manager
+
     def _connect(self):
         if not HAS_NCCLIENT:
-            raise AnsibleError(
-                'The required "ncclient" python library is required to use the netconf connection type: %s.\n'
-                'Please run pip install ncclient' % to_native(NCCLIENT_IMP_ERR)
-            )
+            raise AnsibleError("%s: %s" % (missing_required_lib("ncclient"), to_native(NCCLIENT_IMP_ERR)))
 
         self.queue_message('log', 'ssh connection done, starting ncclient')
 

lib/ansible/plugins/netconf/__init__.py

@@ -42,15 +42,6 @@ except ImportError:
     from xml.etree.ElementTree import Element, SubElement, tostring, fromstring
 
 
-def ensure_connected(func):
-    @wraps(func)
-    def wrapped(self, *args, **kwargs):
-        if not self._connection._connected:
-            self._connection._connect()
-        return func(self, *args, **kwargs)
-    return wrapped
-
-
 def ensure_ncclient(func):
     @wraps(func)
     def wrapped(self, *args, **kwargs):
@@ -118,10 +109,8 @@ class NetconfBase(AnsiblePlugin):
 
     @property
     def m(self):
-        return self._connection._manager
+        return self._connection.manager
 
-    @ensure_ncclient
-    @ensure_connected
     def rpc(self, name):
         """
         RPC to be execute on remote device
@@ -136,7 +125,6 @@ class NetconfBase(AnsiblePlugin):
             msg = exc.xml
             raise Exception(to_xml(msg))
 
-    @ensure_connected
     def get_config(self, source=None, filter=None):
         """
         Retrieve all or part of a specified configuration
@@ -153,7 +141,6 @@ class NetconfBase(AnsiblePlugin):
         resp = self.m.get_config(source=source, filter=filter)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
-    @ensure_connected
     def get(self, filter=None, with_defaults=None):
         """
         Retrieve device configuration and state information.
@@ -169,7 +156,6 @@ class NetconfBase(AnsiblePlugin):
         response = resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
         return response
 
-    @ensure_connected
     def edit_config(self, config=None, 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.
@@ -189,7 +175,6 @@ class NetconfBase(AnsiblePlugin):
                                   error_option=error_option)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
-    @ensure_connected
     def validate(self, source='candidate'):
         """
         Validate the contents of the specified configuration.
@@ -200,7 +185,6 @@ class NetconfBase(AnsiblePlugin):
         resp = self.m.validate(source=source)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
-    @ensure_connected
     def copy_config(self, source, target):
         """
         Create or replace an entire configuration datastore with the contents of another complete configuration datastore.
@@ -212,7 +196,6 @@ class NetconfBase(AnsiblePlugin):
         resp = self.m.copy_config(source, target)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
-    @ensure_connected
     def dispatch(self, rpc_command=None, source=None, filter=None):
         """
         Execute rpc on the remote device eg. dispatch('clear-arp-table')
@@ -240,7 +223,6 @@ class NetconfBase(AnsiblePlugin):
 
         return result
 
-    @ensure_connected
     def lock(self, target="candidate"):
         """
         Allows the client to lock the configuration system of a device.
@@ -251,7 +233,6 @@ class NetconfBase(AnsiblePlugin):
         resp = self.m.lock(target=target)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
-    @ensure_connected
     def unlock(self, target="candidate"):
         """
         Release a configuration lock, previously obtained with the lock operation.
@@ -262,7 +243,6 @@ class NetconfBase(AnsiblePlugin):
         resp = self.m.unlock(target=target)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
-    @ensure_connected
     def discard_changes(self):
         """
         Revert the candidate configuration to the currently running configuration.
@@ -272,7 +252,6 @@ class NetconfBase(AnsiblePlugin):
         resp = self.m.discard_changes()
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
-    @ensure_connected
     def commit(self, confirmed=False, timeout=None, persist=None):
         """
         Commit the candidate configuration as the device's new current configuration.
@@ -291,7 +270,6 @@ class NetconfBase(AnsiblePlugin):
         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, identifier=None, version=None, format=None):
         """
         Retrieve a named schema, with optional revision and type.
@@ -303,7 +281,6 @@ class NetconfBase(AnsiblePlugin):
         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 delete_config(self, target):
         """
         delete a configuration datastore
@@ -313,7 +290,6 @@ class NetconfBase(AnsiblePlugin):
         resp = self.m.delete_config(target)
         return resp.data_xml if hasattr(resp, 'data_xml') else resp.xml
 
-    @ensure_connected
     def locked(self, target):
         return self.m.locked(target)
 
@@ -387,7 +363,7 @@ class NetconfBase(AnsiblePlugin):
         if operations['supports_startup']:
             operations['lock_datastore'].append('startup')
 
-        operations['supports_lock'] = True if len(operations['lock_datastore']) else False
+        operations['supports_lock'] = bool(operations['lock_datastore'])
 
         return operations
 

lib/ansible/plugins/netconf/ce.py

@@ -22,10 +22,9 @@ __metaclass__ = type
 import json
 import re
 
-from ansible.module_utils._text import to_text, to_bytes, to_native
+from ansible.module_utils._text import to_text, to_bytes
 from ansible.errors import AnsibleConnectionFailure
-from ansible.plugins.netconf import NetconfBase
-from ansible.plugins.netconf import ensure_connected, ensure_ncclient
+from ansible.plugins.netconf import NetconfBase, ensure_ncclient
 
 try:
     from ncclient import manager
@@ -66,14 +65,12 @@ class Netconf(NetconfBase):
 
         return device_info
 
-    @ensure_connected
     def execute_rpc(self, name):
         """RPC to be execute on remote device
            :name: Name of rpc in string format"""
         return self.rpc(name)
 
     @ensure_ncclient
-    @ensure_connected
     def load_configuration(self, *args, **kwargs):
         """Loads given configuration on device
         :format: Format of configuration (xml, text, set)
@@ -121,7 +118,7 @@ class Netconf(NetconfBase):
                 ssh_config=obj._ssh_config
             )
         except SSHUnknownHostError as exc:
-            raise AnsibleConnectionFailure(to_native(exc))
+            raise AnsibleConnectionFailure(to_text(exc))
 
         guessed_os = None
         for c in m.server_capabilities:
@@ -132,7 +129,6 @@ class Netconf(NetconfBase):
         m.close_session()
         return guessed_os
 
-    @ensure_connected
     def get_configuration(self, *args, **kwargs):
         """Retrieve all or part of a specified configuration.
            :format: format in configuration should be retrieved
@@ -140,14 +136,12 @@ class Netconf(NetconfBase):
            (by default entire configuration is retrieved)"""
         return self.m.get_configuration(*args, **kwargs).data_xml
 
-    @ensure_connected
     def compare_configuration(self, *args, **kwargs):
         """Compare configuration
            :rollback: rollback id"""
         return self.m.compare_configuration(*args, **kwargs).data_xml
 
     @ensure_ncclient
-    @ensure_connected
     def execute_action(self, xml_str):
         """huawei execute-action"""
         con_obj = None
@@ -158,18 +152,15 @@ class Netconf(NetconfBase):
 
         return con_obj.xml
 
-    @ensure_connected
     def halt(self):
         """reboot the device"""
         return self.m.halt().data_xml
 
-    @ensure_connected
     def reboot(self):
         """reboot the device"""
         return self.m.reboot().data_xml
 
     @ensure_ncclient
-    @ensure_connected
     def get(self, *args, **kwargs):
         try:
             if_rpc_reply = kwargs.pop('if_rpc_reply', False)
@@ -180,7 +171,6 @@ class Netconf(NetconfBase):
             raise Exception(to_xml(exc.xml))
 
     @ensure_ncclient
-    @ensure_connected
     def get_config(self, *args, **kwargs):
         try:
             return self.m.get_config(*args, **kwargs).data_xml
@@ -188,7 +178,6 @@ class Netconf(NetconfBase):
             raise Exception(to_xml(exc.xml))
 
     @ensure_ncclient
-    @ensure_connected
     def edit_config(self, *args, **kwargs):
         try:
             return self.m.edit_config(*args, **kwargs).xml
@@ -196,7 +185,6 @@ class Netconf(NetconfBase):
             raise Exception(to_xml(exc.xml))
 
     @ensure_ncclient
-    @ensure_connected
     def execute_nc_cli(self, *args, **kwargs):
         try:
             return self.m.cli(*args, **kwargs).xml
@@ -204,23 +192,19 @@ class Netconf(NetconfBase):
             raise Exception(to_xml(exc.xml))
 
     @ensure_ncclient
-    @ensure_connected
     def commit(self, *args, **kwargs):
         try:
             return self.m.commit(*args, **kwargs).data_xml
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
-    @ensure_connected
     def validate(self, *args, **kwargs):
         return self.m.validate(*args, **kwargs).data_xml
 
-    @ensure_connected
     def discard_changes(self, *args, **kwargs):
         return self.m.discard_changes(*args, **kwargs).data_xml
 
     @ensure_ncclient
-    @ensure_connected
     def dispatch_rpc(self, rpc_command=None, source=None, filter=None):
         """
         Execute rpc on the remote device eg. dispatch('get-next')

lib/ansible/plugins/netconf/iosxr.py

@@ -28,8 +28,7 @@ from ansible.module_utils._text import to_native
 from ansible.module_utils.network.common.netconf import remove_namespaces
 from ansible.module_utils.network.iosxr.iosxr import build_xml, etree_find
 from ansible.errors import AnsibleConnectionFailure
-from ansible.plugins.netconf import NetconfBase
-from ansible.plugins.netconf import ensure_connected, ensure_ncclient
+from ansible.plugins.netconf import NetconfBase, ensure_ncclient
 
 try:
     from ncclient import manager
@@ -42,7 +41,6 @@ except (ImportError, AttributeError):  # paramiko and gssapi are incompatible an
 
 
 class Netconf(NetconfBase):
-    @ensure_connected
     def get_device_info(self):
         device_info = {}
         device_info['network_os'] = 'iosxr'
@@ -122,8 +120,6 @@ class Netconf(NetconfBase):
         return guessed_os
 
     # TODO: change .xml to .data_xml, when ncclient supports data_xml on all platforms
-    @ensure_ncclient
-    @ensure_connected
     def get(self, filter=None, remove_ns=False):
         if isinstance(filter, list):
             filter = tuple(filter)
@@ -137,8 +133,6 @@ class Netconf(NetconfBase):
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
-    @ensure_ncclient
-    @ensure_connected
     def get_config(self, source=None, filter=None, remove_ns=False):
         if isinstance(filter, list):
             filter = tuple(filter)
@@ -152,8 +146,6 @@ class Netconf(NetconfBase):
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
-    @ensure_ncclient
-    @ensure_connected
     def edit_config(self, config=None, format='xml', target='candidate', default_operation=None, test_option=None, error_option=None, remove_ns=False):
         if config is None:
             raise ValueError('config value must be provided')
@@ -168,8 +160,6 @@ class Netconf(NetconfBase):
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
-    @ensure_ncclient
-    @ensure_connected
     def commit(self, confirmed=False, timeout=None, persist=None, remove_ns=False):
         try:
             resp = self.m.commit(confirmed=confirmed, timeout=timeout, persist=persist)
@@ -181,8 +171,6 @@ class Netconf(NetconfBase):
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
-    @ensure_ncclient
-    @ensure_connected
     def validate(self, source="candidate", remove_ns=False):
         try:
             resp = self.m.validate(source=source)
@@ -194,8 +182,6 @@ class Netconf(NetconfBase):
         except RPCError as exc:
             raise Exception(to_xml(exc.xml))
 
-    @ensure_ncclient
-    @ensure_connected
     def discard_changes(self, remove_ns=False):
         try:
             resp = self.m.discard_changes()

lib/ansible/plugins/netconf/junos.py

@@ -25,8 +25,7 @@ import re
 from ansible.module_utils._text import to_text, to_native
 from ansible.module_utils.six import string_types
 from ansible.errors import AnsibleConnectionFailure
-from ansible.plugins.netconf import NetconfBase
-from ansible.plugins.netconf import ensure_connected, ensure_ncclient
+from ansible.plugins.netconf import NetconfBase, ensure_ncclient
 
 try:
     from ncclient import manager
@@ -60,7 +59,6 @@ class Netconf(NetconfBase):
 
         return device_info
 
-    @ensure_connected
     def execute_rpc(self, name):
         """
         RPC to be execute on remote device
@@ -70,7 +68,6 @@ class Netconf(NetconfBase):
         return self.rpc(name)
 
     @ensure_ncclient
-    @ensure_connected
     def load_configuration(self, format='xml', action='merge', target='candidate', config=None):
         """
         Load given configuration on device
@@ -136,7 +133,6 @@ class Netconf(NetconfBase):
         m.close_session()
         return guessed_os
 
-    @ensure_connected
     def get_configuration(self, format='xml', filter=None):
         """
         Retrieve all or part of a specified configuration.
@@ -153,7 +149,6 @@ class Netconf(NetconfBase):
 
         return self.m.get_configuration(format=format, filter=filter).data_xml
 
-    @ensure_connected
     def compare_configuration(self, rollback=0):
         """
         Compare the candidate configuration with running configuration
@@ -164,12 +159,10 @@ class Netconf(NetconfBase):
         """
         return self.m.compare_configuration(rollback=rollback).data_xml
 
-    @ensure_connected
     def halt(self):
         """reboot the device"""
         return self.m.halt().data_xml
 
-    @ensure_connected
     def reboot(self):
         """reboot the device"""
         return self.m.reboot().data_xml
@@ -179,7 +172,6 @@ class Netconf(NetconfBase):
     # ncclient generic rpc() method to execute rpc on remote host.
     # Remove below method after the issue in ncclient is fixed.
     @ensure_ncclient
-    @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.