Introduce command-dict and validator

Refactor `vm.utils._run_command_over_ssh' making it accept a dict to
specify command as either local script file, inline script or remote
script path. Both local and inline scripts require `interpreter' being
specified. Any of the above can be a list specifying e.g. environment
variables (for `interpreter') or command args.

Introduce `valid_command' validator that checks whether the specified
command dictionary is a valid one.

Change-Id: I3c495297251c87529b9adf1e769cfe277338f5fc
Implements: blueprint vm-workloads-framework

rally/benchmark/validation.py

@@ -31,6 +31,9 @@ from rally import osclients
 from rally.plugins.openstack.context import flavors as flavors_ctx
 from rally.verification.tempest import tempest
 
+# TODO(boris-42): make the validators usable as a functions as well.
+# At the moment validators can only be used as decorators.
+
 
 class ValidationResult(object):
 
@@ -165,6 +168,87 @@ def file_exists(config, clients, deployment, param_name, mode=os.R_OK,
                            param_name, required)
 
 
+def check_command_dict(command):
+    """Check command-specifying dict `command', raise ValueError on error."""
+
+    # NOTE(pboldin): Here we check for the values not for presence of the keys
+    # due to template-driven configuration generation that can leave keys
+    # defined but values empty.
+    if len([1 for k in ("remote_path", "script_file", "script_inline")
+           if command.get(k)]) != 1:
+        raise ValueError(
+            "Exactly one of script_inline, script_file or remote_path"
+            " is expected: %r" % command)
+    if ((command.get("script_file") or command.get("script_inline")) and
+       "interpreter" not in command):
+        raise ValueError(
+            "An `interpreter' is required for both script_file and"
+            " script_inline: %r" % command)
+
+
+@validator
+def valid_command(config, clients, deployment, param_name, required=True):
+    """Checks that parameter is a proper command-specifying dictionary.
+
+    Ensure that the command dictionary either specifies remote command path
+    via `remote_path' (optionally copied from a local file specified by
+    `local_path`), an inline script via `script_inline' or a local script
+    file path using `script_file'. `script_file' and `local_path' are checked
+    to be accessible like in `file_exists' validator.
+
+    The `script_inline' and `script_file' both require an `interpreter' value
+    to specify the interpreter script should be run with.
+
+    Note that `interpreter' and `remote_path' can be an array specifying
+    environment variables and args.
+
+    Examples::
+
+        # Run a `local_script.pl' file sending it to a remote Perl interpreter
+        command = {
+            "script_file": "local_script.pl",
+            "interpreter": "/usr/bin/perl"
+        }
+
+        # Run an inline script sending it to a remote interpreter
+        command = {
+            "script_inline": "echo 'Hello, World!'",
+            "interpreter": "/bin/sh"
+        }
+
+        # Run a remote command
+        command = {
+            "remote_path": "/bin/false"
+        }
+
+        # Run an inline script sending it to a remote interpreter
+        command = {
+            "script_inline": "echo \"Hello, ${NAME:-World}\"",
+            "interpreter": ["NAME=Earth", "/bin/sh"]
+        }
+
+    :param param_name: Name of parameter to validate
+    :param required: Boolean indicating that the command dictionary is required
+    """
+    # TODO(pboldin): Make that a `jsonschema' check once generic validator
+    # is available.
+
+    command = config.get("args", {}).get(param_name)
+    if command is None:
+        return ValidationResult(not required,
+                                "Command dicitionary is required")
+    try:
+        check_command_dict(command)
+    except ValueError as e:
+        return ValidationResult(False, str(e))
+
+    if command.get("script_file"):
+        return _file_access_ok(command["script_file"], os.R_OK,
+                               param_name + ".script_file", True)
+
+    return ValidationResult(True)
+
+
 def _get_validated_image(config, clients, param_name):
     image_context = config.get("context", {}).get("images", {})
     image_args = config.get("args", {}).get(param_name)

rally/plugins/openstack/scenarios/vm/utils.py

@@ -21,10 +21,10 @@ import six
 
 from rally.benchmark.scenarios import base
 from rally.benchmark import utils as bench_utils
+from rally.benchmark import validation
 from rally.common.i18n import _
 from rally.common import log as logging
 from rally.common import sshutils
-from rally import exceptions
 from rally.plugins.openstack.wrappers import network as network_wrapper
 
 LOG = logging.getLogger(__name__)
@@ -40,32 +40,33 @@ class VMScenario(base.Scenario):
     """
 
     @base.atomic_action_timer("vm.run_command_over_ssh")
-    def _run_command_over_ssh(self, ssh, interpreter, script,
+    def _run_command_over_ssh(self, ssh, command):
-                              is_file=True):
         """Run command inside an instance.
 
         This is a separate function so that only script execution is timed.
 
         :param ssh: A SSHClient instance.
-        :param interpreter: The interpreter that will be used to execute
+        :param command: Dictionary specifying command to execute.
-                the script.
+            See `validation.valid_command' docstring for details.
-        :param script: Path to the script file or its content in a StringIO.
+
-        :param is_file: if True, script represent a path,
-                        else, script contains an inline script.
         :returns: tuple (exit_status, stdout, stderr)
         """
-        if not is_file:
+        validation.check_command_dict(command)
-            stdin = script
-        elif isinstance(script, six.string_types):
-            stdin = open(script, "rb")
-        elif isinstance(script, six.moves.StringIO):
-            stdin = script
-        else:
-            raise exceptions.ScriptError(
-                "Either file path or StringIO expected, given %s" %
-                type(script).__name__)
 
-        return ssh.execute(interpreter, stdin=stdin)
+        # NOTE(pboldin): Here we `get' the values and not check for the keys
+        # due to template-driven configuration generation that can leave keys
+        # defined but values empty.
+        if command.get("script_file") or command.get("script_inline"):
+            cmd = command["interpreter"]
+            if command.get("script_file"):
+                stdin = open(command["script_file"], "rb")
+            elif command.get("script_inline"):
+                stdin = six.moves.StringIO(command["script_inline"])
+        elif command.get("remote_path"):
+            cmd = command["remote_path"]
+            stdin = None
+
+        return ssh.execute(cmd, stdin=stdin)
 
     def _boot_server_with_fip(self, image, flavor,
                               use_floating_ip=True, floating_network=None,
@@ -135,30 +136,30 @@ class VMScenario(base.Scenario):
             timeout=120
         )
 
-    def _run_command(self, server_ip, port, username, password, interpreter,
+    def _run_command(self, server_ip, port, username, password, command,
-                     script, pkey=None, is_file=True):
+                     pkey=None):
         """Run command via SSH on server.
 
-        Create SSH connection for server, wait for server to become
+        Create SSH connection for server, wait for server to become available
-        available (there is a delay between server being set to ACTIVE
+        (there is a delay between server being set to ACTIVE and sshd being
-        and sshd being available). Then call run_command_over_ssh to actually
+        available). Then call run_command_over_ssh to actually execute the
-        execute the command.
+        command.
+
         :param server_ip: server ip address
         :param port: ssh port for SSH connection
         :param username: str. ssh username for server
         :param password: Password for SSH authentication
-        :param interpreter: server's interpreter to execute the script
+        :param command: Dictionary specifying command to execute.
-        :param script: script to run on server
+                See `valiation.valid_command' docstring for explanation.
         :param pkey: key for SSH authentication
-        :param is_file: if True, script represent a path,
+
-                        else, script contains an inline script.
+        :returns: tuple (exit_status, stdout, stderr)
         """
         pkey = pkey if pkey else self.context["user"]["keypair"]["private"]
         ssh = sshutils.SSH(username, server_ip, port=port,
                            pkey=pkey, password=password)
         self._wait_for_ssh(ssh)
-        return self._run_command_over_ssh(ssh, interpreter,
+        return self._run_command_over_ssh(ssh, command)
-                                          script, is_file)
 
     @staticmethod
     def _ping_ip_address(host):

rally/plugins/openstack/scenarios/vm/vmtasks.py

@@ -84,8 +84,9 @@ class VMTasks(nova_utils.NovaScenario, vm_utils.VMScenario,
             key_name=self.context["user"]["keypair"]["name"],
             **kwargs)
         try:
-            code, out, err = self._run_command(fip["ip"], port, username,
+            code, out, err = self._run_command(
-                                               password, interpreter, script)
+                fip["ip"], port, username, password,
+                command={"script_file": script, "interpreter": interpreter})
             if code:
                 raise exceptions.ScriptError(
                     "Error running script %(script)s. "

tests/unit/benchmark/test_validation.py

@@ -132,6 +132,77 @@ class ValidatorsTestCase(test.TestCase):
         mock__file_access_ok.assert_called_once_with(
             "test_file", os.R_OK, "p", False)
 
+    def test_check_command_valid(self):
+
+        e = self.assertRaises(
+            ValueError, validation.check_command_dict,
+            {"script_file": "foo", "remote_path": "bar"})
+        self.assertIn("Exactly one of ", str(e))
+
+        e = self.assertRaises(
+            ValueError, validation.check_command_dict,
+            {"script_file": "foobar"})
+        self.assertIn("An `interpreter' is required for", str(e))
+
+        e = self.assertRaises(
+            ValueError, validation.check_command_dict,
+            {"script_inline": "foobar"})
+        self.assertIn("An `interpreter' is required for", str(e))
+
+        command = {"script_inline": "foobar", "interpreter": "foo"}
+        result = validation.check_command_dict(command)
+        self.assertIsNone(result)
+
+    @mock.patch("rally.benchmark.validation._file_access_ok")
+    def test_valid_command(self, mock__file_access_ok):
+        validator = self._unwrap_validator(validation.valid_command,
+                                           param_name="p")
+
+        mock__file_access_ok.return_value = validation.ValidationResult(True)
+        command = {"script_file": "foobar", "interpreter": "foo"}
+        result = validator({"args": {"p": command}}, None, None)
+        self.assertTrue(result.is_valid, result.msg)
+        mock__file_access_ok.assert_called_once_with(
+            "foobar", os.R_OK, "p.script_file", True)
+
+    def test_valid_command_required(self):
+        validator = self._unwrap_validator(validation.valid_command,
+                                           param_name="p")
+
+        result = validator({"args": {"p": None}}, None, None)
+        self.assertFalse(result.is_valid, result.msg)
+
+    @mock.patch("rally.benchmark.validation._file_access_ok")
+    def test_valid_command_unreadable_script_file(self, mock__file_access_ok):
+        mock__file_access_ok.return_value = validation.ValidationResult(False)
+
+        validator = self._unwrap_validator(validation.valid_command,
+                                           param_name="p")
+
+        command = {"script_file": "foobar", "interpreter": "foo"}
+        result = validator({"args": {"p": command}}, None, None)
+        self.assertFalse(result.is_valid, result.msg)
+
+    @mock.patch("rally.benchmark.validation.check_command_dict")
+    def test_valid_command_fail_check_command_dict(self,
+                                                   mock_check_command_dict):
+        validator = self._unwrap_validator(validation.valid_command,
+                                           param_name="p")
+
+        mock_check_command_dict.side_effect = ValueError("foobar")
+        command = {"foo": "bar"}
+        result = validator({"args": {"p": command}}, None, None)
+        self.assertFalse(result.is_valid, result.msg)
+        self.assertEqual("foobar", result.msg)
+
+    def test_valid_command_script_inline(self):
+        validator = self._unwrap_validator(validation.valid_command,
+                                           param_name="p")
+
+        command = {"script_inline": "bar", "interpreter": "/bin/sh"}
+        result = validator({"args": {"p": command}}, None, None)
+        self.assertTrue(result.is_valid, result.msg)
+
     def test__get_validated_image_no_value_in_config(self):
         result = validation._get_validated_image({}, None, "non_existing")
         self.assertFalse(result[0].is_valid, result[0].msg)

tests/unit/plugins/openstack/scenarios/vm/test_utils.py

@@ -19,9 +19,7 @@ import subprocess
 import mock
 import netaddr
 from oslotest import mockpatch
-import six
 
-from rally import exceptions
 from rally.plugins.openstack.scenarios.vm import utils
 from tests.unit import test
 
@@ -38,26 +36,55 @@ class VMScenarioTestCase(test.TestCase):
 
     @mock.patch("%s.open" % VMTASKS_UTILS,
                 side_effect=mock.mock_open(), create=True)
-    def test__run_command_over_ssh(self, mock_open):
+    def test__run_command_over_ssh_script_file(self, mock_open):
         mock_ssh = mock.MagicMock()
         vm_scenario = utils.VMScenario()
-        vm_scenario._run_command_over_ssh(mock_ssh, "interpreter", "script")
+        vm_scenario._run_command_over_ssh(
-        mock_ssh.execute.assert_called_once_with("interpreter",
+            mock_ssh,
-                                                 stdin=mock_open.side_effect())
+            {
+                "script_file": "foobar",
+                "interpreter": ["interpreter", "interpreter_arg"],
+            }
+        )
+        mock_ssh.execute.assert_called_once_with(
+            ["interpreter", "interpreter_arg"],
+            stdin=mock_open.side_effect())
+        mock_open.assert_called_once_with("foobar", "rb")
 
-    def test__run_command_over_ssh_stringio(self):
+    @mock.patch("%s.six.moves.StringIO" % VMTASKS_UTILS)
+    def test__run_command_over_ssh_script_inline(self, mock_stringio):
         mock_ssh = mock.MagicMock()
         vm_scenario = utils.VMScenario()
-        script = six.moves.StringIO("script")
+        vm_scenario._run_command_over_ssh(
-        vm_scenario._run_command_over_ssh(mock_ssh, "interpreter", script)
+            mock_ssh,
-        mock_ssh.execute.assert_called_once_with("interpreter",
+            {
-                                                 stdin=script)
+                "script_inline": "foobar",
+                "interpreter": ["interpreter", "interpreter_arg"],
+            }
+        )
+        mock_ssh.execute.assert_called_once_with(
+            ["interpreter", "interpreter_arg"],
+            stdin=mock_stringio.return_value)
+        mock_stringio.assert_called_once_with("foobar")
+
+    def test__run_command_over_ssh_remote_path(self):
+        mock_ssh = mock.MagicMock()
+        vm_scenario = utils.VMScenario()
+        vm_scenario._run_command_over_ssh(
+            mock_ssh,
+            {
+                "remote_path": ["foo", "bar"],
+            }
+        )
+        mock_ssh.execute.assert_called_once_with(
+            ["foo", "bar"],
+            stdin=None)
 
     def test__run_command_over_ssh_fails(self):
         vm_scenario = utils.VMScenario()
-        self.assertRaises(exceptions.ScriptError,
+        self.assertRaises(ValueError,
                           vm_scenario._run_command_over_ssh,
-                          None, "interpreter", 10)
+                          None, command={})
 
     def test__wait_for_ssh(self):
         ssh = mock.MagicMock()
@@ -85,35 +112,17 @@ class VMScenarioTestCase(test.TestCase):
 
         vm_scenario = utils.VMScenario()
         vm_scenario.context = {"user": {"keypair": {"private": "ssh"}}}
-        vm_scenario._run_command("1.2.3.4", 22, "username",
+        vm_scenario._run_command("1.2.3.4", 22, "username", "password",
-                                 "password", "int", "/path/to/foo/script.sh",
+                                 command={"script_file": "foo",
-                                 is_file=True)
+                                          "interpreter": "bar"})
 
         mock_ssh_class.assert_called_once_with("username", "1.2.3.4", port=22,
                                                pkey="ssh",
                                                password="password")
         mock_ssh_instance.wait.assert_called_once_with()
         mock_run_command_over_ssh.assert_called_once_with(
-            mock_ssh_instance, "int", "/path/to/foo/script.sh", True)
+            mock_ssh_instance,
-
+            {"script_file": "foo", "interpreter": "bar"})
-    @mock.patch(VMTASKS_UTILS + ".sshutils.SSH")
-    def test__run_command_inline_script(self, mock_ssh):
-        mock_ssh_instance = mock.MagicMock()
-        mock_ssh.return_value = mock_ssh_instance
-        mock_ssh_instance.execute.return_value = "foobar"
-        vm_scenario = utils.VMScenario()
-        vm_scenario._wait_for_ssh = mock.Mock()
-        vm_scenario.context = {"user": {"keypair": {"private": "foo_pkey"}}}
-        result = vm_scenario._run_command("foo_ip", "foo_port", "foo_username",
-                                          "foo_password", "foo_interpreter",
-                                          "foo_script", is_file=False)
-        mock_ssh.assert_called_once_with("foo_username", "foo_ip",
-                                         port="foo_port", pkey="foo_pkey",
-                                         password="foo_password")
-        vm_scenario._wait_for_ssh.assert_called_once_with(mock_ssh_instance)
-        mock_ssh_instance.execute.assert_called_once_with("foo_interpreter",
-                                                          stdin="foo_script")
-        self.assertEqual(result, "foobar")
 
     @mock.patch(VMTASKS_UTILS + ".sys")
     @mock.patch("subprocess.Popen")

tests/unit/plugins/openstack/scenarios/vm/test_vmtasks.py

@@ -37,8 +37,9 @@ class VMTasksTestCase(test.TestCase):
 
     def test_boot_runcommand_delete(self):
         self.scenario.boot_runcommand_delete(
-            "foo_image", "foo_flavor", "foo_script",
+            "foo_image", "foo_flavor",
-            "foo_interpreter", "foo_username",
+            script="foo_script", interpreter="foo_interpreter",
+            username="foo_username",
             password="foo_password",
             use_floating_ip="use_fip",
             floating_network="ext_network",
@@ -56,7 +57,8 @@ class VMTasksTestCase(test.TestCase):
 
         self.scenario._run_command.assert_called_once_with(
             "foo_ip", 22, "foo_username", "foo_password",
-            "foo_interpreter", "foo_script")
+            command={"script_file": "foo_script",
+                     "interpreter": "foo_interpreter"})
         self.scenario._delete_server_with_fip.assert_called_once_with(
             "foo_server", self.ip, force_delete="foo_force")