On Wed, Dec 4, 2013 at 2:50 PM, Santi Raffa <[email protected]> wrote:
> The shared file and gluster disk templates should not report their disk > space information like file does, because they do not behave the same. > > If a cluster pulls from the same, shared source of storage then it is > not correct, or useful, to report per-node disk availability, as the > information is not node-specific. > > This change introduces the Shared File storage type for which no > per-node measuring of disk resources is made. > > Signed-off-by: Santi Raffa <[email protected]> > --- > lib/backend.py | 9 +++++++++ > lib/bootstrap.py | 6 ++++-- > lib/client/gnt_node.py | 1 + > lib/cmdlib/cluster.py | 7 +++++-- > lib/cmdlib/node.py | 22 ++++++++++++++++------ > lib/storage/container.py | 1 + > lib/utils/storage.py | 4 ++-- > qa/qa_cluster.py | 6 ++++-- > qa/qa_instance.py | 2 +- > src/Ganeti/Constants.hs | 7 +++++-- > src/Ganeti/Types.hs | 9 +++++++-- > test/py/ganeti.backend_unittest.py | 16 ++++++++++------ > test/py/ganeti.utils.storage_unittest.py | 6 ++++-- > 13 files changed, 69 insertions(+), 27 deletions(-) > > diff --git a/lib/backend.py b/lib/backend.py > index 8a0672e..51b76d0 100644 > --- a/lib/backend.py > +++ b/lib/backend.py > @@ -755,6 +755,14 @@ def GetNodeInfo(storage_units, hv_specs): > return (bootid, storage_info, hv_info) > > > +def _DeclineToProvideSharedFileStorageInfo(path, _): > This name is not coherent with the others, and doesn't quite reflect what it's doing. Find a better name. > + """Returns no storage data for the Shared File disk template. > + > + """ > + return {"type": constants.ST_SHARED_FILE, > + "name": path} > + > + > def _GetFileStorageSpaceInfo(path, params): > """Wrapper around filestorage.GetSpaceInfo. > > @@ -778,6 +786,7 @@ _STORAGE_TYPE_INFO_FN = { > constants.ST_FILE: _GetFileStorageSpaceInfo, > constants.ST_LVM_PV: _GetLvmPvSpaceInfo, > constants.ST_LVM_VG: _GetLvmVgSpaceInfo, > + constants.ST_SHARED_FILE: _DeclineToProvideSharedFileStorageInfo, > constants.ST_RADOS: None, > } > > diff --git a/lib/bootstrap.py b/lib/bootstrap.py > index 24f4245..a0185ff 100644 > --- a/lib/bootstrap.py > +++ b/lib/bootstrap.py > @@ -387,13 +387,15 @@ def _PrepareFileBasedStorage( > @param default_dir: default file storage directory when > C{file_storage_dir} > is 'None' > @type file_disk_template: string > - @param file_disk_template: a disk template whose storage type is > 'ST_FILE' > + @param file_disk_template: a disk template whose storage type is > 'ST_FILE' or > + 'ST_SHARED_FILE' > @rtype: string > @returns: the name of the actual file storage directory > > """ > assert (file_disk_template in > - utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE)) > + utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE, > + > constants.ST_SHARED_FILE)) > if file_storage_dir is None: > file_storage_dir = default_dir > if not acceptance_fn: > diff --git a/lib/client/gnt_node.py b/lib/client/gnt_node.py > index ada6a7a..47bfb5c 100644 > --- a/lib/client/gnt_node.py > +++ b/lib/client/gnt_node.py > @@ -89,6 +89,7 @@ _USER_STORAGE_TYPE = { > constants.ST_FILE: "file", > constants.ST_LVM_PV: "lvm-pv", > constants.ST_LVM_VG: "lvm-vg", > + constants.ST_SHARED_FILE: "sharedfile", > } > > _STORAGE_TYPE_OPT = \ > diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py > index 32fe0d3..a16d45a 100644 > --- a/lib/cmdlib/cluster.py > +++ b/lib/cmdlib/cluster.py > @@ -655,7 +655,8 @@ def CheckFileBasedStoragePathVsEnabledDiskTemplates( > > """ > assert (file_disk_template in > - utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE)) > + utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE, > + > constants.ST_SHARED_FILE)) > file_storage_enabled = file_disk_template in enabled_disk_templates > if file_storage_dir is not None: > if file_storage_dir == "": > @@ -2576,7 +2577,9 @@ class LUClusterVerifyGroup(LogicalUnit, > _VerifyErrors): > > """ > assert (file_disk_template in > - > utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE)) > + utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE, > + > constants.ST_SHARED_FILE > + )) > cluster = self.cfg.GetClusterInfo() > if cluster.IsDiskTemplateEnabled(file_disk_template): > self._ErrorIf( > diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py > index c8be6d69..a95fddc 100644 > --- a/lib/cmdlib/node.py > +++ b/lib/cmdlib/node.py > @@ -1112,11 +1112,19 @@ def _GetStorageTypeArgs(cfg, storage_type): > > """ > # Special case for file storage > - if storage_type == constants.ST_FILE: > - # storage.FileStorage wants a list of storage directories > - return [[cfg.GetFileStorageDir(), cfg.GetSharedFileStorageDir()]] > > - return [] > + if storage_type == constants.ST_FILE: > + return [[cfg.GetFileStorageDir()]] > + elif storage_type == constants.ST_SHARED_FILE: > + dts = cfg.GetClusterInfo().enabled_disk_templates > + paths = [] > + if constants.DT_SHARED_FILE in dts: > + paths.append(cfg.GetSharedFileStorageDir()) > + if constants.DT_GLUSTER in dts: > + paths.append(cfg.GetGlusterStorageDir()) > + return [paths] > + else: > + return [] > > > class LUNodeModifyStorage(NoHooksLU): > @@ -1302,13 +1310,15 @@ class LUNodeQueryStorage(NoHooksLU): > self.storage_type = self.op.storage_type > else: > self.storage_type = self._DetermineStorageType() > - if self.storage_type not in constants.STS_REPORT: > + supported_storage_types = set(constants.STS_REPORT) > + supported_storage_types.add(constants.ST_SHARED_FILE) > + if self.storage_type not in supported_storage_types: > Why can't we add ST_SHARED_FILE in STS_REPORT? It does do (dummy) reporting, right? In any case, I think having a special case here is not so great, so either go for and stick to the dummy reporting, or don't claim that it's supported at all (which also defeats the purpose of the _DeclineToProvideSharedFileStorageInfo function, right?). > raise errors.OpPrereqError( > "Storage reporting for storage type '%s' is not supported. > Please" > " use the --storage-type option to specify one of the > supported" > " storage types (%s) or set the default disk template to one > that" > " supports storage reporting." % > - (self.storage_type, utils.CommaJoin(constants.STS_REPORT))) > + (self.storage_type, utils.CommaJoin(supported_storage_types))) > > def Exec(self, feedback_fn): > """Computes the list of nodes and their attributes. > diff --git a/lib/storage/container.py b/lib/storage/container.py > index d77d80b..1c9c747 100644 > --- a/lib/storage/container.py > +++ b/lib/storage/container.py > @@ -468,6 +468,7 @@ _STORAGE_TYPES = { > constants.ST_FILE: FileStorage, > constants.ST_LVM_PV: LvmPvStorage, > constants.ST_LVM_VG: LvmVgStorage, > + constants.ST_SHARED_FILE: FileStorage, > } > > > diff --git a/lib/utils/storage.py b/lib/utils/storage.py > index ab8443a..e3b6cb9 100644 > --- a/lib/utils/storage.py > +++ b/lib/utils/storage.py > @@ -27,11 +27,11 @@ import logging > from ganeti import constants > > > -def GetDiskTemplatesOfStorageType(storage_type): > +def GetDiskTemplatesOfStorageType(*storage_types): > When changing this function to accept multiple parameters, you should also change its name to the plural form. > """Given the storage type, returns a list of disk templates based on > that > storage type.""" > return [dt for dt in constants.DISK_TEMPLATES > - if constants.MAP_DISK_TEMPLATE_STORAGE_TYPE[dt] == storage_type] > + if constants.MAP_DISK_TEMPLATE_STORAGE_TYPE[dt] in > storage_types] > > > def IsDiskTemplateEnabled(disk_template, enabled_disk_templates): > diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py > index 119472b..ef1d40b 100644 > --- a/qa/qa_cluster.py > +++ b/qa/qa_cluster.py > @@ -498,8 +498,10 @@ def TestClusterModifyFileBasedStorageDir( > > # Get some non-file-based disk template to disable file storage > other_disk_template = _GetOtherEnabledDiskTemplate( > - utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE), > - enabled_disk_templates) > + utils.storage.GetDiskTemplatesOfStorageType(constants.ST_FILE, > + constants.ST_SHARED_FILE), > + enabled_disk_templates > + ) > > file_storage_dir = qa_config.get(dir_config_key, default_dir) > invalid_file_storage_dir = "/boot/" > diff --git a/qa/qa_instance.py b/qa/qa_instance.py > index f3e4710..afbc853 100644 > --- a/qa/qa_instance.py > +++ b/qa/qa_instance.py > @@ -135,7 +135,7 @@ def _DestroyInstanceDisks(instance): > vols = info["volumes"] > for node in info["nodes"]: > AssertCommand(["lvremove", "-f"] + vols, node=node) > - elif info["storage-type"] == constants.ST_FILE: > + elif info["storage-type"] in (constants.ST_FILE, > constants.ST_SHARED_FILE): > # Note that this works for both file and sharedfile, and this is > intended. > storage_dir = qa_config.get("file-storage-dir", > pathutils.DEFAULT_FILE_STORAGE_DIR) > diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs > index 654429e..fc5c357 100644 > --- a/src/Ganeti/Constants.hs > +++ b/src/Ganeti/Constants.hs > @@ -659,6 +659,9 @@ stExt = Types.storageTypeToRaw StorageExt > stFile :: String > stFile = Types.storageTypeToRaw StorageFile > > +stSharedFile :: String > +stSharedFile = Types.storageTypeToRaw StorageSharedFile > + > stLvmPv :: String > stLvmPv = Types.storageTypeToRaw StorageLvmPv > > @@ -818,12 +821,12 @@ mapDiskTemplateStorageType = > [(DTBlock, StorageBlock), > (DTDrbd8, StorageLvmVg), > (DTExt, StorageExt), > - (DTSharedFile, StorageFile), > + (DTSharedFile, StorageSharedFile), > (DTFile, StorageFile), > (DTDiskless, StorageDiskless), > (DTPlain, StorageLvmVg), > (DTRbd, StorageRados), > - (DTGluster, StorageFile)] > + (DTGluster, StorageSharedFile)] > > -- | The set of network-mirrored disk templates > dtsIntMirror :: FrozenSet String > diff --git a/src/Ganeti/Types.hs b/src/Ganeti/Types.hs > index 3200336..42201d9 100644 > --- a/src/Ganeti/Types.hs > +++ b/src/Ganeti/Types.hs > @@ -459,6 +459,7 @@ $(THH.makeJSONInstance ''OobStatus) > -- | Storage type. > $(THH.declareLADT ''String "StorageType" > [ ("StorageFile", "file") > + , ("StorageSharedFile", "sharedfile") > , ("StorageLvmPv", "lvm-pv") > , ("StorageLvmVg", "lvm-vg") > , ("StorageDiskless", "diskless") > @@ -481,6 +482,7 @@ data StorageUnitRaw = SURaw StorageType StorageKey > > -- | Full storage unit with storage-type-specific parameters > data StorageUnit = SUFile StorageKey > + | SUSharedFile StorageKey > | SULvmPv StorageKey SPExclusiveStorage > | SULvmVg StorageKey SPExclusiveStorage > | SUDiskless StorageKey > @@ -491,6 +493,7 @@ data StorageUnit = SUFile StorageKey > > instance Show StorageUnit where > show (SUFile key) = showSUSimple StorageFile key > + show (SUSharedFile key) = showSUSimple StorageSharedFile key > show (SULvmPv key es) = showSULvm StorageLvmPv key es > show (SULvmVg key es) = showSULvm StorageLvmVg key es > show (SUDiskless key) = showSUSimple StorageDiskless key > @@ -500,6 +503,7 @@ instance Show StorageUnit where > > instance JSON StorageUnit where > showJSON (SUFile key) = showJSON (StorageFile, key, []::[String]) > + showJSON (SUSharedFile key) = showJSON (StorageSharedFile, key, > []::[String]) > showJSON (SULvmPv key es) = showJSON (StorageLvmPv, key, [es]) > showJSON (SULvmVg key es) = showJSON (StorageLvmVg, key, [es]) > showJSON (SUDiskless key) = showJSON (StorageDiskless, key, > []::[String]) > @@ -525,13 +529,13 @@ showSULvm st sk es = show (storageTypeToRaw st, sk, > [es]) > diskTemplateToStorageType :: DiskTemplate -> StorageType > diskTemplateToStorageType DTExt = StorageExt > diskTemplateToStorageType DTFile = StorageFile > -diskTemplateToStorageType DTSharedFile = StorageFile > +diskTemplateToStorageType DTSharedFile = StorageSharedFile > diskTemplateToStorageType DTDrbd8 = StorageLvmVg > diskTemplateToStorageType DTPlain = StorageLvmVg > diskTemplateToStorageType DTRbd = StorageRados > diskTemplateToStorageType DTDiskless = StorageDiskless > diskTemplateToStorageType DTBlock = StorageBlock > -diskTemplateToStorageType DTGluster = StorageFile > +diskTemplateToStorageType DTGluster = StorageSharedFile > > -- | Equips a raw storage unit with its parameters > addParamsToStorageUnit :: SPExclusiveStorage -> StorageUnitRaw -> > StorageUnit > @@ -539,6 +543,7 @@ addParamsToStorageUnit _ (SURaw StorageBlock key) = > SUBlock key > addParamsToStorageUnit _ (SURaw StorageDiskless key) = SUDiskless key > addParamsToStorageUnit _ (SURaw StorageExt key) = SUExt key > addParamsToStorageUnit _ (SURaw StorageFile key) = SUFile key > +addParamsToStorageUnit _ (SURaw StorageSharedFile key) = SUSharedFile key > addParamsToStorageUnit es (SURaw StorageLvmPv key) = SULvmPv key es > addParamsToStorageUnit es (SURaw StorageLvmVg key) = SULvmVg key es > addParamsToStorageUnit _ (SURaw StorageRados key) = SURados key > diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ > ganeti.backend_unittest.py > index 226c8f3..6651cd9 100755 > --- a/test/py/ganeti.backend_unittest.py > +++ b/test/py/ganeti.backend_unittest.py > @@ -839,20 +839,24 @@ class TestGetNodeInfo(unittest.TestCase): > class TestSpaceReportingConstants(unittest.TestCase): > """Ensures consistency between STS_REPORT and backend. > > - These tests ensure, that the constant 'STS_REPORT' is consitent > + These tests ensure, that the constant 'STS_REPORT' is consistent > with the implementation of invoking space reporting functions > in backend.py. Once space reporting is available for all types, > the constant can be removed and these tests as well. > > """ > + > + REPORTING = set(constants.STS_REPORT) > + REPORTING.add(constants.ST_SHARED_FILE) > Such special handling should not exist. > + > + NOT_REPORTING = set(constants.STORAGE_TYPES) - REPORTING > + > def testAllReportingTypesHaveAReportingFunction(self): > - for storage_type in constants.STS_REPORT: > + for storage_type in TestSpaceReportingConstants.REPORTING: > self.assertTrue(backend._STORAGE_TYPE_INFO_FN[storage_type] is not > None) > > - def testAllNotReportingTypesDoneHaveFunction(self): > - non_reporting_types = set(constants.STORAGE_TYPES)\ > - - set(constants.STS_REPORT) > - for storage_type in non_reporting_types: > + def testAllNotReportingTypesDontHaveFunction(self): > + for storage_type in TestSpaceReportingConstants.NOT_REPORTING: > self.assertEqual(None, backend._STORAGE_TYPE_INFO_FN[storage_type]) > > > diff --git a/test/py/ganeti.utils.storage_unittest.py b/test/py/ > ganeti.utils.storage_unittest.py > index 92b1648..c48cd0c 100755 > --- a/test/py/ganeti.utils.storage_unittest.py > +++ b/test/py/ganeti.utils.storage_unittest.py > @@ -61,7 +61,7 @@ class > TestGetStorageUnitForDiskTemplate(unittest.TestCase): > (storage_type, storage_key) = \ > storage._GetDefaultStorageUnitForDiskTemplate(self._cfg, > > constants.DT_SHARED_FILE) > - self.assertEqual(storage_type, constants.ST_FILE) > + self.assertEqual(storage_type, constants.ST_SHARED_FILE) > self.assertEqual(storage_key, self._cluster.shared_file_storage_dir) > > def testGetDefaultStorageUnitForDiskTemplateDiskless(self): > @@ -80,7 +80,9 @@ class TestGetStorageUnits(unittest.TestCase): > self._cfg = mock.Mock() > > def testGetStorageUnits(self): > - disk_templates = constants.DTS_FILEBASED > + disk_templates = constants.DTS_FILEBASED - frozenset( > + storage.GetDiskTemplatesOfStorageType(constants.ST_SHARED_FILE) > + ) > storage_units = storage.GetStorageUnits(self._cfg, disk_templates) > self.assertEqual(len(storage_units), len(disk_templates)) > > -- > 1.8.4.1 > > -- Thomas Thrainer | Software Engineer | [email protected] | Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
