Skip to content

Commit

Permalink
Merge "Handle recreate of deleted BGPaaS peer prior to destroy" into …
Browse files Browse the repository at this point in the history 
…R3.0
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed May 12, 2016
2 parents a784218 + 547e772 commit 3fa727f
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 5 deletions.
8 changes: 5 additions & 3 deletions src/bgp/bgp_server.cc
View file
Expand Up @@ -181,9 +181,11 @@ class BgpServer::ConfigUpdater {
if (event == BgpConfigManager::CFG_ADD ||
event == BgpConfigManager::CFG_CHANGE) {
BgpPeer *peer = peer_manager->PeerLocate(server_, neighbor_config);
server_->RemovePeer(peer->endpoint(), peer);
peer->ConfigUpdate(neighbor_config);
server_->InsertPeer(peer->endpoint(), peer);
if (peer) {
server_->RemovePeer(peer->endpoint(), peer);
peer->ConfigUpdate(neighbor_config);
server_->InsertPeer(peer->endpoint(), peer);
}
} else if (event == BgpConfigManager::CFG_DELETE) {
BgpPeer *peer = peer_manager->TriggerPeerDeletion(neighbor_config);
if (peer) {
Expand Down
15 changes: 13 additions & 2 deletions src/bgp/routing-instance/peer_manager.cc
View file
Expand Up @@ -19,14 +19,19 @@ using std::make_pair;
using std::string;
using std::vector;

//
// Find or create a BgpPeer for the given BgpNeighborConfig.
// Return NULL if the peer already exists and is being deleted. The BgpPeer
// will eventually get created in this case via PeerResurrect.
//
BgpPeer *PeerManager::PeerLocate(BgpServer *server,
const BgpNeighborConfig *config) {
BgpPeerKey key(config);

BgpPeerNameMap::iterator loc = peers_by_name_.find(config->name());
if (loc != peers_by_name_.end()) {
if (loc->second->IsDeleted())
return loc->second;
return NULL;
RemovePeerByKey(loc->second->peer_key(), loc->second);
InsertPeerByKey(key, loc->second);
return loc->second;
Expand All @@ -50,6 +55,10 @@ BgpPeer *PeerManager::PeerLocate(BgpServer *server,

//
// Resurrect the BgpPeer with given name if we have configuration for it.
// Also insert it into the BgpServer's EndpointToBgpPeerList - in case it's
// a BGPaaS peer. This needs to happen here since we would not have been
// able to do it from BgpServer::ConfigUpdater::ProcessNeighborConfig since
// old incarnation of the BgpPeer still existed at that point.
//
void PeerManager::PeerResurrect(string name) {
CHECK_CONCURRENCY("bgp::Config");
Expand All @@ -64,7 +73,9 @@ void PeerManager::PeerResurrect(string name) {
if (!config)
return;

PeerLocate(server(), config);
BgpPeer *peer = PeerLocate(server(), config);
assert(peer);
server()->InsertPeer(peer->endpoint(), peer);
}

//
Expand Down
88 changes: 88 additions & 0 deletions src/bgp/test/bgp_config_test.cc
View file
Expand Up @@ -672,6 +672,94 @@ TEST_F(BgpConfigTest, BGPaaSNeighbors6) {
task_util::WaitForIdle();
}

//
// Config for neighbor is re-added before the previous incarnation has been
// destroyed. The peer should get resurrected after the old incarnation has
// been destroyed.
//
TEST_F(BgpConfigTest, BGPaaSNeighbors7) {
string content;
content = FileRead("controller/src/bgp/testdata/config_test_36a.xml");
EXPECT_TRUE(parser_.Parse(content));
task_util::WaitForIdle();

RoutingInstance *rti =
server_.routing_instance_mgr()->GetRoutingInstance("test");
TASK_UTIL_ASSERT_TRUE(rti != NULL);
TASK_UTIL_EXPECT_EQ(2, rti->peer_manager()->size());
TASK_UTIL_EXPECT_EQ(0, server_.num_bgp_peer());
TASK_UTIL_EXPECT_EQ(2, server_.num_bgpaas_peer());

// Verify that peers got created.
TASK_UTIL_EXPECT_TRUE(
rti->peer_manager()->PeerLookup("test:vm1:0") != NULL);
BgpPeer *peer1 = rti->peer_manager()->PeerLookup("test:vm1:0");
TASK_UTIL_EXPECT_EQ(1024, peer1->peer_port());
TASK_UTIL_EXPECT_EQ(peer1, server_.FindPeer(peer1->endpoint()));

TASK_UTIL_EXPECT_TRUE(
rti->peer_manager()->PeerLookup("test:vm2:0") != NULL);
BgpPeer *peer2 = rti->peer_manager()->PeerLookup("test:vm2:0");
TASK_UTIL_EXPECT_EQ(1025, peer2->peer_port());
TASK_UTIL_EXPECT_EQ(peer2, server_.FindPeer(peer2->endpoint()));

// Pause deletion of both peers.
PauseDelete(peer1->deleter());
PauseDelete(peer2->deleter());
task_util::WaitForIdle();

// Delete the neighbor config - this should trigger peer deletion.
// The peers can't get destroyed because deletion has been paused.
content = FileRead("controller/src/bgp/testdata/config_test_36e.xml");
EXPECT_TRUE(parser_.Parse(content));
task_util::WaitForIdle();
TASK_UTIL_EXPECT_TRUE(peer1->IsDeleted());
TASK_UTIL_EXPECT_TRUE(peer1->config() == NULL);
TASK_UTIL_EXPECT_TRUE(server_.FindPeer(peer1->endpoint()) == NULL);
TASK_UTIL_EXPECT_TRUE(peer2->IsDeleted());
TASK_UTIL_EXPECT_TRUE(peer2->config() == NULL);
TASK_UTIL_EXPECT_TRUE(server_.FindPeer(peer2->endpoint()) == NULL);

// Recreate neighbor config. The old peers should still be around
// but should not be in the BgpServer::EndpointToBgpPeerList.
content = FileRead("controller/src/bgp/testdata/config_test_36a.xml");
EXPECT_TRUE(parser_.Parse(content));
task_util::WaitForIdle();
TASK_UTIL_EXPECT_TRUE(peer1->IsDeleted());
TASK_UTIL_EXPECT_TRUE(peer1->config() == NULL);
TASK_UTIL_EXPECT_TRUE(server_.FindPeer(peer1->endpoint()) == NULL);
TASK_UTIL_EXPECT_TRUE(peer2->IsDeleted());
TASK_UTIL_EXPECT_TRUE(peer2->config() == NULL);
TASK_UTIL_EXPECT_TRUE(server_.FindPeer(peer2->endpoint()) == NULL);

// Resume deletion of the peers.
ResumeDelete(peer1->deleter());
ResumeDelete(peer2->deleter());
task_util::WaitForIdle();

// Make sure the peers got resurrected.
TASK_UTIL_EXPECT_TRUE(
rti->peer_manager()->PeerLookup("test:vm1:0") != NULL);
peer1 = rti->peer_manager()->PeerLookup("test:vm1:0");
TASK_UTIL_EXPECT_EQ(1024, peer1->peer_port());
TASK_UTIL_EXPECT_EQ(peer1, server_.FindPeer(peer1->endpoint()));

TASK_UTIL_EXPECT_TRUE(
rti->peer_manager()->PeerLookup("test:vm2:0") != NULL);
peer2 = rti->peer_manager()->PeerLookup("test:vm2:0");
TASK_UTIL_EXPECT_EQ(1025, peer2->peer_port());
TASK_UTIL_EXPECT_EQ(peer2, server_.FindPeer(peer2->endpoint()));

// Get rid of the peers.
boost::replace_all(content, "<config>", "<delete>");
boost::replace_all(content, "</config>", "</delete>");
EXPECT_TRUE(parser_.Parse(content));
task_util::WaitForIdle();

TASK_UTIL_EXPECT_EQ(0, db_graph_.edge_count());
TASK_UTIL_EXPECT_EQ(0, db_graph_.vertex_count());
}

TEST_F(BgpConfigTest, MasterNeighborAttributes) {
string content_a = FileRead("controller/src/bgp/testdata/config_test_35a.xml");
EXPECT_TRUE(parser_.Parse(content_a));
Expand Down
65 changes: 65 additions & 0 deletions src/bgp/testdata/config_test_36e.xml
View file
@@ -0,0 +1,65 @@
<?xml version="1.0" encoding="utf-8"?>
<delete>
<routing-instance name="test">
<bgp-router name='bgpaas-server'>
<router-type>bgpaas-server</router-type>
<autonomous-system>64512</autonomous-system>
<session to='vm1:0'>
<family-attributes>
<address-family>inet</address-family>
</family-attributes>
<family-attributes>
<address-family>inet6</address-family>
</family-attributes>
<family-attributes>
<address-family>inet-vpn</address-family>
</family-attributes>
</session>
<session to='vm2:0'>
<family-attributes>
<address-family>inet</address-family>
</family-attributes>
<family-attributes>
<address-family>inet6</address-family>
</family-attributes>
<family-attributes>
<address-family>inet-vpn</address-family>
</family-attributes>
</session>
</bgp-router>
<bgp-router name='vm1'>
<router-type>bgpaas-client</router-type>
<autonomous-system>65001</autonomous-system>
<address>10.0.0.1</address>
<source-port>1024</source-port>
<session to='bgpaas-server:0'>
<family-attributes>
<address-family>inet</address-family>
</family-attributes>
<family-attributes>
<address-family>inet6</address-family>
</family-attributes>
<family-attributes>
<address-family>inet-vpn</address-family>
</family-attributes>
</session>
</bgp-router>
<bgp-router name='vm2'>
<router-type>bgpaas-client</router-type>
<autonomous-system>65002</autonomous-system>
<address>10.0.0.2</address>
<source-port>1025</source-port>
<session to='bgpaas-server:0'>
<family-attributes>
<address-family>inet</address-family>
</family-attributes>
<family-attributes>
<address-family>inet6</address-family>
</family-attributes>
<family-attributes>
<address-family>inet-vpn</address-family>
</family-attributes>
</session>
</bgp-router>
</routing-instance>
</delete>

0 comments on commit 3fa727f

Please sign in to comment.