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.

Compared to upstream commit, the fixture in test_keepalived was adjusted
for kilo's version of fixtures package.

Conflicts:
    neutron/agent/linux/keepalived.py

Closes-Bug: 1561046
Change-Id: Ib6b6f2fe76fe82253f195c9eab6b243d9eb76fa2
(cherry picked from commit e98fabb5836b12bc40a2b64a2668893ea73c2320)

2 files changed, 74 insertions, 2 deletions

diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py
index cc8a324d79..b0b764a94a 100644
--- a/neutron/agent/linux/keepalived.py
+++ b/neutron/agent/linux/keepalived.py
@@ -20,6 +20,7 @@ import netaddr
 from oslo_config import cfg
 from oslo_log import log as logging
 
+from neutron.i18n import _, _LE
 from neutron.agent.linux import external_process
 from neutron.agent.linux import utils
 from neutron.common import exceptions
@@ -320,6 +321,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:
@@ -334,7 +347,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))
@@ -378,10 +391,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 a2747ce00a..2079e9fd4e 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.agent.linux import external_process
@@ -71,3 +75,54 @@ 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):
+        super(SleepyProcessFixture, self).setUp()
+        self.process = Process(target=self.yawn)
+
+    def destroy(self):
+        self.process.terminate()
+
+    @property
+    def pid(self):
+        return self.process.pid