From 1ce03fb1f2453e8a55941bcc95ab9c934a631fd1 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Tue, 26 Jul 2011 13:12:11 +0200
Subject: [PATCH] Add ht-based result checks to opcodes

This adds the infrastructure necessary to check opcode results using
ht-based functions. Checks are added for two opcodes.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 lib/errors.py                   |    6 ++++++
 lib/mcpu.py                     |   13 +++++++++++--
 lib/opcodes.py                  |   13 +++++++++++++
 test/ganeti.opcodes_unittest.py |    1 +
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/lib/errors.py b/lib/errors.py
index 9de3b6d..df743d7 100644
--- a/lib/errors.py
+++ b/lib/errors.py
@@ -189,6 +189,12 @@ class OpExecError(GenericError):
   """
 
 
+class OpResultError(GenericError):
+  """Issue with OpCode result.
+
+  """
+
+
 class OpCodeUnknown(GenericError):
   """Unknown opcode submitted.
 
diff --git a/lib/mcpu.py b/lib/mcpu.py
index 7788959..33a9092 100644
--- a/lib/mcpu.py
+++ b/lib/mcpu.py
@@ -373,8 +373,8 @@ class Processor(object):
         assert lu.needed_locks is not None, "needed_locks not set by LU"
 
         try:
-          return self._LockAndExecLU(lu, locking.LEVEL_INSTANCE, calc_timeout,
-                                     priority)
+          result = self._LockAndExecLU(lu, locking.LEVEL_INSTANCE, calc_timeout,
+                                       priority)
         finally:
           if self._ec_id:
             self.context.cfg.DropECReservations(self._ec_id)
@@ -383,6 +383,15 @@ class Processor(object):
     finally:
       self._cbs = None
 
+    resultcheck_fn = op.OP_RESULT
+    if not (resultcheck_fn is None or resultcheck_fn(result)):
+      logging.error("Expected opcode result matching %s, got %s",
+                    resultcheck_fn, result)
+      raise errors.OpResultError("Opcode result does not match %s" %
+                                 resultcheck_fn)
+
+    return result
+
   def Log(self, *args):
     """Forward call to feedback callback function.
 
diff --git a/lib/opcodes.py b/lib/opcodes.py
index 1a4c00a..e00d651 100644
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -419,6 +419,9 @@ def _BuildJobDepCheck(relative):
 
 TNoRelativeJobDependencies = _BuildJobDepCheck(False)
 
+#: List of submission status and job ID as returned by C{SubmitManyJobs}
+TJobIdList = ht.TListOf(ht.TItems([ht.TBool, ht.TOr(ht.TString, ht.TJobId)]))
+
 
 class OpCode(BaseOpCode):
   """Abstract OpCode.
@@ -433,6 +436,7 @@ class OpCode(BaseOpCode):
                       method for details).
   @cvar OP_PARAMS: List of opcode attributes, the default values they should
                    get if not already defined, and types they must match.
+  @cvar OP_RESULT: Callable to verify opcode result
   @cvar WITH_LU: Boolean that specifies whether this should be included in
       mcpu's dispatch table
   @ivar dry_run: Whether the LU should be run in dry-run mode, i.e. just
@@ -454,6 +458,7 @@ class OpCode(BaseOpCode):
     (COMMENT_ATTR, None, ht.TMaybeString,
      "Comment describing the purpose of the opcode"),
     ]
+  OP_RESULT = None
 
   def __getstate__(self):
     """Specialized getstate for opcodes.
@@ -595,6 +600,9 @@ class OpClusterVerifyDisks(OpCode):
   """Verify the cluster disks.
 
   """
+  OP_RESULT = ht.TStrictDict(True, True, {
+    constants.JOB_IDS_KEY: TJobIdList,
+    })
 
 
 class OpGroupVerifyDisks(OpCode):
@@ -619,6 +627,11 @@ class OpGroupVerifyDisks(OpCode):
   OP_PARAMS = [
     _PGroupName,
     ]
+  OP_RESULT = \
+    ht.TAnd(ht.TIsLength(3),
+            ht.TItems([ht.TDictOf(ht.TString, ht.TString),
+                       ht.TListOf(ht.TString),
+                       ht.TDictOf(ht.TString, ht.TListOf(ht.TString))]))
 
 
 class OpClusterRepairDiskSizes(OpCode):
diff --git a/test/ganeti.opcodes_unittest.py b/test/ganeti.opcodes_unittest.py
index d6bdfb4..a9bbd17 100755
--- a/test/ganeti.opcodes_unittest.py
+++ b/test/ganeti.opcodes_unittest.py
@@ -49,6 +49,7 @@ class TestOpcodes(unittest.TestCase):
       self.assertEqual(cls.OP_ID, opcodes._NameToId(cls.__name__))
       self.assertFalse(compat.any(cls.OP_ID.startswith(prefix)
                                   for prefix in opcodes._SUMMARY_PREFIX.keys()))
+      self.assertTrue(cls.OP_RESULT is None or callable(cls.OP_RESULT))
 
       self.assertRaises(TypeError, cls, unsupported_parameter="some value")
 
-- 
1.7.10.4