LGTM, thanks.

On Fri, Dec 6, 2013 at 12:53 PM, Santi Raffa <[email protected]> wrote:

> On Thu, Dec 5, 2013 at 1:30 PM, Thomas Thrainer <[email protected]>
> wrote:
> > The point is that most probably multiple hypervisors will expect the same
> > URI, as they are pretty standard. With one method it's easy to just
> return
> > the same URI for all of them.
>
> Let's worry about that once Gluster gets support from more than just KVM :)
>
> > What's about the other IP's further down?
>
> Those were copy-pasted from the "KVM documentation" of this feature
> [0] as much as possible in order to test for correctness. But I guess
> we can feel daring and fix those, too. I tried to simplify things a
> little to boot.
>
> [0]: https://github.com/qemu/qemu/commit/8d6d8
>
> diff --git a/test/py/ganeti.storage.gluster_unittest.py
> b/test/py/ganeti.storage.gluster_unittest.py
> index 0c47e13..2037eac 100644
> --- a/test/py/ganeti.storage.gluster_unittest.py
> +++ b/test/py/ganeti.storage.gluster_unittest.py
> @@ -35,21 +35,19 @@ import testutils
>
>  class TestGlusterVolume(testutils.GanetiTestCase):
>
> +  testAddrIpv = {4: "203.0.113.42",
> +                 6: "2001:DB8::74:65:28:6:69",
> +                }
> +
>    @staticmethod
> -  def _MakeVolume(ipv=4, addr=None, port=9001,
> +  def _MakeVolume(addr=None, port=9001,
>                    run_cmd=NotImplemented,
>                    vol_name="pinky"):
>
> -    address = {4: "203.0.113.42",
> -               6: "2001:DB8::74:65:28:6:69",
> -              }
> +    addr = addr if addr is not None else TestGlusterVolume.testAddrIpv[4]
>
> -    return gluster.GlusterVolume(address[ipv] if not addr else addr,
> -                                 port,
> -                                 str(vol_name),
> -                                 _run_cmd=run_cmd,
> -                                 _mount_point="/invalid"
> -                                )
> +    return gluster.GlusterVolume(addr, port, vol_name, _run_cmd=run_cmd,
> +                                 _mount_point="/invalid")
>
>    def setUp(self):
>      testutils.GanetiTestCase.setUp(self)
> @@ -78,43 +76,55 @@ class TestGlusterVolume(testutils.GanetiTestCase):
>      # within the limits of our implementation (no transport specification,
>      #                                          no default port version).
>
> -    vol_1 = TestGlusterVolume._MakeVolume(addr="1.2.3.4",
> -                                         port=24007,
> -                                         vol_name="testvol")
> -    self.assertEqual(vol_1.GetKVMMountString("dir/a.img"),
> -                     "gluster://1.2.3.4:24007/testvol/dir/a.img")
> -
> -    vol_2 = TestGlusterVolume._MakeVolume(addr="1:2:3:4:5::8",
> -                                         port=24007,
> -                                         vol_name="testvol")
> -    self.assertEqual(vol_2.GetKVMMountString("dir/a.img"),
> -                     "gluster://[1:2:3:4:5::8]:24007/testvol/dir/a.img")
> +    vol_1 =
> TestGlusterVolume._MakeVolume(addr=TestGlusterVolume.testAddrIpv[4],
> +                                          port=24007,
> +                                          vol_name="testvol")
> +    self.assertEqual(
> +      vol_1.GetKVMMountString("dir/a.img"),
> +      "gluster://203.0.113.42:24007/testvol/dir/a.img"
> +    )
> +
> +    vol_2 =
> TestGlusterVolume._MakeVolume(addr=TestGlusterVolume.testAddrIpv[6],
> +                                          port=24007,
> +                                          vol_name="testvol")
> +    self.assertEqual(
> +      vol_2.GetKVMMountString("dir/a.img"),
> +      "gluster://[2001:db8:0:74:65:28:6:69]:24007/testvol/dir/a.img"
> +    )
>
>      vol_3 = TestGlusterVolume._MakeVolume(addr="localhost",
> -                                         port=9001,
> -                                         vol_name="testvol")
> -    self.assertEqual(vol_3.GetKVMMountString("dir/a.img"),
> -                     "gluster://127.0.0.1:9001/testvol/dir/a.img")
> +                                          port=9001,
> +                                          vol_name="testvol")
> +    self.assertEqual(
> +      vol_3.GetKVMMountString("dir/a.img"),
> +      "gluster://127.0.0.1:9001/testvol/dir/a.img"
> +    )
>
>    def testFUSEMountStrings(self):
> -    vol_1 = TestGlusterVolume._MakeVolume(addr="1.2.3.4",
> -                                         port=24007,
> -                                         vol_name="testvol")
> -    self.assertEqual(vol_1._GetFUSEMountString(),
> -                     "1.2.3.4:24007:testvol")
> -
> -    vol_2 = TestGlusterVolume._MakeVolume(addr="1:2:3:4:5::8",
> -                                         port=24007,
> -                                         vol_name="testvol")
> +    vol_1 =
> TestGlusterVolume._MakeVolume(addr=TestGlusterVolume.testAddrIpv[4],
> +                                          port=24007,
> +                                          vol_name="testvol")
> +    self.assertEqual(
> +      vol_1._GetFUSEMountString(),
> +      "203.0.113.42:24007:testvol"
> +    )
> +
> +    vol_2 =
> TestGlusterVolume._MakeVolume(addr=TestGlusterVolume.testAddrIpv[6],
> +                                          port=24007,
> +                                          vol_name="testvol")
>      # This _ought_ to work.
> https://bugzilla.redhat.com/show_bug.cgi?id=764188
> -    self.assertEqual(vol_2._GetFUSEMountString(),
> -                     "1:2:3:4:5::8:24007:testvol")
> +    self.assertEqual(
> +      vol_2._GetFUSEMountString(),
> +      "2001:db8:0:74:65:28:6:69:24007:testvol"
> +    )
>
>      vol_3 = TestGlusterVolume._MakeVolume(addr="localhost",
> -                                         port=9001,
> -                                         vol_name="testvol")
> -    self.assertEqual(vol_3._GetFUSEMountString(),
> -                     "127.0.0.1:9001:testvol")
> +                                          port=9001,
> +                                          vol_name="testvol")
> +    self.assertEqual(
> +      vol_3._GetFUSEMountString(),
> +      "127.0.0.1:9001:testvol"
> +    )
>
>
>  if __name__ == "__main__":
>
>
> --
> Raffa Santi
> 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
>



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