]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Tweak test_keepalived_respawns test logic
authorarmando-migliaccio <armamig@gmail.com>
Fri, 11 Sep 2015 04:54:33 +0000 (21:54 -0700)
committerarmando-migliaccio <armamig@gmail.com>
Sat, 12 Sep 2015 08:37:20 +0000 (01:37 -0700)
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

neutron/tests/functional/agent/linux/test_keepalived.py

index cfb49348bebf48c9f9ac5fe2abb0f1a593f4428f..4c0b9605da66cacddee6665d43e422613a5e5845 100644 (file)
@@ -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")))