On Thu, Aug 22, 2013 at 10:16 AM, Helga Velroyen <[email protected]> wrote:
> As in the previous patch, the option '--no-drbd-storage' > is deprectated, because it is subsumed by the non-inclusion > of 'drbd' in the list of enabled disk templates. > > Signed-off-by: Helga Velroyen <[email protected]> > --- > lib/client/gnt_cluster.py | 21 ++++---- > lib/cmdlib/cluster.py | 73 > +++++++++++++++++++-------- > test/py/cmdlib/cluster_unittest.py | 38 ++++++++++++-- > test/py/ganeti.client.gnt_cluster_unittest.py | 41 +++++++++++---- > 4 files changed, 131 insertions(+), 42 deletions(-) > > diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py > index daf2595..887e7b6 100644 > --- a/lib/client/gnt_cluster.py > +++ b/lib/client/gnt_cluster.py > @@ -1044,17 +1044,19 @@ def _GetVgName(opts, enabled_disk_templates): > return vg_name > > > -def _GetDrbdHelper(opts): > +def _GetDrbdHelper(opts, enabled_disk_templates): > """Determine the DRBD usermode helper. > > """ > drbd_helper = opts.drbd_helper > - if not opts.drbd_storage and opts.drbd_helper: > - raise errors.OpPrereqError( > - "Options --no-drbd-storage and --drbd-usermode-helper conflict.") > - > - if not opts.drbd_storage: > - drbd_helper = "" > + if enabled_disk_templates: > When can `enabled_disk_templates` be None? Note that [] would work... > + drbd_enabled = constants.DT_DRBD8 in enabled_disk_templates > + # This raises an exception for historic reasons. It might be a good > idea > + # to allow users to set a DRBD helper when DRBD storage is not > enabled. > + if not drbd_enabled and opts.drbd_helper: > + raise errors.OpPrereqError( > + "Setting a DRBD usermode helper when DRBD is not enabled is" > + " not allowed.") > return drbd_helper > > > @@ -1068,7 +1070,8 @@ def SetClusterParams(opts, args): > @return: the desired exit code > > """ > - if not (opts.vg_name is not None or opts.drbd_helper or > + if not (opts.vg_name is not None or > + opts.drbd_helper is not None or > opts.enabled_hypervisors or opts.hvparams or > opts.beparams or opts.nicparams or > opts.ndparams or opts.diskparams or > @@ -1103,7 +1106,7 @@ def SetClusterParams(opts, args): > vg_name = _GetVgName(opts, enabled_disk_templates) > > try: > - drbd_helper = _GetDrbdHelper(opts) > + drbd_helper = _GetDrbdHelper(opts, enabled_disk_templates) > except errors.OpPrereqError, e: > ToStderr(str(e)) > return 1 > diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py > index b563409..b0714b3 100644 > --- a/lib/cmdlib/cluster.py > +++ b/lib/cmdlib/cluster.py > @@ -851,37 +851,66 @@ class LUClusterSetParams(LogicalUnit): > CheckIpolicyVsDiskTemplates(cluster.ipolicy, > enabled_disk_templates) > > - def _CheckDrbdHelper(self, node_uuids): > + def _CheckDrbdHelperOnNodes(self, drbd_helper, node_uuids): > + """Checks whether the set DRBD helper actually exists on the nodes. > + > + @type drbd_helper: string > + @param drbd_helper: path of the drbd usermode helper binary > + @type node_uuids: list of strings > + @param node_uuids: list of node UUIDs to check for the helper > + > + """ > + # checks given drbd helper on all nodes > + helpers = self.rpc.call_drbd_helper(node_uuids) > + for (_, ninfo) in self.cfg.GetMultiNodeInfo(node_uuids): > + if ninfo.offline: > + self.LogInfo("Not checking drbd helper on offline node %s", > + ninfo.name) > + continue > + msg = helpers[ninfo.uuid].fail_msg > + if msg: > + raise errors.OpPrereqError("Error checking drbd helper on node" > + " '%s': %s" % (ninfo.name, msg), > + errors.ECODE_ENVIRON) > + node_helper = helpers[ninfo.uuid].payload > + if node_helper != drbd_helper: > + raise errors.OpPrereqError("Error on node '%s': drbd helper is > %s" % > + (ninfo.name, node_helper), > + errors.ECODE_ENVIRON) > + > + def _CheckDrbdHelper(self, node_uuids, drbd_enabled, drbd_gets_enabled): > """Check the DRBD usermode helper. > > @type node_uuids: list of strings > @param node_uuids: a list of nodes' UUIDs > + @type drbd_enabled: boolean > + @param drbd_enabled: whether DRBD will be enabled after this operation > + (no matter if it was disabled before or not) > + @type drbd_gets_enabled: boolen > + @param drbd_gets_enabled: true if DRBD was disabled before this > + operation, but will be enabled afterwards > > """ > - if self.op.drbd_helper is not None and not self.op.drbd_helper: > + if self.op.drbd_helper == '': > + if drbd_enabled: > + raise errors.OpPrereqError("Cannot disable drbd helper while" > + " DRBD is enabled.") > if self.cfg.HasAnyDiskOfType(constants.LD_DRBD8): > raise errors.OpPrereqError("Cannot disable drbd helper while" > " drbd-based instances exist", > errors.ECODE_INVAL) > > - if self.op.drbd_helper: > - # checks given drbd helper on all nodes > - helpers = self.rpc.call_drbd_helper(node_uuids) > - for (_, ninfo) in self.cfg.GetMultiNodeInfo(node_uuids): > - if ninfo.offline: > - self.LogInfo("Not checking drbd helper on offline node %s", > - ninfo.name) > - continue > - msg = helpers[ninfo.uuid].fail_msg > - if msg: > - raise errors.OpPrereqError("Error checking drbd helper on node" > - " '%s': %s" % (ninfo.name, msg), > - errors.ECODE_ENVIRON) > - node_helper = helpers[ninfo.uuid].payload > - if node_helper != self.op.drbd_helper: > - raise errors.OpPrereqError("Error on node '%s': drbd helper is > %s" % > - (ninfo.name, node_helper), > - errors.ECODE_ENVIRON) > + else: > + if self.op.drbd_helper is not None and drbd_enabled: > + self._CheckDrbdHelperOnNodes(self.op.drbd_helper, node_uuids) > + else: > + if drbd_gets_enabled: > + current_drbd_helper = > self.cfg.GetClusterInfo().drbd_usermode_helper > + if current_drbd_helper is not None: > + self._CheckDrbdHelperOnNodes(current_drbd_helper, node_uuids) > + else: > + raise errors.OpPrereqError("Cannot enable DRBD without a" > + " DRBD usermode helper set.") > > def CheckPrereq(self): > """Check prerequisites. > @@ -912,7 +941,9 @@ class LUClusterSetParams(LogicalUnit): > self.LogWarning, self.op.shared_file_storage_dir, > enabled_disk_templates) > > - self._CheckDrbdHelper(node_uuids) > + drbd_enabled = constants.DT_DRBD8 in enabled_disk_templates > + drbd_gets_enabled = constants.DT_DRBD8 in new_enabled_disk_templates > + self._CheckDrbdHelper(node_uuids, drbd_enabled, drbd_gets_enabled) > > # validate params changes > if self.op.beparams: > diff --git a/test/py/cmdlib/cluster_unittest.py > b/test/py/cmdlib/cluster_unittest.py > index ff9f14e..680eadf 100644 > --- a/test/py/cmdlib/cluster_unittest.py > +++ b/test/py/cmdlib/cluster_unittest.py > @@ -501,7 +501,7 @@ class TestLUClusterSetParams(CmdlibTestCase): > > self.mcpu.assertLogContainsRegex("but did not enable") > > - def testResetDrbdHelper(self): > + def testResetDrbdHelperDrbdDisabled(self): > drbd_helper = "" > self.cfg.SetEnabledDiskTemplates([constants.DT_DISKLESS]) > op = opcodes.OpClusterSetParams(drbd_helper=drbd_helper) > @@ -509,13 +509,45 @@ class TestLUClusterSetParams(CmdlibTestCase): > > self.assertEqual(None, self.cluster.drbd_usermode_helper) > > + def testResetDrbdHelperDrbdEnabled(self): > + drbd_helper = "" > + self.cluster.enabled_disk_templates = [constants.DT_DRBD8] > See previous patch. > + op = opcodes.OpClusterSetParams(drbd_helper=drbd_helper) > + self.ExecOpCodeExpectOpPrereqError( > + op, "Cannot disable drbd helper while DRBD is enabled.") > + > + def testEnableDrbdNoHelper(self): > + self.cluster.enabled_disk_templates = [constants.DT_DISKLESS] > As above. > + self.cluster.drbd_usermode_helper = None > + enabled_disk_templates = [constants.DT_DRBD8] > + op = opcodes.OpClusterSetParams( > + enabled_disk_templates=enabled_disk_templates) > + self.ExecOpCodeExpectOpPrereqError( > + op, "Cannot enable DRBD without a DRBD usermode helper set") > + > + def testEnableDrbdHelperSet(self): > + drbd_helper = "/bin/random_helper" > + self.rpc.call_drbd_helper.return_value = \ > + self.RpcResultsBuilder() \ > + .AddSuccessfulNode(self.master, drbd_helper) \ > + .Build() > + self.cfg.SetEnabledDiskTemplates([constants.DT_DISKLESS]) > Use this consistently. I'm glad that it was not as well hidden as I feared ;-). > + self.cluster.drbd_usermode_helper = drbd_helper > + enabled_disk_templates = [constants.DT_DRBD8] > + op = opcodes.OpClusterSetParams( > + enabled_disk_templates=enabled_disk_templates, > + ipolicy={constants.IPOLICY_DTS: enabled_disk_templates}) > + self.ExecOpCode(op) > + > + self.assertEqual(drbd_helper, self.cluster.drbd_usermode_helper) > + > def testDrbdHelperAlreadySet(self): > drbd_helper = "/bin/true" > self.rpc.call_drbd_helper.return_value = \ > self.RpcResultsBuilder() \ > .AddSuccessfulNode(self.master, "/bin/true") \ > .Build() > - self.cluster.enabled_disk_templates = [constants.DT_DISKLESS] > + self.cfg.SetEnabledDiskTemplates([constants.DT_DISKLESS]) > op = opcodes.OpClusterSetParams(drbd_helper=drbd_helper) > self.ExecOpCode(op) > > @@ -529,7 +561,7 @@ class TestLUClusterSetParams(CmdlibTestCase): > .AddSuccessfulNode(self.master, "/bin/true") \ > .Build() > self.cluster.drbd_usermode_helper = "/bin/false" > - self.cluster.enabled_disk_templates = [constants.DT_DRBD8] > + self.cfg.SetEnabledDiskTemplates([constants.DT_DRBD8]) > op = opcodes.OpClusterSetParams(drbd_helper=drbd_helper) > self.ExecOpCode(op) > > diff --git a/test/py/ganeti.client.gnt_cluster_unittest.py b/test/py/ > ganeti.client.gnt_cluster_unittest.py > index 66aafcc..6aa9c62 100755 > --- a/test/py/ganeti.client.gnt_cluster_unittest.py > +++ b/test/py/ganeti.client.gnt_cluster_unittest.py > @@ -310,31 +310,54 @@ class InitDrbdHelper(unittest.TestCase): > > class GetDrbdHelper(unittest.TestCase): > > + def setUp(self): > + unittest.TestCase.setUp(self) > + self.enabled_disk_templates = [] > + > + def enableDrbd(self): > + self.enabled_disk_templates = [constants.DT_DRBD8] > + > That's really too similar to InitDrbdHelper... > + def disableDrbd(self): > + self.enabled_disk_templates = [constants.DT_DISKLESS] > + > def testNoDrbdNoHelper(self): > opts = mock.Mock() > - opts.drbd_storage = False > + self.disableDrbd() > + opts.drbd_helper = None > + helper = gnt_cluster._GetDrbdHelper(opts, self.enabled_disk_templates) > + self.assertEquals(None, helper) > + > + def testNoTemplateInfoNoHelper(self): > + opts = mock.Mock() > opts.drbd_helper = None > - helper = gnt_cluster._GetDrbdHelper(opts) > - self.assertEquals("", helper) > + helper = gnt_cluster._GetDrbdHelper(opts, None) > + self.assertEquals(None, helper) > + > + def testNoTemplateInfoHelper(self): > + opts = mock.Mock() > + opts.drbd_helper = "/bin/true" > + helper = gnt_cluster._GetDrbdHelper(opts, None) > + self.assertEquals(opts.drbd_helper, helper) > > def testNoDrbdHelper(self): > opts = mock.Mock() > - opts.drbd_storage = None > + self.disableDrbd() > opts.drbd_helper = "/bin/true" > - self.assertRaises(errors.OpPrereqError, gnt_cluster._GetDrbdHelper, > opts) > + self.assertRaises(errors.OpPrereqError, gnt_cluster._GetDrbdHelper, > opts, > + self.enabled_disk_templates) > > def testDrbdNoHelper(self): > opts = mock.Mock() > - opts.drbd_storage = True > + self.enableDrbd() > opts.drbd_helper = None > - helper = gnt_cluster._GetDrbdHelper(opts) > + helper = gnt_cluster._GetDrbdHelper(opts, self.enabled_disk_templates) > self.assertEquals(None, helper) > > def testDrbdHelper(self): > opts = mock.Mock() > - opts.drbd_storage = True > + self.enableDrbd() > opts.drbd_helper = "/bin/true" > - helper = gnt_cluster._GetDrbdHelper(opts) > + helper = gnt_cluster._GetDrbdHelper(opts, self.enabled_disk_templates) > self.assertEquals(opts.drbd_helper, helper) > -- > 1.8.3 > > Rest LGMT (with InitDrbdHelper and GetDrbdHelper merged as suggested in a previous patch). Thanks, Thomas -- 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
