Bug 4709 - Add keepalive response timeout to KeepaliveSalFacade of netconf connector.

Change-Id: I8b1a5b6a2003b0d35c8acc463621145e04164e4f
Signed-off-by: adetalhouet <adetalhouet@inocybe.com>

diff --git a/opendaylight/md-sal/sal-netconf-connector/pom.xml b/opendaylight/md-sal/sal-netconf-connector/pom.xml
index 5ff7daca431fa496d77f7ffb5f860e786aca5758..b60b9674141c46482aef489368e321cd9accbb73 100644 (file)
--- a/opendaylight/md-sal/sal-netconf-connector/pom.xml
+++ b/opendaylight/md-sal/sal-netconf-connector/pom.xml
@@ -175,8 +175,8 @@
       <scope>test</scope>
     </dependency>
     <dependency>
-      <groupId>org.opendaylight.yangtools</groupId>
-      <artifactId>mockito-configuration</artifactId>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-all</artifactId>
       <scope>test</scope>
     </dependency>
   </dependencies>

diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/KeepaliveSalFacade.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/KeepaliveSalFacade.java
index 4b7b46acd14a607c745cfd3ca6aa9a44fd01c63b..58e2ea38b87c11bfde07547260cdaa79f1896bcf 100644 (file)
--- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/KeepaliveSalFacade.java
+++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/KeepaliveSalFacade.java
@@ -128,7 +128,7 @@ public final class KeepaliveSalFacade implements RemoteDeviceHandler<NetconfSess
     private void scheduleKeepalive() {
         Preconditions.checkState(currentDeviceRpc != null);
         LOG.trace("{}: Scheduling next keepalive in {} {}", id, keepaliveDelaySeconds, TimeUnit.SECONDS);
-        currentKeepalive = executor.schedule(new Keepalive(), keepaliveDelaySeconds, TimeUnit.SECONDS);
+        currentKeepalive = executor.schedule(new Keepalive(currentKeepalive), keepaliveDelaySeconds, TimeUnit.SECONDS);
     }
 
     @Override
@@ -168,12 +168,22 @@ public final class KeepaliveSalFacade implements RemoteDeviceHandler<NetconfSess
      */
     private class Keepalive implements Runnable, FutureCallback<DOMRpcResult> {
 
+        private final ScheduledFuture<?> previousKeepalive;
+
+        public Keepalive(final ScheduledFuture<?> previousKeepalive) {
+            this.previousKeepalive = previousKeepalive;
+        }
+
         @Override
         public void run() {
             LOG.trace("{}: Invoking keepalive RPC", id);
 
             try {
-                Futures.addCallback(currentDeviceRpc.invokeRpc(PATH, KEEPALIVE_PAYLOAD), this);
+                if(previousKeepalive != null && !previousKeepalive.isDone()) {
+                    onFailure(new IllegalStateException("Previous keepalive timed out"));
+                } else {
+                    Futures.addCallback(currentDeviceRpc.invokeRpc(PATH, KEEPALIVE_PAYLOAD), this);
+                }
             } catch (NullPointerException e) {
                 LOG.debug("{}: Skipping keepalive while reconnecting", id);
                 // Empty catch block intentional

diff --git a/opendaylight/md-sal/sal-netconf-connector/src/test/java/org/opendaylight/controller/sal/connect/netconf/sal/KeepaliveSalFacadeTest.java b/opendaylight/md-sal/sal-netconf-connector/src/test/java/org/opendaylight/controller/sal/connect/netconf/sal/KeepaliveSalFacadeTest.java
index f1693e66897e146862a61097b51c3d9b52c1aecf..c3e94be746e328985901578de21bf4ad4ec616fc 100644 (file)
--- a/opendaylight/md-sal/sal-netconf-connector/src/test/java/org/opendaylight/controller/sal/connect/netconf/sal/KeepaliveSalFacadeTest.java
+++ b/opendaylight/md-sal/sal-netconf-connector/src/test/java/org/opendaylight/controller/sal/connect/netconf/sal/KeepaliveSalFacadeTest.java
@@ -18,11 +18,16 @@ import static org.mockito.Mockito.verify;
 
 import java.net.InetSocketAddress;
 import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
 
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Matchers;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
@@ -50,7 +55,7 @@ public class KeepaliveSalFacadeTest {
     @Mock
     private RemoteDeviceHandler<NetconfSessionPreferences> underlyingSalFacade;
 
-    private static java.util.concurrent.ScheduledExecutorService executorService;
+    private ScheduledExecutorService executorServiceSpy;
 
     @Mock
     private NetconfDeviceCommunicator listener;
@@ -59,9 +64,11 @@ public class KeepaliveSalFacadeTest {
 
     private DOMRpcService proxyRpc;
 
+    @Mock
+    private ScheduledFuture currentKeepalive;
+
     @Before
     public void setUp() throws Exception {
-        executorService = Executors.newScheduledThreadPool(1);
 
         MockitoAnnotations.initMocks(this);
 
@@ -69,11 +76,25 @@ public class KeepaliveSalFacadeTest {
         doReturn("mockedRpc").when(deviceRpc).toString();
         doNothing().when(underlyingSalFacade).onDeviceConnected(
                 any(SchemaContext.class), any(NetconfSessionPreferences.class), any(DOMRpcService.class));
+
+        ScheduledExecutorService executorService = Executors.newScheduledThreadPool(1);
+        executorServiceSpy = Mockito.spy(executorService);
+        doAnswer(new Answer<ScheduledFuture>() {
+            @Override
+            public ScheduledFuture answer(InvocationOnMock invocationOnMock)
+                    throws Throwable {
+                invocationOnMock.callRealMethod();
+                return currentKeepalive;
+            }
+        }).when(executorServiceSpy).schedule(Mockito.<Runnable> any(),
+                Mockito.anyLong(), Matchers.<TimeUnit> any());
+
+        Mockito.when(currentKeepalive.isDone()).thenReturn(true);
     }
 
     @After
     public void tearDown() throws Exception {
-        executorService.shutdownNow();
+        executorServiceSpy.shutdownNow();
     }
 
     @Test
@@ -81,10 +102,12 @@ public class KeepaliveSalFacadeTest {
         final DOMRpcResult result = new DefaultDOMRpcResult(Builders.containerBuilder().withNodeIdentifier(
                 new YangInstanceIdentifier.NodeIdentifier(NetconfMessageTransformUtil.NETCONF_RUNNING_QNAME)).build());
 
-        doReturn(Futures.immediateCheckedFuture(result)).when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class));
+        doReturn(Futures.immediateCheckedFuture(result))
+                 .when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class));
 
         final KeepaliveSalFacade keepaliveSalFacade =
-                new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorService, 1L);
+                new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 1L);
+
         keepaliveSalFacade.setListener(listener);
 
         keepaliveSalFacade.onDeviceConnected(null, null, deviceRpc);
@@ -111,7 +134,7 @@ public class KeepaliveSalFacadeTest {
                 .when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class));
 
         final KeepaliveSalFacade keepaliveSalFacade =
-                new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorService, 1L);
+                new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 1L);
         keepaliveSalFacade.setListener(listener);
 
         keepaliveSalFacade.onDeviceConnected(null, null, deviceRpc);
@@ -147,6 +170,12 @@ public class KeepaliveSalFacadeTest {
 
         // 1 failed that results in disconnect, 3 total with previous fail
         verify(listener, timeout(15000).times(3)).disconnect();
+
+
+        Mockito.when(currentKeepalive.isDone()).thenReturn(false);
+        keepaliveSalFacade.onDeviceConnected(null, null, deviceRpc);
+        // 1 failed that results in disconnect, 4 total with previous fail
+        verify(listener, timeout(15000).times(4)).disconnect();
     }
 
     @Test
@@ -163,7 +192,7 @@ public class KeepaliveSalFacadeTest {
                 .when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class));
 
         final KeepaliveSalFacade keepaliveSalFacade =
-                new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorService, 100L);
+                new KeepaliveSalFacade(REMOTE_DEVICE_ID, underlyingSalFacade, executorServiceSpy, 100L);
         keepaliveSalFacade.setListener(listener);
 
         keepaliveSalFacade.onDeviceConnected(null, null, deviceRpc);