From 22ab9a17d238298ba03068e76d9d3be68c8aa9ad Mon Sep 17 00:00:00 2001
From: adetalhouet <adetalhouet@inocybe.com>
Date: Thu, 3 Dec 2015 11:03:49 -0500
Subject: [PATCH] Bug 4712 - Fix Keepalive task to check the response of a
 keepalive request

If response is null, tear down the session.

Change-Id: I8ca2302f2f5f0fad09a9ef7ab4511ffb863de343
Signed-off-by: adetalhouet <adetalhouet@inocybe.com>
---
 .../netconf/sal/KeepaliveSalFacade.java       |  9 +++++--
 .../netconf/sal/KeepaliveSalFacadeTest.java   | 24 +++++++++++++++----
 2 files changed, 27 insertions(+), 6 deletions(-)

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 6c29f1d338..4b7b46acd1 100644
--- 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
@@ -185,8 +185,13 @@ public final class KeepaliveSalFacade implements RemoteDeviceHandler<NetconfSess
 
         @Override
         public void onSuccess(final DOMRpcResult result) {
-            LOG.debug("{}: Keepalive RPC successful with response: {}", id, result.getResult());
-            scheduleKeepalive();
+            if (result != null && result.getResult() != null) {
+                LOG.debug("{}: Keepalive RPC successful with response: {}", id, result.getResult());
+                scheduleKeepalive();
+            } else {
+                LOG.warn("{} Keepalive RPC returned null with response: {}. Reconnecting netconf session", id, result);
+                reconnect();
+            }
         }
 
         @Override
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 71d1b831f0..f1693e6689 100644
--- 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
@@ -16,9 +16,9 @@ import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
-import com.google.common.util.concurrent.Futures;
 import java.net.InetSocketAddress;
 import java.util.concurrent.Executors;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -41,6 +41,8 @@ import org.opendaylight.yangtools.yang.data.impl.schema.Builders;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.opendaylight.yangtools.yang.model.api.SchemaPath;
 
+import com.google.common.util.concurrent.Futures;
+
 public class KeepaliveSalFacadeTest {
 
     private static final RemoteDeviceId REMOTE_DEVICE_ID = new RemoteDeviceId("test", new InetSocketAddress("localhost", 22));
@@ -98,10 +100,13 @@ public class KeepaliveSalFacadeTest {
         final DOMRpcResult result = new DefaultDOMRpcResult(Builders.containerBuilder().withNodeIdentifier(
                 new YangInstanceIdentifier.NodeIdentifier(NetconfMessageTransformUtil.NETCONF_RUNNING_QNAME)).build());
 
-        final DOMRpcResult resultFail = new DefaultDOMRpcResult(mock(RpcError.class));
+        RpcError error = mock(RpcError.class);
+        doReturn("Failure").when(error).toString();
+
+        final DOMRpcResult resultFailwithResultAndError = new DefaultDOMRpcResult(mock(NormalizedNode.class), error);
 
         doReturn(Futures.immediateCheckedFuture(result))
-                .doReturn(Futures.immediateCheckedFuture(resultFail))
+                .doReturn(Futures.immediateCheckedFuture(resultFailwithResultAndError))
                 .doReturn(Futures.immediateFailedCheckedFuture(new IllegalStateException("illegal-state")))
                 .when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class));
 
@@ -121,7 +126,7 @@ public class KeepaliveSalFacadeTest {
 
         // Reconnect with same keepalive responses
         doReturn(Futures.immediateCheckedFuture(result))
-                .doReturn(Futures.immediateCheckedFuture(resultFail))
+                .doReturn(Futures.immediateCheckedFuture(resultFailwithResultAndError))
                 .doReturn(Futures.immediateFailedCheckedFuture(new IllegalStateException("illegal-state")))
                 .when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class));
 
@@ -131,6 +136,17 @@ public class KeepaliveSalFacadeTest {
         verify(listener, timeout(15000).times(2)).disconnect();
         // 6 attempts now total
         verify(deviceRpc, times(3 * 2)).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class));
+
+
+        final DOMRpcResult resultFailwithError = new DefaultDOMRpcResult(error);
+
+        doReturn(Futures.immediateCheckedFuture(resultFailwithError))
+                .when(deviceRpc).invokeRpc(any(SchemaPath.class), any(NormalizedNode.class));
+
+        keepaliveSalFacade.onDeviceConnected(null, null, deviceRpc);
+
+        // 1 failed that results in disconnect, 3 total with previous fail
+        verify(listener, timeout(15000).times(3)).disconnect();
     }
 
     @Test
-- 
2.36.6