Taking the haproxy process id from config.pid file

The haproxy config is updated by providing the new configuration file
and old haproxy process id. This starts a new haproxy process letting
the old haproxy process to handle the old sessions. New haproxy handles
the new sessions. If a new configuration update comes while both old and
new haproxy processes are runnning, another haproxy process needs to be started
by providing the latest running haproxy process id.

In the existing update code, the haproxy process id is found by "ps"
output. This is resulating in not getting the latest haproxy process id,
but an older process id. This is leading to second haproxy process never
getting killed leaving two haproxies running for ever.

The latest haproxy process that is handling the new session is always
updated in a file by haproxy itself. As a fix, this process id used to
start a new haproxy rather taking the pid from "ps". When the
loadbalancer VIP is deleted, Agent deletes the directory where haproxy
config file, stats file and pid file are stored. This can result in
vrouter_netns.py script failing as pid is not found to stop the haproxy.
To overcome this, the config file, pid file and sock file are stored not
in an explicit directory corresponding to the pool. Only config fike and
sock file are deleted by Agent and pid file is deleted by vrouter_netns
script.

Change-Id: Id7e1a74a4f076f02c052f860ab5b183adf950640
closes-bug: #1495702

src/nodemgr/haproxy_stats.py

@@ -33,7 +33,7 @@ def __init__(self):
         pass
 
     def get_stats(self, pool_id):
-        sock_path = os.path.join(LB_BASE_DIR, pool_id, 'etc/haproxy/haproxy.cfg.sock')
+        sock_path  = LB_BASE_DIR  + pool_id  + ".haproxy.cfg.sock";
         if not os.path.exists(sock_path):
             sys.stderr.write('\nStats socket not found for pool ' + pool_id)
             return {}

src/vnsw/agent/oper/instance_manager.cc

@@ -100,17 +100,19 @@ class InstanceManager::NamespaceStaleCleaner {
             //If Loadbalncer, delete the config files as well
             if (prop.service_type == ServiceInstance::LoadBalancer) {
 
-                std::stringstream cfg_dir_path;
-                cfg_dir_path <<
-                    manager_->loadbalancer_config_path_ << prop.pool_id;
+                std::stringstream cfg_path;
+                cfg_path <<
+                    manager_->loadbalancer_config_path_ << prop.pool_id
+                    << ".haproxy.cfg";
 
                 boost::system::error_code error;
-                if (fs::exists(cfg_dir_path.str())) {
-                    fs::remove_all(cfg_dir_path.str(), error);
-                    if (error) {
-                        LOG(ERROR, "Stale Haproxy cfg fle delete error"
-                                    << error.message());
-                    }
+                if (fs::exists(cfg_path.str())) {
+                    fs::remove_all(cfg_path.str(), error);
+                }
+
+                cfg_path << ".sock";
+                if (fs::exists(cfg_path.str())) {
+                    fs::remove_all(cfg_path.str(), error);
                 }
             }
         }
@@ -619,7 +621,7 @@ void InstanceManager::StopStaleNetNS(ServiceInstance::Properties &props) {
     cmd_str << " " << UuidToString(boost::uuids::nil_uuid());
     if (props.service_type == ServiceInstance::LoadBalancer) {
         cmd_str << " --cfg-file " << loadbalancer_config_path_default <<
-            props.pool_id << "/etc/haproxy/haproxy.cfg";
+            props.pool_id << ".haproxy.cfg";
         cmd_str << " --pool-id " << props.pool_id;
     }
 
@@ -710,36 +712,41 @@ void InstanceManager::LoadbalancerObserver(
     DBTablePartBase *db_part, DBEntryBase *entry) {
     Loadbalancer *loadbalancer = static_cast<Loadbalancer *>(entry);
     std::stringstream pathgen;
-        pathgen << loadbalancer_config_path_ << loadbalancer->uuid();
-
-        boost::system::error_code error;
-        if (!loadbalancer->IsDeleted() && loadbalancer->properties() != NULL) {
-            pathgen << "/etc/haproxy";
-            boost::filesystem::path dir(pathgen.str());
-            if (!boost::filesystem::exists(dir, error)) {
-#if 0
-                if (error) {
-                    LOG(ERROR, error.message());
-                    return;
-                }
-#endif
-                boost::filesystem::create_directories(dir, error);
-                if (error) {
-                    LOG(ERROR, error.message());
-                    return;
-                }
+
+    pathgen << loadbalancer_config_path_ << loadbalancer->uuid();
+    pathgen << ".haproxy.cfg";
+
+    boost::system::error_code error;
+    if (!loadbalancer->IsDeleted() && loadbalancer->properties() != NULL) {
+        boost::filesystem::path dir(loadbalancer_config_path_);
+        if (!boost::filesystem::exists(dir, error)) {
+            boost::filesystem::create_directories(dir, error);
+            if (error) {
+                LOG(ERROR, error.message());
+                return;
             }
-            pathgen << "/haproxy.cfg";
-            haproxy_->GenerateConfig(pathgen.str(), loadbalancer->uuid(),
+        }
+
+        haproxy_->GenerateConfig(pathgen.str(), loadbalancer->uuid(),
                                      *loadbalancer->properties());
-        } else {
-             boost::filesystem::remove_all(pathgen.str(), error);
-             if (error) {
-                 LOG(ERROR, error.message());
-                 return;
-             }
+    } else {
+        boost::filesystem::path file(pathgen.str());
+        if (boost::filesystem::exists(file, error)) {
+            boost::filesystem::remove_all(pathgen.str(), error);
+            if (error) {
+                LOG(ERROR, error.message());
+                return;
+            }
+
+            pathgen << ".sock";
+            boost::filesystem::remove_all(pathgen.str(), error);
+            if (error) {
+                LOG(ERROR, error.message());
+                return;
+            }
         }
     }
+}
 
 bool InstanceManager::StaleTimeout() {
 

src/vnsw/agent/oper/netns_instance_adapter.cc

@@ -34,7 +34,7 @@ InstanceTask* NetNSInstanceAdapter::CreateStartTask(const ServiceInstance::Prope
 
     if (props.service_type == ServiceInstance::LoadBalancer) {
         cmd_str << " --cfg-file " << loadbalancer_config_path_ <<
-            props.pool_id << "/etc/haproxy/haproxy.cfg";
+            props.pool_id << ".haproxy.cfg";
         cmd_str << " --pool-id " << props.pool_id;
     }
 
@@ -68,7 +68,7 @@ InstanceTask* NetNSInstanceAdapter::CreateStopTask(const ServiceInstance::Proper
     cmd_str << " " << UuidToString(props.vmi_outside);
     if (props.service_type == ServiceInstance::LoadBalancer) {
         cmd_str << " --cfg-file " << loadbalancer_config_path_ <<
-            props.pool_id << "/etc/haproxy/haproxy.cfg";
+            props.pool_id << ".haproxy.cfg";
         cmd_str << " --pool-id " << props.pool_id;
     }
 

src/vnsw/agent/oper/test/instance_manager_test.cc

@@ -544,7 +544,7 @@ TEST_F(InstanceManagerTest, LoadbalancerConfig) {
 
     stringstream pathgen;
     pathgen << loadbalancer_config_path() << lbid
-            << "/etc/haproxy/haproxy.cfg";
+            << ".haproxy.cfg";
     boost::filesystem::path config(pathgen.str());
     std::time_t old_time =
         boost::filesystem::last_write_time(pathgen.str());
@@ -554,7 +554,7 @@ TEST_F(InstanceManagerTest, LoadbalancerConfig) {
     pathgen.str("");
     pathgen.clear();
     pathgen << loadbalancer_config_path() << lbid
-            << "/etc/haproxy/haproxy.cfg";
+            << ".haproxy.cfg";
     boost::filesystem::path config1(pathgen.str());
 
     //Make sure that both files exists

src/vnsw/opencontrail-vrouter-netns/opencontrail_vrouter_netns/vrouter_netns.py

@@ -32,6 +32,7 @@
 import subprocess
 import requests
 import json
+import os
 
 from linux import ip_lib
 
@@ -129,8 +130,8 @@ def set_snat(self):
                                  'dev', str(self.nic_left['name'])])
 
     def _get_lbaas_pid(self):
-        cmd = """ps aux | grep  \'%(process)s -f %(file)s\' | grep -v grep 
-              """ % {'process':self.LBAAS_PROCESS, 'file':self.cfg_file}
+        pid_file = self.cfg_file + ".pid"
+        cmd = """cat %(file)s """ % {'file':pid_file}
         try:
             if "check_output" not in dir(subprocess):
                 s = _check_output(cmd)
@@ -139,9 +140,7 @@ def _get_lbaas_pid(self):
 
         except subprocess.CalledProcessError:
             return None
-        words = s.split()
-        pid = int(words[1])
-        return pid
+        return int(s)
 
     def set_lbaas(self):
         if not self.ip_ns.netns.exists(self.namespace):
@@ -179,9 +178,21 @@ def release_lbaas(self):
                     s = _check_output(cmd)
                 else:
                     s = subprocess.check_output(cmd, shell=True)
-                print ("Haproxy process with pid %d config file %s killed" %(pid, self.cfg_file), file=sys.stderr)
             except subprocess.CalledProcessError:
                 print ("SIGKILL Error for pid %d %s" %(pid, self.cfg_file), file=sys.stderr)
+
+            pfile = self.cfg_file + ".pid"
+            if not os.path.exists(pfile):
+                return
+
+            cmd = """rm -f %(pid_file)s""" % {'pid_file':pfile}
+            try:
+                if "check_output" not in dir(subprocess):
+                    s = _check_output(cmd)
+                else:
+                    s = subprocess.check_output(cmd, shell=True)
+            except subprocess.CalledProcessError:
+                print ("Error in deleting pidfile %s %s" %(pfile, self.cfg_file), file=sys.stderr)
         try:
             self.ip_ns.netns.execute(['route', 'del', 'default'])
         except RuntimeError: