From 98dfcaffc6a0d39e7e1bec2babefb23085c0693c Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Tue, 8 May 2012 14:29:07 +0200
Subject: [PATCH] Fix exception re-raising in Python Luxi clients
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit e687ec01 (present in 2.5 since the 2.5 beta 3) did consistency
fixes across the code-base. Unfortunately this was done without enough
checks on the actual meaning of one of the fixes, which means error
re-raising in lib/errors.py is broken.

The problem is that:

  raise cls, args

is different than:

  raise cls(args)

And our unit-tests didn't catch this (this patch updates the tests).

This breakage is usually trivial, like wrong error messages:

  $ gnt-instance remove no-such-instance
  Failure: prerequisites not met for this operation:
  ("Instance 'no-such-instance' not known", 'unknown_entity')

versus:

  $ gnt-instance remove no-such-instance
  Failure: prerequisites not met for this operation:
  error type: unknown_entity, error details:
  Instance 'no-such-instance' not known

or:

  $ gnt-instance add … no-such-instance
  Failure: prerequisites not met for this operation:
  ('The given name (no-such-instance) does not resolve: Name or service not known', 'resolver_error')

versus:

  $ gnt-instance add … no-such-instance
  Failure: prerequisites not met for this operation:
  error type: resolver_error, error details:
  The given name (no-such-instance) does not resolve: Name or service not known

But in some cases where we rely on a certain data representation
(e.g. HooksAbort), this actually breaks because we try to iterate over
the wrong type:

  File "/usr/lib/python2.6/dist-packages/ganeti/cli.py", line 1907, in FormatError
     for node, script, out in err.args[0]:
  ValueError: need more than 1 value to unpack

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
---
 lib/errors.py                  |  5 +++--
 test/ganeti.errors_unittest.py | 19 +++++++++++++++----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/lib/errors.py b/lib/errors.py
index 181288618..948920c27 100644
--- a/lib/errors.py
+++ b/lib/errors.py
@@ -1,7 +1,7 @@
 #
 #
 
-# Copyright (C) 2006, 2007, 2008, 2009, 2010 Google Inc.
+# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Google Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -482,4 +482,5 @@ def MaybeRaise(result):
   error = GetEncodedError(result)
   if error:
     (errcls, args) = error
-    raise errcls(args)
+    # pylint: disable=W0142
+    raise errcls(*args)
diff --git a/test/ganeti.errors_unittest.py b/test/ganeti.errors_unittest.py
index ec25912c6..f10f661c7 100755
--- a/test/ganeti.errors_unittest.py
+++ b/test/ganeti.errors_unittest.py
@@ -1,7 +1,7 @@
 #!/usr/bin/python
 #
 
-# Copyright (C) 2010 Google Inc.
+# Copyright (C) 2010, 2012 Google Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -53,14 +53,25 @@ class TestErrors(testutils.GanetiTestCase):
                            ("GenericError", (True, 100, "foo", ["x", "y"])))
 
   def testMaybeRaise(self):
+    testvals = [None, 1, 2, 3, "Hello World", (1, ), (1, 2, 3),
+                ("NoErrorClassName", []), ("NoErrorClassName", None),
+                ("GenericError", [1, 2, 3], None), ("GenericError", 1)]
     # These shouldn't raise
-    for i in [None, 1, 2, 3, "Hello World", (1, ), (1, 2, 3),
-              ("NoErrorClassName", []), ("NoErrorClassName", None),
-              ("GenericError", [1, 2, 3], None), ("GenericError", 1)]:
+    for i in testvals:
       errors.MaybeRaise(i)
 
     self.assertRaises(errors.GenericError, errors.MaybeRaise,
                       ("GenericError", ["Hello"]))
+    # Check error encoding
+    for i in testvals:
+      src = errors.GenericError(i)
+      try:
+        errors.MaybeRaise(errors.EncodeException(src))
+      except errors.GenericError, dst:
+        self.assertEqual(src.args, dst.args)
+        self.assertEqual(src.__class__, dst.__class__)
+      else:
+        self.fail("Exception %s not raised" % repr(src))
 
   def testGetEncodedError(self):
     self.assertEqualValues(errors.GetEncodedError(["GenericError",
-- 
GitLab