From c89eb67de10a11c29c181fad11c749305f8a8638 Mon Sep 17 00:00:00 2001 From: Helga Velroyen Date: Wed, 7 Aug 2013 17:52:38 +0200 Subject: [PATCH] ClusterSetParams: move vg-name checks from to CheckPrereq This fixes a bug in the logic of 'gnt-cluster modify'. Some checks that should better be done in 'CheckPrereq' were actually done in 'Exec'. This lead to a strange behavior in case the execution failed, because an inconsistent state of the cluster's config was kept in memory. Signed-off-by: Helga Velroyen Reviewed-by: Thomas Thrainer --- lib/cmdlib/cluster.py | 54 +++++++++++++++++------------ test/py/ganeti.cmdlib.cluster_unittest.py | 12 +++++++ 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py index 2b93e93..32e24de 100644 --- a/lib/cmdlib/cluster.py +++ b/lib/cmdlib/cluster.py @@ -741,16 +741,28 @@ class LUClusterSetParams(LogicalUnit): unset whether there are instances still using it. """ + lvm_is_enabled = utils.IsLvmEnabled(enabled_disk_templates) + lvm_gets_enabled = utils.LvmGetsEnabled(enabled_disk_templates, + new_enabled_disk_templates) + current_vg_name = self.cfg.GetVGName() + + if self.op.vg_name == '': + if lvm_is_enabled: + raise errors.OpPrereqError("Cannot unset volume group if lvm-based" + " disk templates are or get enabled.") + + if self.op.vg_name is None: + if current_vg_name is None and lvm_is_enabled: + raise errors.OpPrereqError("Please specify a volume group when" + " enabling lvm-based disk-templates.") + if self.op.vg_name is not None and not self.op.vg_name: if self.cfg.HasAnyDiskOfType(constants.LD_LV): raise errors.OpPrereqError("Cannot disable lvm storage while lvm-based" " instances exist", errors.ECODE_INVAL) - if (self.op.vg_name is not None and - utils.IsLvmEnabled(enabled_disk_templates)) or \ - (self.cfg.GetVGName() is not None and - utils.LvmGetsEnabled(enabled_disk_templates, - new_enabled_disk_templates)): + if (self.op.vg_name is not None and lvm_is_enabled) or \ + (self.cfg.GetVGName() is not None and lvm_gets_enabled): self._CheckVgNameOnNodes(node_uuids) def _CheckVgNameOnNodes(self, node_uuids): @@ -774,22 +786,32 @@ class LUClusterSetParams(LogicalUnit): (self.cfg.GetNodeName(node_uuid), vgstatus), errors.ECODE_ENVIRON) - def _GetEnabledDiskTemplates(self, cluster): + @staticmethod + def _GetEnabledDiskTemplatesInner(op_enabled_disk_templates, + old_enabled_disk_templates): """Determines the enabled disk templates and the subset of disk templates that are newly enabled by this operation. """ enabled_disk_templates = None new_enabled_disk_templates = [] - if self.op.enabled_disk_templates: - enabled_disk_templates = self.op.enabled_disk_templates + if op_enabled_disk_templates: + enabled_disk_templates = op_enabled_disk_templates new_enabled_disk_templates = \ list(set(enabled_disk_templates) - - set(cluster.enabled_disk_templates)) + - set(old_enabled_disk_templates)) else: - enabled_disk_templates = cluster.enabled_disk_templates + enabled_disk_templates = old_enabled_disk_templates return (enabled_disk_templates, new_enabled_disk_templates) + def _GetEnabledDiskTemplates(self, cluster): + """Determines the enabled disk templates and the subset of disk templates + that are newly enabled by this operation. + + """ + return self._GetEnabledDiskTemplatesInner(self.op.enabled_disk_templates, + cluster.enabled_disk_templates) + def _CheckIpolicy(self, cluster, enabled_disk_templates): """Checks the ipolicy. @@ -1069,26 +1091,14 @@ class LUClusterSetParams(LogicalUnit): """ if self.op.vg_name is not None: - if self.op.vg_name and not \ - utils.IsLvmEnabled(self.cluster.enabled_disk_templates): - feedback_fn("Note that you specified a volume group, but did not" - " enable any lvm disk template.") new_volume = self.op.vg_name if not new_volume: - if utils.IsLvmEnabled(self.cluster.enabled_disk_templates): - raise errors.OpPrereqError("Cannot unset volume group if lvm-based" - " disk templates are enabled.") new_volume = None if new_volume != self.cfg.GetVGName(): self.cfg.SetVGName(new_volume) else: feedback_fn("Cluster LVM configuration already in desired" " state, not changing") - else: - if utils.IsLvmEnabled(self.cluster.enabled_disk_templates) and \ - not self.cfg.GetVGName(): - raise errors.OpPrereqError("Please specify a volume group when" - " enabling lvm-based disk-templates.") def _SetFileStorageDir(self, feedback_fn): """Set the file storage directory. diff --git a/test/py/ganeti.cmdlib.cluster_unittest.py b/test/py/ganeti.cmdlib.cluster_unittest.py index 7667362..819b401 100755 --- a/test/py/ganeti.cmdlib.cluster_unittest.py +++ b/test/py/ganeti.cmdlib.cluster_unittest.py @@ -76,5 +76,17 @@ class TestCheckFileStoragePath(unittest.TestCase): NotImplemented, "", self.enabled_disk_templates) +class TestGetEnabledDiskTemplates(unittest.TestCase): + + def testNoNew(self): + op_dts = [constants.DT_DISKLESS] + old_dts = [constants.DT_DISKLESS] + (enabled_dts, new_dts) =\ + cluster.LUClusterSetParams._GetEnabledDiskTemplatesInner( + op_dts, old_dts) + self.assertEqual(enabled_dts, old_dts) + self.assertEqual(new_dts, []) + + if __name__ == "__main__": testutils.GanetiTestProgram() -- 1.7.10.4