diff --git a/src/bgp/bgp_config_listener.cc b/src/bgp/bgp_config_listener.cc index c9ca663a5d2..311cd5d4194 100644 --- a/src/bgp/bgp_config_listener.cc +++ b/src/bgp/bgp_config_listener.cc @@ -45,7 +45,8 @@ void BgpConfigListener::DependencyTrackerInit() { policy->insert(make_pair("bgp-peering", bgp_peering_react)); ReactionMap bgp_router_react = map_list_of - ("self", list_of("bgp-peering")); + ("self", list_of("bgp-peering")) + ("instance-bgp-router", list_of("bgp-peering")); policy->insert(make_pair("bgp-router", bgp_router_react)); ReactionMap connection_react = map_list_of diff --git a/src/bgp/test/bgp_config_listener_test.cc b/src/bgp/test/bgp_config_listener_test.cc index 9793d724049..dfcb357970d 100644 --- a/src/bgp/test/bgp_config_listener_test.cc +++ b/src/bgp/test/bgp_config_listener_test.cc @@ -738,12 +738,12 @@ TEST_F(BgpConfigListenerTest, BgpRouterChange2) { } // -// Link event for an uninteresting link of a bgp-router object. +// Link event for the instance-bgp-router link of a bgp-router object. // -TEST_F(BgpConfigListenerTest, BgpRouterUninterestingLinkChange) { +TEST_F(BgpConfigListenerTest, BgpRouterInstanceLinkChange) { - // Initialize config with local router and add link to master instance. - string content = ReadFile("controller/src/bgp/testdata/config_listener_test_0.xml"); + // Initialize config with local router and 3 remote routers. + string content = ReadFile("controller/src/bgp/testdata/config_listener_test_1.xml"); EXPECT_TRUE(parser_.Parse(content)); string instance(BgpConfigManager::kMasterInstance); string router = instance + ":local"; @@ -759,17 +759,21 @@ TEST_F(BgpConfigListenerTest, BgpRouterUninterestingLinkChange) { "bgp-router", router, "routing-instance", instance); task_util::WaitForIdle(); - // Edge list should be empty since both edges are not interesting. - // Reaction map for bgp-router has no entry for instance-bgp-router. - // Reaction map for routing-instance has no entry for instance-bgp-router. + // Only one edge should get added to the edge list. + // This is the instance-bgp-router edge from bgp-router. + // The instance-bgp-router edge from the routing-instance is not + // interesting since the reaction map for routing-instance doesn't + // have an entry for instance-bgp-router. TASK_UTIL_EXPECT_EQ(0, GetChangeListCount()); TASK_UTIL_EXPECT_EQ(0, GetNodeListCount()); - TASK_UTIL_EXPECT_EQ(0, GetEdgeListCount()); + TASK_UTIL_EXPECT_EQ(1, GetEdgeListCount()); // Perform propagation and verify change list. - // Nothing should get added since the node and edge lists are empty. + // The bgp-routers are on the change list since the propagate list for + // instance-bgp-router in bgp-router contains bgp-peering. PerformChangeListPropagation(); - TASK_UTIL_EXPECT_EQ(0, GetChangeListCount()); + TASK_UTIL_EXPECT_EQ(3, GetChangeListCount()); + TASK_UTIL_EXPECT_EQ(3, GetChangeListCount("bgp-peering")); ResumeChangeListPropagation(); TASK_UTIL_EXPECT_EQ(0, GetChangeListCount()); @@ -1439,43 +1443,6 @@ TEST_F(BgpConfigListenerTest, RoutingInstanceVirtualNetworkChange3) { TASK_UTIL_EXPECT_EQ(0, GetChangeListCount()); } -// -// Link event for an uninteresting link of a routing-instance object. -// -TEST_F(BgpConfigListenerTest, RoutingInstanceUninterestingLinkChange) { - - // Initialize config with local router and add link to master instance. - string content = ReadFile("controller/src/bgp/testdata/config_listener_test_0.xml"); - EXPECT_TRUE(parser_.Parse(content)); - string instance(BgpConfigManager::kMasterInstance); - string router = instance + ":local"; - ifmap_test_util::IFMapMsgLink(&db_, "routing-instance", instance, - "bgp-router", router, "instance-bgp-router"); - task_util::WaitForIdle(); - TASK_UTIL_EXPECT_EQ(0, GetChangeListCount()); - - // Pause propagation, notify instance-bgp-router link. - PauseChangeListPropagation(); - ifmap_test_util::IFMapLinkNotify(&db_, &graph_, - "routing-instance", instance, "bgp-router", router); - task_util::WaitForIdle(); - - // Edge list should be empty since both edges are not interesting. - // Reaction map for bgp-router has no entry for instance-bgp-router. - // Reaction map for routing-instance has no entry for instance-bgp-router. - TASK_UTIL_EXPECT_EQ(0, GetChangeListCount()); - TASK_UTIL_EXPECT_EQ(0, GetNodeListCount()); - TASK_UTIL_EXPECT_EQ(0, GetEdgeListCount()); - - // Perform propagation and verify change list. - // Nothing should get added since the node and edge lists are empty. - PerformChangeListPropagation(); - TASK_UTIL_EXPECT_EQ(0, GetChangeListCount()); - - ResumeChangeListPropagation(); - TASK_UTIL_EXPECT_EQ(0, GetChangeListCount()); -} - // // Node event for a virtual-network object associated with routing-instances. // There are no other virtual-networks or routing-instances. diff --git a/src/bgp/test/bgp_ifmap_config_manager_test.cc b/src/bgp/test/bgp_ifmap_config_manager_test.cc index c10f2350d6f..e617bfbca34 100644 --- a/src/bgp/test/bgp_ifmap_config_manager_test.cc +++ b/src/bgp/test/bgp_ifmap_config_manager_test.cc @@ -1871,6 +1871,106 @@ TEST_F(BgpIfmapConfigManagerShowTest, ShowBGPaaSPeerings) { TASK_UTIL_EXPECT_EQ(0, db_graph_.vertex_count()); } +TEST_F(BgpIfmapConfigManagerTest, AddBgpPeeringBeforeInstanceBgpRouterLink1) { + // Add client bgp-router with parameters. + string bgp_router_id1 = string("test") + ":client"; + autogen::BgpRouterParams *params1 = new autogen::BgpRouterParams; + params1->Clear(); + params1->autonomous_system = 100; + params1->identifier = "10.1.1.100"; + params1->address = "127.0.0.100"; + params1->router_type = "bgpaas-client"; + ifmap_test_util::IFMapMsgPropertyAdd(&db_, "bgp-router", bgp_router_id1, + "bgp-router-parameters", params1); + task_util::WaitForIdle(); + + // Add server bgp-router with parameters. + string bgp_router_id2 = string("test") + ":server"; + autogen::BgpRouterParams *params2 = new autogen::BgpRouterParams; + params2->Clear(); + params2->autonomous_system = 100; + params2->router_type = "bgpaas-server"; + ifmap_test_util::IFMapMsgPropertyAdd(&db_, "bgp-router", bgp_router_id2, + "bgp-router-parameters", params2); + task_util::WaitForIdle(); + + // Now add a bgp-peering link between the bgp-routers. + // Verify that's there's no peering config created as there's no parent + // routing instance. + string peering = "attr(" + bgp_router_id1 + "," + bgp_router_id2 + ")"; + ifmap_test_util::IFMapMsgLink(&db_, + "bgp-router", bgp_router_id1, "bgp-router", bgp_router_id2, + "bgp-peering", 0, new autogen::BgpPeeringAttributes()); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_EQ(0, GetPeeringCount()); + TASK_UTIL_EXPECT_TRUE(FindPeeringConfig(peering) == NULL); + + // Now add a link between the bgp-routers and the routing-instance. + // Verify that's the peering config gets created due to notification + // of the instance-bgp-router links. + ifmap_test_util::IFMapMsgLink(&db_, "routing-instance", "test", + "bgp-router", bgp_router_id1, "instance-bgp-router"); + ifmap_test_util::IFMapMsgLink(&db_, "routing-instance", "test", + "bgp-router", bgp_router_id2, "instance-bgp-router"); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_EQ(1, GetPeeringCount()); + TASK_UTIL_EXPECT_TRUE(FindPeeringConfig(peering) != NULL); +} + +TEST_F(BgpIfmapConfigManagerTest, AddBgpPeeringBeforeInstanceBgpRouterLink2) { + // Add client bgp-router with parameters. + string bgp_router_id1 = string("test") + ":client"; + autogen::BgpRouterParams *params1 = new autogen::BgpRouterParams; + params1->Clear(); + params1->autonomous_system = 100; + params1->identifier = "10.1.1.100"; + params1->address = "127.0.0.100"; + params1->router_type = "bgpaas-client"; + ifmap_test_util::IFMapMsgPropertyAdd(&db_, "bgp-router", bgp_router_id1, + "bgp-router-parameters", params1); + task_util::WaitForIdle(); + + // Add server bgp-router with parameters. + string bgp_router_id2 = string("test") + ":server"; + autogen::BgpRouterParams *params2 = new autogen::BgpRouterParams; + params2->Clear(); + params2->autonomous_system = 100; + params2->router_type = "bgpaas-server"; + ifmap_test_util::IFMapMsgPropertyAdd(&db_, "bgp-router", bgp_router_id2, + "bgp-router-parameters", params2); + task_util::WaitForIdle(); + + // Now add a bgp-peering link between the bgp-routers. + // Verify that's there's no peering config created as there's no parent + // routing instance. + string peering = "attr(" + bgp_router_id1 + "," + bgp_router_id2 + ")"; + ifmap_test_util::IFMapMsgLink(&db_, + "bgp-router", bgp_router_id1, "bgp-router", bgp_router_id2, + "bgp-peering", 0, new autogen::BgpPeeringAttributes()); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_EQ(0, GetPeeringCount()); + TASK_UTIL_EXPECT_TRUE(FindPeeringConfig(peering) == NULL); + + // Now add the parent routing-instance. + // Verify that's there's no peering config created sine there's no + // notification to re-evaluate the bgp-peering. + ifmap_test_util::IFMapMsgNodeAdd(&db_, "routing-instance", "test"); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_EQ(0, GetPeeringCount()); + TASK_UTIL_EXPECT_TRUE(FindPeeringConfig(peering) == NULL); + + // Now add a link between the bgp-routers and the routing-instance. + // Verify that's the peering config gets created due to notification + // of the instance-bgp-router links. + ifmap_test_util::IFMapMsgLink(&db_, "routing-instance", "test", + "bgp-router", bgp_router_id1, "instance-bgp-router"); + ifmap_test_util::IFMapMsgLink(&db_, "routing-instance", "test", + "bgp-router", bgp_router_id2, "instance-bgp-router"); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_EQ(1, GetPeeringCount()); + TASK_UTIL_EXPECT_TRUE(FindPeeringConfig(peering) != NULL); +} + TEST_F(BgpIfmapConfigManagerTest, AddBgpRouterBeforeParentLink) { string bgp_router_id = string(BgpConfigManager::kMasterInstance) + ":local"; // Add bgp-router with parameters.