From 912737ba13f35a062ce81c2de147aacd8ffb7ead Mon Sep 17 00:00:00 2001 From: Helga Velroyen Date: Tue, 2 Apr 2013 13:16:46 +0200 Subject: [PATCH] gnt-cluster modify/init: deprecate --no-lvm-storage This patch does three things: 1. It deprecates the option '--no-lvm-storage' of 'gnt-cluster modify' and 'gnt-cluster init'. Technically, it is not fully removed, but kept in order to warn the user that it is no longer supported and that she should use --enabled-disk-templates instead. 2. The consistency check between '--no-lvm-storage' and '--vg-name' is replaced by checks between '--enabled-disk-templates' and '--vg-name'. There are these cases: - vg name, lvm disk template enabled = ok - no vg name, lvm disk template enabled = error - vg name, no lvm enabled = warning - no vg name, no lvm enabled = ok I added quite a lot of tests for all these and the transitions from each case to another to the QA. 3. The check whether or not the volume group is available on all nodes is now done only in these cases: - the volume group name gets set and lvm is already enabled - lvm is getting enabled and the volume group was set before Signed-off-by: Helga Velroyen Reviewed-by: Guido Trotter --- lib/bootstrap.py | 6 +-- lib/client/gnt_cluster.py | 70 ++++++++++++++++++-------- lib/cmdlib.py | 115 ++++++++++++++++++++++++++++++++---------- qa/qa_cluster.py | 122 +++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 250 insertions(+), 63 deletions(-) diff --git a/lib/bootstrap.py b/lib/bootstrap.py index 0d7a4bc..3557838 100644 --- a/lib/bootstrap.py +++ b/lib/bootstrap.py @@ -461,14 +461,12 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914 else: master_netmask = ipcls.iplen - if vg_name is not None: + if vg_name: # Check if volume group is valid vgstatus = utils.CheckVolumeGroupSize(utils.ListVolumeGroups(), vg_name, constants.MIN_VG_SIZE) if vgstatus: - raise errors.OpPrereqError("Error: %s\nspecify --no-lvm-storage if" - " you are not using lvm" % vgstatus, - errors.ECODE_INVAL) + raise errors.OpPrereqError("Error: %s" % vgstatus, errors.ECODE_INVAL) if drbd_helper is not None: try: diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py index 4183b9b..c4141ec 100644 --- a/lib/client/gnt_cluster.py +++ b/lib/client/gnt_cluster.py @@ -62,6 +62,19 @@ _EPO_PING_TIMEOUT = 1 # 1 second _EPO_REACHABLE_TIMEOUT = 15 * 60 # 15 minutes +def _CheckNoLvmStorageOptDeprecated(opts): + """Checks if the legacy option '--no-lvm-storage' is used. + + """ + if not opts.lvm_storage: + ToStderr("The option --no-lvm-storage is no longer supported. If you want" + " to disable lvm-based storage cluster-wide, use the option" + " --enabled-disk-templates to disable all of these lvm-base disk " + " templates: %s" % + utils.CommaJoin(utils.GetLvmDiskTemplates())) + return 1 + + @UsesRPC def InitCluster(opts, args): """Initialize the cluster. @@ -74,13 +87,28 @@ def InitCluster(opts, args): @return: the desired exit code """ - if not opts.lvm_storage and opts.vg_name: - ToStderr("Options --no-lvm-storage and --vg-name conflict.") + if _CheckNoLvmStorageOptDeprecated(opts): return 1 + enabled_disk_templates = opts.enabled_disk_templates + if enabled_disk_templates: + enabled_disk_templates = enabled_disk_templates.split(",") + else: + enabled_disk_templates = list(constants.DEFAULT_ENABLED_DISK_TEMPLATES) - vg_name = opts.vg_name - if opts.lvm_storage and not opts.vg_name: - vg_name = constants.DEFAULT_VG + vg_name = None + if opts.vg_name is not None: + vg_name = opts.vg_name + if vg_name: + if not utils.IsLvmEnabled(enabled_disk_templates): + ToStdout("You specified a volume group with --vg-name, but you did not" + " enable any disk template that uses lvm.") + else: + if utils.IsLvmEnabled(enabled_disk_templates): + ToStderr("LVM disk templates are enabled, but vg name not set.") + return 1 + else: + if utils.IsLvmEnabled(enabled_disk_templates): + vg_name = constants.DEFAULT_VG if not opts.drbd_storage and opts.drbd_helper: ToStderr("Options --no-drbd-storage and --drbd-usermode-helper conflict.") @@ -193,12 +221,6 @@ def InitCluster(opts, args): hv_state = dict(opts.hv_state) - enabled_disk_templates = opts.enabled_disk_templates - if enabled_disk_templates: - enabled_disk_templates = enabled_disk_templates.split(",") - else: - enabled_disk_templates = list(constants.DEFAULT_ENABLED_DISK_TEMPLATES) - bootstrap.InitCluster(cluster_name=args[0], secondary_ip=opts.secondary_ip, vg_name=vg_name, @@ -945,8 +967,7 @@ def SetClusterParams(opts, args): @return: the desired exit code """ - if not (not opts.lvm_storage or opts.vg_name or - not opts.drbd_storage or opts.drbd_helper or + if not (opts.vg_name is not None or opts.drbd_helper or opts.enabled_hypervisors or opts.hvparams or opts.beparams or opts.nicparams or opts.ndparams or opts.diskparams or @@ -975,13 +996,22 @@ def SetClusterParams(opts, args): ToStderr("Please give at least one of the parameters.") return 1 - vg_name = opts.vg_name - if not opts.lvm_storage and opts.vg_name: - ToStderr("Options --no-lvm-storage and --vg-name conflict.") + if _CheckNoLvmStorageOptDeprecated(opts): return 1 - if not opts.lvm_storage: - vg_name = "" + enabled_disk_templates = None + if opts.enabled_disk_templates: + enabled_disk_templates = opts.enabled_disk_templates.split(",") + + # consistency between vg name and enabled disk templates + vg_name = None + if opts.vg_name is not None: + vg_name = opts.vg_name + if enabled_disk_templates: + if vg_name and not utils.IsLvmEnabled(enabled_disk_templates): + ToStdout("You specified a volume group with --vg-name, but you did not" + " enable any of the following lvm-based disk templates: %s" % + utils.CommaJoin(utils.GetLvmDiskTemplates())) drbd_helper = opts.drbd_helper if not opts.drbd_storage and opts.drbd_helper: @@ -995,10 +1025,6 @@ def SetClusterParams(opts, args): if hvlist is not None: hvlist = hvlist.split(",") - enabled_disk_templates = opts.enabled_disk_templates - if enabled_disk_templates: - enabled_disk_templates = enabled_disk_templates.split(",") - # a list of (name, dict) we can pass directly to dict() (or []) hvparams = dict(opts.hvparams) for hv_params in hvparams.values(): diff --git a/lib/cmdlib.py b/lib/cmdlib.py index d6f16d0..bba8277 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -4212,11 +4212,10 @@ class LUClusterSetParams(LogicalUnit): mn = self.cfg.GetMasterNode() return ([mn], [mn]) - def CheckPrereq(self): - """Check prerequisites. - - This checks whether the given params don't conflict and - if the given volume group is valid. + def _CheckVgName(self, node_list, enabled_disk_templates, + new_enabled_disk_templates): + """Check the consistency of the vg name on all nodes and in case it gets + unset whether there are instances still using it. """ if self.op.vg_name is not None and not self.op.vg_name: @@ -4224,6 +4223,55 @@ class LUClusterSetParams(LogicalUnit): 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)): + self._CheckVgNameOnNodes(node_list) + + def _CheckVgNameOnNodes(self, node_list): + """Check the status of the volume group on each node. + + """ + vglist = self.rpc.call_vg_list(node_list) + for node in node_list: + msg = vglist[node].fail_msg + if msg: + # ignoring down node + self.LogWarning("Error while gathering data on node %s" + " (ignoring node): %s", node, msg) + continue + vgstatus = utils.CheckVolumeGroupSize(vglist[node].payload, + self.op.vg_name, + constants.MIN_VG_SIZE) + if vgstatus: + raise errors.OpPrereqError("Error on node '%s': %s" % + (node, vgstatus), errors.ECODE_ENVIRON) + + def _GetEnabledDiskTemplates(self, cluster): + """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 + new_enabled_disk_templates = \ + list(set(enabled_disk_templates) + - set(cluster.enabled_disk_templates)) + else: + enabled_disk_templates = cluster.enabled_disk_templates + return (enabled_disk_templates, new_enabled_disk_templates) + + def CheckPrereq(self): + """Check prerequisites. + + This checks whether the given params don't conflict and + if the given volume group is valid. + + """ if self.op.drbd_helper is not None and not self.op.drbd_helper: if self.cfg.HasAnyDiskOfType(constants.LD_DRBD8): raise errors.OpPrereqError("Cannot disable drbd helper while" @@ -4231,23 +4279,13 @@ class LUClusterSetParams(LogicalUnit): errors.ECODE_INVAL) node_list = self.owned_locks(locking.LEVEL_NODE) + self.cluster = cluster = self.cfg.GetClusterInfo() - # if vg_name not None, checks given volume group on all nodes - if self.op.vg_name: - vglist = self.rpc.call_vg_list(node_list) - for node in node_list: - msg = vglist[node].fail_msg - if msg: - # ignoring down node - self.LogWarning("Error while gathering data on node %s" - " (ignoring node): %s", node, msg) - continue - vgstatus = utils.CheckVolumeGroupSize(vglist[node].payload, - self.op.vg_name, - constants.MIN_VG_SIZE) - if vgstatus: - raise errors.OpPrereqError("Error on node '%s': %s" % - (node, vgstatus), errors.ECODE_ENVIRON) + (enabled_disk_templates, new_enabled_disk_templates) = \ + self._GetEnabledDiskTemplates(cluster) + + self._CheckVgName(node_list, enabled_disk_templates, + new_enabled_disk_templates) if self.op.drbd_helper: # checks given drbd helper on all nodes @@ -4266,7 +4304,6 @@ class LUClusterSetParams(LogicalUnit): raise errors.OpPrereqError("Error on node '%s': drbd helper is %s" % (node, node_helper), errors.ECODE_ENVIRON) - self.cluster = cluster = self.cfg.GetClusterInfo() # validate params changes if self.op.beparams: objects.UpgradeBeParams(self.op.beparams) @@ -4468,20 +4505,47 @@ class LUClusterSetParams(LogicalUnit): " because instance '%s' is using it." % (instance.disk_template, instance.name)) - def Exec(self, feedback_fn): - """Change the parameters of the cluster. + + def _SetVgName(self, feedback_fn): + """Determines and sets the new volume group name. """ 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 Exec(self, feedback_fn): + """Change the parameters of the cluster. + + """ + if self.op.enabled_disk_templates: + self.cluster.enabled_disk_templates = \ + list(set(self.op.enabled_disk_templates)) + + self._SetVgName(feedback_fn) + if self.op.drbd_helper is not None: + if not constants.DT_DRBD8 in self.cluster.enabled_disk_templates: + feedback_fn("Note that you specified a drbd user helper, but did" + " enabled the drbd disk template.") new_helper = self.op.drbd_helper if not new_helper: new_helper = None @@ -4497,9 +4561,6 @@ class LUClusterSetParams(LogicalUnit): if self.op.enabled_hypervisors is not None: self.cluster.hvparams = self.new_hvparams self.cluster.enabled_hypervisors = self.op.enabled_hypervisors - if self.op.enabled_disk_templates: - self.cluster.enabled_disk_templates = \ - list(set(self.op.enabled_disk_templates)) if self.op.beparams: self.cluster.beparams[constants.PP_DEFAULT] = self.new_beparams if self.op.nicparams: diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py index c4de57a..061b13c 100644 --- a/qa/qa_cluster.py +++ b/qa/qa_cluster.py @@ -192,9 +192,13 @@ def TestClusterInit(rapi_user, rapi_secret): if master.secondary: cmd.append("--secondary-ip=%s" % master.secondary) - vgname = qa_config.get("vg-name", None) - if vgname: - cmd.append("--vg-name=%s" % vgname) + if utils.IsLvmEnabled(qa_config.GetEnabledDiskTemplates()): + vgname = qa_config.get("vg-name", constants.DEFAULT_VG) + if vgname: + cmd.append("--vg-name=%s" % vgname) + else: + raise qa_error.Error("Please specify a volume group if you enable" + " lvm-based disk templates in the QA.") master_netdev = qa_config.get("master-netdev", None) if master_netdev: @@ -398,6 +402,7 @@ def TestClusterModifyDiskTemplates(): _TestClusterModifyDiskTemplatesArguments(default_disk_template, enabled_disk_templates) + _TestClusterModifyDiskTemplatesVgName(enabled_disk_templates) _RestoreEnabledDiskTemplates() nodes = qa_config.AcquireManyNodes(2) @@ -420,10 +425,12 @@ def _RestoreEnabledDiskTemplates(): other tests. """ + vgname = qa_config.get("vg-name", constants.DEFAULT_VG) AssertCommand( ["gnt-cluster", "modify", - "--enabled-disk-template=%s" % - ",".join(qa_config.GetEnabledDiskTemplates())], + "--enabled-disk-templates=%s" % + ",".join(qa_config.GetEnabledDiskTemplates()), + "--vg-name=%s" % vgname], fail=False) @@ -434,11 +441,7 @@ def _TestClusterModifyDiskTemplatesArguments(default_disk_template, of instances. """ - AssertCommand( - ["gnt-cluster", "modify", - "--enabled-disk-template=%s" % - ",".join(enabled_disk_templates)], - fail=False) + _RestoreEnabledDiskTemplates() # bogus templates AssertCommand(["gnt-cluster", "modify", @@ -452,6 +455,105 @@ def _TestClusterModifyDiskTemplatesArguments(default_disk_template, (default_disk_template, default_disk_template)], fail=False) + # interaction with --drbd-usermode-helper option + drbd_usermode_helper = qa_config.get("drbd-usermode-helper", None) + if not drbd_usermode_helper: + drbd_usermode_helper = "/bin/true" + # specifying a helper when drbd gets disabled + AssertCommand(["gnt-cluster", "modify", + "--drbd-usermode-helper=%s" % drbd_usermode_helper, + "--enabled-disk-templates=%s" % constants.DT_DISKLESS], + fail=False) + if constants.DT_DRBD8 in enabled_disk_templates: + # specifying a vg name when lvm is enabled + AssertCommand(["gnt-cluster", "modify", + "--drbd-usermode-helper=%s" % drbd_usermode_helper, + "--enabled-disk-templates=%s" % + ",".join(enabled_disk_templates)], + fail=False) + + +def _TestClusterModifyDiskTemplatesVgName(enabled_disk_templates): + """Tests argument handling of 'gnt-cluster modify' with respect to + the parameter '--enabled-disk-templates' and '--vg-name'. This test is + independent of instances. + + """ + if not utils.IsLvmEnabled(enabled_disk_templates): + # These tests only make sense if lvm is enabled for QA + return + + # determine an LVM and a non-LVM disk template for the tests + non_lvm_templates = list(set(enabled_disk_templates) + - set(utils.GetLvmDiskTemplates())) + lvm_template = list(set(enabled_disk_templates) + .intersection(set(utils.GetLvmDiskTemplates())))[0] + non_lvm_template = None + if non_lvm_templates: + non_lvm_template = non_lvm_templates[0] + else: + # If no non-lvm disk template is available for QA, choose 'diskless' and + # hope for the best. + non_lvm_template = constants.ST_DISKLESS + + vgname = qa_config.get("vg-name", constants.DEFAULT_VG) + + # Clean start: unset volume group name, disable lvm storage + AssertCommand( + ["gnt-cluster", "modify", + "--enabled-disk-templates=%s" % non_lvm_template, + "--vg-name="], + fail=False) + + # Try to enable lvm, when no volume group is given + AssertCommand( + ["gnt-cluster", "modify", + "--enabled-disk-templates=%s" % lvm_template], + fail=True) + + # Set volume group, with lvm still disabled: just a warning + AssertCommand(["gnt-cluster", "modify", "--vg-name=%s" % vgname], fail=False) + + # Try unsetting vg name and enabling lvm at the same time + AssertCommand( + ["gnt-cluster", "modify", + "--enabled-disk-templates=%s" % lvm_template, + "--vg-name="], + fail=True) + + # Enable lvm with vg name present + AssertCommand( + ["gnt-cluster", "modify", + "--enabled-disk-templates=%s" % lvm_template], + fail=False) + + # Try unsetting vg name with lvm still enabled + AssertCommand(["gnt-cluster", "modify", "--vg-name="], fail=True) + + # Disable lvm with vg name still set + AssertCommand( + ["gnt-cluster", "modify", "--enabled-disk-templates=%s" % non_lvm_template], + fail=False) + + # Try unsetting vg name with lvm disabled + AssertCommand(["gnt-cluster", "modify", "--vg-name="], fail=False) + + # Set vg name and enable lvm at the same time + AssertCommand( + ["gnt-cluster", "modify", + "--enabled-disk-templates=%s" % lvm_template, + "--vg-name=%s" % vgname], + fail=False) + + # Unset vg name and disable lvm at the same time + AssertCommand( + ["gnt-cluster", "modify", + "--enabled-disk-templates=%s" % non_lvm_template, + "--vg-name="], + fail=False) + + _RestoreEnabledDiskTemplates() + def _TestClusterModifyUsedDiskTemplate(instance_template, enabled_disk_templates): -- 1.7.10.4