Issue: When both discovery IP and service IP is configured in contrai…

…l-vrouter-agent.conf, the expected_connections count (used by NodeStatus UVE to declare whether agent is Functional or not) computed by agent was incorrect.

Fix: While updating expected_connections count we should consider whether agent has subscribed for a given discovery service or not.
Closes-Bug: #1396164

Change-Id: I93837ea036276580c55903f7b2d8e36bbd66ca7d

src/vnsw/agent/cmn/agent_param.cc

@@ -807,17 +807,18 @@ void AgentParam::set_test_mode(bool mode) {
 }
 
 AgentParam::AgentParam(Agent *agent) :
-        vhost_(), eth_port_(), xmpp_instance_count_(), xmpp_server_1_(),
-        xmpp_server_2_(), dns_server_1_(), dns_server_2_(),
+        xmpp_server_1_(), xmpp_server_2_(), dns_server_1_(), dns_server_2_(),
+        dss_server_(), collector_server_list_(),
+        vhost_(), eth_port_(), xmpp_instance_count_(),
         dns_port_1_(ContrailPorts::DnsServerPort()),
         dns_port_2_(ContrailPorts::DnsServerPort()),
-        dss_server_(), mgmt_ip_(), mode_(MODE_KVM), xen_ll_(),
+        mgmt_ip_(), mode_(MODE_KVM), xen_ll_(),
         tunnel_type_(), metadata_shared_secret_(), max_vm_flows_(),
         linklocal_system_flows_(), linklocal_vm_flows_(),
         flow_cache_timeout_(), config_file_(), program_name_(),
         log_file_(), log_local_(false), log_flow_(false),
         log_level_(), log_category_(), use_syslog_(false),
-        collector_server_list_(), http_server_port_(), host_name_(),
+        http_server_port_(), host_name_(),
         agent_stats_interval_(AgentStatsInterval),
         flow_stats_interval_(FlowStatsInterval),
         vmware_physical_port_(""), test_mode_(false), debug_(false), tree_(),

src/vnsw/agent/cmn/agent_param.h

@@ -128,6 +128,13 @@ class AgentParam  {
     void InitVhostAndXenLLPrefix();
     void set_test_mode(bool mode);
     bool test_mode() const { return test_mode_; }
+protected:
+    Ip4Address xmpp_server_1_;
+    Ip4Address xmpp_server_2_;
+    Ip4Address dns_server_1_;
+    Ip4Address dns_server_2_;
+    Ip4Address dss_server_;
+    std::vector<std::string> collector_server_list_;
 private:
     void ComputeFlowLimits();
     void InitFromSystem();
@@ -198,13 +205,8 @@ class AgentParam  {
     PortInfo vhost_;
     std::string eth_port_;
     uint16_t xmpp_instance_count_;
-    Ip4Address xmpp_server_1_;
-    Ip4Address xmpp_server_2_;
-    Ip4Address dns_server_1_;
-    Ip4Address dns_server_2_;
     uint16_t dns_port_1_;
     uint16_t dns_port_2_;
-    Ip4Address dss_server_;
     Ip4Address mgmt_ip_;
     Mode mode_;
     PortInfo xen_ll_;
@@ -228,7 +230,6 @@ class AgentParam  {
     std::string log_category_;
     bool use_syslog_;
     std::string syslog_facility_;
-    std::vector<std::string> collector_server_list_;
     uint16_t http_server_port_;
     std::string host_name_;
     int agent_stats_interval_;

src/vnsw/agent/test/SConscript

@@ -30,7 +30,8 @@ env.Append(LIBS = [
         'task_test',
         ])
 
-test_lib_srcs = ['test_agent_init.cc',
+test_lib_srcs = ['agent_param_test.cc',
+                 'test_agent_init.cc',
                  'test_init.cc', 
                  'test_util.cc',
                  '../pkt/test/test_pkt_util.cc',

src/vnsw/agent/test/agent_param_test.cc

@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved.
+ */
+
+#include <agent_param_test.h>
+
+AgentParamTest::AgentParamTest(Agent *agent) : AgentParam(agent) {
+}
+
+AgentParamTest::~AgentParamTest() {
+}
+
+Ip4Address AgentParamTest::StrToIp(const char *ip) {
+    boost::system::error_code ec;
+    Ip4Address addr = Ip4Address::from_string(ip, ec);
+    if (ec.value() == 0) {
+        return addr;
+    } else {
+        return Ip4Address(0);
+    }
+}
+
+void AgentParamTest::set_xmpp_server_1(const char *ip) {
+    xmpp_server_1_ = StrToIp(ip);
+}
+
+void AgentParamTest::set_xmpp_server_2(const char *ip) {
+    xmpp_server_2_ = StrToIp(ip);
+}
+
+void AgentParamTest::set_dns_server_1(const char *ip) {
+    dns_server_1_ = StrToIp(ip);
+}
+
+void AgentParamTest::set_dns_server_2(const char *ip) {
+    dns_server_2_ = StrToIp(ip);
+}
+
+void AgentParamTest::set_collector_server_list(const char *ip) {
+    std::string element(ip);
+    if (element.length()) {
+        collector_server_list_.push_back(element);
+    } else {
+        collector_server_list_.clear();
+    }
+}
+
+void AgentParamTest::set_discovery_server(const char *ip) {
+    dss_server_ = StrToIp(ip);
+}

src/vnsw/agent/test/agent_param_test.h

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved.
+ */
+
+#ifndef vnsw_agent_param_test_hpp
+#define vnsw_agent_param_test_hpp
+
+#include <cmn/agent_cmn.h>
+#include <cmn/agent_param.h>
+
+// Class handling agent configuration parameters from config file and 
+// arguments
+class AgentParamTest : public AgentParam  {
+public:
+    AgentParamTest(Agent *agent);
+    virtual ~AgentParamTest();
+    Ip4Address StrToIp(const char *ip);
+    void set_xmpp_server_1(const char*);
+    void set_xmpp_server_2(const char*);
+    void set_dns_server_1(const char*);
+    void set_dns_server_2(const char*);
+    void set_discovery_server(const char*);
+    void set_collector_server_list(const char*);
+};
+
+#endif // vnsw_agent_param_test_hpp

src/vnsw/agent/test/test_init.h

@@ -69,6 +69,7 @@
 #include <oper/agent_path.h>
 #include <controller/controller_route_path.h>
 
+#include "agent_param_test.h"
 #include "test_agent_init.h"
 using namespace std;
 
@@ -549,7 +550,7 @@ class TestClient {
                                                    this, _1, _2));
     };
     TestAgentInit *agent_init() { return &agent_init_; }
-    AgentParam *param() { return &param_; }
+    AgentParamTest *param() { return &param_; }
     Agent *agent() { return agent_; }
     void set_agent(Agent *agent) { agent_ = agent; }
     void delete_agent() {
@@ -577,7 +578,7 @@ class TestClient {
     std::vector<const NextHop *> comp_nh_list_;
     Agent *agent_;
     TestAgentInit agent_init_;
-    AgentParam param_;
+    AgentParamTest param_;
 };
 
 TestClient *TestInit(const char *init_file = NULL, bool ksync_init = false, 

src/vnsw/agent/uve/agent_uve.cc

@@ -81,14 +81,27 @@ void AgentUve::Init() {
 uint8_t AgentUve::ExpectedConnections(uint8_t &num_control_nodes,
                                       uint8_t &num_dns_servers) {
     uint8_t count = 0;
+    AgentParam *cfg = agent_->params();
 
-    /* If Discovery server is configured, increment the count to
-     * 3 for 3 possible discovery services */
+    /* If Discovery server is configured, increment the count by 1 for each
+     * possible discovery service if we subscribe to that service. We subscribe
+     * to a discovery service only if the service IP is not configured
+     * explicitly */
     if (!agent_->discovery_server().empty()) {
         // Discovery:Collector
+        if (cfg->collector_server_list().size() == 0) {
+            count++;
+        }
         // Discovery:dns-server
+        if (!cfg->dns_server_1().to_ulong() &&
+            !cfg->dns_server_2().to_ulong()) {
+            count++;
+        }
         // Discovery:xmpp-server
-        count += 3;
+        if (!cfg->xmpp_server_1().to_ulong() &&
+            !cfg->xmpp_server_2().to_ulong() ) {
+            count++;
+        }
     }
     for (int i = 0; i < MAX_XMPP_SERVERS; i++) {
         if (!agent_->controller_ifmap_xmpp_server(i).empty()) {
@@ -101,7 +114,10 @@ uint8_t AgentUve::ExpectedConnections(uint8_t &num_control_nodes,
         }
     }
     //Increment 1 for collector service
-    count++;
+    if (!agent_->discovery_server().empty() ||
+        cfg->collector_server_list().size() != 0) {
+        count++;
+    }
     return count;
 }
 

src/vnsw/agent/uve/agent_uve.h

@@ -48,6 +48,7 @@ class AgentUve {
     void Init();
     void RegisterDBClients();
     static AgentUve *GetInstance() {return singleton_;}
+    uint8_t ExpectedConnections(uint8_t &num_c_nodes, uint8_t &num_d_servers);
 protected:
     boost::scoped_ptr<VnUveTable> vn_uve_table_;
     boost::scoped_ptr<VmUveTable> vm_uve_table_;
@@ -58,7 +59,6 @@ class AgentUve {
     void VrouterAgentProcessState(
         const std::vector<process::ConnectionInfo> &c,
         process::ProcessState::type &state, std::string &message);
-    uint8_t ExpectedConnections(uint8_t &num_c_nodes, uint8_t &num_d_servers);
     void UpdateState(const process::ConnectionInfo &info, 
                       process::ProcessState::type &c,
                       std::string &message);

src/vnsw/agent/uve/test/test_uve.cc

@@ -460,9 +460,74 @@ TEST_F(UveTest, SandeshTest) {
     EXPECT_EQ(3, flow_stats_interval_);
 }
 
+/* Discovery IP and service IPs are not configured */
+TEST_F(UveTest, NodeStatus_ExpectedConnections_0) {
+    Agent *agent = Agent::GetInstance();
+    AgentUve *uve = static_cast<AgentUve *>(agent->uve());
+
+    uint8_t num_c_nodes, num_d_servers;
+    int expected_conns = uve->ExpectedConnections(num_c_nodes, num_d_servers);
+    EXPECT_EQ(expected_conns, 0);
+}
+
+/* Discovery IP is NOT configured and all service IPs are configured */
+TEST_F(UveTest, NodeStatus_ExpectedConnections_1) {
+    Agent *agent = Agent::GetInstance();
+    AgentParamTest *params = static_cast<AgentParamTest *>(agent->params());
+    AgentUve *uve = static_cast<AgentUve *>(agent->uve());
+    params->set_discovery_server("0.0.0.0");
+    params->set_xmpp_server_1("1.1.1.1");
+    params->set_xmpp_server_2("1.1.1.2");
+    params->set_dns_server_1("1.1.1.1");
+    params->set_dns_server_2("1.1.1.2");
+    params->set_collector_server_list("1.1.1.1:8086");
+    agent->CopyConfig(params);
+
+    uint8_t num_c_nodes, num_d_servers;
+    int expected_conns = uve->ExpectedConnections(num_c_nodes, num_d_servers);
+    EXPECT_EQ(expected_conns, 5);
+}
+
+/* Discovery IP is configured and none of the service IPs are configured */
+TEST_F(UveTest, NodeStatus_ExpectedConnections_2) {
+    Agent *agent = Agent::GetInstance();
+    AgentParamTest *params = static_cast<AgentParamTest *>(agent->params());
+    AgentUve *uve = static_cast<AgentUve *>(agent->uve());
+    params->set_discovery_server("1.1.1.1");
+    params->set_xmpp_server_1("0.0.0.0");
+    params->set_xmpp_server_2("0.0.0.0");
+    params->set_dns_server_1("0.0.0.0");
+    params->set_dns_server_2("0.0.0.0");
+    params->set_collector_server_list("");
+    agent->CopyConfig(params);
+
+    uint8_t num_c_nodes, num_d_servers;
+    int expected_conns = uve->ExpectedConnections(num_c_nodes, num_d_servers);
+    EXPECT_EQ(expected_conns, 8);
+}
+
+/* Both Discovery IP and service IPs are configured */
+TEST_F(UveTest, NodeStatus_ExpectedConnections_3) {
+    Agent *agent = Agent::GetInstance();
+    AgentParamTest *params = static_cast<AgentParamTest *>(agent->params());
+    AgentUve *uve = static_cast<AgentUve *>(agent->uve());
+    params->set_discovery_server("1.1.1.1");
+    params->set_xmpp_server_1("1.1.1.1");
+    params->set_xmpp_server_2("1.1.1.2");
+    params->set_dns_server_1("1.1.1.1");
+    params->set_dns_server_2("1.1.1.2");
+    params->set_collector_server_list("1.1.1.1:8086");
+    agent->CopyConfig(params);
+
+    uint8_t num_c_nodes, num_d_servers;
+    int expected_conns = uve->ExpectedConnections(num_c_nodes, num_d_servers);
+    EXPECT_EQ(expected_conns, 5);
+}
+
 int main(int argc, char **argv) {
     GETUSERARGS();
-    client = TestInit(init_file, ksync_init, true, false);
+    client = TestInit("controller/src/vnsw/agent/uve/test/vnswa_cfg.ini",
+                      ksync_init, true, false);
     UveTest::TestSetup(vrf_array, 2);
 
     int ret = RUN_ALL_TESTS();

src/vnsw/agent/uve/test/vnswa_cfg.ini

@@ -7,7 +7,7 @@
 [CONTROL-NODE]
 # IP address to be used to connect to control-node. If IP is not configured 
 # for server1, value provided by discovery service will be used.
-server=127.0.0.1
+# server=127.0.0.1
 
 [DEFAULT]
 # IP address and port to be used to connect to collector. If these are not
@@ -53,7 +53,7 @@ server=127.0.0.1
 # IP address and port to be used to connect to dns-node. Maximum of 2 IP
 # addresses (separated by a space) can be provided. If no IP is configured then
 # the value provided by discovery service will be used.
-server=127.0.0.1:53
+# server=127.0.0.1:53
 
 [HYPERVISOR]
 # Hypervisor type. Possible values are kvm, xen and vmware