Improve handling of storage info

This patch improves the processing of storage information
as result of the 'node_info' call in the following way:
- It removes the override for LVM storage. This was needed
  in MakeLegacyNode info to be compatible to the
  iallocator, which no longer uses it.
- When no storage information is available (for example
  when only disk templates are enabled, for which no
  storage reporting exists), then this is handled more
  gracefully.

Signed-off-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Thomas Thrainer <thomasth@google.com>

diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
index e61a8b9..08a6441 100644 (file)
--- a/lib/cmdlib/node.py
+++ b/lib/cmdlib/node.py
@@ -1183,15 +1183,15 @@ class NodeQuery(QueryBase):
       # filter out non-vm_capable nodes
       toquery_node_uuids = [node.uuid for node in all_info.values()
                             if node.vm_capable and node.uuid in node_uuids]
+      lvm_enabled = utils.storage.IsLvmEnabled(
+          lu.cfg.GetClusterInfo().enabled_disk_templates)
       # FIXME: this per default asks for storage space information for all
       # enabled disk templates. Fix this by making it possible to specify
       # space report fields for specific disk templates.
       raw_storage_units = utils.storage.GetStorageUnitsOfCluster(
-          lu.cfg, include_spindles=True)
+          lu.cfg, include_spindles=lvm_enabled)
       storage_units = rpc.PrepareStorageUnitsForNodes(
           lu.cfg, raw_storage_units, toquery_node_uuids)
-      lvm_enabled = utils.storage.IsLvmEnabled(
-          lu.cfg.GetClusterInfo().enabled_disk_templates)
       default_hypervisor = lu.cfg.GetHypervisorType()
       hvparams = lu.cfg.GetClusterInfo().hvparams[default_hypervisor]
       hvspecs = [(default_hypervisor, hvparams)]
@@ -1199,7 +1199,7 @@ class NodeQuery(QueryBase):
                                         hvspecs)
       live_data = dict(
           (uuid, rpc.MakeLegacyNodeInfo(nresult.payload,
-                                        require_vg_info=lvm_enabled))
+                                        require_spindles=lvm_enabled))
           for (uuid, nresult) in node_data.items()
           if not nresult.fail_msg and nresult.payload)
     else:

diff --git a/lib/rpc.py b/lib/rpc.py
index dd766b4..5ce530a 100644 (file)
--- a/lib/rpc.py
+++ b/lib/rpc.py
@@ -610,17 +610,15 @@ def _AddSpindlesToLegacyNodeInfo(result, space_info):
   if lvm_pv_info:
     result["spindles_free"] = lvm_pv_info["storage_free"]
     result["spindles_total"] = lvm_pv_info["storage_size"]
+  else:
+    raise errors.OpExecError("No spindle storage information available.")
 
 
-def _AddDefaultStorageInfoToLegacyNodeInfo(result, space_info,
-                                           require_vg_info=True):
+def _AddDefaultStorageInfoToLegacyNodeInfo(result, space_info):
   """Extracts the storage space information of the default storage type from
   the space info and adds it to the result dictionary.
 
   @see: C{_AddSpindlesToLegacyNodeInfo} for parameter information.
-  @type require_vg_info: boolean
-  @param require_vg_info: indicates whether volume group information is
-    required or not
 
   """
   # Check if there is at least one row for non-spindle storage info.
@@ -633,41 +631,29 @@ def _AddDefaultStorageInfoToLegacyNodeInfo(result, space_info,
   else:
     default_space_info = space_info[0]
 
-  if require_vg_info:
-    # if lvm storage is required, ignore the actual default and look for LVM
-    lvm_info_found = False
-    for space_entry in space_info:
-      if space_entry["type"] == constants.ST_LVM_VG:
-        default_space_info = space_entry
-        lvm_info_found = True
-        continue
-    if not lvm_info_found:
-      raise errors.OpExecError("LVM volume group info required, but not"
-                               " provided.")
-
   if default_space_info:
     result["name"] = default_space_info["name"]
     result["storage_free"] = default_space_info["storage_free"]
     result["storage_size"] = default_space_info["storage_size"]
 
 
-def MakeLegacyNodeInfo(data, require_vg_info=True):
+def MakeLegacyNodeInfo(data, require_spindles=False):
   """Formats the data returned by L{rpc.RpcRunner.call_node_info}.
 
   Converts the data into a single dictionary. This is fine for most use cases,
   but some require information from more than one volume group or hypervisor.
 
-  @param require_vg_info: raise an error if the returnd vg_info
-      doesn't have any values
+  @param require_spindles: add spindle storage information to the legacy node
+      info
 
   """
   (bootid, space_info, (hv_info, )) = data
 
   ret = utils.JoinDisjointDicts(hv_info, {"bootid": bootid})
 
-  _AddSpindlesToLegacyNodeInfo(ret, space_info)
-  _AddDefaultStorageInfoToLegacyNodeInfo(ret, space_info,
-                                         require_vg_info=require_vg_info)
+  if require_spindles:
+    _AddSpindlesToLegacyNodeInfo(ret, space_info)
+  _AddDefaultStorageInfoToLegacyNodeInfo(ret, space_info)
 
   return ret
 

diff --git a/test/py/ganeti.rpc_unittest.py b/test/py/ganeti.rpc_unittest.py
index d2eee4b..670d555 100755 (executable)
--- a/test/py/ganeti.rpc_unittest.py
+++ b/test/py/ganeti.rpc_unittest.py
@@ -936,24 +936,23 @@ class TestLegacyNodeInfo(unittest.TestCase):
     KEY_VG1: VAL_VG1,
     KEY_VG2: VAL_VG2,
     KEY_HV: VAL_HV,
-    KEY_SP1: VAL_SP1,
-    KEY_SP2: VAL_SP2,
     }
 
   def testStandard(self):
     result = rpc.MakeLegacyNodeInfo(self.STD_LST)
     self.assertEqual(result, self.STD_DICT)
 
-  def testReqVg(self):
+  def testSpindlesRequired(self):
     my_lst = [self.VAL_BOOT, [], [self.DICT_HV]]
-    self.assertRaises(errors.OpExecError, rpc.MakeLegacyNodeInfo, my_lst)
+    self.assertRaises(errors.OpExecError, rpc.MakeLegacyNodeInfo, my_lst,
+        require_spindles=True)
 
-  def testNoReqVg(self):
+  def testNoSpindlesRequired(self):
     my_lst = [self.VAL_BOOT, [], [self.DICT_HV]]
-    result = rpc.MakeLegacyNodeInfo(my_lst, require_vg_info = False)
+    result = rpc.MakeLegacyNodeInfo(my_lst, require_spindles = False)
     self.assertEqual(result, {self.KEY_BOOT: self.VAL_BOOT,
                               self.KEY_HV: self.VAL_HV})
-    result = rpc.MakeLegacyNodeInfo(self.STD_LST, require_vg_info = False)
+    result = rpc.MakeLegacyNodeInfo(self.STD_LST, require_spindles = False)
     self.assertEqual(result, self.STD_DICT)
 
 
@@ -964,48 +963,31 @@ class TestAddDefaultStorageInfoToLegacyNodeInfo(unittest.TestCase):
     self.total_storage_file = 42
     self.free_storage_lvm = 69
     self.total_storage_lvm = 666
-    self.node_info = [{"name": "mynode",
+    self.node_info = [{"name": "myfile",
                        "type": constants.ST_FILE,
                        "storage_free": self.free_storage_file,
                        "storage_size": self.total_storage_file},
-                      {"name": "mynode",
+                      {"name": "myvg",
                        "type": constants.ST_LVM_VG,
                        "storage_free": self.free_storage_lvm,
                        "storage_size": self.total_storage_lvm},
-                      {"name": "mynode",
+                      {"name": "myspindle",
                        "type": constants.ST_LVM_PV,
                        "storage_free": 33,
                        "storage_size": 44}]
 
   def testAddDefaultStorageInfoToLegacyNodeInfo(self):
     result = {}
-    has_lvm = False
-    rpc._AddDefaultStorageInfoToLegacyNodeInfo(result, self.node_info, has_lvm)
+    rpc._AddDefaultStorageInfoToLegacyNodeInfo(result, self.node_info)
     self.assertEqual(self.free_storage_file, result["storage_free"])
     self.assertEqual(self.total_storage_file, result["storage_size"])
 
-  def testAddDefaultStorageInfoToLegacyNodeInfoOverrideDefault(self):
-    result = {}
-    has_lvm = True
-    rpc._AddDefaultStorageInfoToLegacyNodeInfo(result, self.node_info, has_lvm)
-    self.assertEqual(self.free_storage_lvm, result["storage_free"])
-    self.assertEqual(self.total_storage_lvm, result["storage_size"])
-
   def testAddDefaultStorageInfoToLegacyNodeInfoNoDefaults(self):
     result = {}
-    has_lvm = False
-    rpc._AddDefaultStorageInfoToLegacyNodeInfo(result, self.node_info[-1:],
-                                               has_lvm)
+    rpc._AddDefaultStorageInfoToLegacyNodeInfo(result, self.node_info[-1:])
     self.assertFalse("storage_free" in result)
     self.assertFalse("storage_size" in result)
 
-  def testAddDefaultStorageInfoToLegacyNodeInfoNoLvm(self):
-    result = {}
-    has_lvm = True
-    self.assertRaises(errors.OpExecError,
-                      rpc._AddDefaultStorageInfoToLegacyNodeInfo,
-                      result, self.node_info[-1:], has_lvm)
-
 
 if __name__ == "__main__":
   testutils.GanetiTestProgram()