On Tue, Mar 4, 2014 at 9:54 AM, Michele Tartara <[email protected]> wrote:
> On Tue, Mar 4, 2014 at 9:12 AM, Thomas Thrainer <[email protected]> wrote:
>> I think that's not needed. The return value of the method (lu_result) is
>> correctly updated if there are results from nodes. The case which happened
>> in issue 742 was that an offline node was incorrectly contacted. However,
>> the result of this attempt didn't make it in the return value of
>> HooksCallBack(...), because in line 3252 the result of offline nodes is
>> discarded. So while fixing issue 742, this instances of the bug should go
>> away.
>>
>> However, the result of the node is also discarded if it gave an error
>> message (also checked in line 3252). This sounds fishy to me, as the return
>> value should be False as well if the RPC call resulted in an error... So
>> maybe something like this would make more sense:
>>
>> if test: # test is defined in line 3249
>> lu_result = False
>> continue
>> if res.offline:
>> continue
>>
>> What do you think?
>>
>> Cheers,
>> Thomas
>
> I think that both approaches can work. In either cases, when an error
> is detected, self._ErrorIf is invoked, and that function sets
> self.bad. So, using "not self.bad" should be enough.
> My favorite approach would be to not use lu_result at all in the
> function, and just use it in the return statement to propagate the
> previous Exec result, but your approach is ok as well, so I'll send
> and interdiff to implement it.
Here it is:
diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
index fa45eb6..f6e44e8 100644
--- a/lib/cmdlib/cluster.py
+++ b/lib/cmdlib/cluster.py
@@ -3249,9 +3249,11 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors):
test = msg and not res.offline
self._ErrorIf(test, constants.CV_ENODEHOOKS, node_name,
"Communication failure in hooks execution: %s", msg)
- if res.offline or msg:
- # No need to investigate payload if node is offline or gave
- # an error.
+ if test:
+ lu_result = False
+ continue
+ if res.offline:
+ # No need to investigate payload if node is offline
continue
for script, hkr, output in res.payload:
test = hkr == constants.HKR_FAIL
@@ -3262,7 +3264,7 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors):
feedback_fn("%s" % output)
lu_result = False
- return lu_result and not self.bad
+ return lu_result
class LUClusterVerifyDisks(NoHooksLU):
Cheers,
Michele
--
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
