From: armando-migliaccio <armamig@gmail.com>
Date: Fri, 11 Sep 2015 04:54:33 +0000 (-0700)
Subject: Tweak test_keepalived_respawns test logic
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=5405d9742b94f203389f555c56727a66925e9454;p=openstack-build%2Fneutron-build.git

Tweak test_keepalived_respawns test logic

This test initial design is problematic: it spawns keepalived,
it asserts the process is up, then it attempts to kill it.

However, this is when problems may arise:

a) it does so by using the disable method on the process - we
   should be more rude than that if we want to simulate a crash!

b) keepalived may be forking while it is starting and it is
   possible that for a moment the ppid changes and the process
   owner invoking the kill has no rights to kill the spawned
   process. This is the most plausible explaination I could find
   as to why kill returns 1 with no standard error

c) it does not verify that the process has indeed disappeared
   (what if the pm.disable didn't work?) - this means that the
   test can pass, and yet the monitor may not work.

Bottom line: this test relied on the correctness of the very code
that was meant to validate...and that's not cool. To this aim, we
wait for the process to be active, kill the process with a kill -9
and verify that the process after the kill is indeed different.

Closes-bug: #1490043

Change-Id: Idaf419a1464d9d0d75b9106a7acd5cd960a7c623
---

diff --git a/neutron/tests/functional/agent/linux/test_keepalived.py b/neutron/tests/functional/agent/linux/test_keepalived.py
index cfb49348b..4c0b9605d 100644
--- a/neutron/tests/functional/agent/linux/test_keepalived.py
+++ b/neutron/tests/functional/agent/linux/test_keepalived.py
@@ -53,21 +53,21 @@ class KeepalivedManagerTestCase(base.BaseTestCase,
         self.assertEqual(self.expected_config.get_config_str(),
                          self.manager.get_conf_on_disk())
 
-    def _log_pid(self, pid):
-        # TODO(amuller): Remove when bug 1490043 is solved.
-        LOG.info(utils.execute(['ps', '-F', pid]))
-
     def test_keepalived_respawns(self):
         self.manager.spawn()
         process = self.manager.get_process()
         pid = process.pid
-        self._log_pid(pid)
-        self.assertTrue(process.active)
-        self._log_pid(pid)
-        process.disable(sig='15')
-
         utils.wait_until_true(
             lambda: process.active,
             timeout=5,
             sleep=0.01,
+            exception=RuntimeError(_("Keepalived didn't spawn")))
+
+        # force process crash, and see that when it comes back
+        # it's indeed a different process
+        utils.execute(['kill', '-9', pid], run_as_root=True)
+        utils.wait_until_true(
+            lambda: process.active and pid != process.pid,
+            timeout=5,
+            sleep=0.01,
             exception=RuntimeError(_("Keepalived didn't respawn")))