Close device's source resolution on teardown

Checking the current connection status is not enough to guarantee that
schema setup was completed for the initial session-up event.

To resolve this close ListenableFuture for device schema resolution
when the 'onRemoteSessionDown' method is called.

This prevents unnecessary duplicate notification, creating duplicate
SchemaContext and avoid confusion in the connection state.

JIRA: NETCONF-1173
Change-Id: I7f84da4a9197db52a0c33d2640e0c2543e587d7b
(cherry picked from commit e9518fdc4410b5407318d4b3bcc7fa333e296c55)
Signed-off-by: Sangwook Ha <sangwook.ha@verizon.com>
Signed-off-by: Peter Suna <peter.suna@pantheon.tech>
Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>

diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDevice.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDevice.java
index 4ffafe55b29528219898f0c3ba2df8847f40d803..ca821798b1f724536fb3e46fa91329dc0becde13 100644 (file)
--- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDevice.java
+++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDevice.java
@@ -31,6 +31,7 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.CancellationException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
@@ -116,6 +117,8 @@ public class NetconfDevice
     private final EventExecutor eventExecutor;
     private final NetconfNodeAugmentedOptional nodeOptional;
 
+    @GuardedBy("this")
+    private ListenableFuture<List<Object>> schemaFuturesList;
     @GuardedBy("this")
     private boolean connected = false;
 
@@ -151,8 +154,8 @@ public class NetconfDevice
     }
 
     @Override
-    public void onRemoteSessionUp(final NetconfSessionPreferences remoteSessionCapabilities,
-                                  final NetconfDeviceCommunicator listener) {
+    public synchronized 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
@@ -178,6 +181,7 @@ public class NetconfDevice
         // Potentially acquire mount point list and interpret it
         final ListenableFuture<MountPointContext> futureContext = Futures.transformAsync(futureSchema,
             schemaContext -> createMountPointContext(schemaContext, baseSchema, listener), processingExecutor);
+        schemaFuturesList = Futures.allAsList(sourceResolverFuture, futureSchema, futureContext);
 
         Futures.addCallback(futureContext, new FutureCallback<MountPointContext>() {
             @Override
@@ -188,8 +192,12 @@ public class NetconfDevice
 
             @Override
             public void onFailure(final Throwable cause) {
-                LOG.warn("{}: Unexpected error resolving device sources", id, cause);
+                if (cause instanceof CancellationException) {
+                    LOG.warn("{}: Device communicator was tear down since the schema setup started", id);
+                    return;
+                }
 
+                LOG.warn("{}: Unexpected error resolving device sources", id, cause);
                 // No more sources, fail or try to reconnect
                 if (cause instanceof EmptySchemaContextException) {
                     if (nodeOptional != null && nodeOptional.getIgnoreMissingSchemaSources().getAllowed()) {
@@ -342,10 +350,14 @@ public class NetconfDevice
     }
 
     @Override
-    public void onRemoteSessionDown() {
+    public synchronized void onRemoteSessionDown() {
         setConnected(false);
+        if (schemaFuturesList != null && !schemaFuturesList.isDone()) {
+            if (!schemaFuturesList.cancel(true)) {
+                LOG.warn("The cleanup of Schema Futures for device {} was unsuccessful.", id);
+            }
+        }
         notificationHandler.onRemoteSchemaDown();
-
         salFacade.onDeviceDisconnected();
         sourceRegistrations.forEach(SchemaSourceRegistration::close);
         sourceRegistrations.clear();

diff --git a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDeviceTest.java b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDeviceTest.java
index 656b6697873b03d7bb4bf06aae7d1aa93c7174c4..12dcf90b218f3664650889d36bf4e493eec8359d 100644 (file)
--- a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDeviceTest.java
+++ b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfDeviceTest.java
@@ -372,6 +372,45 @@ public class NetconfDeviceTest extends AbstractTestModelTest {
         verify(facade, after(1000).never()).onDeviceConnected(any(), any(), any(), any(DOMActionService.class));
     }
 
+    @Test
+    public void testNetconfDeviceReconnectBeforeSchemaSetup() throws Exception {
+        final RemoteDeviceHandler<NetconfSessionPreferences>  facade = getFacade();
+
+        final EffectiveModelContextFactory schemaContextProviderFactory = mock(EffectiveModelContextFactory.class);
+        final SettableFuture<SchemaContext> schemaFuture = SettableFuture.create();
+        doReturn(schemaFuture).when(schemaContextProviderFactory).createEffectiveModelContext(anyCollection());
+
+        final NetconfDevice.SchemaResourcesDTO schemaResourcesDTO = new NetconfDevice.SchemaResourcesDTO(
+            getSchemaRegistry(), getSchemaRepository(), schemaContextProviderFactory, STATE_SCHEMAS_RESOLVER);
+        final NetconfDevice device = new NetconfDeviceBuilder()
+            .setReconnectOnSchemasChange(true)
+            .setSchemaResourcesDTO(schemaResourcesDTO)
+            .setGlobalProcessingExecutor(getExecutor())
+            .setId(getId())
+            .setSalFacade(facade)
+            .setBaseSchemas(BASE_SCHEMAS)
+            .build();
+        final NetconfSessionPreferences sessionCaps = getSessionCaps(true,
+            List.of(TEST_NAMESPACE + "?module=" + TEST_MODULE + "&amp;revision=" + TEST_REVISION));
+
+        final NetconfDeviceCommunicator listener = getListener();
+        // session up, start schema resolution
+        device.onRemoteSessionUp(sessionCaps, listener);
+        // session down
+        device.onRemoteSessionDown();
+        verify(facade, timeout(5000)).onDeviceDisconnected();
+        // session back up, start another schema resolution
+        device.onRemoteSessionUp(sessionCaps, listener);
+        // complete schema setup
+        schemaFuture.set(SCHEMA_CONTEXT);
+        // schema setup performed twice
+        verify(schemaContextProviderFactory, timeout(5000)).createEffectiveModelContext(anyCollection());
+        // onDeviceConnected called once
+        verify(facade, timeout(5000)).onDeviceConnected(
+            any(MountPointContext.class), any(NetconfSessionPreferences.class), any(DOMRpcService.class),
+            isNull());
+    }
+
     @Test
     public void testNetconfDeviceAvailableCapabilitiesBuilding() throws Exception {
         final RemoteDeviceHandler<NetconfSessionPreferences> facade = getFacade();