Commit c89eb67d authored by Helga Velroyen's avatar Helga Velroyen

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: default avatarHelga Velroyen <helgav@google.com>
Reviewed-by: default avatarThomas Thrainer <thomasth@google.com>
parent 3f8567e1
......@@ -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.
......
......@@ -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()
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