From 59aa4e8b91f45b11cca93730c7f6b9239828b429 Mon Sep 17 00:00:00 2001 From: Yaroslav Lobankov Date: Tue, 9 Aug 2016 19:21:58 +0300 Subject: [PATCH] [Verify] Fixing issue with system-wide Tempest installation The issue was noticed in a Rally docker container. In the docker container, as we know, Rally is installed in the global system environment. When we try to execute $ rally verify install --system-wide Tempest installation will be broken on $ pip install --no-deps -e ./ because of permission issues. The command requires the 'sudo' prefix. So it looks like the best solution for resolving the issue is that user will take care of installing Tempest package like he does for Tempest requirements. Change-Id: I3c606d1f659600f027ecaf6fe99eae26c329c9a3 --- rally/api.py | 6 ++-- rally/cli/commands/verify.py | 16 +++++++---- rally/verification/tempest/tempest.py | 37 +++++++++++++++---------- tests/unit/verification/test_tempest.py | 19 ++++++------- 4 files changed, 44 insertions(+), 34 deletions(-) diff --git a/rally/api.py b/rally/api.py index e79dc84ab7..706d626494 100644 --- a/rally/api.py +++ b/rally/api.py @@ -455,7 +455,8 @@ class Verification(object): :param source: Path/URL to repo to clone Tempest from :param version: Commit ID or tag to checkout before Tempest installation - :param system_wide: Whether or not to create a Tempest virtual env + :param system_wide: Whether or not to install Tempest package and + create a Tempest virtual env """ deployment_uuid = objects.Deployment.get(deployment)["uuid"] verifier = tempest.Tempest(deployment_uuid, source=source, @@ -481,7 +482,8 @@ class Verification(object): :param source: Path/URL to repo to clone Tempest from :param version: Commit ID or tag to checkout before Tempest installation - :param system_wide: Whether or not to create a Tempest virtual env + :param system_wide: Whether or not to install Tempest package and + create a Tempest virtual env """ deployment_uuid = objects.Deployment.get(deployment)["uuid"] verifier = tempest.Tempest(deployment_uuid, source=source, diff --git a/rally/cli/commands/verify.py b/rally/cli/commands/verify.py index e3c1d70da2..295e9ee088 100644 --- a/rally/cli/commands/verify.py +++ b/rally/cli/commands/verify.py @@ -420,8 +420,9 @@ class VerifyCommands(object): help="Commit ID or tag to checkout before Tempest " "installation") @cliutils.args("--system-wide", dest="system_wide", - help="Don't create a virtual env for Tempest. Note " - "that all Tempest requirements have to be already " + help="Not to install Tempest package and not to create " + "a virtual env for Tempest. Note that Tempest package " + "and all Tempest requirements have to be already " "installed in the local env!", required=False, action="store_true") @envutils.with_default_deployment(cli_arg_name="deployment") @@ -433,7 +434,8 @@ class VerifyCommands(object): :param source: Path/URL to repo to clone Tempest from :param version: Commit ID or tag to checkout before Tempest installation - :param system_wide: Whether or not to create a Tempest virtual env + :param system_wide: Whether or not to install Tempest package and + create a Tempest virtual env """ api.Verification.install_tempest(deployment, source, version, system_wide) @@ -458,8 +460,9 @@ class VerifyCommands(object): help="Commit ID or tag to checkout before Tempest " "installation") @cliutils.args("--system-wide", dest="system_wide", - help="Don't create a virtual env for Tempest. Note " - "that all Tempest requirements have to be already " + help="Not to install Tempest package and not to create " + "a virtual env for Tempest. Note that Tempest package " + "and all Tempest requirements have to be already " "installed in the local env!", required=False, action="store_true") @envutils.with_default_deployment(cli_arg_name="deployment") @@ -471,7 +474,8 @@ class VerifyCommands(object): :param source: Path/URL to repo to clone Tempest from :param version: Commit ID or tag to checkout before Tempest installation - :param system_wide: Whether or not to create a Tempest virtual env + :param system_wide: Whether or not to install Tempest package and + create a Tempest virtual env """ api.Verification.reinstall_tempest(deployment, source, version, system_wide) diff --git a/rally/verification/tempest/tempest.py b/rally/verification/tempest/tempest.py index 4d2774d9e0..8f31547b50 100644 --- a/rally/verification/tempest/tempest.py +++ b/rally/verification/tempest/tempest.py @@ -41,7 +41,7 @@ class TempestSetupFailure(exceptions.RallyException): def check_output(*args, **kwargs): - print_debug_output = kwargs.pop("print_debug_output", True) + debug = kwargs.pop("debug", True) kwargs["stderr"] = subprocess.STDOUT try: output = subprocess.check_output(*args, **kwargs) @@ -50,8 +50,9 @@ def check_output(*args, **kwargs): LOG.error("Error output: '%s'" % encodeutils.safe_decode(e.output)) raise - if print_debug_output: + if debug: LOG.debug("Subprocess output: '%s'" % encodeutils.safe_decode(output)) + return output @@ -193,10 +194,8 @@ class Tempest(object): cwd=self.path()) # NOTE(kun): Using develop mode installation is for running # multiple Tempest instances. - check_output([self.venv_wrapper, - "pip", "install", "-r", - "requirements.txt", "-r", - "test-requirements.txt"], cwd=self.path()) + check_output([self.venv_wrapper, "pip", "install", "-e", "./"], + cwd=self.path()) except subprocess.CalledProcessError: if os.path.exists(self.path(".venv")): shutil.rmtree(self.path(".venv")) @@ -268,14 +267,14 @@ class Tempest(object): if not self._is_git_repo(self.base_repo): self._clone() shutil.copytree(self.base_repo, self.path()) - if self.version: - cmd = ["git", "checkout", self.version] - subprocess.check_call(cmd, cwd=self.path("tempest")) - cmd = ["pip", "install", "--no-deps", "-e", "./"] + + if self.version: + check_output(["git", "checkout", self.version], + cwd=self.path()) + if not self._system_wide: self._install_venv() - cmd.insert(0, self.path("tools/with_venv.sh")) - check_output(cmd, cwd=self.path()) + self._initialize_testr() except subprocess.CalledProcessError as e: self.uninstall() @@ -313,10 +312,18 @@ class Tempest(object): def list_plugins(self): """List all installed Tempest plugins for local Tempest repo.""" - cmd = ["tempest", "list-plugins"] + cmd_list_plugins = ["tempest", "list-plugins"] if not self._system_wide: - cmd.insert(0, self.path("tools/with_venv.sh")) - return check_output(cmd, cwd=self.path(), print_debug_output=False) + cmd_list_plugins.insert(0, self.path("tools/with_venv.sh")) + else: + cmd_pip_list = ["pip", "list"] + if "tempest" not in check_output(cmd_pip_list, + cwd=self.path(), debug=False): + return _("Cannot list Tempest plugins because Tempest " + "package is not installed in your environment. " + "Please, install Tempest package and try again.") + + return check_output(cmd_list_plugins, cwd=self.path(), debug=False) def uninstall_plugin(self, repo_name): """Uninstall Tempest plugin for local Tempest repo.""" diff --git a/tests/unit/verification/test_tempest.py b/tests/unit/verification/test_tempest.py index aa6fa1234d..48338c3aea 100644 --- a/tests/unit/verification/test_tempest.py +++ b/tests/unit/verification/test_tempest.py @@ -98,8 +98,8 @@ class TempestUtilsTestCase(BaseTestCase): @mock.patch("os.path.isdir", return_value=False) @mock.patch("%s.tempest.check_output" % TEMPEST_PATH, return_value="some_output") - def test__venv_install_when_venv_not_exist(self, mock_check_output, - mock_isdir, mock_sys): + def test__venv_install_when_venv_does_not_exist(self, mock_check_output, + mock_isdir, mock_sys): mock_sys.version_info = "not_py27_env" self.verifier._install_venv() @@ -107,9 +107,8 @@ class TempestUtilsTestCase(BaseTestCase): mock_check_output.assert_has_calls([ mock.call(["virtualenv", "-p", mock_sys.executable, ".venv"], cwd="/tmp"), - mock.call(["/tmp/tools/with_venv.sh", "pip", "install", "-r", - "requirements.txt", "-r", "test-requirements.txt"], - cwd="/tmp") + mock.call(["/tmp/tools/with_venv.sh", "pip", + "install", "-e", "./"], cwd="/tmp") ]) @mock.patch("%s.tempest.sys" % TEMPEST_PATH) @@ -234,7 +233,6 @@ class TempestInstallAndUninstallTestCase(BaseTestCase): @mock.patch(TEMPEST_PATH + ".tempest.Tempest._initialize_testr") @mock.patch(TEMPEST_PATH + ".tempest.Tempest._install_venv") @mock.patch(TEMPEST_PATH + ".tempest.check_output") - @mock.patch(TEMPEST_PATH + ".tempest.subprocess.check_call") @mock.patch("shutil.copytree") @mock.patch(TEMPEST_PATH + ".tempest.Tempest._clone") @mock.patch("os.path.exists", return_value=False) @@ -242,8 +240,7 @@ class TempestInstallAndUninstallTestCase(BaseTestCase): return_value=False) def test_install_successful(self, mock_tempest__is_git_repo, mock_exists, mock_tempest__clone, mock_copytree, - mock_check_call, mock_check_output, - mock_tempest__install_venv, + mock_check_output, mock_tempest__install_venv, mock_tempest__initialize_testr, mock_tempest_base_repo): mock_tempest_base_repo.__get__ = mock.Mock(return_value="fake_dir") @@ -258,9 +255,9 @@ class TempestInstallAndUninstallTestCase(BaseTestCase): mock_copytree.assert_called_once_with( self.verifier.base_repo, self.verifier.path()) - cwd = self.verifier.path("tempest") + cwd = self.verifier.path() expected = [mock.call(["git", "checkout", "3f4c8d44"], cwd=cwd)] - self.assertEqual(expected, mock_check_call.mock_calls) + self.assertEqual(expected, mock_check_output.mock_calls) mock_tempest__install_venv.assert_called_once_with() mock_tempest__initialize_testr.assert_called_once_with() @@ -347,7 +344,7 @@ class TempestInstallPluginsTestCase(BaseTestCase): cmd = [self.verifier.venv_wrapper, "tempest", "list-plugins"] mock_tempest_check_output.assert_called_with( - cmd, cwd=self.verifier.path(), print_debug_output=False) + cmd, cwd=self.verifier.path(), debug=False) @mock.patch("shutil.rmtree") @mock.patch("os.path.exists")