Fix race condition in BGPaaS peering creation - take 2

If the internal ifmap configuration object for the routing-instance
for the bgp-routers that comprise a bgp-peering does not exist when
processing the bgp-peering, the ifmap config manager does create the
internal ifmap configuration object for the bgp-peering.

This causes the control node to not create the internal ifmap config
object for some bgp-peerings when the control node gets restarted in
a scaled bgpaas configuration. This happens for bgp-peerings which
get processed before the corresponding routing-instance has been
processed.

The previous fix did not cover the case where the notification for a
routing-instance has not been processed when the notification for the
instance-bgp-router link is processed.

Fix this by modifying dependency tracker policy to re-evaluate all of
the bgp-routers for a routing-instance when processing a notification
for the routing-instance itself.  This in turn causes a re-evaluation
of all the bgp-peerings for those bgp-routers.

Change-Id: Idc4425c87037aac1eb96f23394bfae536c17546c
Closes-Bug: 1662678

src/bgp/bgp_config_listener.cc

@@ -54,6 +54,7 @@ void BgpConfigListener::DependencyTrackerInit() {
     policy->insert(make_pair("connection", connection_react));
 
     ReactionMap rt_instance_react = map_list_of<string, PropagateList>
+        ("self", list_of("instance-bgp-router"))
         ("instance-target", list_of("self")("connection"))
         ("connection", list_of("self"))
         ("virtual-network-routing-instance", list_of("self"))

src/bgp/test/bgp_config_listener_test.cc

@@ -769,7 +769,7 @@ TEST_F(BgpConfigListenerTest, BgpRouterInstanceLinkChange) {
     TASK_UTIL_EXPECT_EQ(1, GetEdgeListCount());
 
     // Perform propagation and verify change list.
-    // The bgp-routers are on the change list since the propagate list for
+    // The bgp-peerings are on the change list since the propagate list for
     // instance-bgp-router in bgp-router contains bgp-peering.
     PerformChangeListPropagation();
     TASK_UTIL_EXPECT_EQ(3, GetChangeListCount());
@@ -780,11 +780,12 @@ TEST_F(BgpConfigListenerTest, BgpRouterInstanceLinkChange) {
 }
 
 //
-// Node event for a routing-instance object without any connections.
+// Node event for a routing-instance object with bgp-routers and bgp-peerings.
 //
 TEST_F(BgpConfigListenerTest, RoutingInstanceChange1) {
 
-    // Initialize config with local router and 3 remote routers.
+    // Initialize config with local router and 3 remote routers, hence 6
+    // peerings.
     string content = ReadFile("controller/src/bgp/testdata/config_listener_test_1.xml");
     EXPECT_TRUE(parser_.Parse(content));
     task_util::WaitForIdle();
@@ -795,17 +796,20 @@ TEST_F(BgpConfigListenerTest, RoutingInstanceChange1) {
     string id_name(BgpConfigManager::kMasterInstance);
     ifmap_test_util::IFMapNodeNotify(&db_, "routing-instance", id_name);
 
-    // The routing-instance should be on the change list but not on the node
-    // list since there's no entry for self in the reaction map.
+    // The routing-instance should be on the change list and on the node list
+    // since there's an entry for self in the reaction map.
     TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
-    TASK_UTIL_EXPECT_EQ(0, GetNodeListCount());
+    TASK_UTIL_EXPECT_EQ(1, GetNodeListCount());
     TASK_UTIL_EXPECT_EQ(0, GetEdgeListCount());
 
     // Perform propagation and verify change list.
-    // Nothing else should get added since the node and edge lists are empty.
+    // The bgp-peerings are on the change list since the propagate list for
+    // self in routing-instance contains instance-bgp-router and propagate
+    // list for instance-bgp-router in bgp-router contains bgp-peering.
     PerformChangeListPropagation();
-    TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
+    TASK_UTIL_EXPECT_EQ(7, GetChangeListCount());
     TASK_UTIL_EXPECT_EQ(1, GetChangeListCount("routing-instance"));
+    TASK_UTIL_EXPECT_EQ(6, GetChangeListCount("bgp-peering"));
 
     ResumeChangeListPropagation();
     TASK_UTIL_EXPECT_EQ(0, GetChangeListCount());
@@ -833,14 +837,15 @@ TEST_F(BgpConfigListenerTest, RoutingInstanceChange2) {
     string id_name("red");
     ifmap_test_util::IFMapNodeNotify(&db_, "routing-instance", id_name);
 
-    // The routing-instance should be on the change list but not on the node
-    // list since there's no entry for self in the reaction map.
+    // The routing-instance should be on the change list and on the node list
+    // since there's an entry for self in the reaction map.
     TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
-    TASK_UTIL_EXPECT_EQ(0, GetNodeListCount());
+    TASK_UTIL_EXPECT_EQ(1, GetNodeListCount());
     TASK_UTIL_EXPECT_EQ(0, GetEdgeListCount());
 
     // Perform propagation and verify change list.
-    // Nothing else should get added since the node and edge lists are empty.
+    // Nothing else should get added since the routing-instance doesn't have
+    // any bgp-routers and bgp-peerings.
     PerformChangeListPropagation();
     TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
     TASK_UTIL_EXPECT_EQ(1, GetChangeListCount("routing-instance"));
@@ -1659,7 +1664,7 @@ TEST_F(BgpConfigListenerTest, RoutingPolicyUpdate_1) {
     task_util::WaitForIdle();
 
     TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
-    TASK_UTIL_EXPECT_EQ(0, GetNodeListCount());
+    TASK_UTIL_EXPECT_EQ(1, GetNodeListCount());
     TASK_UTIL_EXPECT_EQ(0, GetEdgeListCount());
 
     PerformChangeListPropagation();
@@ -1904,7 +1909,7 @@ TEST_F(BgpConfigListenerTest, RoutingPolicyUpdate_8) {
     task_util::WaitForIdle();
 
     TASK_UTIL_EXPECT_EQ(1, GetChangeListCount());
-    TASK_UTIL_EXPECT_EQ(0, GetNodeListCount());
+    TASK_UTIL_EXPECT_EQ(1, GetNodeListCount());
     TASK_UTIL_EXPECT_EQ(0, GetEdgeListCount());
 
     PerformChangeListPropagation();