I think we should actually throw an exception here, as it should not happen that one asks for an instance which is not in the config. I sent another patch for this. Disregard this one.
On Mon, 23 Mar 2015 at 19:02 Petr Pudlak <[email protected]> wrote: > On Mon, Mar 23, 2015 at 06:28:38PM +0100, 'Helga Velroyen' via > ganeti-devel wrote: > >This patches fixes a problem which occurred when one tries > >to lookup an instance in an empty nodegroup. > > > >Signed-off-by: Helga Velroyen <[email protected]> > >--- > > lib/cmdlib/cluster.py | 2 +- > > lib/config.py | 3 ++- > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > >diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py > >index 30e35c4..6cb1401 100644 > >--- a/lib/cmdlib/cluster.py > >+++ b/lib/cmdlib/cluster.py > >@@ -2892,7 +2892,7 @@ class LUClusterVerifyGroup(LogicalUnit, > _VerifyErrors): > > if test: > > nimg.hyp_fail = True > > else: > >- nimg.instances = [inst.uuid for (_, inst) in > >+ nimg.instances = [uuid for (uuid, _) in > > self.cfg.GetMultiInstanceInfoByName(idata)] > > > > def _UpdateNodeInfo(self, ninfo, nresult, nimg, vg_name): > >diff --git a/lib/config.py b/lib/config.py > >index b6f1373..f273e1e 100644 > >--- a/lib/config.py > >+++ b/lib/config.py > >@@ -1782,7 +1782,8 @@ class ConfigWriter(object): > > result = [] > > for name in inst_names: > > instance = self._UnlockedGetInstanceInfoByName(name) > >- result.append((instance.uuid, instance)) > >+ if instance: > >+ result.append((instance.uuid, instance)) > > This is definitely a good change that it prevents the function from > crashing, when a non-existent instance name is given. But then it can > happen > that the resulting list is shorter than the list with names and the order > won't match. So I'd suggest to further improving that, probably by > appending > None if an instance doesn't exist, or alternatively updating the > documentation. > > > return result > > > > @locking.ssynchronized(_config_lock, shared=1) > >-- > >2.2.0.rc0.207.ga3a616c > > > > Rest LGTM, thanks >
