cluster modify: deprecate --no-drbd-storage

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 <helgav@google.com>
Reviewed-by: Thomas Thrainer <thomasth@google.com>

diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 3c82732..a73a1b7 100644 (file)
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -1037,17 +1037,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:
+    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
 
 
@@ -1061,7 +1063,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
@@ -1096,7 +1099,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 4886a47..f9d6268 100644 (file)
--- 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 (file)
--- 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]
+    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]
+    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])
+    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..c34432a 100755 (executable)
--- a/test/py/ganeti.client.gnt_cluster_unittest.py
+++ b/test/py/ganeti.client.gnt_cluster_unittest.py
@@ -260,7 +260,7 @@ class TestEpo(unittest.TestCase):
     self.assertEqual(result, constants.EXIT_FAILURE)
 
 
-class InitDrbdHelper(unittest.TestCase):
+class DrbdHelperTestCase(unittest.TestCase):
 
   def setUp(self):
     unittest.TestCase.setUp(self)
@@ -272,6 +272,9 @@ class InitDrbdHelper(unittest.TestCase):
   def disableDrbd(self):
     self.enabled_disk_templates = [constants.DT_DISKLESS]
 
+
+class InitDrbdHelper(DrbdHelperTestCase):
+
   def testNoDrbdNoHelper(self):
     opts = mock.Mock()
     opts.drbd_helper = None
@@ -308,33 +311,46 @@ class InitDrbdHelper(unittest.TestCase):
     self.assertEquals(opts.drbd_helper, helper)
 
 
-class GetDrbdHelper(unittest.TestCase):
+class GetDrbdHelper(DrbdHelperTestCase):
 
   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)