BGPCEP-724 Make BGP Session recoverable

This patch make BGP Session recoverable when anything wrong
happens during netty session negotiation. When an exception
is captured, the BGP session will be terminated.

Change-Id: Id3fc7ec282a2151e3bd6bdbfa8930e141b3ceaff
Signed-off-by: Kevin Wang <kevixw@gmail.com>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
index 43ba857b83d94440557c20323bce779016a8107d..25223f28677363c5e892da2551873720dbf826f6 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
@@ -209,8 +209,11 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
 
     @Override
     public synchronized void close() {
-        if (this.state != State.IDLE && !this.terminationReasonNotified) {
-            this.writeAndFlush(new NotifyBuilder().setErrorCode(BGPError.CEASE.getCode()).setErrorSubcode(BGPError.CEASE.getSubcode()).build());
+        if (this.state != State.IDLE) {
+            if (!this.terminationReasonNotified) {
+                this.writeAndFlush(new NotifyBuilder().setErrorCode(BGPError.CEASE.getCode())
+                        .setErrorSubcode(BGPError.CEASE.getSubcode()).build());
+            }
             this.closeWithoutMessage();
         }
     }
@@ -262,9 +265,9 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
 
     private synchronized void notifyTerminationReasonAndCloseWithoutMessage(final Short errorCode, final Short errorSubcode) {
         this.terminationReasonNotified = true;
-        this.listener.onSessionTerminated(this, new BGPTerminationReason(
-            BGPError.forValue(errorCode, errorSubcode)));
         this.closeWithoutMessage();
+        this.listener.onSessionTerminated(this, new BGPTerminationReason(
+                BGPError.forValue(errorCode, errorSubcode)));
     }
 
     synchronized void endOfInput() {
@@ -326,7 +329,8 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
      *
      * @param e BGPDocumentedException
      */
-    private synchronized void terminate(final BGPDocumentedException e) {
+    @VisibleForTesting
+    synchronized void terminate(final BGPDocumentedException e) {
         final BGPError error = e.getError();
         final byte[] data = e.getData();
         final NotifyBuilder builder = new NotifyBuilder().setErrorCode(error.getCode()).setErrorSubcode(error.getSubcode());
@@ -416,8 +420,13 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
     protected synchronized void sessionUp() {
         this.sessionStats.startSessionStopwatch();
         this.state = State.UP;
-        this.sessionState.setSessionState(this.state);
-        this.listener.onSessionUp(this);
+        try {
+            this.sessionState.setSessionState(this.state);
+            this.listener.onSessionUp(this);
+        } catch (final Exception e) {
+            handleException(e);
+            throw e;
+        }
     }
 
     public synchronized State getState() {
@@ -477,11 +486,20 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
 
     @Override
     public synchronized void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) {
+        handleException(cause);
+    }
+
+    /**
+     * Handle exception occurred in the BGP session. The session in error state should be closed
+     * properly so that it can be restored later.
+     */
+    @VisibleForTesting
+    void handleException(final Throwable cause) {
         LOG.warn("BGP session encountered error", cause);
         if (cause.getCause() instanceof BGPDocumentedException) {
             this.terminate((BGPDocumentedException) cause.getCause());
         } else {
-            this.close();
+            this.terminate(new BGPDocumentedException(BGPError.CEASE));
         }
     }
 

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImplTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImplTest.java
index 0245ce31450156de3d34d8d8d4807cd5912d3305..5d5edb15ad1fc0538b4f11f33c02bcfd702f01a7 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImplTest.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImplTest.java
@@ -40,6 +40,8 @@ import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
 import org.opendaylight.protocol.bgp.parser.BGPError;
 import org.opendaylight.protocol.bgp.parser.BgpExtendedMessageUtil;
 import org.opendaylight.protocol.bgp.parser.BgpTableTypeImpl;
+import org.opendaylight.protocol.bgp.rib.spi.BGPSessionListener;
+import org.opendaylight.protocol.bgp.rib.spi.BGPTerminationReason;
 import org.opendaylight.protocol.bgp.rib.spi.State;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.AsNumber;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;
@@ -247,4 +249,27 @@ public class BGPSessionImplTest {
         Assert.assertEquals(BGPError.HOLD_TIMER_EXPIRED.getSubcode(), error.getErrorSubcode().shortValue());
         Mockito.verify(this.speakerListener).close();
     }
+
+    @Test
+    public void testSessionRecoveryOnException() throws Exception {
+        final BGPSessionListener listener = mock(BGPSessionListener.class);
+        Mockito.doThrow(new RuntimeException("Mocked runtime exception."))
+                .when(listener).onSessionUp(Matchers.any());
+        this.bgpSession = Mockito.spy(new BGPSessionImpl(listener, this.speakerListener, this.classicOpen,
+                this.classicOpen.getHoldTimer(), null));
+        this.bgpSession.setChannelExtMsgCoder(this.classicOpen);
+
+        Mockito.verify(this.bgpSession, Mockito.never()).handleException(Matchers.any());
+        Mockito.verify(this.bgpSession, Mockito.never()).writeAndFlush(Matchers.any(Notification.class));
+        Mockito.verify(this.bgpSession, Mockito.never()).terminate(Matchers.any(BGPDocumentedException.class));
+        try {
+            this.bgpSession.sessionUp();
+            Assert.fail();  // expect the exception to be populated
+        } catch (final RuntimeException ignored) {}
+        Assert.assertNotEquals(State.UP, this.bgpSession.getState());
+        Mockito.verify(this.bgpSession).handleException(Matchers.any());
+        Mockito.verify(this.bgpSession).writeAndFlush(Matchers.any(Notification.class));
+        Mockito.verify(this.bgpSession).terminate(Matchers.any(BGPDocumentedException.class));
+        Mockito.verify(listener).onSessionTerminated(this.bgpSession, new BGPTerminationReason(BGPError.CEASE));
+    }
 }

diff --git a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/BGPTerminationReason.java b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/BGPTerminationReason.java
index 323993d539d79c042e04001b874c7f8e34fba6da..bff89dd6817a99acabb27e803b732c4387bcae71 100644 (file)
--- a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/BGPTerminationReason.java
+++ b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/BGPTerminationReason.java
@@ -8,6 +8,7 @@
 package org.opendaylight.protocol.bgp.rib.spi;
 
 import com.google.common.base.MoreObjects;
+import java.util.Objects;
 import org.opendaylight.protocol.bgp.parser.BGPError;
 
 public final class BGPTerminationReason {
@@ -21,6 +22,17 @@ public final class BGPTerminationReason {
         return this.error.toString();
     }
 
+    @Override
+    public int hashCode() {
+        return Objects.hash(this.error);
+    }
+
+    @Override
+    public boolean equals(final Object o) {
+        return (o instanceof BGPTerminationReason) &&
+                Objects.equals(this.error, ((BGPTerminationReason) o).error);
+    }
+
     @Override
     public String toString() {
         return MoreObjects.toStringHelper(this)