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

@@ -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

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