Clean up NetconfDevice shutdown

When we fail to setup schemas, we should not be calling
onRemoteSessionDown(), as it ends up emitting two changes to
NetconfDeviceListener -- onDeviceDisconnected() and onDeviceFailed().

JIRA: NETCONF-1097
Change-Id: I1b9a3751accfa15791ac2c84a11ad31d51256f16
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit c775691891f930cba1f6f920cadd1fb4cf61d31b)

diff --git a/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDevice.java b/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDevice.java
index c087d22e44d3a5d6020ca85d4fffd4e6151d9993..4c0bdeec221c62dee666c90d4f7cb57a20a6c3cc 100644 (file)
--- a/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDevice.java
+++ b/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDevice.java
@@ -104,9 +104,6 @@ public class NetconfDevice implements RemoteDevice<NetconfDeviceCommunicator> {
     @GuardedBy("this")
     private boolean connected = false;
 
-    // Message transformer is constructed once the schemas are available
-    private NetconfMessageTransformer messageTransformer;
-
     public NetconfDevice(final SchemaResourcesDTO schemaResourcesDTO, final BaseNetconfSchemas baseSchemas,
             final RemoteDeviceId id, final RemoteDeviceHandler salFacade, final Executor globalProcessingExecutor,
             final boolean reconnectOnSchemasChange) {
@@ -132,11 +129,9 @@ public class NetconfDevice implements RemoteDevice<NetconfDeviceCommunicator> {
     @Override
     public void onRemoteSessionUp(final NetconfSessionPreferences remoteSessionCapabilities,
                                   final NetconfDeviceCommunicator listener) {
-        // SchemaContext setup has to be performed in a dedicated thread since
-        // we are in a netty thread in this method
-        // Yang models are being downloaded in this method and it would cause a
-        // deadlock if we used the netty thread
-        // http://netty.io/wiki/thread-model.html
+        // SchemaContext setup has to be performed in a dedicated thread since we are in a Netty thread in this method
+        // YANG models are being downloaded in this method and it would cause a deadlock if we used the netty thread
+        // https://netty.io/wiki/thread-model.html
         setConnected(true);
         LOG.debug("{}: Session to remote device established with {}", id, remoteSessionCapabilities);
 
@@ -221,7 +216,7 @@ public class NetconfDevice implements RemoteDevice<NetconfDeviceCommunicator> {
             return;
         }
 
-        messageTransformer = new NetconfMessageTransformer(deviceSchema.mountContext(), true,
+        final var messageTransformer = new NetconfMessageTransformer(deviceSchema.mountContext(), true,
             resolveBaseSchema(remoteSessionCapabilities.isNotificationsSupported()));
 
         // Order is important here: salFacade has to see the device come up and then the notificationHandler can deliver
@@ -235,26 +230,17 @@ public class NetconfDevice implements RemoteDevice<NetconfDeviceCommunicator> {
     }
 
     private void handleSalInitializationFailure(final RemoteDeviceCommunicator listener, final Throwable cause) {
-        // FIXME: pick one of these messages
         LOG.warn("{}: Unexpected error resolving device sources", id, cause);
-        LOG.error("{}: Initialization in sal failed, disconnecting from device", id, cause);
         listener.close();
-        onRemoteSessionDown();
-        resetMessageTransformer();
-
-        // FIXME: this causes salFacade to see onDeviceDisconnected() and then onDeviceFailed(), which is quite weird
+        cleanupInitialization();
         salFacade.onDeviceFailed(cause);
     }
 
-    /**
-     * Set the transformer to null as is in initial state.
-     */
-    private void resetMessageTransformer() {
-        updateTransformer(null);
-    }
-
-    private synchronized void updateTransformer(final NetconfMessageTransformer transformer) {
-        messageTransformer = transformer;
+    private synchronized void cleanupInitialization() {
+        connected = false;
+        notificationHandler.onRemoteSchemaDown();
+        sourceRegistrations.forEach(Registration::close);
+        sourceRegistrations.clear();
     }
 
     private synchronized void setConnected(final boolean connected) {
@@ -305,13 +291,8 @@ public class NetconfDevice implements RemoteDevice<NetconfDeviceCommunicator> {
 
     @Override
     public void onRemoteSessionDown() {
-        setConnected(false);
-        notificationHandler.onRemoteSchemaDown();
-
+        cleanupInitialization();
         salFacade.onDeviceDisconnected();
-        sourceRegistrations.forEach(Registration::close);
-        sourceRegistrations.clear();
-        resetMessageTransformer();
     }
 
     @Override

diff --git a/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceTest.java b/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceTest.java
index 03212717cbd695ab0d11c7d192f40be989f646f3..d73d1e47ee634fa20019503d25a5a31321f5e521 100644 (file)
--- a/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceTest.java
+++ b/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceTest.java
@@ -173,7 +173,6 @@ public class NetconfDeviceTest extends AbstractTestModelTest {
         final NetconfSessionPreferences sessionCaps = getSessionCaps(false, List.of(TEST_CAPABILITY));
         device.onRemoteSessionUp(sessionCaps, listener);
 
-        verify(facade, timeout(5000)).onDeviceDisconnected();
         final var captor = ArgumentCaptor.forClass(Throwable.class);
         verify(facade, timeout(5000)).onDeviceFailed(captor.capture());
         assertThat(captor.getValue(), instanceOf(EmptySchemaContextException.class));