On Wed, Dec 4, 2013 at 2:50 PM, Santi Raffa <[email protected]> wrote:
> Gluster still does not mount anything autonomously, but this commit > changes where Gluster expects its mountpoint to be. > > Signed-off-by: Santi Raffa <[email protected]> > --- > lib/cmdlib/instance.py | 34 > ++++++++++++++++++------------ > lib/storage/gluster.py | 9 +++++--- > lib/utils/storage.py | 2 +- > src/Ganeti/Storage/Utils.hs | 5 ++--- > test/py/cmdlib/instance_unittest.py | 2 +- > test/py/ganeti.storage.gluster_unittest.py | 3 ++- > 6 files changed, 33 insertions(+), 22 deletions(-) > > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > index 4f512a4..99bc9cf 100644 > --- a/lib/cmdlib/instance.py > +++ b/lib/cmdlib/instance.py > @@ -849,25 +849,33 @@ class LUInstanceCreate(LogicalUnit): > # build the full file storage dir path > joinargs = [] > > - if self.op.disk_template in (constants.DT_SHARED_FILE, > - constants.DT_GLUSTER): > - get_fsd_fn = self.cfg.GetSharedFileStorageDir > - else: > - get_fsd_fn = self.cfg.GetFileStorageDir > + cfg_storage = { > + constants.DT_FILE: self.cfg.GetFileStorageDir, > + constants.DT_SHARED_FILE: self.cfg.GetSharedFileStorageDir, > + constants.DT_GLUSTER: self.cfg.GetGlusterStorageDir, > + }.get(self.op.disk_template, lambda: None)() > I'm not quite sure why we should write this simple logic in this way. Is it really easier to read than an if-elif-elif-else cascade? I doubt so. And the few extra characters/lines won't hurt, IMHO. > + > + if not cfg_storage: > + raise errors.OpPrereqError( > + "Cluster file storage dir for {tpl} storage type not > defined".format( > + tpl=repr(self.op.disk_template) > + ), > + errors.ECODE_STATE > + ) > > - cfg_storagedir = get_fsd_fn() > - if not cfg_storagedir: > - raise errors.OpPrereqError("Cluster file storage dir not defined", > - errors.ECODE_STATE) > - joinargs.append(cfg_storagedir) > + joinargs.append(cfg_storage) > > if self.op.file_storage_dir is not None: > joinargs.append(self.op.file_storage_dir) > > - joinargs.append(self.op.instance_name) > + if self.op.disk_template != constants.DT_GLUSTER: > + joinargs.append(self.op.instance_name) > > - # pylint: disable=W0142 > - self.instance_file_storage_dir = utils.PathJoin(*joinargs) > + if len(joinargs) > 1: > + # pylint: disable=W0142 > + self.instance_file_storage_dir = utils.PathJoin(*joinargs) > + else: > + self.instance_file_storage_dir = joinargs[0] > > def CheckPrereq(self): # pylint: disable=R0914 > """Check prerequisites. > diff --git a/lib/storage/gluster.py b/lib/storage/gluster.py > index 45e3e60..e5829c8 100644 > --- a/lib/storage/gluster.py > +++ b/lib/storage/gluster.py > @@ -53,7 +53,8 @@ class GlusterVolume(object): > > """ > > - def __init__(self, server_addr, port, volume, _run_cmd=utils.RunCmd): > + def __init__(self, server_addr, port, volume, _run_cmd=utils.RunCmd, > + _mount_point=None): > """Creates a Gluster volume object. > > @type server_addr: str > @@ -72,8 +73,10 @@ class GlusterVolume(object): > port = netutils.ValidatePortNumber(port) > self._port = port > self._volume = volume > - self.mount_point = io.PathJoin(constants.GLUSTER_MOUNTPOINT, > - self._volume) > + if _mount_point: # tests > + self.mount_point = _mount_point > + else: > + self.mount_point = ssconf.SimpleStore().GetGlusterStorageDir() > > self._run_cmd = _run_cmd > > diff --git a/lib/utils/storage.py b/lib/utils/storage.py > index bf56953..ab8443a 100644 > --- a/lib/utils/storage.py > +++ b/lib/utils/storage.py > @@ -92,7 +92,7 @@ def _GetDefaultStorageUnitForDiskTemplate(cfg, > disk_template): > elif disk_template == constants.DT_SHARED_FILE: > return (storage_type, cluster.shared_file_storage_dir) > elif disk_template == constants.DT_GLUSTER: > - return (storage_type, constants.GLUSTER_MOUNTPOINT) > + return (storage_type, cluster.gluster_storage_dir) > else: > return (storage_type, None) > > diff --git a/src/Ganeti/Storage/Utils.hs b/src/Ganeti/Storage/Utils.hs > index 5848e8d..50bce37 100644 > --- a/src/Ganeti/Storage/Utils.hs > +++ b/src/Ganeti/Storage/Utils.hs > @@ -31,7 +31,6 @@ module Ganeti.Storage.Utils > import Ganeti.Config > import Ganeti.Objects > import Ganeti.Types > -import Ganeti.Constants > import qualified Ganeti.Types as T > > import Control.Monad > @@ -47,8 +46,8 @@ getDefaultStorageKey cfg T.DTFile = > Just (clusterFileStorageDir $ configCluster cfg) > getDefaultStorageKey cfg T.DTSharedFile = > Just (clusterSharedFileStorageDir $ configCluster cfg) > -getDefaultStorageKey _ T.DTGluster = > - Just glusterMountpoint > +getDefaultStorageKey cfg T.DTGluster = > + Just (clusterGlusterStorageDir $ configCluster cfg) > getDefaultStorageKey _ _ = Nothing > > -- | Get the cluster's default spindle storage unit > diff --git a/test/py/cmdlib/instance_unittest.py > b/test/py/cmdlib/instance_unittest.py > index 9d4153b..448d7c9 100644 > --- a/test/py/cmdlib/instance_unittest.py > +++ b/test/py/cmdlib/instance_unittest.py > @@ -446,7 +446,7 @@ class TestLUInstanceCreate(CmdlibTestCase): > self.cluster.file_storage_dir = None > op = self.CopyOpCode(self.file_op) > self.ExecOpCodeExpectOpPrereqError( > - op, "Cluster file storage dir not defined") > + op, "Cluster file storage dir for 'file' storage type not defined") > > def testFileInstanceAdditionalPath(self): > op = self.CopyOpCode(self.file_op, > diff --git a/test/py/ganeti.storage.gluster_unittest.py b/test/py/ > ganeti.storage.gluster_unittest.py > index bd31637..55a189f 100644 > --- a/test/py/ganeti.storage.gluster_unittest.py > +++ b/test/py/ganeti.storage.gluster_unittest.py > @@ -47,7 +47,8 @@ class TestGlusterVolume(testutils.GanetiTestCase): > return gluster.GlusterVolume(address[ipv] if not addr else addr, > port, > str(vol_name), > - _run_cmd=run_cmd > + _run_cmd=run_cmd, > + _mount_point="/invalid" > ) > > def setUp(self): > -- > 1.8.4.1 > > Rest LGTM, thanks. -- 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
