General suggestion: this seems like the ideal bit of code to put in a separate function and unit test, and writing it that way would have made it very clear which corner cases you were trying to account for.
As-is, this LGTM with the nitpick. Should you have the time, a test or two would be appreciated though. On Tue, Nov 10, 2015 at 3:56 PM, 'Oleg Ponomarev' via ganeti-devel < [email protected]> wrote: > NoHooksLU opcodes don't provide node lists on which the hooks should be > executed. Some opcodes porivde node lists which don't contain the master > Nitpick: s/porivde/provide/ > node. Thus, always add the master node to the node list for global hooks. > > Signed-off-by: Oleg Ponomarev <[email protected]> > --- > lib/hooksmaster.py | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/lib/hooksmaster.py b/lib/hooksmaster.py > index b092c2c..b56c54e 100644 > --- a/lib/hooksmaster.py > +++ b/lib/hooksmaster.py > @@ -214,25 +214,24 @@ class HooksMaster(object): > > """ > if phase == constants.HOOKS_PHASE_PRE: > - if node_names is None: > - node_names = self.pre_nodes > env = self.pre_env > elif phase == constants.HOOKS_PHASE_POST: > - if node_names is None: > - node_names = self.post_nodes > - if node_names is not None and self.prepare_post_nodes_fn is not > None: > - node_names = > frozenset(self.prepare_post_nodes_fn(list(node_names))) > env = self._BuildEnv(phase) > else: > raise AssertionError("Unknown phase '%s'" % phase) > > - if glob: > - if node_names is None or not node_names: > - node_names = frozenset([self.master_name]) > - elif self.master_name not in list(node_names): > - new_nodes = list(node_names) > - new_nodes.append(self.master_name) > - node_names = frozenset(new_nodes) > + if node_names is None: > + node_names = set([self.master_name]) if glob else set() > + if phase == constants.HOOKS_PHASE_PRE and self.pre_nodes: > + node_names.update(self.pre_nodes) > + elif phase == constants.HOOKS_PHASE_POST: > + if self.post_nodes is not None: > + if self.prepare_post_nodes_fn is not None: > + node_names.update( > + self.prepare_post_nodes_fn(list(self.post_nodes))) > + else: > + node_names.update(self.post_nodes) > + node_names = list(node_names) > > if not node_names: > # empty node list, we should not attempt to run this as either > -- > 2.6.0.rc2.230.g3dd15c0 > > Hrvoje Ribicic Ganeti Engineering Google Germany GmbH Dienerstr. 12, 80331, München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank. This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.
