LGTM, thanks!
On Tue, Dec 17, 2013 at 10:28 AM, Santi Raffa <[email protected]> wrote: > This commit teaches Gluster what a volume is and how to use it. > > Signed-off-by: Santi Raffa <[email protected]> > --- > lib/storage/gluster.py | 241 > ++++++++++++++++++++++++ > test/py/ganeti.storage.filestorage_unittest.py | 2 +- > test/py/ganeti.storage.gluster_unittest.py | 105 +++++++++++ > 3 files changed, 347 insertions(+), 1 deletion(-) > > diff --git a/lib/storage/gluster.py b/lib/storage/gluster.py > index e90fdac..8f4a2a6 100644 > --- a/lib/storage/gluster.py > +++ b/lib/storage/gluster.py > @@ -25,12 +25,253 @@ behaves essentially like a regular file system. > Unlike RBD, there are no > special provisions for block device abstractions (yet). > > """ > +import logging > +import os > +import socket > + > +from ganeti import utils > from ganeti import errors > +from ganeti import netutils > +from ganeti import constants > > +from ganeti.utils import io > from ganeti.storage import base > from ganeti.storage.filestorage import FileDeviceHelper > > > +class GlusterVolume(object): > + """This class represents a Gluster volume. > + > + Volumes are uniquely identified by: > + > + - their IP address > + - their port > + - the volume name itself > + > + Two GlusterVolume objects x, y with same IP address, port and volume > name > + are considered equal. > + > + """ > + > + def __init__(self, server_addr, port, volume, _run_cmd=utils.RunCmd): > + """Creates a Gluster volume object. > + > + @type server_addr: str > + @param server_addr: The address to connect to > + > + @type port: int > + @param port: The port to connect to (Gluster standard is 24007) > + > + @type volume: str > + @param volume: The gluster volume to use for storage. > + > + """ > + self.server_addr = server_addr > + server_ip = netutils.Hostname.GetIP(self.server_addr) > + self._server_ip = server_ip > + port = netutils.ValidatePortNumber(port) > + self._port = port > + self._volume = volume > + self.mount_point = io.PathJoin(constants.GLUSTER_MOUNTPOINT, > + self._volume) > + > + self._run_cmd = _run_cmd > + > + @property > + def server_ip(self): > + return self._server_ip > + > + @property > + def port(self): > + return self._port > + > + @property > + def volume(self): > + return self._volume > + > + def __eq__(self, other): > + return (self.server_ip, self.port, self.volume) == \ > + (other.server_ip, other.port, other.volume) > + > + def __repr__(self): > + return """GlusterVolume("{ip}", {port}, "{volume}")""" \ > + .format(ip=self.server_ip, port=self.port, > volume=self.volume) > + > + def __hash__(self): > + return (self.server_ip, self.port, self.volume).__hash__() > + > + def _IsMounted(self): > + """Checks if we are mounted or not. > + > + @rtype: bool > + @return: True if this volume is mounted. > + > + """ > + if not os.path.exists(self.mount_point): > + return False > + > + return os.path.ismount(self.mount_point) > + > + def _GuessMountFailReasons(self): > + """Try and give reasons why the mount might've failed. > + > + @rtype: str > + @return: A semicolon-separated list of problems found with the > current setup > + suitable for display to the user. > + > + """ > + > + reasons = [] > + > + # Does the mount point exist? > + if not os.path.exists(self.mount_point): > + reasons.append("%r: does not exist" % self.mount_point) > + > + # Okay, it exists, but is it a directory? > + elif not os.path.isdir(self.mount_point): > + reasons.append("%r: not a directory" % self.mount_point) > + > + # If, for some unfortunate reason, this folder exists before mounting: > + # > + # /var/run/ganeti/gluster/gv0/10.0.0.1:30000:gv0/ > + # '--------- cwd ------------' > + # > + # and you _are_ trying to mount the gluster volume gv0 on > 10.0.0.1:30000, > + # then the mount.glusterfs command parser gets confused and this > command: > + # > + # mount -t glusterfs 10.0.0.1:30000:gv0 /var/run/ganeti/gluster/gv0 > + # '-- remote end --' '------ mountpoint -------' > + # > + # gets parsed instead like this: > + # > + # mount -t glusterfs 10.0.0.1:30000:gv0 /var/run/ganeti/gluster/gv0 > + # '-- mountpoint --' '----- syntax error ------' > + # > + # and if there _is_ a gluster server running locally at the default > remote > + # end, localhost:24007, then this is not a network error and > therefore... no > + # usage message gets printed out. All you get is a Byson parser error > in the > + # gluster log files about an unexpected token in line 1, "". (That's > stdin.) > + # > + # Not that we rely on that output in any way whatsoever... > + > + parser_confusing = io.PathJoin(self.mount_point, > + self._GetFUSEMountString()) > + if os.path.exists(parser_confusing): > + reasons.append("%r: please delete, rename or move." % > parser_confusing) > + > + # Let's try something else: can we connect to the server? > + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) > + try: > + sock.connect((self.server_ip, self.port)) > + sock.close() > + except socket.error as err: > + reasons.append("%s:%d: %s" % (self.server_ip, self.port, > err.strerror)) > + > + reasons.append("try running 'gluster volume info %s' on %s to ensure" > + " it exists, it is started and it is using the tcp" > + " transport" % (self.volume, self.server_ip)) > + > + return "; ".join(reasons) > + > + def _GetFUSEMountString(self): > + """Return the string FUSE needs to mount this volume. > + > + @rtype: str > + """ > + > + return "{ip}:{port}:{volume}" \ > + .format(ip=self.server_ip, port=self.port, > volume=self.volume) > + > + def GetKVMMountString(self, path): > + """Return the string KVM needs to use this volume. > + > + @rtype: str > + """ > + > + ip = self.server_ip > + if netutils.IPAddress.GetAddressFamily(ip) == socket.AF_INET6: > + ip = "[%s]" % ip > + return "gluster://{ip}:{port}/{volume}/{path}" \ > + .format(ip=ip, port=self.port, volume=self.volume, > path=path) > + > + def Mount(self): > + """Try and mount the volume. No-op if the volume is already mounted. > + > + @raises BlockDeviceError: if the mount was unsuccessful > + > + @rtype: context manager > + @return: A simple context manager that lets you use this volume for > + short lived operations like so:: > + > + with volume.mount(): > + # Do operations on volume > + # Volume is now unmounted > + > + """ > + > + class _GlusterVolumeContextManager(object): > + > + def __init__(self, volume): > + self.volume = volume > + > + def __enter__(self): > + # We're already mounted. > + return self > + > + def __exit__(self, *exception_information): > + self.volume.Unmount() > + return False # do not swallow exceptions. > + > + if self._IsMounted(): > + return _GlusterVolumeContextManager(self) > + > + command = ["mount", > + "-t", "glusterfs", > + self._GetFUSEMountString(), > + self.mount_point > + ] > + > + io.Makedirs(self.mount_point) > + self._run_cmd(" ".join(command), > + # Why set cwd? Because it's an area we control. If, > + # for some unfortunate reason, this folder exists: > + # "/%s/" % _GetFUSEMountString() > + # ...then the gluster parser gets confused and treats > + # _GetFUSEMountString() as your mount point and > + # self.mount_point becomes a syntax error. > + cwd=self.mount_point) > + > + # mount.glusterfs exits with code 0 even after failure. > + # https://bugzilla.redhat.com/show_bug.cgi?id=1031973 > + if not self._IsMounted(): > + reasons = self._GuessMountFailReasons() > + if not reasons: > + reasons = "%r failed." % (" ".join(command)) > + base.ThrowError("%r: mount failure: %s", > + self.mount_point, > + reasons) > + > + return _GlusterVolumeContextManager(self) > + > + def Unmount(self): > + """Try and unmount the volume. > + > + Failures are logged but otherwise ignored. > + > + @raises BlockDeviceError: if the volume was not mounted to begin with. > + """ > + > + if not self._IsMounted(): > + base.ThrowError("%r: should be mounted but isn't.", > self.mount_point) > + > + result = self._run_cmd(["umount", > + self.mount_point]) > + > + if result.failed: > + logging.warning("Failed to unmount %r from %r: %s", > + self, self.mount_point, result.fail_reason) > + > + > class GlusterStorage(base.BlockDev): > """File device using the Gluster backend. > > diff --git a/test/py/ganeti.storage.filestorage_unittest.py b/test/py/ > ganeti.storage.filestorage_unittest.py > index 09b9885..d0f63a4 100755 > --- a/test/py/ganeti.storage.filestorage_unittest.py > +++ b/test/py/ganeti.storage.filestorage_unittest.py > @@ -227,7 +227,7 @@ class TestFileDeviceHelper(testutils.GanetiTestCase): > def _Make(path, create_with_size=None, create_folders=False): > skip_checks = lambda path: None > if create_with_size: > - return filestorage.FileDeviceHelper.Create( > + return filestorage.FileDeviceHelper.CreateFile( > path, create_with_size, create_folders=create_folders, > _file_path_acceptance_fn=skip_checks > ) > diff --git a/test/py/ganeti.storage.gluster_unittest.py b/test/py/ > ganeti.storage.gluster_unittest.py > index 6814cb9..2037eac 100644 > --- a/test/py/ganeti.storage.gluster_unittest.py > +++ b/test/py/ganeti.storage.gluster_unittest.py > @@ -20,7 +20,112 @@ > > """Script for unittesting the ganeti.storage.gluster module""" > > +import os > +import shutil > +import tempfile > +import unittest > +import mock > + > +from ganeti import errors > +from ganeti.storage import filestorage > +from ganeti.storage import gluster > +from ganeti import utils > + > import testutils > > +class TestGlusterVolume(testutils.GanetiTestCase): > + > + testAddrIpv = {4: "203.0.113.42", > + 6: "2001:DB8::74:65:28:6:69", > + } > + > + @staticmethod > + def _MakeVolume(addr=None, port=9001, > + run_cmd=NotImplemented, > + vol_name="pinky"): > + > + addr = addr if addr is not None else TestGlusterVolume.testAddrIpv[4] > + > + return gluster.GlusterVolume(addr, port, vol_name, _run_cmd=run_cmd, > + _mount_point="/invalid") > + > + def setUp(self): > + testutils.GanetiTestCase.setUp(self) > + > + # Create some volumes. > + self.vol_a = TestGlusterVolume._MakeVolume() > + self.vol_a_clone = TestGlusterVolume._MakeVolume() > + self.vol_b = TestGlusterVolume._MakeVolume(vol_name="pinker") > + > + def testEquality(self): > + self.assertEqual(self.vol_a, self.vol_a_clone) > + > + def testInequality(self): > + self.assertNotEqual(self.vol_a, self.vol_b) > + > + def testHostnameResolution(self): > + vol_1 = TestGlusterVolume._MakeVolume(addr="localhost") > + self.assertEqual(vol_1.server_ip, "127.0.0.1") > + self.assertRaises(errors.ResolverError, lambda: \ > + TestGlusterVolume._MakeVolume(addr="E_NOENT")) > + > + def testKVMMountStrings(self): > + # The only source of documentation I can find is: > + # https://github.com/qemu/qemu/commit/8d6d89c > + # This test gets as close as possible to the examples given there, > + # within the limits of our implementation (no transport specification, > + # no default port version). > + > + 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" > + ) > + > + def testFUSEMountStrings(self): > + 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(), > + "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" > + ) > + > + > if __name__ == "__main__": > testutils.GanetiTestProgram() > -- > 1.7.10.4 > > -- 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
