cluster init: deprecate --no-drbd-storage

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

diff --git a/lib/bootstrap.py b/lib/bootstrap.py
index ed001aa..cb2f0f4 100644 (file)
--- 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):
   """Checks the DRBD usermode helper.
 
   @type drbd_helper: string
@@ -480,13 +480,16 @@ def _InitCheckDrbdHelper(drbd_helper):
     use
 
   """
+  if not drbd_enabled:
+    return
+
   if drbd_helper is not None:
     try:
       curr_helper = drbd.DRBD8.GetUsermodeHelper()
     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"
@@ -592,7 +595,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 f3fb7a6..3c82732 100644 (file)
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -108,16 +108,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
 
@@ -141,7 +151,7 @@ def InitCluster(opts, args):
 
   try:
     vg_name = _InitVgName(opts, enabled_disk_templates)
-    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 f6278ae..b772825 100755 (executable)
--- a/test/py/ganeti.bootstrap_unittest.py
+++ b/test/py/ganeti.bootstrap_unittest.py
@@ -134,33 +134,43 @@ class TestRestrictIpolicyToEnabledDiskTemplates(unittest.TestCase):
 class TestInitCheckDrbdHelper(unittest.TestCase):
 
   @testutils.patch_object(drbd.DRBD8, "GetUsermodeHelper")
+  def testNoDrbd(self, drbd_mock_get_usermode_helper):
+    drbd_enabled = False
+    drbd_helper = None
+    bootstrap._InitCheckDrbdHelper(drbd_helper, drbd_enabled)
+
+  @testutils.patch_object(drbd.DRBD8, "GetUsermodeHelper")
   def testHelperNone(self, drbd_mock_get_usermode_helper):
+    drbd_enabled = True
     current_helper = "/bin/helper"
     drbd_helper = None
     drbd_mock_get_usermode_helper.return_value = current_helper
-    bootstrap._InitCheckDrbdHelper(drbd_helper)
+    bootstrap._InitCheckDrbdHelper(drbd_helper, drbd_enabled)
 
   @testutils.patch_object(drbd.DRBD8, "GetUsermodeHelper")
   def testHelperOk(self, drbd_mock_get_usermode_helper):
+    drbd_enabled = True
     current_helper = "/bin/helper"
     drbd_helper = "/bin/helper"
     drbd_mock_get_usermode_helper.return_value = current_helper
-    bootstrap._InitCheckDrbdHelper(drbd_helper)
+    bootstrap._InitCheckDrbdHelper(drbd_helper, drbd_enabled)
 
   @testutils.patch_object(drbd.DRBD8, "GetUsermodeHelper")
   def testWrongHelper(self, drbd_mock_get_usermode_helper):
+    drbd_enabled = True
     current_helper = "/bin/otherhelper"
     drbd_helper = "/bin/helper"
     drbd_mock_get_usermode_helper.return_value = current_helper
     self.assertRaises(errors.OpPrereqError,
-        bootstrap._InitCheckDrbdHelper, drbd_helper)
+        bootstrap._InitCheckDrbdHelper, drbd_helper, drbd_enabled)
 
   @testutils.patch_object(drbd.DRBD8, "GetUsermodeHelper")
   def testHelperCheckFails(self, drbd_mock_get_usermode_helper):
+    drbd_enabled = True
     drbd_helper = "/bin/helper"
     drbd_mock_get_usermode_helper.side_effect=errors.BlockDeviceError
     self.assertRaises(errors.OpPrereqError,
-        bootstrap._InitCheckDrbdHelper, drbd_helper)
+        bootstrap._InitCheckDrbdHelper, drbd_helper, drbd_enabled)
 
 
 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 (executable)
--- 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)