cluster init: check for enabled disk templates

The purpose of this patch is to remove the usage of the
'ENABLE_FILE_STORAGE' constant. To get there, we do some
refactoring, add unit tests and add a FIXME for a forgotten
test regarding the file storage location.

Signed-off-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Klaus Aehlig <aehlig@google.com>

diff --git a/Makefile.am b/Makefile.am
index 49cd09e..7397c34 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -1213,6 +1213,7 @@ python_tests = \
        test/py/ganeti.asyncnotifier_unittest.py \
        test/py/ganeti.backend_unittest-runasroot.py \
        test/py/ganeti.backend_unittest.py \
+       test/py/ganeti.bootstrap_unittest.py \
        test/py/ganeti.cli_unittest.py \
        test/py/ganeti.client.gnt_cluster_unittest.py \
        test/py/ganeti.client.gnt_instance_unittest.py \

diff --git a/lib/bootstrap.py b/lib/bootstrap.py
index 2052f28..eb15841 100644 (file)
--- a/lib/bootstrap.py
+++ b/lib/bootstrap.py
@@ -334,7 +334,18 @@ def RunNodeSetupCmd(cluster_name, node, basecmd, debug, verbose,
   _WaitForSshDaemon(node, netutils.GetDaemonPort(constants.SSH), family)
 
 
-def _InitFileStorage(file_storage_dir):
+def _PrepareFileStorage(enabled_disk_templates, file_storage_dir):
+  """Checks if file storage is enabled and inits the dir.
+
+  """
+  if utils.storage.IsFileStorageEnabled(enabled_disk_templates):
+    file_storage_dir = _InitFileStorageDir(file_storage_dir)
+  else:
+    file_storage_dir = ""
+  return file_storage_dir
+
+
+def _InitFileStorageDir(file_storage_dir):
   """Initialize if needed the file storage.
 
   @param file_storage_dir: the user-supplied value
@@ -360,9 +371,26 @@ def _InitFileStorage(file_storage_dir):
     raise errors.OpPrereqError("The file storage directory '%s' is not"
                                " a directory." % file_storage_dir,
                                errors.ECODE_ENVIRON)
+
+  # FIXME: check here if the file_storage_dir is in the set of allowed dirs
   return file_storage_dir
 
 
+def _InitCheckEnabledDiskTemplates(enabled_disk_templates):
+  """Checks the sanity of the enabled disk templates.
+
+  """
+  if not enabled_disk_templates:
+    raise errors.OpPrereqError("Enabled disk templates list must contain at"
+                               " least one member", errors.ECODE_INVAL)
+  invalid_disk_templates = \
+    set(enabled_disk_templates) - constants.DISK_TEMPLATES
+  if invalid_disk_templates:
+    raise errors.OpPrereqError("Enabled disk templates list contains invalid"
+                               " entries: %s" % invalid_disk_templates,
+                               errors.ECODE_INVAL)
+
+
 def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914
                 master_netmask, master_netdev, file_storage_dir,
                 shared_file_storage_dir, candidate_pool_size, secondary_ip=None,
@@ -396,15 +424,7 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914
                                " entries: %s" % invalid_hvs,
                                errors.ECODE_INVAL)
 
-  if not enabled_disk_templates:
-    raise errors.OpPrereqError("Enabled disk templates list must contain at"
-                               " least one member", errors.ECODE_INVAL)
-  invalid_disk_templates = \
-    set(enabled_disk_templates) - constants.DISK_TEMPLATES
-  if invalid_disk_templates:
-    raise errors.OpPrereqError("Enabled disk templates list contains invalid"
-                               " entries: %s" % invalid_disk_templates,
-                               errors.ECODE_INVAL)
+  _InitCheckEnabledDiskTemplates(enabled_disk_templates)
 
   try:
     ipcls = netutils.IPAddress.GetClassFromIpVersion(primary_ip_version)
@@ -489,13 +509,11 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914
                              " had exitcode %s and error '%s'" %
                              (result.cmd, result.exit_code, result.output))
 
-  if constants.ENABLE_FILE_STORAGE:
-    file_storage_dir = _InitFileStorage(file_storage_dir)
-  else:
-    file_storage_dir = ""
+  file_storage_dir = _PrepareFileStorage(enabled_disk_templates,
+                                         file_storage_dir)
 
   if constants.ENABLE_SHARED_FILE_STORAGE:
-    shared_file_storage_dir = _InitFileStorage(shared_file_storage_dir)
+    shared_file_storage_dir = _InitFileStorageDir(shared_file_storage_dir)
   else:
     shared_file_storage_dir = ""
 

diff --git a/test/py/ganeti.bootstrap_unittest.py b/test/py/ganeti.bootstrap_unittest.py
new file mode 100755 (executable)
index 0000000..4730d5a
--- /dev/null
+++ b/test/py/ganeti.bootstrap_unittest.py
@@ -0,0 +1,73 @@
+#!/usr/bin/python
+#
+
+# Copyright (C) 2013 Google Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+
+"""Script for testing ganeti.bootstrap"""
+
+import shutil
+import tempfile
+import unittest
+
+from ganeti import bootstrap
+from ganeti import constants
+from ganeti import errors
+
+import testutils
+
+
+class TestPrepareFileStorage(unittest.TestCase):
+  def setUp(self):
+    self.tmpdir = tempfile.mkdtemp()
+
+  def tearDown(self):
+    shutil.rmtree(self.tmpdir)
+
+  def testFileStorageEnabled(self):
+    enabled_disk_templates = [constants.DT_FILE]
+    file_storage_dir = bootstrap._PrepareFileStorage(
+        enabled_disk_templates, self.tmpdir)
+    self.assertEqual(self.tmpdir, file_storage_dir)
+
+  def testFileStorageDisabled(self):
+    # anything != DT_FILE would do here
+    enabled_disk_templates = [constants.DT_PLAIN]
+    file_storage_dir = bootstrap._PrepareFileStorage(
+        enabled_disk_templates, self.tmpdir)
+    self.assertEqual('', file_storage_dir)
+
+
+class TestInitCheckEnabledDiskTemplates(unittest.TestCase):
+  def testValidTemplates(self):
+    enabled_disk_templates = list(constants.DISK_TEMPLATES)
+    bootstrap._InitCheckEnabledDiskTemplates(enabled_disk_templates)
+
+  def testInvalidTemplates(self):
+    enabled_disk_templates = ["pinkbunny"]
+    self.assertRaises(errors.OpPrereqError,
+        bootstrap._InitCheckEnabledDiskTemplates, enabled_disk_templates)
+
+  def testEmptyTemplates(self):
+    enabled_disk_templates = []
+    self.assertRaises(errors.OpPrereqError,
+        bootstrap._InitCheckEnabledDiskTemplates, enabled_disk_templates)
+
+
+if __name__ == "__main__":
+  testutils.GanetiTestProgram()