From 3fb4f7404062b0e6212c380ed77e34345d6c4a3e Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Wed, 10 Jun 2009 18:23:50 +0200
Subject: [PATCH] Convert hooks_runner rpc to new style result

This also converts (and fixes) unittests and mock objects to deal with
this change, and the custom hook verifier in cmdlib.LUClusterVerify.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
 lib/backend.py                |  7 ++++---
 lib/cmdlib.py                 |  8 +++++---
 lib/mcpu.py                   | 12 +++++++-----
 test/ganeti.hooks_unittest.py | 36 +++++++++++++++++++++--------------
 test/mocks.py                 |  4 ++--
 5 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/lib/backend.py b/lib/backend.py
index f012dcffa..eaf559b8f 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -2557,7 +2557,8 @@ class HooksRunner(object):
     elif phase == constants.HOOKS_PHASE_POST:
       suffix = "post"
     else:
-      raise errors.ProgrammerError("Unknown hooks phase: '%s'" % phase)
+      _Fail("Unknown hooks phase '%s'", phase)
+
     rr = []
 
     subdir = "%s-%s.d" % (hpath, suffix)
@@ -2566,7 +2567,7 @@ class HooksRunner(object):
       dir_contents = utils.ListVisibleFiles(dir_name)
     except OSError, err:
       # FIXME: must log output in case of failures
-      return rr
+      return True, rr
 
     # we use the standard python sort order,
     # so 00name is the recommended naming scheme
@@ -2585,7 +2586,7 @@ class HooksRunner(object):
           rrval = constants.HKR_SUCCESS
       rr.append(("%s/%s" % (subdir, relname), rrval, output))
 
-    return rr
+    return True, rr
 
 
 class IAllocatorRunner(object):
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index ea3632797..d2eb0fdde 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -1242,14 +1242,16 @@ class LUVerifyCluster(LogicalUnit):
         for node_name in hooks_results:
           show_node_header = True
           res = hooks_results[node_name]
-          if res.failed or res.data is False or not isinstance(res.data, list):
+          msg = res.RemoteFailMsg()
+          if msg:
             if res.offline:
               # no need to warn or set fail return value
               continue
-            feedback_fn("    Communication failure in hooks execution")
+            feedback_fn("    Communication failure in hooks execution: %s" %
+                        msg)
             lu_result = 1
             continue
-          for script, hkr, output in res.data:
+          for script, hkr, output in res.payload:
             if hkr == constants.HKR_FAIL:
               # The node header is only shown once, if there are
               # failing hooks on that node
diff --git a/lib/mcpu.py b/lib/mcpu.py
index 2e75f5643..a41577152 100644
--- a/lib/mcpu.py
+++ b/lib/mcpu.py
@@ -340,12 +340,14 @@ class HooksMaster(object):
         raise errors.HooksFailure("Communication failure")
       for node_name in results:
         res = results[node_name]
-        if res.failed or res.data is False or not isinstance(res.data, list):
-          if not res.offline:
-            self.proc.LogWarning("Communication failure to node %s" %
-                                 node_name)
+        if res.offline:
           continue
-        for script, hkr, output in res.data:
+        msg = res.RemoteFailMsg()
+        if msg:
+          self.proc.LogWarning("Communication failure to node %s: %s",
+                               node_name, msg)
+          continue
+        for script, hkr, output in res.payload:
           if hkr == constants.HKR_FAIL:
             errs.append((node_name, script, output))
       if errs:
diff --git a/test/ganeti.hooks_unittest.py b/test/ganeti.hooks_unittest.py
index 55fef1962..5f820c059 100755
--- a/test/ganeti.hooks_unittest.py
+++ b/test/ganeti.hooks_unittest.py
@@ -75,7 +75,8 @@ class TestHooksRunner(unittest.TestCase):
   def testEmpty(self):
     """Test no hooks"""
     for phase in (constants.HOOKS_PHASE_PRE, constants.HOOKS_PHASE_POST):
-      self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}), [])
+      self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
+                           (True, []))
 
   def testSkipNonExec(self):
     """Test skip non-exec file"""
@@ -85,7 +86,7 @@ class TestHooksRunner(unittest.TestCase):
       f.close()
       self.torm.append((fname, False))
       self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
-                           [(self._rname(fname), HKR_SKIP, "")])
+                           (True, [(self._rname(fname), HKR_SKIP, "")]))
 
   def testSkipInvalidName(self):
     """Test skip script with invalid name"""
@@ -97,7 +98,7 @@ class TestHooksRunner(unittest.TestCase):
       os.chmod(fname, 0700)
       self.torm.append((fname, False))
       self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
-                           [(self._rname(fname), HKR_SKIP, "")])
+                           (True, [(self._rname(fname), HKR_SKIP, "")]))
 
   def testSkipDir(self):
     """Test skip directory"""
@@ -106,7 +107,7 @@ class TestHooksRunner(unittest.TestCase):
       os.mkdir(fname)
       self.torm.append((fname, True))
       self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
-                           [(self._rname(fname), HKR_SKIP, "")])
+                           (True, [(self._rname(fname), HKR_SKIP, "")]))
 
   def testSuccess(self):
     """Test success execution"""
@@ -118,7 +119,7 @@ class TestHooksRunner(unittest.TestCase):
       self.torm.append((fname, False))
       os.chmod(fname, 0700)
       self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
-                           [(self._rname(fname), HKR_SUCCESS, "")])
+                           (True, [(self._rname(fname), HKR_SUCCESS, "")]))
 
   def testSymlink(self):
     """Test running a symlink"""
@@ -127,7 +128,7 @@ class TestHooksRunner(unittest.TestCase):
       os.symlink("/bin/true", fname)
       self.torm.append((fname, False))
       self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
-                           [(self._rname(fname), HKR_SUCCESS, "")])
+                           (True, [(self._rname(fname), HKR_SUCCESS, "")]))
 
   def testFail(self):
     """Test success execution"""
@@ -139,7 +140,7 @@ class TestHooksRunner(unittest.TestCase):
       self.torm.append((fname, False))
       os.chmod(fname, 0700)
       self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
-                           [(self._rname(fname), HKR_FAIL, "")])
+                           (True, [(self._rname(fname), HKR_FAIL, "")]))
 
   def testCombined(self):
     """Test success, failure and skip all in one test"""
@@ -156,7 +157,8 @@ class TestHooksRunner(unittest.TestCase):
         self.torm.append((fname, False))
         os.chmod(fname, 0700)
         expect.append((self._rname(fname), rs, ""))
-      self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}), expect)
+      self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
+                           (True, expect))
 
   def testOrdering(self):
     for phase in (constants.HOOKS_PHASE_PRE, constants.HOOKS_PHASE_POST):
@@ -172,7 +174,8 @@ class TestHooksRunner(unittest.TestCase):
         self.torm.append((fname, False))
         expect.append((self._rname(fname), HKR_SUCCESS, ""))
       expect.sort()
-      self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}), expect)
+      self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, {}),
+                           (True, expect))
 
   def testEnv(self):
     """Test environment execution"""
@@ -184,7 +187,8 @@ class TestHooksRunner(unittest.TestCase):
       env_snt = {"PHASE": phase}
       env_exp = "PHASE=%s" % phase
       self.failUnlessEqual(self.hr.RunHooks(self.hpath, phase, env_snt),
-                           [(self._rname(fname), HKR_SUCCESS, env_exp)])
+                           (True, [(self._rname(fname), HKR_SUCCESS,
+                                    env_exp)]))
 
 
 class TestHooksMaster(unittest.TestCase):
@@ -213,8 +217,10 @@ class TestHooksMaster(unittest.TestCase):
     @return: script execution failure from all nodes
 
     """
-    return dict([(node, rpc.RpcResult([("utest", constants.HKR_FAIL, "err")],
-                  node=node, call='FakeScriptFail')) for node in node_list])
+    rr = rpc.RpcResult
+    return dict([(node, rr((True, [("utest", constants.HKR_FAIL, "err")]),
+                           node=node, call='FakeScriptFail'))
+                  for node in node_list])
 
   @staticmethod
   def _call_script_succeed(node_list, hpath, phase, env):
@@ -224,8 +230,10 @@ class TestHooksMaster(unittest.TestCase):
     @return: script execution from all nodes
 
     """
-    return dict([(node, rpc.RpcResult([("utest", constants.HKR_SUCCESS, "ok")],
-                  node=node, call='FakeScriptOk')) for node in node_list])
+    rr = rpc.RpcResult
+    return dict([(node, rr(True, [("utest", constants.HKR_SUCCESS, "ok")],
+                           node=node, call='FakeScriptOk'))
+                 for node in node_list])
 
   def setUp(self):
     self.op = opcodes.OpCode()
diff --git a/test/mocks.py b/test/mocks.py
index 249cd7252..58e68c3c3 100644
--- a/test/mocks.py
+++ b/test/mocks.py
@@ -55,10 +55,10 @@ class FakeConfig:
 class FakeProc:
     """Fake processor object"""
 
-    def LogWarning(self, msg):
+    def LogWarning(self, msg, *args, **kwargs):
         pass
 
-    def LogInfo(self, msg):
+    def LogInfo(self, msg, *args, **kwargs):
         pass
 
 class FakeContext:
-- 
GitLab