From a0281358d14e627b4e718d8b016e149ce8ed749f Mon Sep 17 00:00:00 2001 From: Ajay Lele Date: Fri, 13 Dec 2019 20:09:13 -0800 Subject: [PATCH] Fix CSIT regression due to BGPCEP-878 fix 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 Change-Id: I5f8ac4ac4992e4641769243c91fac03bd0a96094 --- .../protocol/bgp/rib/impl/config/BgpPeer.java | 4 ++++ .../bgp/rib/impl/config/BgpPeerTest.java | 19 +++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java index 425023b96c..38eb0e86ff 100644 --- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java @@ -149,6 +149,10 @@ public class BgpPeer implements PeerBean, BGPPeerStateConsumer { public synchronized void restart(final RIB rib, final InstanceIdentifier 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); } diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeerTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeerTest.java index 44cc622c96..552d391efb 100644 --- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeerTest.java +++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeerTest.java @@ -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(); -- 2.36.6