Adding the list back ---------- Forwarded message ---------- From: Guido Trotter <[email protected]> Date: 4 February 2013 18:10 Subject: Re: [PATCH devel-2.7 1/2] "exclusive_storage" cannot be changed on single nodes To: Bernardo Dal Seno <[email protected]>
On Mon, Feb 4, 2013 at 6:00 PM, Bernardo Dal Seno <[email protected]> wrote: > There's never been support for a configuration where nodes in the same node > group have different values of the exclusive_storage flag. This patch > disables the possibility to change the flag for individual nodes. > > Signed-off-by: Bernardo Dal Seno <[email protected]> > --- > lib/cmdlib.py | 23 ++++++++++++++++++++--- > lib/constants.py | 4 ++++ > man/ganeti.rst | 4 +++- > qa/ganeti-qa.py | 2 +- > qa/qa_cluster.py | 18 ------------------ > qa/qa_node.py | 19 +++++++++++++++++++ > 6 files changed, 47 insertions(+), 23 deletions(-) > > diff --git a/lib/cmdlib.py b/lib/cmdlib.py > index d14bb65..6572587 100644 > --- a/lib/cmdlib.py > +++ b/lib/cmdlib.py > @@ -1035,6 +1035,24 @@ def _CheckGlobalHvParams(params): > raise errors.OpPrereqError(msg, errors.ECODE_INVAL) > > > +def _CheckNotGlobalNdp(params): > + """Make sure that no one of the given node paramters is global. > + > + If a global parameter is found, an L{errors.OpPrereqError} exception is > + raised. This is used to avoid setting global parameters for individual > nodes. > + > + @type params: dictionary > + @param params: Node parameters to check > + > + """ > + used_globals = constants.NDC_GLOBALS.intersection(params) > + if used_globals: > + msg = ("The following node parameters are global and cannot" > + " be customized at node level, please modify them at" > + " cluster or group level: %s" % utils.CommaJoin(used_globals)) > + raise errors.OpPrereqError(msg, errors.ECODE_INVAL) > + This is very similar to _CheckGlobalHvParams, I wonder if we should make it the same... > + > def _CheckNodeOnline(lu, node, msg=None): > """Ensure that a given node is online. > > @@ -6139,6 +6157,7 @@ class LUNodeAdd(LogicalUnit): > > if self.op.ndparams: > utils.ForceDictType(self.op.ndparams, constants.NDS_PARAMETER_TYPES) > + _CheckNotGlobalNdp(self.op.ndparams) > > if self.op.hv_state: > self.new_hv_state = _MergeAndVerifyHvState(self.op.hv_state, None) > @@ -6164,9 +6183,6 @@ class LUNodeAdd(LogicalUnit): > if vg_name is not None: > vparams = {constants.NV_PVLIST: [vg_name]} > excl_stor = _IsExclusiveStorageEnabledNode(cfg, self.new_node) > - if self.op.ndparams: > - excl_stor = self.op.ndparams.get(constants.ND_EXCLUSIVE_STORAGE, > - excl_stor) > cname = self.cfg.GetClusterName() > result = rpcrunner.call_node_verify_light([node], vparams, cname)[node] > (errmsgs, _) = _CheckNodePVs(result.payload, excl_stor) > @@ -6564,6 +6580,7 @@ class LUNodeSetParams(LogicalUnit): > if self.op.ndparams: > new_ndparams = _GetUpdatedParams(self.node.ndparams, self.op.ndparams) > utils.ForceDictType(new_ndparams, constants.NDS_PARAMETER_TYPES) > + _CheckNotGlobalNdp(self.op.ndparams) > self.new_ndparams = new_ndparams > > if self.op.hv_state: > diff --git a/lib/constants.py b/lib/constants.py > index 4e04dff..e8fadf0 100644 > --- a/lib/constants.py > +++ b/lib/constants.py > @@ -2019,6 +2019,10 @@ NDC_DEFAULTS = { > ND_EXCLUSIVE_STORAGE: False, > } > > +NDC_GLOBALS = compat.UniqueFrozenset([ > + ND_EXCLUSIVE_STORAGE, > + ]) > + Could you also modify objects.py to delete the params if they are found, like we do it for global hv params, please? > DISK_LD_DEFAULTS = { > LD_DRBD8: { > LDP_RESYNC_RATE: CLASSIC_DRBD_SYNC_SPEED, > diff --git a/man/ganeti.rst b/man/ganeti.rst > index 03fe652..16800d1 100644 > --- a/man/ganeti.rst > +++ b/man/ganeti.rst > @@ -120,7 +120,9 @@ exclusive_storage > When this Boolean flag is enabled, physical disks on the node are > assigned to instance disks in an exclusive manner, so as to lower I/O > interference between instances. See the `Partitioned Ganeti > - <design-partitioned.rst>`_ design document for more details. > + <design-partitioned.rst>`_ design document for more details. This > + parameter cannot be set on individual nodes, as its value must be > + the same within each node group. > > > Hypervisor State Parameters > diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py > index 62f9550..5c2388b 100755 > --- a/qa/ganeti-qa.py > +++ b/qa/ganeti-qa.py > @@ -452,7 +452,7 @@ def RunExclusiveStorageTests(): > node = qa_config.AcquireNode() > try: > old_es = qa_cluster.TestSetExclStorCluster(False) > - qa_cluster.TestExclStorSingleNode(node) > + qa_node.TestExclStorSingleNode(node) > > qa_cluster.TestSetExclStorCluster(True) > qa_cluster.TestExclStorSharedPv(node) > diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py > index 09434a7..ffb0971 100644 > --- a/qa/qa_cluster.py > +++ b/qa/qa_cluster.py > @@ -667,24 +667,6 @@ def TestSetExclStorCluster(newvalue): > return oldvalue > > > -def _BuildSetESCmd(value, node_name): > - return ["gnt-node", "modify", "--node-parameters", > - "exclusive_storage=%s" % value, node_name] > - > - > -def TestExclStorSingleNode(node): > - """cluster-verify reports exclusive_storage set only on one node. > - > - """ > - node_name = node["primary"] > - es_val = _GetBoolClusterField("exclusive_storage") > - assert not es_val > - AssertCommand(_BuildSetESCmd(True, node_name)) > - AssertClusterVerify(fail=True, errors=[constants.CV_EGROUPMIXEDESFLAG]) > - AssertCommand(_BuildSetESCmd("default", node_name)) > - AssertClusterVerify() > - > - > def TestExclStorSharedPv(node): > """cluster-verify reports LVs that share the same PV with > exclusive_storage. > > diff --git a/qa/qa_node.py b/qa/qa_node.py > index 647af19..ac7fc90 100644 > --- a/qa/qa_node.py > +++ b/qa/qa_node.py > @@ -422,3 +422,22 @@ def TestNodeListFields(): > def TestNodeListDrbd(node): > """gnt-node list-drbd""" > AssertCommand(["gnt-node", "list-drbd", node["primary"]]) > + > + > +def _BuildSetESCmd(action, value, node_name): > + cmd = ["gnt-node"] > + if action == "add": > + cmd.extend(["add", "--readd"]) > + else: > + cmd.append("modify") > + cmd.extend(["--node-parameters", "exclusive_storage=%s" % value, > node_name]) > + return cmd > + > + > +def TestExclStorSingleNode(node): > + """gnt-node add/modify cannot change the exclusive_storage flag. > + > + """ > + for action in ["add", "modify"]: > + for value in (True, False, "default"): > + AssertCommand(_BuildSetESCmd(action, value, node["primary"]), > fail=True) Thanks! Guido
