From e98fabb5836b12bc40a2b64a2668893ea73c2320 Mon Sep 17 00:00:00 2001
From: Hynek Mlnarik <hmlnarik@redhat.com>
Date: Wed, 23 Mar 2016 14:51:59 +0100
Subject: [PATCH] Remove obsolete keepalived PID files before start

keepalived refuses to start and claims "daemon already started"
when there is already a process with the same PID as found in
either the VRRP or the main process PID file. This happens even
in case when the new process is not keepalived. The situation
can happen when the neutron node is reset and the obsolete PID
files are not cleaned before neutron is started.

This commit adds PID file cleanup before keepalived start.

Closes-Bug: 1561046
Change-Id: Ib6b6f2fe76fe82253f195c9eab6b243d9eb76fa2
---
 neutron/agent/linux/keepalived.py             | 22 ++++++--
 .../functional/agent/linux/test_keepalived.py | 54 +++++++++++++++++++
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py
index 3c10f6a885c..ed02a9a65c8 100644
--- a/neutron/agent/linux/keepalived.py
+++ b/neutron/agent/linux/keepalived.py
@@ -20,7 +20,7 @@ import netaddr
 from oslo_config import cfg
 from oslo_log import log as logging
 
-from neutron._i18n import _
+from neutron._i18n import _, _LE
 from neutron.agent.linux import external_process
 from neutron.common import exceptions
 from neutron.common import utils as common_utils
@@ -367,6 +367,18 @@ class KeepalivedManager(object):
 
         return config_path
 
+    @staticmethod
+    def _safe_remove_pid_file(pid_file):
+        try:
+            os.remove(pid_file)
+        except OSError as e:
+            if e.errno != errno.ENOENT:
+                LOG.error(_LE("Could not delete file %s, keepalived can "
+                              "refuse to start."), pid_file)
+
+    def get_vrrp_pid_file_name(self, base_pid_file):
+        return '%s-vrrp' % base_pid_file
+
     def get_conf_on_disk(self):
         config_path = self.get_full_config_file_path('keepalived.conf')
         try:
@@ -381,7 +393,7 @@ class KeepalivedManager(object):
 
         keepalived_pm = self.get_process()
         vrrp_pm = self._get_vrrp_process(
-            '%s-vrrp' % keepalived_pm.get_pid_file_name())
+            self.get_vrrp_pid_file_name(keepalived_pm.get_pid_file_name()))
 
         keepalived_pm.default_cmd_callback = (
             self._get_keepalived_process_callback(vrrp_pm, config_path))
@@ -424,10 +436,14 @@ class KeepalivedManager(object):
             # and spawn keepalived successfully.
             if vrrp_pm.active:
                 vrrp_pm.disable()
+
+            self._safe_remove_pid_file(pid_file)
+            self._safe_remove_pid_file(self.get_vrrp_pid_file_name(pid_file))
+
             cmd = ['keepalived', '-P',
                    '-f', config_path,
                    '-p', pid_file,
-                   '-r', '%s-vrrp' % pid_file]
+                   '-r', self.get_vrrp_pid_file_name(pid_file)]
             return cmd
 
         return callback
diff --git a/neutron/tests/functional/agent/linux/test_keepalived.py b/neutron/tests/functional/agent/linux/test_keepalived.py
index eacbbaf0222..ac1fe80cc92 100644
--- a/neutron/tests/functional/agent/linux/test_keepalived.py
+++ b/neutron/tests/functional/agent/linux/test_keepalived.py
@@ -13,6 +13,10 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import fixtures
+from multiprocessing import Process
+import time
+
 from oslo_config import cfg
 
 from neutron._i18n import _
@@ -76,3 +80,53 @@ class KeepalivedManagerTestCase(base.BaseTestCase,
 
     def test_keepalived_respawn_with_unexpected_exit(self):
         self._test_keepalived_respawns(False)
+
+    def _test_keepalived_spawns_conflicting_pid(self, process, pid_file):
+        # Test the situation when keepalived PID file contains PID of an
+        # existing non-keepalived process. This situation can happen e.g.
+        # after hard node reset.
+
+        spawn_process = SleepyProcessFixture()
+        self.useFixture(spawn_process)
+
+        with open(pid_file, "w") as f_pid_file:
+            f_pid_file.write("%s" % spawn_process.pid)
+
+        self.manager.spawn()
+        utils.wait_until_true(
+            lambda: process.active,
+            timeout=5,
+            sleep=0.1,
+            exception=RuntimeError(_("Keepalived didn't spawn")))
+
+    def test_keepalived_spawns_conflicting_pid_base_process(self):
+        process = self.manager.get_process()
+        pid_file = process.get_pid_file_name()
+        self._test_keepalived_spawns_conflicting_pid(process, pid_file)
+
+    def test_keepalived_spawns_conflicting_pid_vrrp_subprocess(self):
+        process = self.manager.get_process()
+        pid_file = process.get_pid_file_name()
+        self._test_keepalived_spawns_conflicting_pid(
+            process,
+            self.manager.get_vrrp_pid_file_name(pid_file))
+
+
+class SleepyProcessFixture(fixtures.Fixture):
+
+    def __init__(self):
+        super(SleepyProcessFixture, self).__init__()
+
+    @staticmethod
+    def yawn():
+        time.sleep(60)
+
+    def _setUp(self):
+        self.process = Process(target=self.yawn)
+
+    def destroy(self):
+        self.process.terminate()
+
+    @property
+    def pid(self):
+        return self.process.pid