Commit 6c00b2c7 authored by Helga Velroyen's avatar Helga Velroyen
Browse files

Streamlining handling of spindles and default templates

This rather lenghy patch comprises a couple of refactorings
to achieve the following goals:
- 'gnt-node info' should only report space information about
  spindles, when exclusive storage is enabled on the node
  (so far it always did when lvm-based storage was enabled).
- 'gnt-node info' should report the disks' space information
  for the default disk template's storage. So far it did it
  always for LVM, which is irrelevant for a cluster running
  for example file-based instances.
- Performance tweaking of storage reporting and gnt-node
- Consider the future work on storage pools for any changes.

This is is achieved by the following refactorings:
- Move the decision whether or not to ask for spindle
  from cmdlib-level to rpc level.
- Move the decision for which disk template's storage
  information to ask for to cmdlib level (instead of
  requesting information for all storage units and
  later discarding most of them).
- Adjustments to storage utility functions.
- Affected unit tests are adjusted as well.
Signed-off-by: default avatarHelga Velroyen <>
Reviewed-by: default avatarKlaus Aehlig <>
parent 4d1429e4
......@@ -1205,13 +1205,9 @@ 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 =
# 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 =
lu.cfg, include_spindles=lvm_enabled)
default_template = lu.cfg.GetClusterInfo().enabled_disk_templates[0]
raw_storage_units =
lu.cfg, [default_template])
storage_units = rpc.PrepareStorageUnitsForNodes(
lu.cfg, raw_storage_units, toquery_node_uuids)
default_hypervisor = lu.cfg.GetHypervisorType()
......@@ -1220,8 +1216,7 @@ class NodeQuery(QueryBase):
node_data = lu.rpc.call_node_info(toquery_node_uuids, storage_units,
live_data = dict(
(uuid, rpc.MakeLegacyNodeInfo(nresult.payload,
(uuid, rpc.MakeLegacyNodeInfo(nresult.payload, default_template))
for (uuid, nresult) in node_data.items()
if not nresult.fail_msg and nresult.payload)
......@@ -415,13 +415,7 @@ class IAllocator(object):
@return: the result of the node info RPC call
if disk_templates:
storage_units_raw =,
# FIXME: eliminate this case
storage_units_raw =
self.cfg, include_spindles=True)
storage_units_raw =, disk_templates)
storage_units = rpc.PrepareStorageUnitsForNodes(self.cfg, storage_units_raw,
hvspecs = [(hypervisor_name, cluster_info.hvparams[hypervisor_name])]
......@@ -587,17 +581,14 @@ class IAllocator(object):
total_disk = template_space_info["storage_size"]
free_disk = template_space_info["storage_free"]
total_spindles = 0
free_spindles = 0
if disk_template in constants.DTS_LVM:
lvm_pv_info =
space_info, constants.ST_LVM_PV)
if not lvm_pv_info:
raise errors.OpExecError("Node '%s' didn't return LVM pv space info."
% (node_name))
total_spindles = lvm_pv_info["storage_size"]
free_spindles = lvm_pv_info["storage_free"]
total_spindles = 0
free_spindles = 0
if lvm_pv_info:
total_spindles = lvm_pv_info["storage_size"]
free_spindles = lvm_pv_info["storage_free"]
return (total_disk, free_disk, total_spindles, free_spindles)
......@@ -586,49 +586,43 @@ def _AddSpindlesToLegacyNodeInfo(result, space_info):
result["spindles_free"] = lvm_pv_info["storage_free"]
result["spindles_total"] = lvm_pv_info["storage_size"]
raise errors.OpExecError("No spindle storage information available.")
result["spindles_free"] = 0
result["spindles_total"] = 0
def _AddDefaultStorageInfoToLegacyNodeInfo(result, space_info):
"""Extracts the storage space information of the default storage type from
def _AddStorageInfoToLegacyNodeInfoByTemplate(
result, space_info, disk_template):
"""Extracts the storage space information of the disk template from
the space info and adds it to the result dictionary.
@see: C{_AddSpindlesToLegacyNodeInfo} for parameter information.
# Check if there is at least one row for non-spindle storage info.
no_defaults = (len(space_info) < 1) or \
(space_info[0]["type"] == constants.ST_LVM_PV and len(space_info) == 1)
default_space_info = None
if no_defaults:
logging.warning("No storage info provided for default storage type.")
disk_info =
space_info, disk_template)
result["name"] = disk_info["name"]
result["storage_free"] = disk_info["storage_free"]
result["storage_size"] = disk_info["storage_size"]
default_space_info = space_info[0]
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"]
# FIXME: consider displaying '-' in this case
result["storage_free"] = 0
result["storage_size"] = 0
def MakeLegacyNodeInfo(data, require_spindles=False):
def MakeLegacyNodeInfo(data, disk_template):
"""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_spindles: add spindle storage information to the legacy node
(bootid, space_info, (hv_info, )) = data
ret = utils.JoinDisjointDicts(hv_info, {"bootid": bootid})
if require_spindles:
_AddSpindlesToLegacyNodeInfo(ret, space_info)
_AddDefaultStorageInfoToLegacyNodeInfo(ret, space_info)
_AddSpindlesToLegacyNodeInfo(ret, space_info)
_AddStorageInfoToLegacyNodeInfoByTemplate(ret, space_info, disk_template)
return ret
......@@ -95,50 +95,10 @@ def _GetDefaultStorageUnitForDiskTemplate(cfg, disk_template):
return (storage_type, None)
def _GetDefaultStorageUnitForSpindles(cfg):
"""Creates a 'spindle' storage unit, by retrieving the volume group
name and associating it to the lvm-pv storage type.
@rtype: (string, string)
@return: tuple (storage_type, storage_key), where storage type is
'lvm-pv' and storage_key the name of the default volume group
return (constants.ST_LVM_PV, cfg.GetVGName())
def GetStorageUnitsOfCluster(cfg, include_spindles=False):
"""Examines the cluster's configuration and returns a list of storage
units and their storage keys, ordered by the order in which they
are enabled.
@type cfg: L{config.ConfigWriter}
@param cfg: Cluster configuration
@type include_spindles: boolean
@param include_spindles: flag to include an extra storage unit for physical
@rtype: list of tuples (string, string)
@return: list of storage units, each storage unit being a tuple of
(storage_type, storage_key); storage_type is in
C{constants.STORAGE_TYPES} and the storage_key a string to
identify an entity of that storage type, for example a volume group
name for LVM storage or a file for file storage.
cluster_config = cfg.GetClusterInfo()
storage_units = []
for disk_template in cluster_config.enabled_disk_templates:
if constants.MAP_DISK_TEMPLATE_STORAGE_TYPE[disk_template]\
in constants.STS_REPORT:
_GetDefaultStorageUnitForDiskTemplate(cfg, disk_template))
if include_spindles:
included_storage_types = set([st for (st, _) in storage_units])
if not constants.ST_LVM_PV in included_storage_types:
return storage_units
def DiskTemplateSupportsSpaceReporting(disk_template):
"""Check whether the disk template supports storage space reporting."""
return (constants.MAP_DISK_TEMPLATE_STORAGE_TYPE[disk_template]
in constants.STS_REPORT)
def GetStorageUnits(cfg, disk_templates):
......@@ -162,14 +122,9 @@ def GetStorageUnits(cfg, disk_templates):
storage_units = []
for disk_template in disk_templates:
if constants.MAP_DISK_TEMPLATE_STORAGE_TYPE[disk_template]\
in constants.STS_REPORT:
if DiskTemplateSupportsSpaceReporting(disk_template):
_GetDefaultStorageUnitForDiskTemplate(cfg, disk_template))
if len(set(disk_templates) & constants.DTS_LVM) > 0:
return storage_units
......@@ -943,58 +943,22 @@ class TestLegacyNodeInfo(unittest.TestCase):
def testStandard(self):
result = rpc.MakeLegacyNodeInfo(self.STD_LST)
def testWithSpindles(self):
result = rpc.MakeLegacyNodeInfo(self.STD_LST, constants.DT_PLAIN)
self.assertEqual(result, self.STD_DICT)
def testSpindlesRequired(self):
my_lst = [self.VAL_BOOT, [], [self.DICT_HV]]
self.assertRaises(errors.OpExecError, rpc.MakeLegacyNodeInfo, my_lst,
def testNoSpindlesRequired(self):
my_lst = [self.VAL_BOOT, [], [self.DICT_HV]]
result = rpc.MakeLegacyNodeInfo(my_lst, require_spindles = False)
self.assertEqual(result, {self.KEY_BOOT: self.VAL_BOOT,
result = rpc.MakeLegacyNodeInfo(self.STD_LST, require_spindles = False)
self.assertEqual(result, self.STD_DICT)
class TestAddDefaultStorageInfoToLegacyNodeInfo(unittest.TestCase):
def setUp(self):
self.free_storage_file = 23
self.total_storage_file = 42
self.free_storage_lvm = 69
self.total_storage_lvm = 666
self.node_info = [{"name": "myfile",
"type": constants.ST_FILE,
"storage_free": self.free_storage_file,
"storage_size": self.total_storage_file},
{"name": "myvg",
"type": constants.ST_LVM_VG,
"storage_free": self.free_storage_lvm,
"storage_size": self.total_storage_lvm},
{"name": "myspindle",
"type": constants.ST_LVM_PV,
"storage_free": 33,
"storage_size": 44}]
def testAddDefaultStorageInfoToLegacyNodeInfo(self):
result = {}
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 testAddDefaultStorageInfoToLegacyNodeInfoNoDefaults(self):
result = {}
rpc._AddDefaultStorageInfoToLegacyNodeInfo(result, self.node_info[-1:])
self.assertFalse("storage_free" in result)
self.assertFalse("storage_size" in result)
def testNoSpindles(self):
my_lst = [self.VAL_BOOT, [self.DICT_VG], [self.DICT_HV]]
result = rpc.MakeLegacyNodeInfo(my_lst, constants.DT_PLAIN)
expected_dict = dict((k,v) for k, v in self.STD_DICT.iteritems())
expected_dict[self.KEY_SPINDLES_FREE] = 0
expected_dict[self.KEY_SPINDLES_TOTAL] = 0
self.assertEqual(result, expected_dict)
if __name__ == "__main__":
......@@ -26,7 +26,6 @@ import mock
import unittest
from ganeti import constants
from ganeti import objects
from ganeti.utils import storage
import testutils
......@@ -72,39 +71,6 @@ class TestGetStorageUnitForDiskTemplate(unittest.TestCase):
self.assertEqual(storage_type, constants.ST_DISKLESS)
self.assertEqual(storage_key, None)
def testGetDefaultStorageUnitForSpindles(self):
(storage_type, storage_key) = \
self.assertEqual(storage_type, constants.ST_LVM_PV)
self.assertEqual(storage_key, self._default_vg_name)
class TestGetStorageUnitsOfCluster(unittest.TestCase):
def setUp(self):
storage._GetDefaultStorageUnitForDiskTemplate = \
mock.Mock(return_value=("foo", "bar"))
self._cluster_cfg = objects.Cluster()
self._enabled_disk_templates = \
[constants.DT_DRBD8, constants.DT_PLAIN, constants.DT_FILE,
self._cluster_cfg.enabled_disk_templates = \
self._cfg = mock.Mock()
self._cfg.GetClusterInfo = mock.Mock(return_value=self._cluster_cfg)
self._cfg.GetVGName = mock.Mock(return_value="some_vg_name")
def testGetStorageUnitsOfCluster(self):
storage_units = storage.GetStorageUnitsOfCluster(self._cfg)
self.assertEqual(len(storage_units), len(self._enabled_disk_templates))
def testGetStorageUnitsOfClusterWithSpindles(self):
storage_units = storage.GetStorageUnitsOfCluster(
self._cfg, include_spindles=True)
self.assertEqual(len(storage_units), len(self._enabled_disk_templates) + 1)
self.assertTrue(constants.ST_LVM_PV in [st for (st, sk) in storage_units])
class TestGetStorageUnits(unittest.TestCase):
......@@ -121,7 +87,7 @@ class TestGetStorageUnits(unittest.TestCase):
def testGetStorageUnitsLvm(self):
disk_templates = [constants.DT_PLAIN, constants.DT_DRBD8]
storage_units = storage.GetStorageUnits(self._cfg, disk_templates)
self.assertEqual(len(storage_units), len(disk_templates) + 1)
self.assertEqual(len(storage_units), len(disk_templates))
class TestLookupSpaceInfoByStorageType(unittest.TestCase):
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment