diff --git a/src/vnsw/agent/controller/controller.sandesh b/src/vnsw/agent/controller/controller.sandesh index 1b9b9c152ee..dbba55a050b 100644 --- a/src/vnsw/agent/controller/controller.sandesh +++ b/src/vnsw/agent/controller/controller.sandesh @@ -74,9 +74,12 @@ traceobject sandesh AgentXmppSession { } trace sandesh AgentXmppDiscoveryConnection { - 1: string servertype; - 2: string server; - 3: string message; + 1: string message; + 2: "index ="; + 3: u16 index; + 4: "server ="; + 5: string server; + 6: string data; } trace sandesh AgentXmppWalker { diff --git a/src/vnsw/agent/controller/controller_init.cc b/src/vnsw/agent/controller/controller_init.cc index 53fe0b29af8..c1a6ce0750d 100644 --- a/src/vnsw/agent/controller/controller_init.cc +++ b/src/vnsw/agent/controller/controller_init.cc @@ -50,9 +50,9 @@ void VNController::XmppServerConnect() { AgentXmppChannel *ch = agent_->controller_xmpp_channel(count); if (ch) { // Channel is created, do not disturb - CONTROLLER_TRACE(DiscoveryConnection, "XMPP Server", - ch->GetXmppServer(), - "is already present, ignore discovery response"); + CONTROLLER_TRACE(DiscoveryConnection, + "XMPP Server is already present, ignore discovery response", + count, ch->GetXmppServer(), ""); count++; continue; } @@ -118,9 +118,9 @@ void VNController::DnsXmppServerConnect() { AgentDnsXmppChannel *ch = agent_->dns_xmpp_channel(count); if (ch) { // Channel is up and running, do not disturb - CONTROLLER_TRACE(DiscoveryConnection, "DNS Server", - ch->GetXmppServer(), - "is already present, ignore discovery response"); + CONTROLLER_TRACE(DiscoveryConnection, + "DNS Server is already present, ignore discovery response", + count, ch->GetXmppServer(), ""); count++; continue; } @@ -301,88 +301,99 @@ void VNController::DisConnectControllerIfmapServer(uint8_t idx) { agent_->reset_controller_ifmap_xmpp_server(idx); } +bool VNController::AgentXmppServerExists(const std::string &server_ip, + std::vector resp) { + + std::vector::iterator iter; + for (iter = resp.begin(); iter != resp.end(); iter++) { + DSResponse dr = *iter; + if (dr.ep.address().to_string().compare(server_ip) == 0) { + return true; + } + } + return false; +} + void VNController::ApplyDiscoveryXmppServices(std::vector resp) { - std::listAgentXmppChannelList; std::vector::iterator iter; + int8_t count = -1; for (iter = resp.begin(); iter != resp.end(); iter++) { DSResponse dr = *iter; - - CONTROLLER_TRACE(DiscoveryConnection, "XMPP Server", dr.ep.address().to_string(), - "Discovery Server Response"); + count ++; + + CONTROLLER_TRACE(DiscoveryConnection, "XMPP Discovery Server Response", + count, dr.ep.address().to_string(), integerToString(dr.ep.port())); + AgentXmppChannel *chnl = FindAgentXmppChannel(dr.ep.address().to_string()); if (chnl) { - AgentXmppChannelList.push_back(chnl); if (chnl->GetXmppChannel() && chnl->GetXmppChannel()->GetPeerState() == xmps::READY) { - CONTROLLER_TRACE(DiscoveryConnection, "XMPP Server", - chnl->GetXmppServer(), "is UP and running, ignore"); + CONTROLLER_TRACE(DiscoveryConnection, + "XMPP Server is READY and running, ignore", count, + chnl->GetXmppServer(), ""); continue; } else { - CONTROLLER_TRACE(DiscoveryConnection, "XMPP Server", - chnl->GetXmppServer(), "is NOT_READY, ignore"); + CONTROLLER_TRACE(DiscoveryConnection, + "XMPP Server is NOT_READY, ignore", count, + chnl->GetXmppServer(), ""); continue; - } + } } else { - if (agent_->controller_ifmap_xmpp_server(0).empty()) { - agent_->set_controller_ifmap_xmpp_server(dr.ep.address().to_string(), 0); - agent_->set_controller_ifmap_xmpp_port(dr.ep.port(), 0); - CONTROLLER_TRACE(DiscoveryConnection, "Set Xmpp Channel[0] = ", - dr.ep.address().to_string(), ""); - } else if (agent_->controller_ifmap_xmpp_server(1).empty()) { - agent_->set_controller_ifmap_xmpp_server(dr.ep.address().to_string(), 1); - agent_->set_controller_ifmap_xmpp_port(dr.ep.port(), 1); - CONTROLLER_TRACE(DiscoveryConnection, "Set Xmpp Channel[1] = ", - dr.ep.address().to_string(), ""); - } else if (agent_->controller_xmpp_channel(0) && - (agent_->controller_xmpp_channel(0)->GetXmppChannel()-> - GetPeerState() == xmps::NOT_READY)) { - - DisConnectControllerIfmapServer(0); - CONTROLLER_TRACE(DiscoveryConnection, - "Refresh Xmpp Channel[0] = ", dr.ep.address().to_string(), ""); - agent_->set_controller_ifmap_xmpp_server(dr.ep.address().to_string(),0); - agent_->set_controller_ifmap_xmpp_port(dr.ep.port(), 0); + for (uint8_t xs_idx = 0; xs_idx < MAX_XMPP_SERVERS; xs_idx++) { - } else if (agent_->controller_xmpp_channel(1) && - (agent_->controller_xmpp_channel(1)->GetXmppChannel()-> - GetPeerState() == xmps::NOT_READY)) { + if (agent_->controller_ifmap_xmpp_server(xs_idx).empty()) { - DisConnectControllerIfmapServer(1); + CONTROLLER_TRACE(DiscoveryConnection, "Set Xmpp Channel", + xs_idx, dr.ep.address().to_string(), + integerToString(dr.ep.port())); - CONTROLLER_TRACE(DiscoveryConnection, - "Refresh Xmpp Channel[1] = ", dr.ep.address().to_string(), ""); - agent_->set_controller_ifmap_xmpp_server(dr.ep.address().to_string(), 1); - agent_->set_controller_ifmap_xmpp_port(dr.ep.port(), 1); - } + agent_->set_controller_ifmap_xmpp_server( + dr.ep.address().to_string(), xs_idx); + agent_->set_controller_ifmap_xmpp_port(dr.ep.port(), xs_idx); + break; + + } else if (agent_->controller_xmpp_channel(xs_idx) && + (agent_->controller_xmpp_channel(xs_idx)->GetXmppChannel()-> + GetPeerState() == xmps::NOT_READY)) { + + if (AgentXmppServerExists( + agent_->controller_ifmap_xmpp_server(xs_idx), resp)) { + + CONTROLLER_TRACE(DiscoveryConnection, + "Retain Xmpp Channel ", xs_idx, + agent_->controller_ifmap_xmpp_server(xs_idx), ""); + continue; + } + + CONTROLLER_TRACE(DiscoveryConnection, + "ReSet Xmpp Channel ", xs_idx, + agent_->controller_ifmap_xmpp_server(xs_idx), + dr.ep.address().to_string()); + + DisConnectControllerIfmapServer(xs_idx); + agent_->set_controller_ifmap_xmpp_server( + dr.ep.address().to_string(),xs_idx); + agent_->set_controller_ifmap_xmpp_port(dr.ep.port(), xs_idx); + break; + } + } } } /* Remove stale Servers */ for (uint8_t idx = 0; idx < MAX_XMPP_SERVERS; idx++) { if (agent_->controller_xmpp_channel(idx) != NULL) { - std::list::iterator it; - for (it = AgentXmppChannelList.begin(); - it != AgentXmppChannelList.end(); it++) { - - if (agent_->controller_xmpp_channel(idx) == *it) { - // in the list, nothing to do - break; - } - } - - if (it == AgentXmppChannelList.end()) { - // not in list, cleanup only if DOWN as new set of hints from - // discovery shud take effect only if agent detects the server DOWN - if (agent_->controller_xmpp_channel(idx)->GetXmppChannel()-> - GetPeerState() == xmps::NOT_READY) { - CONTROLLER_TRACE(DiscoveryConnection, "Cleanup Older Xmpp ", - agent_->controller_ifmap_xmpp_server(idx), ""); - DisConnectControllerIfmapServer(idx); - agent_->reset_controller_ifmap_xmpp_server(idx); - } + + if (!AgentXmppServerExists( + agent_->controller_ifmap_xmpp_server(idx), resp)) { + + CONTROLLER_TRACE(DiscoveryConnection, "Cleanup Older Xmpp ", + idx, agent_->controller_ifmap_xmpp_server(idx), ""); + DisConnectControllerIfmapServer(idx); + agent_->reset_controller_ifmap_xmpp_server(idx); } } } @@ -432,61 +443,67 @@ void VNController::DisConnectDnsServer(uint8_t idx) { void VNController::ApplyDiscoveryDnsXmppServices(std::vector resp) { - std::listAgentDnsXmppChannelList; std::vector::iterator iter; + int8_t count = -1; for (iter = resp.begin(); iter != resp.end(); iter++) { DSResponse dr = *iter; - - CONTROLLER_TRACE(DiscoveryConnection, "DNS Server", dr.ep.address().to_string(), - "Discovery Server Response"); + count++; + + CONTROLLER_TRACE(DiscoveryConnection, "DNS Discovery Server Response", count, + dr.ep.address().to_string(), integerToString(dr.ep.port())); + AgentDnsXmppChannel *chnl = FindAgentDnsXmppChannel(dr.ep.address().to_string()); if (chnl) { - AgentDnsXmppChannelList.push_back(chnl); if (chnl->GetXmppChannel() && chnl->GetXmppChannel()->GetPeerState() == xmps::READY) { - CONTROLLER_TRACE(DiscoveryConnection, "DNS Server", - chnl->GetXmppServer(), "is UP and running, ignore"); + CONTROLLER_TRACE(DiscoveryConnection, + "DNS Server is READY and running, ignore", count, + chnl->GetXmppServer(), ""); continue; } else { - CONTROLLER_TRACE(DiscoveryConnection, "DNS Server", - chnl->GetXmppServer(), "is NOT_READY, ignore"); + CONTROLLER_TRACE(DiscoveryConnection, + "DNS Server is NOT_READY, ignore", count, + chnl->GetXmppServer(), ""); continue; } } else { - if (agent_->dns_server(0).empty()) { - agent_->set_dns_server(dr.ep.address().to_string(), 0); - agent_->set_dns_server_port(dr.ep.port(), 0); - CONTROLLER_TRACE(DiscoveryConnection, "Set Dns Xmpp Channel[0] = ", - dr.ep.address().to_string(), integerToString(dr.ep.port())); - } else if (agent_->dns_server(1).empty()) { - agent_->set_dns_server(dr.ep.address().to_string(), 1); - agent_->set_dns_server_port(dr.ep.port(), 1); - CONTROLLER_TRACE(DiscoveryConnection, "Set Dns Xmpp Channel[1] = ", - dr.ep.address().to_string(), integerToString(dr.ep.port())); - } else if (agent_->dns_xmpp_channel(0) && - (agent_->dns_xmpp_channel(0)->GetXmppChannel()->GetPeerState() - == xmps::NOT_READY)) { - - DisConnectDnsServer(0); - - CONTROLLER_TRACE(DiscoveryConnection, - "Refresh Dns Xmpp Channel[0] = ", - dr.ep.address().to_string(), integerToString(dr.ep.port())); - agent_->set_dns_server(dr.ep.address().to_string(), 0); - agent_->set_dns_server_port(dr.ep.port(), 0); - - } else if (agent_->dns_xmpp_channel(1) && - (agent_->dns_xmpp_channel(1)->GetXmppChannel()->GetPeerState() - == xmps::NOT_READY)) { - - DisConnectDnsServer(1); - CONTROLLER_TRACE(DiscoveryConnection, - "Refresh Dns Xmpp Channel[1] = ", - dr.ep.address().to_string(), integerToString(dr.ep.port())); - agent_->set_dns_server(dr.ep.address().to_string(), 1); - agent_->set_dns_server_port(dr.ep.port(), 1); + for (uint8_t xs_idx = 0; xs_idx < MAX_XMPP_SERVERS; xs_idx++) { + + if (agent_->dns_server(xs_idx).empty()) { + + CONTROLLER_TRACE(DiscoveryConnection, + "Set Dns Xmpp Channel ", xs_idx, + dr.ep.address().to_string(), integerToString(dr.ep.port())); + + agent_->set_dns_server(dr.ep.address().to_string(), xs_idx); + agent_->set_dns_server_port(dr.ep.port(), xs_idx); + break; + + } else if (agent_->dns_xmpp_channel(xs_idx) && + (agent_->dns_xmpp_channel(xs_idx)->GetXmppChannel()->GetPeerState() + == xmps::NOT_READY)) { + + if (AgentXmppServerExists( + agent_->dns_server(xs_idx), resp)) { + + CONTROLLER_TRACE(DiscoveryConnection, + "Retain Dns Xmpp Channel ", xs_idx, + agent_->dns_server(xs_idx), ""); + continue; + } + + CONTROLLER_TRACE(DiscoveryConnection, + "ReSet Dns Xmpp Channel ", xs_idx, + agent_->dns_server(xs_idx), + dr.ep.address().to_string()); + + DisConnectDnsServer(xs_idx); + agent_->set_dns_server(dr.ep.address().to_string(), xs_idx); + agent_->set_dns_server_port(dr.ep.port(), xs_idx); + break; + } } } } @@ -494,26 +511,14 @@ void VNController::ApplyDiscoveryDnsXmppServices(std::vector resp) { /* Remove stale Servers */ for (uint8_t idx = 0; idx < MAX_XMPP_SERVERS; idx++) { if (agent_->dns_xmpp_channel(idx) != NULL) { - std::list::iterator it; - for (it = AgentDnsXmppChannelList.begin(); - it != AgentDnsXmppChannelList.end(); it++) { - - if (agent_->dns_xmpp_channel(idx) == *it) { - // in the list, nothing to do - break; - } - } - - if (it == AgentDnsXmppChannelList.end()) { - // not in list, cleanup only if DOWN as new set of hints from - // discovery shud take effect only if agent detects the server DOWN - if (agent_->dns_xmpp_channel(idx)->GetXmppChannel()->GetPeerState() - == xmps::NOT_READY) { - CONTROLLER_TRACE(DiscoveryConnection, "Cleanup Older Dns Xmpp ", - agent_->dns_server(idx), ""); - DisConnectDnsServer(idx); - agent_->reset_dns_server(idx); - } + + if (AgentXmppServerExists( + agent_->dns_server(idx), resp)) { + + CONTROLLER_TRACE(DiscoveryConnection, "Cleanup Older Dns Xmpp", + idx, agent_->dns_server(idx), ""); + DisConnectDnsServer(idx); + agent_->reset_dns_server(idx); } } } diff --git a/src/vnsw/agent/controller/controller_init.h b/src/vnsw/agent/controller/controller_init.h index 6d607117776..07d8b3f3a75 100644 --- a/src/vnsw/agent/controller/controller_init.h +++ b/src/vnsw/agent/controller/controller_init.h @@ -78,6 +78,8 @@ class VNController { AgentDnsXmppChannel *FindAgentDnsXmppChannel(const std::string &server_ip); void DeleteConnectionInfo(const std::string &addr, bool is_dns) const; const std::string MakeConnectionPrefix(bool is_dns) const; + bool AgentXmppServerExists(const std::string &server_ip, + std::vector resp); Agent *agent_; uint64_t multicast_sequence_number_; diff --git a/src/vnsw/agent/test/test_xmpp_discovery.cc b/src/vnsw/agent/test/test_xmpp_discovery.cc index 6e8d1bcb446..db20d00d4ef 100644 --- a/src/vnsw/agent/test/test_xmpp_discovery.cc +++ b/src/vnsw/agent/test/test_xmpp_discovery.cc @@ -276,17 +276,21 @@ TEST_F(AgentXmppUnitTest, XmppConnection_Discovery) { client->WaitForIdle(); //wait for connection establishment - EXPECT_TRUE(agent_->controller_ifmap_xmpp_port(1) == xs2->GetPort()); - WAIT_FOR(1000, 10000, - agent_->controller_xmpp_channel(1)->GetXmppChannel()->GetPeerState() - == xmps::READY); + EXPECT_TRUE(agent_->controller_ifmap_xmpp_port(0) == + (uint32_t)xs2->GetPort()); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(0)->GetXmppChannel()->GetPeerState() + == xmps::READY); client->WaitForIdle(); + EXPECT_TRUE(agent_->controller_xmpp_channel(1) == NULL); //Bring down Xmpp Server xs2->Shutdown(); - WAIT_FOR(1000, 10000, - agent_->controller_xmpp_channel(1)->GetXmppChannel()->GetPeerState() + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(0)->GetXmppChannel()->GetPeerState() == xmps::NOT_READY); + client->WaitForIdle(); + EXPECT_TRUE(agent_->controller_xmpp_channel(1) == NULL); //Discovery indicating new Xmpp Server ds_response.clear(); @@ -304,6 +308,7 @@ TEST_F(AgentXmppUnitTest, XmppConnection_Discovery) { == xmps::READY); // Wait until older XmppClient, XmppChannel is cleaned client->WaitForIdle(); + EXPECT_TRUE(agent_->controller_xmpp_channel(1) == NULL); //Discovery indicating new Xmpp Server ds_response.clear(); @@ -378,4 +383,147 @@ TEST_F(AgentXmppUnitTest, XmppConnection_Discovery) { client->WaitForIdle(); } + +TEST_F(AgentXmppUnitTest, XmppConnection_Discovery_TimedOut) { + + client->Reset(); + client->WaitForIdle(); + + XmppServerConnectionInit(); + + // Simulate Discovery response for Xmpp Server + std::vector ds_response; + DSResponse resp; + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.1")); + int port = xs1->GetPort(); + resp.ep.port(xs1->GetPort()); + ds_response.push_back(resp); + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.2")); + resp.ep.port(xs2->GetPort()); + ds_response.push_back(resp); + + agent_->controller()->ApplyDiscoveryXmppServices(ds_response); + client->WaitForIdle(); + + //wait for connection establishment + EXPECT_TRUE(agent_->controller_ifmap_xmpp_port(0) == + (uint32_t)xs1->GetPort()); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(0)->GetXmppChannel()->GetPeerState() + == xmps::READY); + + EXPECT_TRUE(agent_->controller_ifmap_xmpp_port(1) == + (uint32_t)xs2->GetPort()); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(1)->GetXmppChannel()->GetPeerState() + == xmps::READY); + + // Pull down both servers + xs1->Shutdown(); + client->WaitForIdle(); + xs2->Shutdown(); + client->WaitForIdle(); + + // Simulate Discovery response for Xmpp Server + ds_response.clear(); + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.1")); + resp.ep.port(port); + ds_response.push_back(resp); + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.3")); + resp.ep.port(xs3->GetPort()); + ds_response.push_back(resp); + + agent_->controller()->ApplyDiscoveryXmppServices(ds_response); + client->WaitForIdle(); + + //wait for connection establishment + EXPECT_TRUE(agent_->controller_ifmap_xmpp_port(0) == port); + ASSERT_STREQ(agent_->controller_ifmap_xmpp_server(0).c_str(), "127.0.0.1"); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(0)->GetXmppChannel()->GetPeerState() + == xmps::NOT_READY); + + EXPECT_TRUE(agent_->controller_ifmap_xmpp_port(1) == + (uint32_t)xs3->GetPort()); + ASSERT_STREQ(agent_->controller_ifmap_xmpp_server(1).c_str(), "127.0.0.3"); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(1)->GetXmppChannel()->GetPeerState() + == xmps::READY); + + // Wait until older XmppClient, XmppChannel is cleaned + client->WaitForIdle(); + + xs3->Shutdown(); + client->WaitForIdle(); + + // Simulate Discovery response for Xmpp Server + ds_response.clear(); + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.4")); + resp.ep.port(xs4->GetPort()); + ds_response.push_back(resp); + + agent_->controller()->ApplyDiscoveryXmppServices(ds_response); + client->WaitForIdle(); + + EXPECT_TRUE(agent_->controller_ifmap_xmpp_port(0) == + (uint32_t)xs4->GetPort()); + ASSERT_STREQ(agent_->controller_ifmap_xmpp_server(0).c_str(), "127.0.0.4"); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(0)->GetXmppChannel()->GetPeerState() + == xmps::READY); + EXPECT_TRUE(agent_->controller_xmpp_channel(1) == NULL); + + xs4->Shutdown(); + client->WaitForIdle(); + xs5->Shutdown(); + client->WaitForIdle(); + + // Simulate Discovery response for Xmpp Server + ds_response.clear(); + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.4")); + resp.ep.port(xs4->GetPort()); // Dummy port place-holder + ds_response.push_back(resp); + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.5")); + resp.ep.port(xs4->GetPort()); + ds_response.push_back(resp); + + agent_->controller()->ApplyDiscoveryXmppServices(ds_response); + client->WaitForIdle(); + + ASSERT_STREQ(agent_->controller_ifmap_xmpp_server(0).c_str(), "127.0.0.4"); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(0)->GetXmppChannel()->GetPeerState() + == xmps::NOT_READY); + + ASSERT_STREQ(agent_->controller_ifmap_xmpp_server(1).c_str(), "127.0.0.5"); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(1)->GetXmppChannel()->GetPeerState() + == xmps::NOT_READY); + + // Simulate Discovery response for Xmpp Server + ds_response.clear(); + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.5")); + resp.ep.port(xs6->GetPort()); // Dummy port place-holder + ds_response.push_back(resp); + resp.ep.address(boost::asio::ip::address::from_string("127.0.0.6")); + resp.ep.port(xs6->GetPort()); + ds_response.push_back(resp); + + agent_->controller()->ApplyDiscoveryXmppServices(ds_response); + client->WaitForIdle(); + + ASSERT_STREQ(agent_->controller_ifmap_xmpp_server(0).c_str(), "127.0.0.6"); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(0)->GetXmppChannel()->GetPeerState() + == xmps::READY); + + ASSERT_STREQ(agent_->controller_ifmap_xmpp_server(1).c_str(), "127.0.0.5"); + WAIT_FOR(1000, 10000, + agent_->controller_xmpp_channel(1)->GetXmppChannel()->GetPeerState() + == xmps::NOT_READY); + + xs6->Shutdown(); + client->WaitForIdle(); +} + }