From c50509bc6f21b3b20b7d0fa6e6860d8e8336bc32 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 29 Sep 2020 02:50:38 +0100 Subject: [PATCH] Respect ``use_shared_home="false"`` when running tools in container When a tool doesn't specify a `profile` (or sets a `profile` < 18.01) but use ``use_shared_home="false"`` to ask to set a HOME within the job's directory (normally because the underlying tool writes to the home directory), this works as expected when resolving dependencies with conda. But when running the same tool in a Docker container (e.g. via `planemo test --biocontainers`), the job script didn't export a temporary HOME into the container (which instead setting `profile="18.01"` correctly does). Also add several integration tests for `use_shared_home`, of which `DockerizedJobsIntegrationTestCase.test_container_job_environment_explicit_isolated_home` would fail without this fix. --- lib/galaxy/tool_util/parser/xml.py | 7 ------ ...job_environment_explicit_isolated_home.xml | 25 +++++++++++++++++++ test/functional/tools/samples_tool_conf.xml | 1 + test/integration/test_containerized_jobs.py | 16 ++++++++++-- test/integration/test_job_environments.py | 23 ++++++++++++++--- 5 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 test/functional/tools/job_environment_explicit_isolated_home.xml diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 0a4b9613f74..1f9763a0e8f 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -177,13 +177,6 @@ class XmlToolSource(ToolSource): # break or modify any configurations by default. return "job_tmp_if_explicit" - def parse_docker_env_pass_through(self): - if self.parse_profile() < "18.01": - return ["GALAXY_SLOTS"] - else: - # Pass home, etc... - return super(XmlToolSource, self).parse_docker_env_pass_through() - def parse_interpreter(self): interpreter = None command_el = self._command_el diff --git a/test/functional/tools/job_environment_explicit_isolated_home.xml b/test/functional/tools/job_environment_explicit_isolated_home.xml new file mode 100644 index 00000000000..ddcbc2e529c --- /dev/null +++ b/test/functional/tools/job_environment_explicit_isolated_home.xml @@ -0,0 +1,25 @@ + + + busybox:ubuntu-14.04 + + '$user_id'; +echo `id -g` > '$group_id'; +echo `pwd` > '$pwd'; +echo "\$HOME" > '$home'; +echo "\$TMP" > '$tmp'; +echo "\$SOME_ENV_VAR" > '$some_env_var'; + ]]> + + + + + + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index a7ae1aabffa..121f8db0d06 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -68,6 +68,7 @@ + diff --git a/test/integration/test_containerized_jobs.py b/test/integration/test_containerized_jobs.py index fe04ecf2537..7650d90090e 100644 --- a/test/integration/test_containerized_jobs.py +++ b/test/integration/test_containerized_jobs.py @@ -109,10 +109,22 @@ class DockerizedJobsIntegrationTestCase(integration_util.IntegrationTestCase, Ru assert job_env.group_id == str(egid), job_env.group_id assert job_env.pwd.startswith(self.jobs_directory) assert job_env.pwd.endswith("/working") - # Should we change env_pass_through to just always include TMP and HOME for docker? - # I'm not sure, if yes this would change. assert not job_env.home.endswith('/home') + def test_container_job_environment_explicit_shared_home(self): + job_env = self._run_and_get_environment_properties("job_environment_explicit_shared_home") + + assert job_env.pwd.startswith(self.jobs_directory) + assert job_env.pwd.endswith("/working") + assert not job_env.home.endswith('/home') + + def test_container_job_environment_explicit_isolated_home(self): + job_env = self._run_and_get_environment_properties("job_environment_explicit_isolated_home") + + assert job_env.pwd.startswith(self.jobs_directory) + assert job_env.pwd.endswith("/working") + assert job_env.home.endswith('/home') + def test_build_mulled(self): if not which('docker'): raise unittest.SkipTest("Docker not found on PATH, required for building images via involucro") diff --git a/test/integration/test_job_environments.py b/test/integration/test_job_environments.py index 7b2892e4524..78bcf3f2246 100644 --- a/test/integration/test_job_environments.py +++ b/test/integration/test_job_environments.py @@ -77,7 +77,7 @@ class DefaultJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTest assert job_env.pwd.startswith(self.jobs_directory) assert job_env.pwd.endswith("/working") - # Newer tools have isolated home directories in job_directory/home + # Newer tools get isolated home directories in job_directory/home job_directory = os.path.dirname(job_env.pwd) assert job_env.home == os.path.join(job_directory, "home"), job_env.home @@ -98,12 +98,22 @@ class DefaultJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTest @skip_without_tool("job_environment_explicit_shared_home") def test_default_environment_force_legacy_home(self): - # Home should not overridden because we haven't set legacy_home_dir in job_conf - # or app, so it should just HOME. + # Home should not be overridden because we haven't set legacy_home_dir in job_conf + # or app, so it should just be HOME. job_env = self._run_and_get_environment_properties("job_environment_explicit_shared_home") home = os.getenv("HOME") assert job_env.home == home, job_env.home + @skip_without_tool("job_environment_explicit_isolated_home") + def test_default_environment_explicit_isolated_home(self): + # A tool with no profile but setting `use_shared_home="false"` must get + # an isolated home directory in job_directory/home + job_env = self._run_and_get_environment_properties("job_environment_explicit_isolated_home") + assert job_env.pwd.startswith(self.jobs_directory) + assert job_env.pwd.endswith("/working") + job_directory = os.path.dirname(job_env.pwd) + assert job_env.home == os.path.join(job_directory, "home"), job_env.home + class TmpDirToTrueJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase): @@ -170,6 +180,13 @@ class SharedHomeJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationT job_env = self._run_and_get_environment_properties("job_environment_explicit_shared_home") assert job_env.home == self.shared_home_directory, job_env.home + @skip_without_tool("job_environment_explicit_isolated_home") + def test_default_environment_explicit_isolated_home(self): + # shared_home_dir ignored for tools setting `use_shared_home="false"` + job_env = self._run_and_get_environment_properties("job_environment_explicit_isolated_home") + job_directory = os.path.dirname(job_env.pwd) + assert job_env.home == os.path.join(job_directory, "home"), job_env.home + class JobIOEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase):