Remove force_master support from LUOobCommand

As per discussion on the man-page[1] update, this functionality should be
removed and replaced by just give the command to run if the user insists
of power cycle/power off the master and refuse to operate.

[1] http://groups.google.com/group/ganeti-devel/browse_thread/thread/95d4879a747cc295

This also removes the --all flag from gnt-node power because it was
buggy and would have added too much logic for special cases.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

diff --git a/lib/client/gnt_node.py b/lib/client/gnt_node.py
index 7210c2a..fe95a6b 100644 (file)
--- a/lib/client/gnt_node.py
+++ b/lib/client/gnt_node.py
@@ -114,16 +114,10 @@ IGNORE_STATUS_OPT = cli_option("--ignore-status", default=False,
                                help=("Ignore the Node(s) offline status"
                                      " (potentially DANGEROUS)"))
 
-FORCE_MASTER_OPT = cli_option("--force-master", default=False,
-                              action="store_true", dest="force_master",
-                              help=("Operate on the master node too"
-                                    " (potentially DANGEROUS)"))
-
 OOB_TIMEOUT_OPT = cli_option("--oob-timeout", dest="oob_timeout", type="int",
                          default=constants.OOB_TIMEOUT,
                          help="Maximum time to wait for out-of-band helper")
 
-
 def ConvertStorageType(user_storage_type):
   """Converts a user storage type to its internal name.
 
@@ -495,7 +489,6 @@ def PowerNode(opts, args):
   @return: the desired exit code
 
   """
-  client = GetClient()
   command = args.pop(0)
 
   if opts.no_headers:
@@ -507,33 +500,27 @@ def PowerNode(opts, args):
     ToStderr("power subcommand %s not supported." % command)
     return constants.EXIT_FAILURE
 
-  nodes = [node for (node, ) in client.QueryNodes(args, ["name"], False)]
   oob_command = "power-%s" % command
 
   if oob_command in _OOB_COMMAND_ASK:
-    if not args and not opts.show_all:
-      ToStderr("Please provide at least one node or use --all for this command"
-               " as this is a potentially harmful command")
-      return constants.EXIT_FAILURE
-    elif args and opts.show_all:
-      ToStderr("Please provide either nodes or use --all, can not use both at"
-               " the same time")
+    if not args:
+      ToStderr("Please provide at least one node for this command")
       return constants.EXIT_FAILURE
-    elif not opts.force and not ConfirmOperation(nodes, "nodes",
+    elif not opts.force and not ConfirmOperation(args, "nodes",
                                                  "power %s" % command):
       return constants.EXIT_FAILURE
+    assert len(args) > 0
 
   opcodelist = []
   if not opts.ignore_status and oob_command == constants.OOB_POWER_OFF:
     # TODO: This is a little ugly as we can't catch and revert
-    for node in nodes:
+    for node in args:
       opcodelist.append(opcodes.OpNodeSetParams(node_name=node, offline=True,
                                                 auto_promote=opts.auto_promote))
 
-  opcodelist.append(opcodes.OpOobCommand(node_names=nodes,
+  opcodelist.append(opcodes.OpOobCommand(node_names=args,
                                          command=oob_command,
                                          ignore_status=opts.ignore_status,
-                                         force_master=opts.force_master,
                                          timeout=opts.oob_timeout))
 
   cli.SetGenericOpcodeOpts(opcodelist, opts)
@@ -870,8 +857,7 @@ commands = {
     [ArgChoice(min=1, max=1, choices=_LIST_POWER_COMMANDS),
      ArgNode()],
     [SUBMIT_OPT, AUTO_PROMOTE_OPT, PRIORITY_OPT, IGNORE_STATUS_OPT,
-     FORCE_MASTER_OPT, FORCE_OPT, NOHDR_OPT, SEP_OPT, ALL_OPT,
-     OOB_TIMEOUT_OPT],
+     FORCE_OPT, NOHDR_OPT, SEP_OPT, OOB_TIMEOUT_OPT],
     "on|off|cycle|status [nodes...]",
     "Change power state of node by calling out-of-band helper."),
   'remove': (

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 4bddc78..bd2751b 100644 (file)
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -3216,6 +3216,7 @@ class LUOobCommand(NoHooksLU):
 
   """
   REG_BGL = False
+  _SKIP_MASTER = (constants.OOB_POWER_OFF, constants.OOB_POWER_CYCLE)
 
   def CheckPrereq(self):
     """Check prerequisites.
@@ -3230,26 +3231,31 @@ class LUOobCommand(NoHooksLU):
     self.nodes = []
     self.master_node = self.cfg.GetMasterNode()
 
-    if self.op.command in (constants.OOB_POWER_OFF, constants.OOB_POWER_CYCLE):
-      # This does two things, it checks if master is in the list and if so and
-      # force_master is set it puts it to the end so the master is done last
-      try:
+    if self.op.node_names:
+      if self.op.command in self._SKIP_MASTER:
+        if self.master_node in self.op.node_names:
+          master_node_obj = self.cfg.GetNodeInfo(self.master_node)
+          master_oob_handler = _SupportsOob(self.cfg, master_node_obj)
+
+          if master_oob_handler:
+            additional_text = ("Run '%s %s %s' if you want to operate on the"
+                               " master regardless") % (master_oob_handler,
+                                                        self.op.command,
+                                                        self.master_node)
+          else:
+            additional_text = "The master node does not support out-of-band"
+
+          raise errors.OpPrereqError(("Operating on the master node %s is not"
+                                      " allowed for %s\n%s") %
+                                     (self.master_node, self.op.command,
+                                      additional_text), errors.ECODE_INVAL)
+    else:
+      self.op.node_names = self.cfg.GetNodeList()
+      if self.op.command in self._SKIP_MASTER:
         self.op.node_names.remove(self.master_node)
-      except ValueError:
-        pass
-      else:
-        if self.op.force_master:
-          self.op.node_names.append(self.master_node)
-        else:
-          self.LogWarning("Master %s was skipped, use the force master"
-                          " option to operate on the master too",
-                          self.master_node)
-          if not self.op.node_names:
-            raise errors.OpPrereqError("No nodes left to operate on, aborting",
-                                       errors.ECODE_INVAL)
 
-      assert (self.master_node not in self.op.node_names or
-              self.op.node_names[-1] == self.master_node)
+    if self.op.command in self._SKIP_MASTER:
+      assert self.master_node not in self.op.node_names
 
     for node_name in self.op.node_names:
       node = self.cfg.GetNodeInfo(node_name)
@@ -3273,11 +3279,12 @@ class LUOobCommand(NoHooksLU):
     if self.op.node_names:
       self.op.node_names = [_ExpandNodeName(self.cfg, name)
                             for name in self.op.node_names]
+      lock_names = self.op.node_names
     else:
-      self.op.node_names = self.cfg.GetNodeList()
+      lock_names = locking.ALL_SET
 
     self.needed_locks = {
-      locking.LEVEL_NODE: self.op.node_names,
+      locking.LEVEL_NODE: lock_names,
       }
 
   def Exec(self, feedback_fn):

diff --git a/lib/opcodes.py b/lib/opcodes.py
index 24f93df..5b16e18 100644 (file)
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -658,7 +658,6 @@ class OpOobCommand(OpCode):
     ("command", None, ht.TElemOf(constants.OOB_COMMANDS), None),
     ("timeout", constants.OOB_TIMEOUT, ht.TInt, None),
     ("ignore_status", False, ht.TBool, None),
-    ("force_master", False, ht.TBool, None),
     ]
 
 

diff --git a/qa/qa_node.py b/qa/qa_node.py
index b71766e..6859cb9 100644 (file)
--- a/qa/qa_node.py
+++ b/qa/qa_node.py
@@ -253,7 +253,6 @@ def TestOutOfBand():
   node = qa_config.AcquireNode(exclude=master)
 
   master_name = master["primary"]
-  full_master_name = qa_utils.ResolveNodeName(master)
   node_name = node["primary"]
   full_node_name = qa_utils.ResolveNodeName(node)
 
@@ -276,15 +275,9 @@ def TestOutOfBand():
     # Power off on master without options should fail
     AssertCommand(["gnt-node", "power", "-f", "off", master_name], fail=True)
     # With force master it should still fail
-    AssertCommand(["gnt-node", "power", "-f", "--force-master", "off",
-                   master_name], fail=True)
     AssertCommand(["gnt-node", "power", "-f",  "--ignore-status", "off",
                    master_name],
                   fail=True)
-    # This should work again
-    AssertCommand(["gnt-node", "power", "-f", "--ignore-status",
-                   "--force-master", "off", master_name])
-    _AssertOobCall(verify_path, "power-off %s" % full_master_name)
 
     # Verify we can't transform back to online when not yet powered on
     AssertCommand(["gnt-node", "modify", "-O", "no", node_name],
@@ -400,43 +393,6 @@ def TestOutOfBand():
       AssertCommand(["gnt-node", "modify", "--node-parameters",
                      "oob_program=default", node_name])
       AssertCommand(["rm", "-f", oob_path2, verify_path2])
-
-    verify_path3 = qa_utils.UploadData(master["primary"], "")
-    oob_script = ("#!/bin/bash\n"
-                  "echo \"$@\" >> %s\n") % verify_path3
-    oob_path3 = qa_utils.UploadData(master["primary"], oob_script, mode=0700)
-
-    AssertCommand(["gnt-cluster", "modify", "--node-parameters",
-                   "oob_program=%s" % oob_path3])
-
-    non_master_nodes = []
-    for node in qa_config.get('nodes'):
-      if node != master:
-        non_master_nodes.append(qa_utils.ResolveNodeName(node))
-
-    try:
-      # Without the --force-master it should not have the master in it
-      verify_content = ["power-cycle %s" % name for name in non_master_nodes]
-      AssertCommand(["gnt-node", "power", "-f", "--all", "cycle"])
-      _AssertOobCall(verify_path3, "\n".join(verify_content))
-
-      # Empty file
-      _UpdateOobFile(verify_path3, "")
-      # With the --force-master it should have the master at last position
-      verify_content.append("power-cycle %s" % full_master_name)
-      AssertCommand(["gnt-node", "power", "--force-master", "-f", "--all",
-                     "cycle"])
-      _AssertOobCall(verify_path3, "\n".join(verify_content))
-
-      # Empty file
-      _UpdateOobFile(verify_path3, "")
-      # With master as first argument it should still be called last
-      cmd = ["gnt-node", "power", "--force-master", "-f", "cycle", master_name]
-      cmd.extend(non_master_nodes)
-      AssertCommand(cmd)
-      _AssertOobCall(verify_path3, "\n".join(verify_content))
-    finally:
-      AssertCommand(["rm", "-f", oob_path3, verify_path3])
   finally:
     AssertCommand(["gnt-cluster", "modify", "--node-parameters",
                    "oob_program="])