Fix CSIT regression due to BGPCEP-878 fix 24/86424/2
authorAjay Lele <ajayslele@gmail.com>
Sat, 14 Dec 2019 04:09:13 +0000 (20:09 -0800)
committerAjay Lele <ajayslele@gmail.com>
Sat, 14 Dec 2019 21:34:23 +0000 (13:34 -0800)
Handle scenario in BGPClusterSingletonService#restartNeighbors()
where BgpPeer#restart() is called after BgpPeer#closeServiceInstance()
without BgpPeer#restart() getting called in between.

JIRA: BGPCEP-878
Signed-off-by: Ajay Lele <ajayslele@gmail.com>
Change-Id: I5f8ac4ac4992e4641769243c91fac03bd0a96094

bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeerTest.java

index 425023b96ccb4bf5efcc9917547f1c2cd2a3467f..38eb0e86ff69bec7bb9a229ec3e5553734e8d2f1 100644 (file)
@@ -149,6 +149,10 @@ public class BgpPeer implements PeerBean, BGPPeerStateConsumer {
     public synchronized void restart(final RIB rib, final InstanceIdentifier<Bgp> bgpIid,
             final PeerGroupConfigLoader peerGroupLoader, final BGPTableTypeRegistryConsumer tableTypeRegistry) {
         Preconditions.checkState(this.currentConfiguration != null);
+        if (this.bgpPeerSingletonService != null) {
+            this.bgpPeerSingletonService.closeServiceInstance();
+            this.bgpPeerSingletonService = null;
+        }
         start(rib, this.currentConfiguration, bgpIid, peerGroupLoader, tableTypeRegistry);
     }
 
index 44cc622c96c04c3f5d53c56244674cd99f73e5d3..552d391efb22e7f7d91816e3d73eaa4a33ee9581 100644 (file)
@@ -160,16 +160,18 @@ public class BgpPeerTest extends AbstractConfig {
         verify(this.serviceRegistration).unregister();
 
         this.bgpPeer.restart(this.rib, null, this.peerGroupLoader, this.tableTypeRegistry);
-        this.bgpPeer.instantiateServiceInstance();
         verify(this.rib, times(2)).createPeerDOMChain(any());
         verify(this.rib, times(4)).getLocalAs();
         verify(this.rib, times(2)).getLocalTables();
+        this.bgpPeer.instantiateServiceInstance();
+        verify(this.bgpPeerRegistry, times(2)).addPeer(any(), any(), any());
+        verify(this.dispatcher, times(2)).createReconnectingClient(any(InetSocketAddress.class),
+                any(), anyInt(), any(KeyMapping.class));
 
         final Neighbor neighborExpected = createNeighborExpected(NEIGHBOR_ADDRESS);
         assertTrue(this.bgpPeer.containsEqualConfiguration(neighborExpected));
         assertFalse(this.bgpPeer.containsEqualConfiguration(createNeighborExpected(
                 new IpAddress(new Ipv4Address("127.0.0.2")))));
-        verify(this.bgpPeerRegistry).removePeer(any(IpAddress.class));
 
         this.bgpPeer.closeServiceInstance();
         verify(this.bgpPeerRegistry, times(2)).removePeer(any());
@@ -183,6 +185,19 @@ public class BgpPeerTest extends AbstractConfig {
         this.bgpPeer.closeServiceInstance();
         verify(this.bgpPeerRegistry, times(3)).removePeer(any());
         verify(this.future, times(3)).cancel(true);
+        verify(this.rib, times(3)).createPeerDOMChain(any());
+
+        this.bgpPeer.restart(this.rib, null, this.peerGroupLoader, this.tableTypeRegistry);
+        verify(this.rib, times(4)).createPeerDOMChain(any());
+        verify(this.rib, times(6)).getLocalAs();
+        verify(this.rib, times(3)).getLocalTables();
+        this.bgpPeer.instantiateServiceInstance();
+        verify(this.bgpPeerRegistry, times(4)).addPeer(any(), any(), any());
+        verify(this.dispatcher, times(4)).createReconnectingClient(any(InetSocketAddress.class),
+                any(), anyInt(), any(KeyMapping.class));
+        this.bgpPeer.closeServiceInstance();
+        verify(this.bgpPeerRegistry, times(4)).removePeer(any());
+        verify(this.future, times(4)).cancel(true);
         this.bgpPeer.close();
         verify(this.serviceRegistration).unregister();