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
>



-- 
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

Reply via email to