From 1a2eb2dc9c1040b7e9f2c1e49a989eb041391529 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Tue, 13 Nov 2012 20:07:55 +0100
Subject: [PATCH] backend: Implement remote commands

As per design document (doc/design-remote-commands.rst), a number of
rather strict tests is applied to any incoming request, a delay is
inserted upon errors and returned error messages are very generic
(unless it's the actual command that failed). There are unit tests for
all of the newly added code.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 Makefile.am                               |   1 +
 lib/backend.py                            | 197 +++++++++++
 test/ganeti.backend_unittest-runasroot.py |  77 +++++
 test/ganeti.backend_unittest.py           | 388 ++++++++++++++++++++++
 4 files changed, 663 insertions(+)
 create mode 100755 test/ganeti.backend_unittest-runasroot.py

diff --git a/Makefile.am b/Makefile.am
index 88f1369df..8a45a335a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -894,6 +894,7 @@ python_tests = \
 	test/cfgupgrade_unittest.py \
 	test/docs_unittest.py \
 	test/ganeti.asyncnotifier_unittest.py \
+	test/ganeti.backend_unittest-runasroot.py \
 	test/ganeti.backend_unittest.py \
 	test/ganeti.bdev_unittest.py \
 	test/ganeti.cli_unittest.py \
diff --git a/lib/backend.py b/lib/backend.py
index 4aa851fff..19711896b 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -87,6 +87,19 @@ _LVSLINE_REGEX = re.compile("^ *([^|]+)\|([^|]+)\|([0-9.]+)\|([^|]{6,})\|?$")
 _MASTER_START = "start"
 _MASTER_STOP = "stop"
 
+#: Maximum file permissions for remote command directory and executables
+_RCMD_MAX_MODE = (stat.S_IRWXU |
+                  stat.S_IRGRP | stat.S_IXGRP |
+                  stat.S_IROTH | stat.S_IXOTH)
+
+#: Delay before returning an error for remote commands
+_RCMD_INVALID_DELAY = 10
+
+#: How long to wait to acquire lock for remote commands (shorter than
+#: L{_RCMD_INVALID_DELAY}) to reduce blockage of noded forks when many
+#: command requests arrive
+_RCMD_LOCK_TIMEOUT = _RCMD_INVALID_DELAY * 0.8
+
 
 class RPCFail(Exception):
   """Class denoting RPC failure.
@@ -3567,6 +3580,190 @@ def PowercycleNode(hypervisor_type):
   hyper.PowercycleNode()
 
 
+def _VerifyRemoteCommandName(cmd):
+  """Verifies a remote command name.
+
+  @type cmd: string
+  @param cmd: Command name
+  @rtype: tuple; (boolean, string or None)
+  @return: The tuple's first element is the status; if C{False}, the second
+    element is an error message string, otherwise it's C{None}
+
+  """
+  if not cmd.strip():
+    return (False, "Missing command name")
+
+  if os.path.basename(cmd) != cmd:
+    return (False, "Invalid command name")
+
+  if not constants.EXT_PLUGIN_MASK.match(cmd):
+    return (False, "Command name contains forbidden characters")
+
+  return (True, None)
+
+
+def _CommonRemoteCommandCheck(path, owner):
+  """Common checks for remote command file system directories and files.
+
+  @type path: string
+  @param path: Path to check
+  @param owner: C{None} or tuple containing UID and GID
+  @rtype: tuple; (boolean, string or C{os.stat} result)
+  @return: The tuple's first element is the status; if C{False}, the second
+    element is an error message string, otherwise it's the result of C{os.stat}
+
+  """
+  if owner is None:
+    # Default to root as owner
+    owner = (0, 0)
+
+  try:
+    st = os.stat(path)
+  except EnvironmentError, err:
+    return (False, "Can't stat(2) '%s': %s" % (path, err))
+
+  if stat.S_IMODE(st.st_mode) & (~_RCMD_MAX_MODE):
+    return (False, "Permissions on '%s' are too permissive" % path)
+
+  if (st.st_uid, st.st_gid) != owner:
+    (owner_uid, owner_gid) = owner
+    return (False, "'%s' is not owned by %s:%s" % (path, owner_uid, owner_gid))
+
+  return (True, st)
+
+
+def _VerifyRemoteCommandDirectory(path, _owner=None):
+  """Verifies remote command directory.
+
+  @type path: string
+  @param path: Path to check
+  @rtype: tuple; (boolean, string or None)
+  @return: The tuple's first element is the status; if C{False}, the second
+    element is an error message string, otherwise it's C{None}
+
+  """
+  (status, value) = _CommonRemoteCommandCheck(path, _owner)
+
+  if not status:
+    return (False, value)
+
+  if not stat.S_ISDIR(value.st_mode):
+    return (False, "Path '%s' is not a directory" % path)
+
+  return (True, None)
+
+
+def _VerifyRemoteCommand(path, cmd, _owner=None):
+  """Verifies a whole remote command and returns its executable filename.
+
+  @type path: string
+  @param path: Directory containing remote commands
+  @type cmd: string
+  @param cmd: Command name
+  @rtype: tuple; (boolean, string)
+  @return: The tuple's first element is the status; if C{False}, the second
+    element is an error message string, otherwise the second element is the
+    absolute path to the executable
+
+  """
+  executable = utils.PathJoin(path, cmd)
+
+  (status, msg) = _CommonRemoteCommandCheck(executable, _owner)
+
+  if not status:
+    return (False, msg)
+
+  if not utils.IsExecutable(executable):
+    return (False, "access(2) thinks '%s' can't be executed" % executable)
+
+  return (True, executable)
+
+
+def _PrepareRemoteCommand(path, cmd,
+                          _verify_dir=_VerifyRemoteCommandDirectory,
+                          _verify_name=_VerifyRemoteCommandName,
+                          _verify_cmd=_VerifyRemoteCommand):
+  """Performs a number of tests on a remote command.
+
+  @type path: string
+  @param path: Directory containing remote commands
+  @type cmd: string
+  @param cmd: Command name
+  @return: Same as L{_VerifyRemoteCommand}
+
+  """
+  # Verify the directory first
+  (status, msg) = _verify_dir(path)
+  if status:
+    # Check command if everything was alright
+    (status, msg) = _verify_name(cmd)
+
+  if not status:
+    return (False, msg)
+
+  # Check actual executable
+  return _verify_cmd(path, cmd)
+
+
+def RunRemoteCommand(cmd,
+                     _lock_timeout=_RCMD_LOCK_TIMEOUT,
+                     _lock_file=pathutils.REMOTE_COMMANDS_LOCK_FILE,
+                     _path=pathutils.REMOTE_COMMANDS_DIR,
+                     _sleep_fn=time.sleep,
+                     _prepare_fn=_PrepareRemoteCommand,
+                     _runcmd_fn=utils.RunCmd,
+                     _enabled=constants.ENABLE_REMOTE_COMMANDS):
+  """Executes a remote command after performing strict tests.
+
+  @type cmd: string
+  @param cmd: Command name
+  @rtype: string
+  @return: Command output
+  @raise RPCFail: In case of an error
+
+  """
+  logging.info("Preparing to run remote command '%s'", cmd)
+
+  if not _enabled:
+    _Fail("Remote commands disabled at configure time")
+
+  lock = None
+  try:
+    cmdresult = None
+    try:
+      lock = utils.FileLock.Open(_lock_file)
+      lock.Exclusive(blocking=True, timeout=_lock_timeout)
+
+      (status, value) = _prepare_fn(_path, cmd)
+
+      if status:
+        cmdresult = _runcmd_fn([value], env={}, reset_env=True,
+                               postfork_fn=lambda _: lock.Unlock())
+      else:
+        logging.error(value)
+    except Exception: # pylint: disable=W0703
+      # Keep original error in log
+      logging.exception("Caught exception")
+
+    if cmdresult is None:
+      logging.info("Sleeping for %0.1f seconds before returning",
+                   _RCMD_INVALID_DELAY)
+      _sleep_fn(_RCMD_INVALID_DELAY)
+
+      # Do not include original error message in returned error
+      _Fail("Executing command '%s' failed" % cmd)
+    elif cmdresult.failed or cmdresult.fail_reason:
+      _Fail("Remote command '%s' failed: %s; output: %s",
+            cmd, cmdresult.fail_reason, cmdresult.output)
+    else:
+      return cmdresult.output
+  finally:
+    if lock is not None:
+      # Release lock at last
+      lock.Close()
+      lock = None
+
+
 class HooksRunner(object):
   """Hook runner.
 
diff --git a/test/ganeti.backend_unittest-runasroot.py b/test/ganeti.backend_unittest-runasroot.py
new file mode 100755
index 000000000..6d99d9ccf
--- /dev/null
+++ b/test/ganeti.backend_unittest-runasroot.py
@@ -0,0 +1,77 @@
+#!/usr/bin/python
+#
+
+# Copyright (C) 2012 Google Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+
+"""Script for testing ganeti.backend (tests requiring root access)"""
+
+import os
+import tempfile
+import shutil
+import errno
+
+from ganeti import constants
+from ganeti import utils
+from ganeti import compat
+from ganeti import backend
+
+import testutils
+
+
+class TestWriteFile(testutils.GanetiTestCase):
+  def setUp(self):
+    self.tmpdir = tempfile.mkdtemp()
+
+  def tearDown(self):
+    shutil.rmtree(self.tmpdir)
+
+  def _PrepareTest(self):
+    tmpname = utils.PathJoin(self.tmpdir, "foobar")
+    os.mkdir(tmpname)
+    os.chmod(tmpname, 0700)
+    return tmpname
+
+  def testCorrectOwner(self):
+    tmpname = self._PrepareTest()
+
+    os.chown(tmpname, 0, 0)
+    (status, value) = backend._CommonRemoteCommandCheck(tmpname, None)
+    self.assertTrue(status)
+    self.assertTrue(value)
+
+  def testWrongOwner(self):
+    tmpname = self._PrepareTest()
+
+    tests = [
+      (1, 0),
+      (0, 1),
+      (100, 50),
+      ]
+
+    for (uid, gid) in tests:
+      self.assertFalse(uid == os.getuid() and gid == os.getgid())
+      os.chown(tmpname, uid, gid)
+
+      (status, errmsg) = backend._CommonRemoteCommandCheck(tmpname, None)
+      self.assertFalse(status)
+      self.assertTrue("foobar' is not owned by " in errmsg)
+
+
+if __name__ == "__main__":
+  testutils.GanetiTestProgram()
diff --git a/test/ganeti.backend_unittest.py b/test/ganeti.backend_unittest.py
index ff9a33c33..bcf933e0e 100755
--- a/test/ganeti.backend_unittest.py
+++ b/test/ganeti.backend_unittest.py
@@ -31,8 +31,10 @@ from ganeti import utils
 from ganeti import constants
 from ganeti import backend
 from ganeti import netutils
+from ganeti import errors
 
 import testutils
+import mocks
 
 
 class TestX509Certificates(unittest.TestCase):
@@ -94,5 +96,391 @@ class TestNodeVerify(testutils.GanetiTestCase):
                 "Result from netutils.TcpPing corrupted")
 
 
+def _DefRemoteCommandOwner():
+  return (os.getuid(), os.getgid())
+
+
+class TestVerifyRemoteCommandName(unittest.TestCase):
+  def testAcceptableName(self):
+    for i in ["foo", "bar", "z1", "000first", "hello-world"]:
+      for fn in [lambda s: s, lambda s: s.upper(), lambda s: s.title()]:
+        (status, msg) = backend._VerifyRemoteCommandName(fn(i))
+        self.assertTrue(status)
+        self.assertTrue(msg is None)
+
+  def testEmptyAndSpace(self):
+    for i in ["", " ", "\t", "\n"]:
+      (status, msg) = backend._VerifyRemoteCommandName(i)
+      self.assertFalse(status)
+      self.assertEqual(msg, "Missing command name")
+
+  def testNameWithSlashes(self):
+    for i in ["/", "./foo", "../moo", "some/name"]:
+      (status, msg) = backend._VerifyRemoteCommandName(i)
+      self.assertFalse(status)
+      self.assertEqual(msg, "Invalid command name")
+
+  def testForbiddenCharacters(self):
+    for i in ["#", ".", "..", "bash -c ls", "'"]:
+      (status, msg) = backend._VerifyRemoteCommandName(i)
+      self.assertFalse(status)
+      self.assertEqual(msg, "Command name contains forbidden characters")
+
+
+class TestVerifyRemoteCommandDirectory(unittest.TestCase):
+  def setUp(self):
+    self.tmpdir = tempfile.mkdtemp()
+
+  def tearDown(self):
+    shutil.rmtree(self.tmpdir)
+
+  def testCanNotStat(self):
+    tmpname = utils.PathJoin(self.tmpdir, "foobar")
+    self.assertFalse(os.path.exists(tmpname))
+    (status, msg) = \
+      backend._VerifyRemoteCommandDirectory(tmpname, _owner=NotImplemented)
+    self.assertFalse(status)
+    self.assertTrue(msg.startswith("Can't stat(2) '"))
+
+  def testTooPermissive(self):
+    tmpname = utils.PathJoin(self.tmpdir, "foobar")
+    os.mkdir(tmpname)
+
+    for mode in [0777, 0706, 0760, 0722]:
+      os.chmod(tmpname, mode)
+      self.assertTrue(os.path.isdir(tmpname))
+      (status, msg) = \
+        backend._VerifyRemoteCommandDirectory(tmpname, _owner=NotImplemented)
+      self.assertFalse(status)
+      self.assertTrue(msg.startswith("Permissions on '"))
+
+  def testNoDirectory(self):
+    tmpname = utils.PathJoin(self.tmpdir, "foobar")
+    utils.WriteFile(tmpname, data="empty\n")
+    self.assertTrue(os.path.isfile(tmpname))
+    (status, msg) = \
+      backend._VerifyRemoteCommandDirectory(tmpname,
+                                            _owner=_DefRemoteCommandOwner())
+    self.assertFalse(status)
+    self.assertTrue(msg.endswith("is not a directory"))
+
+  def testNormal(self):
+    tmpname = utils.PathJoin(self.tmpdir, "foobar")
+    os.mkdir(tmpname)
+    self.assertTrue(os.path.isdir(tmpname))
+    (status, msg) = \
+      backend._VerifyRemoteCommandDirectory(tmpname,
+                                            _owner=_DefRemoteCommandOwner())
+    self.assertTrue(status)
+    self.assertTrue(msg is None)
+
+
+class TestVerifyRemoteCommand(unittest.TestCase):
+  def setUp(self):
+    self.tmpdir = tempfile.mkdtemp()
+
+  def tearDown(self):
+    shutil.rmtree(self.tmpdir)
+
+  def testCanNotStat(self):
+    tmpname = utils.PathJoin(self.tmpdir, "helloworld")
+    self.assertFalse(os.path.exists(tmpname))
+    (status, msg) = \
+      backend._VerifyRemoteCommand(self.tmpdir, "helloworld",
+                                   _owner=NotImplemented)
+    self.assertFalse(status)
+    self.assertTrue(msg.startswith("Can't stat(2) '"))
+
+  def testNotExecutable(self):
+    tmpname = utils.PathJoin(self.tmpdir, "cmdname")
+    utils.WriteFile(tmpname, data="empty\n")
+    (status, msg) = \
+      backend._VerifyRemoteCommand(self.tmpdir, "cmdname",
+                                   _owner=_DefRemoteCommandOwner())
+    self.assertFalse(status)
+    self.assertTrue(msg.startswith("access(2) thinks '"))
+
+  def testExecutable(self):
+    tmpname = utils.PathJoin(self.tmpdir, "cmdname")
+    utils.WriteFile(tmpname, data="empty\n", mode=0700)
+    (status, executable) = \
+      backend._VerifyRemoteCommand(self.tmpdir, "cmdname",
+                                   _owner=_DefRemoteCommandOwner())
+    self.assertTrue(status)
+    self.assertEqual(executable, tmpname)
+
+
+class TestPrepareRemoteCommand(unittest.TestCase):
+  _TEST_PATH = "/tmp/some/test/path"
+
+  def testDirFails(self):
+    def fn(path):
+      self.assertEqual(path, self._TEST_PATH)
+      return (False, "test error 31420")
+
+    (status, msg) = \
+      backend._PrepareRemoteCommand(self._TEST_PATH, "cmd21152",
+                                    _verify_dir=fn,
+                                    _verify_name=NotImplemented,
+                                    _verify_cmd=NotImplemented)
+    self.assertFalse(status)
+    self.assertEqual(msg, "test error 31420")
+
+  def testNameFails(self):
+    def fn(cmd):
+      self.assertEqual(cmd, "cmd4617")
+      return (False, "test error 591")
+
+    (status, msg) = \
+      backend._PrepareRemoteCommand(self._TEST_PATH, "cmd4617",
+                                    _verify_dir=lambda _: (True, None),
+                                    _verify_name=fn,
+                                    _verify_cmd=NotImplemented)
+    self.assertFalse(status)
+    self.assertEqual(msg, "test error 591")
+
+  def testCommandFails(self):
+    def fn(path, cmd):
+      self.assertEqual(path, self._TEST_PATH)
+      self.assertEqual(cmd, "cmd17577")
+      return (False, "test error 25524")
+
+    (status, msg) = \
+      backend._PrepareRemoteCommand(self._TEST_PATH, "cmd17577",
+                                    _verify_dir=lambda _: (True, None),
+                                    _verify_name=lambda _: (True, None),
+                                    _verify_cmd=fn)
+    self.assertFalse(status)
+    self.assertEqual(msg, "test error 25524")
+
+  def testSuccess(self):
+    def fn(path, cmd):
+      return (True, utils.PathJoin(path, cmd))
+
+    (status, executable) = \
+      backend._PrepareRemoteCommand(self._TEST_PATH, "cmd22633",
+                                    _verify_dir=lambda _: (True, None),
+                                    _verify_name=lambda _: (True, None),
+                                    _verify_cmd=fn)
+    self.assertTrue(status)
+    self.assertEqual(executable, utils.PathJoin(self._TEST_PATH, "cmd22633"))
+
+
+def _SleepForRemoteCommand(duration):
+  assert duration > 5
+
+
+def _GenericRemoteCommandError(cmd):
+  return "Executing command '%s' failed" % cmd
+
+
+class TestRunRemoteCommand(unittest.TestCase):
+  def setUp(self):
+    self.tmpdir = tempfile.mkdtemp()
+
+  def tearDown(self):
+    shutil.rmtree(self.tmpdir)
+
+  def testNonExistantLockDirectory(self):
+    lockfile = utils.PathJoin(self.tmpdir, "does", "not", "exist")
+    sleep_fn = testutils.CallCounter(_SleepForRemoteCommand)
+    self.assertFalse(os.path.exists(lockfile))
+    self.assertRaises(backend.RPCFail,
+                      backend.RunRemoteCommand, "test",
+                      _lock_timeout=NotImplemented,
+                      _lock_file=lockfile,
+                      _path=NotImplemented,
+                      _sleep_fn=sleep_fn,
+                      _prepare_fn=NotImplemented,
+                      _runcmd_fn=NotImplemented,
+                      _enabled=True)
+    self.assertEqual(sleep_fn.Count(), 1)
+
+  @staticmethod
+  def _TryLock(lockfile):
+    sleep_fn = testutils.CallCounter(_SleepForRemoteCommand)
+
+    result = False
+    try:
+      backend.RunRemoteCommand("test22717",
+                               _lock_timeout=0.1,
+                               _lock_file=lockfile,
+                               _path=NotImplemented,
+                               _sleep_fn=sleep_fn,
+                               _prepare_fn=NotImplemented,
+                               _runcmd_fn=NotImplemented,
+                               _enabled=True)
+    except backend.RPCFail, err:
+      assert str(err) == _GenericRemoteCommandError("test22717"), \
+             "Did not fail with generic error message"
+      result = True
+
+    assert sleep_fn.Count() == 1
+
+    return result
+
+  def testLockHeldByOtherProcess(self):
+    lockfile = utils.PathJoin(self.tmpdir, "lock")
+
+    lock = utils.FileLock.Open(lockfile)
+    lock.Exclusive(blocking=True, timeout=1.0)
+    try:
+      self.assertTrue(utils.RunInSeparateProcess(self._TryLock, lockfile))
+    finally:
+      lock.Close()
+
+  @staticmethod
+  def _PrepareRaisingException(path, cmd):
+    assert cmd == "test23122"
+    raise Exception("test")
+
+  def testPrepareRaisesException(self):
+    lockfile = utils.PathJoin(self.tmpdir, "lock")
+
+    sleep_fn = testutils.CallCounter(_SleepForRemoteCommand)
+    prepare_fn = testutils.CallCounter(self._PrepareRaisingException)
+
+    try:
+      backend.RunRemoteCommand("test23122",
+                               _lock_timeout=1.0, _lock_file=lockfile,
+                               _path=NotImplemented, _runcmd_fn=NotImplemented,
+                               _sleep_fn=sleep_fn, _prepare_fn=prepare_fn,
+                               _enabled=True)
+    except backend.RPCFail, err:
+      self.assertEqual(str(err), _GenericRemoteCommandError("test23122"))
+    else:
+      self.fail("Didn't fail")
+
+    self.assertEqual(sleep_fn.Count(), 1)
+    self.assertEqual(prepare_fn.Count(), 1)
+
+  @staticmethod
+  def _PrepareFails(path, cmd):
+    assert cmd == "test29327"
+    return ("some error message", None)
+
+  def testPrepareFails(self):
+    lockfile = utils.PathJoin(self.tmpdir, "lock")
+
+    sleep_fn = testutils.CallCounter(_SleepForRemoteCommand)
+    prepare_fn = testutils.CallCounter(self._PrepareFails)
+
+    try:
+      backend.RunRemoteCommand("test29327",
+                               _lock_timeout=1.0, _lock_file=lockfile,
+                               _path=NotImplemented, _runcmd_fn=NotImplemented,
+                               _sleep_fn=sleep_fn, _prepare_fn=prepare_fn,
+                               _enabled=True)
+    except backend.RPCFail, err:
+      self.assertEqual(str(err), _GenericRemoteCommandError("test29327"))
+    else:
+      self.fail("Didn't fail")
+
+    self.assertEqual(sleep_fn.Count(), 1)
+    self.assertEqual(prepare_fn.Count(), 1)
+
+  @staticmethod
+  def _SuccessfulPrepare(path, cmd):
+    return (True, utils.PathJoin(path, cmd))
+
+  def testRunCmdFails(self):
+    lockfile = utils.PathJoin(self.tmpdir, "lock")
+
+    def fn(args, env=NotImplemented, reset_env=NotImplemented,
+           postfork_fn=NotImplemented):
+      self.assertEqual(args, [utils.PathJoin(self.tmpdir, "test3079")])
+      self.assertEqual(env, {})
+      self.assertTrue(reset_env)
+      self.assertTrue(callable(postfork_fn))
+
+      trylock = utils.FileLock.Open(lockfile)
+      try:
+        # See if lockfile is still held
+        self.assertRaises(EnvironmentError, trylock.Exclusive, blocking=False)
+
+        # Call back to release lock
+        postfork_fn(NotImplemented)
+
+        # See if lockfile can be acquired
+        trylock.Exclusive(blocking=False)
+      finally:
+        trylock.Close()
+
+      # Simulate a failed command
+      return utils.RunResult(constants.EXIT_FAILURE, None,
+                             "stdout", "stderr406328567",
+                             utils.ShellQuoteArgs(args),
+                             NotImplemented, NotImplemented)
+
+    sleep_fn = testutils.CallCounter(_SleepForRemoteCommand)
+    prepare_fn = testutils.CallCounter(self._SuccessfulPrepare)
+    runcmd_fn = testutils.CallCounter(fn)
+
+    try:
+      backend.RunRemoteCommand("test3079",
+                               _lock_timeout=1.0, _lock_file=lockfile,
+                               _path=self.tmpdir, _runcmd_fn=runcmd_fn,
+                               _sleep_fn=sleep_fn, _prepare_fn=prepare_fn,
+                               _enabled=True)
+    except backend.RPCFail, err:
+      self.assertTrue(str(err).startswith("Remote command 'test3079' failed:"))
+      self.assertTrue("stderr406328567" in str(err),
+                      msg="Error did not include output")
+    else:
+      self.fail("Didn't fail")
+
+    self.assertEqual(sleep_fn.Count(), 0)
+    self.assertEqual(prepare_fn.Count(), 1)
+    self.assertEqual(runcmd_fn.Count(), 1)
+
+  def testRunCmdSucceeds(self):
+    lockfile = utils.PathJoin(self.tmpdir, "lock")
+
+    def fn(args, env=NotImplemented, reset_env=NotImplemented,
+           postfork_fn=NotImplemented):
+      self.assertEqual(args, [utils.PathJoin(self.tmpdir, "test5667")])
+      self.assertEqual(env, {})
+      self.assertTrue(reset_env)
+
+      # Call back to release lock
+      postfork_fn(NotImplemented)
+
+      # Simulate a successful command
+      return utils.RunResult(constants.EXIT_SUCCESS, None, "stdout14463", "",
+                             utils.ShellQuoteArgs(args),
+                             NotImplemented, NotImplemented)
+
+    sleep_fn = testutils.CallCounter(_SleepForRemoteCommand)
+    prepare_fn = testutils.CallCounter(self._SuccessfulPrepare)
+    runcmd_fn = testutils.CallCounter(fn)
+
+    result = backend.RunRemoteCommand("test5667",
+                                      _lock_timeout=1.0, _lock_file=lockfile,
+                                      _path=self.tmpdir, _runcmd_fn=runcmd_fn,
+                                      _sleep_fn=sleep_fn,
+                                      _prepare_fn=prepare_fn,
+                                      _enabled=True)
+    self.assertEqual(result, "stdout14463")
+
+    self.assertEqual(sleep_fn.Count(), 0)
+    self.assertEqual(prepare_fn.Count(), 1)
+    self.assertEqual(runcmd_fn.Count(), 1)
+
+  def testCommandsDisabled(self):
+    try:
+      backend.RunRemoteCommand("test",
+                               _lock_timeout=NotImplemented,
+                               _lock_file=NotImplemented,
+                               _path=NotImplemented,
+                               _sleep_fn=NotImplemented,
+                               _prepare_fn=NotImplemented,
+                               _runcmd_fn=NotImplemented,
+                               _enabled=False)
+    except backend.RPCFail, err:
+      self.assertEqual(str(err), "Remote commands disabled at configure time")
+    else:
+      self.fail("Did not raise exception")
+
+
 if __name__ == "__main__":
   testutils.GanetiTestProgram()
-- 
GitLab