On Tue, Sep 24, 2013 at 09:32:38AM +0200, Thomas Thrainer wrote:
> On Tue, Sep 24, 2013 at 9:29 AM, Jose A. Lopes <[email protected]> wrote:
> 
> > On Tue, Sep 24, 2013 at 09:25:28AM +0200, Thomas Thrainer wrote:
> > > The physical_id field can't be recreated during downgrades, so don't
> > > expect it to be during the test.
> > >
> > > Signed-off-by: Thomas Thrainer <[email protected]>
> > > ---
> > >  test/py/cfgupgrade_unittest.py | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/test/py/cfgupgrade_unittest.py
> > b/test/py/cfgupgrade_unittest.py
> > > index 09b16c4..c73d6cf 100755
> > > --- a/test/py/cfgupgrade_unittest.py
> > > +++ b/test/py/cfgupgrade_unittest.py
> > > @@ -394,9 +394,17 @@ class TestCfgupgrade(unittest.TestCase):
> > >      newconf = self._LoadConfig()
> > >
> > >      # downgrade from 2.10 to 2.9 does not add physical_id to disks,
> > which is ok
> > > +    # TODO (2.11): Remove this code, it's not required to downgrade
> > from 2.11
> > > +    #              to 2.10
> > > +    def RemovePhysicalId(disk):
> > > +      if "children" in disk:
> > > +        for d in disk["children"]:
> > > +          RemovePhysicalId(d)
> > > +      del disk["physical_id"]
> >
> > Don't you need to test if "physical_id" is present in the dict ?
> >
> >
> Well, that's a test with a well defined input, which happens to have
> "physical_id" in all disk objects. But you are right, for correctness we
> should add it.
> 
> Interdiff:
> 
> diff --git a/test/py/cfgupgrade_unittest.py b/test/py/cfgupgrade_unittest.py
> index c73d6cf..2e0aadf 100755
> --- a/test/py/cfgupgrade_unittest.py
> +++ b/test/py/cfgupgrade_unittest.py
> @@ -400,7 +400,8 @@ class TestCfgupgrade(unittest.TestCase):
>        if "children" in disk:
>          for d in disk["children"]:
>            RemovePhysicalId(d)
> -      del disk["physical_id"]
> +      if "physical_id" in disk:
> +        del disk["physical_id"]
> 
>      for inst in oldconf["instances"].values():
>        for disk in inst["disks"]:
> 
> 
> > > +
> > >      for inst in oldconf["instances"].values():
> > >        for disk in inst["disks"]:
> > > -        del disk["physical_id"]
> > > +        RemovePhysicalId(disk)
> > >
> > >      self.assertEqual(oldconf, newconf)
> > >
> > > --
> > > 1.8.4
> > >
> >
> > Thanks,
> > Jose
> >
> > --
> > Jose Antonio Lopes
> > Ganeti Engineering
> > 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
> > Steuernummer: 48/725/00206
> > Umsatzsteueridentifikationsnummer: DE813741370
> >

LGTM.

Thanks,
Jose

> 
> 
> 
> -- 
> Thomas Thrainer | Software Engineer | [email protected] |
> 
> 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

-- 
Jose Antonio Lopes
Ganeti Engineering
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
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to