On Thu, Aug 22, 2013 at 10:16 AM, Helga Velroyen <[email protected]> wrote:
> Whether or not a particular type of storage is enabled > or not is determined by the list of enabled disk templates > in the cluster's configuration. This makes the option > '--no-drbd-storage' obsolete, because it is subsumed by > not including 'drbd' in the list of enabled disk templates. > > Signed-off-by: Helga Velroyen <[email protected]> > --- > lib/bootstrap.py | 12 ++++++--- > lib/client/gnt_cluster.py | 22 +++++++++++----- > test/py/ganeti.bootstrap_unittest.py | 22 +++++++++++++--- > test/py/ganeti.client.gnt_cluster_unittest.py | 36 > ++++++++++++++++++++------- > 4 files changed, 69 insertions(+), 23 deletions(-) > > diff --git a/lib/bootstrap.py b/lib/bootstrap.py > index fe5a1f4..e0aac86 100644 > --- a/lib/bootstrap.py > +++ b/lib/bootstrap.py > @@ -472,7 +472,7 @@ def _RestrictIpolicyToEnabledDiskTemplates(ipolicy, > enabled_disk_templates): > ipolicy[constants.IPOLICY_DTS] = restricted_disk_templates > > > -def _InitCheckDrbdHelper(drbd_helper, > +def _InitCheckDrbdHelper(drbd_helper, drbd_enabled, > get_helper_fn=drbd.DRBD8.GetUsermodeHelper): > """Checks the DRBD usermode helper. > > @@ -481,13 +481,16 @@ def _InitCheckDrbdHelper(drbd_helper, > use > > """ > + if not drbd_enabled: > + return > + > if drbd_helper is not None: > try: > curr_helper = get_helper_fn() > except errors.BlockDeviceError, err: > raise errors.OpPrereqError("Error while checking drbd helper" > - " (specify --no-drbd-storage if you are > not" > - " using drbd): %s" % str(err), > + " (disable drbd with > --enabled-disk-templates" > + " if you are not using drbd): %s" % > str(err), > errors.ECODE_ENVIRON) > if drbd_helper != curr_helper: > raise errors.OpPrereqError("Error: requiring %s as drbd helper but > %s" > @@ -593,7 +596,8 @@ def InitCluster(cluster_name, mac_prefix, # pylint: > disable=R0913, R0914 > if vgstatus: > raise errors.OpPrereqError("Error: %s" % vgstatus, > errors.ECODE_INVAL) > > - _InitCheckDrbdHelper(drbd_helper) > + drbd_enabled = constants.DT_DRBD8 in enabled_disk_templates > + _InitCheckDrbdHelper(drbd_helper, drbd_enabled) > > logging.debug("Stopping daemons (if any are running)") > result = utils.RunCmd([pathutils.DAEMON_UTIL, "stop-all"]) > diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py > index 90d3220..daf2595 100644 > --- a/lib/client/gnt_cluster.py > +++ b/lib/client/gnt_cluster.py > @@ -110,16 +110,26 @@ def _InitVgName(opts, enabled_disk_templates): > return vg_name > > > -def _InitDrbdHelper(opts): > +def _InitDrbdHelper(opts, enabled_disk_templates): > """Initialize the DRBD usermode helper. > > """ > - if not opts.drbd_storage and opts.drbd_helper: > + drbd_enabled = constants.DT_DRBD8 in enabled_disk_templates > + > + # This raises an exception due to historical reasons, one might consider > + # letting the user set a helper without having DRBD enabled. > + if not drbd_enabled and opts.drbd_helper: > raise errors.OpPrereqError( > - "Options --no-drbd-storage and --drbd-usermode-helper conflict.") > + "Enabling the DRBD disk template and setting a drbd usermode > helper" > + " with --drbd-usermode-helper conflict.") > > - if opts.drbd_storage and not opts.drbd_helper: > - return constants.DEFAULT_DRBD_HELPER > + if drbd_enabled: > + if opts.drbd_helper is None: > + return constants.DEFAULT_DRBD_HELPER > + if opts.drbd_helper == '': > + raise errors.OpPrereqError( > + "Unsetting the drbd usermode helper while enabling DRBD is not" > + " allowed.") > > return opts.drbd_helper > > @@ -148,7 +158,7 @@ def InitCluster(opts, args): > return 1 > > try: > - drbd_helper = _InitDrbdHelper(opts) > + drbd_helper = _InitDrbdHelper(opts, enabled_disk_templates) > except errors.OpPrereqError, e: > ToStderr(str(e)) > return 1 > diff --git a/test/py/ganeti.bootstrap_unittest.py b/test/py/ > ganeti.bootstrap_unittest.py > index a0e5135..577918f 100755 > --- a/test/py/ganeti.bootstrap_unittest.py > +++ b/test/py/ganeti.bootstrap_unittest.py > @@ -132,34 +132,48 @@ class > TestRestrictIpolicyToEnabledDiskTemplates(unittest.TestCase): > > class TestInitCheckDrbdHelper(unittest.TestCase): > > + def testNoDrbd(self): > + drbd_enabled = False > + drbd_helper = None > + mock_helper_fn = mock.Mock() > + mock_helper_fn.side_effect = NotImplemented > + bootstrap._InitCheckDrbdHelper(drbd_helper, drbd_enabled, > + get_helper_fn=mock_helper_fn) > + > def testHelperNone(self): > + drbd_enabled = True > current_helper = "/bin/helper" > drbd_helper = None > mock_helper_fn = mock.Mock(return_value=current_helper) > - bootstrap._InitCheckDrbdHelper(drbd_helper, > get_helper_fn=mock_helper_fn) > + bootstrap._InitCheckDrbdHelper(drbd_helper, drbd_enabled, > + get_helper_fn=mock_helper_fn) > > def testHelperOk(self): > + drbd_enabled = True > current_helper = "/bin/helper" > drbd_helper = "/bin/helper" > mock_helper_fn = mock.Mock(return_value=current_helper) > - bootstrap._InitCheckDrbdHelper(drbd_helper, > get_helper_fn=mock_helper_fn) > + bootstrap._InitCheckDrbdHelper(drbd_helper, drbd_enabled, > + get_helper_fn=mock_helper_fn) > > def testWrongHelper(self): > + drbd_enabled = True > current_helper = "/bin/otherhelper" > drbd_helper = "/bin/helper" > mock_helper_fn = mock.Mock(return_value=current_helper) > self.assertRaises(errors.OpPrereqError, > bootstrap._InitCheckDrbdHelper, > - drbd_helper, get_helper_fn=mock_helper_fn) > + drbd_helper, drbd_enabled, get_helper_fn=mock_helper_fn) > > def testHelperCheckFails(self): > + drbd_enabled = True > current_helper = "/bin/otherhelper" > drbd_helper = "/bin/helper" > mock_helper_fn = mock.Mock(return_value=current_helper) > mock_helper_fn.side_effect = errors.BlockDeviceError > self.assertRaises(errors.OpPrereqError, > bootstrap._InitCheckDrbdHelper, > - drbd_helper, get_helper_fn=mock_helper_fn) > + drbd_helper, drbd_enabled, get_helper_fn=mock_helper_fn) > > > if __name__ == "__main__": > diff --git a/test/py/ganeti.client.gnt_cluster_unittest.py b/test/py/ > ganeti.client.gnt_cluster_unittest.py > index 91e8935..66aafcc 100755 > --- a/test/py/ganeti.client.gnt_cluster_unittest.py > +++ b/test/py/ganeti.client.gnt_cluster_unittest.py > @@ -262,31 +262,49 @@ class TestEpo(unittest.TestCase): > > class InitDrbdHelper(unittest.TestCase): > > + def setUp(self): > + unittest.TestCase.setUp(self) > + self.enabled_disk_templates = [] > + > + def enableDrbd(self): > + self.enabled_disk_templates = [constants.DT_DRBD8] > + > + def disableDrbd(self): > + self.enabled_disk_templates = [constants.DT_DISKLESS] > + > def testNoDrbdNoHelper(self): > opts = mock.Mock() > - opts.drbd_storage = False > opts.drbd_helper = None > - helper = gnt_cluster._InitDrbdHelper(opts) > + self.disableDrbd() > + helper = gnt_cluster._InitDrbdHelper(opts, > self.enabled_disk_templates) > self.assertEquals(None, helper) > > def testNoDrbdHelper(self): > opts = mock.Mock() > - opts.drbd_storage = None > + self.disableDrbd() > opts.drbd_helper = "/bin/true" > - self.assertRaises(errors.OpPrereqError, gnt_cluster._InitDrbdHelper, > opts) > + self.assertRaises(errors.OpPrereqError, gnt_cluster._InitDrbdHelper, > opts, > + self.enabled_disk_templates) > > - def testDrbdNoHelper(self): > + def testDrbdHelperNone(self): > opts = mock.Mock() > - opts.drbd_storage = True > + self.enableDrbd() > opts.drbd_helper = None > - helper = gnt_cluster._InitDrbdHelper(opts) > + helper = gnt_cluster._InitDrbdHelper(opts, > self.enabled_disk_templates) > self.assertEquals(constants.DEFAULT_DRBD_HELPER, helper) > > + def testDrbdHelperEmpty(self): > + opts = mock.Mock() > + self.enableDrbd() > + opts.drbd_helper = '' > + self.assertRaises(errors.OpPrereqError, gnt_cluster._InitDrbdHelper, > opts, > + self.enabled_disk_templates) > + > def testDrbdHelper(self): > opts = mock.Mock() > - opts.drbd_storage = True > + self.enableDrbd() > opts.drbd_helper = "/bin/true" > - helper = gnt_cluster._InitDrbdHelper(opts) > + helper = gnt_cluster._InitDrbdHelper(opts, > self.enabled_disk_templates) > self.assertEquals(opts.drbd_helper, helper) > > > -- > 1.8.3 > > 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
