From 8d8bf5eaffeda0f229bf56cb45e4adbb05b12d57 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sun, 12 Mar 2017 23:40:45 -0400 Subject: [PATCH 1/4] Dockerized job integration testing. Add simple test cases for both mulled containers and explicit Docker image resolution. --- lib/galaxy/config.py | 2 +- test/base/driver_util.py | 2 + .../tools/mulled_example_explicit.xml | 22 ++++++++++ .../tools/mulled_example_simple.xml | 21 ++++++++++ test/functional/tools/samples_tool_conf.xml | 2 + test/integration/dockerized_job_conf.xml | 26 ++++++++++++ test/integration/test_dockerized_jobs.py | 41 +++++++++++++++++++ 7 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 test/functional/tools/mulled_example_explicit.xml create mode 100644 test/functional/tools/mulled_example_simple.xml create mode 100644 test/integration/dockerized_job_conf.xml create mode 100644 test/integration/test_dockerized_jobs.py diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py index 90dc5c1e293..7ff94d69a95 100644 --- a/lib/galaxy/config.py +++ b/lib/galaxy/config.py @@ -480,7 +480,7 @@ class Configuration(object): involucro_path = kwargs.get('involucro_path', None) if involucro_path is None: - involucro_path = os.path.join(tool_dependency_dir, "involucro") + involucro_path = os.path.join(tool_dependency_dir or "database", "involucro") self.involucro_path = resolve_path(involucro_path, self.root) self.involucro_auto_init = string_as_bool(kwargs.get('involucro_auto_init', True)) diff --git a/test/base/driver_util.py b/test/base/driver_util.py index 29e99a8aef7..95c702ee0e4 100644 --- a/test/base/driver_util.py +++ b/test/base/driver_util.py @@ -131,6 +131,8 @@ def setup_galaxy_config( log_format=None, ): """Setup environment and build config for test Galaxy instance.""" + # For certain docker operations this needs to be evaluated out - e.g. for cwltool. + tmpdir = os.path.realpath(tmpdir) if not os.path.exists(tmpdir): os.makedirs(tmpdir) file_path = os.path.join(tmpdir, 'files') diff --git a/test/functional/tools/mulled_example_explicit.xml b/test/functional/tools/mulled_example_explicit.xml new file mode 100644 index 00000000000..f341dae5bda --- /dev/null +++ b/test/functional/tools/mulled_example_explicit.xml @@ -0,0 +1,22 @@ + + + bwa + quay.io/biocontainers/bwa:0.7.15--0 + + + + + $output_1 2>&1 + ]]> + + + + + + + + + \ No newline at end of file diff --git a/test/functional/tools/mulled_example_simple.xml b/test/functional/tools/mulled_example_simple.xml new file mode 100644 index 00000000000..d061297a93f --- /dev/null +++ b/test/functional/tools/mulled_example_simple.xml @@ -0,0 +1,21 @@ + + + bwa + + + + + $output_1 2>&1 + ]]> + + + + + + + + + \ No newline at end of file diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index 25d646b1c33..512e2925aaa 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -159,6 +159,8 @@ + + diff --git a/test/integration/dockerized_job_conf.xml b/test/integration/dockerized_job_conf.xml new file mode 100644 index 00000000000..44097003630 --- /dev/null +++ b/test/integration/dockerized_job_conf.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + true + false + + + + + + + + + + + + diff --git a/test/integration/test_dockerized_jobs.py b/test/integration/test_dockerized_jobs.py new file mode 100644 index 00000000000..88aa843938c --- /dev/null +++ b/test/integration/test_dockerized_jobs.py @@ -0,0 +1,41 @@ +"""Integration tests for running tools in Docker containers.""" + +import os + +from base import integration_util +from base.populators import ( + DatasetPopulator, +) + +SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) +DOCKERIZED_JOB_CONFIG_FILE = os.path.join(SCRIPT_DIRECTORY, "dockerized_job_conf.xml") +# DOCKERIZED_JOB_DEPENDENCY_RESOLVERS_CONF = os.path.join(SCRIPT_DIRECTORY, "dockerzied_dependency_resolvers_conf.xml") + + +class DockerizedJobsIntegrationTestCase(integration_util.IntegrationTestCase): + + framework_tool_and_types = True + + @classmethod + def handle_galaxy_config_kwds(cls, config): + config["job_config_file"] = DOCKERIZED_JOB_CONFIG_FILE + # Disable tool dependency resolution. + config["tool_dependency_dir"] = "none" + config["enable_beta_mulled_containers"] = "true" + + def setUp(self): + super(DockerizedJobsIntegrationTestCase, self).setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + self.history_id = self.dataset_populator.new_history() + + def test_explicit(self): + self.dataset_populator.run_tool("mulled_example_explicit", {}, self.history_id) + self.dataset_populator.wait_for_history(self.history_id, assert_ok=True) + output = self.dataset_populator.get_history_dataset_content(self.history_id) + assert "0.7.15-r1140" in output + + def test_mulled_simple(self): + self.dataset_populator.run_tool("mulled_example_simple", {}, self.history_id) + self.dataset_populator.wait_for_history(self.history_id, assert_ok=True) + output = self.dataset_populator.get_history_dataset_content(self.history_id) + assert "0.7.15-r1140" in output From cd04ff30665ae6f15a6af35152af54d630458e60 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 13 Sep 2017 16:38:00 -0400 Subject: [PATCH 2/4] Set group id in addition to user id when running docker containers by default. Noticed cwltool does this and it is a good idea. It wasn't the bug I was tracking but it is a solid improvement. --- lib/galaxy/tools/deps/docker_util.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/deps/docker_util.py b/lib/galaxy/tools/deps/docker_util.py index 629cc59d81d..a28b2103e71 100644 --- a/lib/galaxy/tools/deps/docker_util.py +++ b/lib/galaxy/tools/deps/docker_util.py @@ -175,8 +175,13 @@ def build_docker_run_command( if set_user: user = set_user if set_user == DEFAULT_SET_USER: - user = str(os.geteuid()) - command_parts.extend(["-u", user]) + # If future-us is ever in here and fixing this for docker-machine just + # use cwltool.docker_id - it takes care of this default nicely. + euid = os.geteuid() + egid = os.getgid() + + user = "%d:%d" % (euid, egid) + command_parts.extend(["--user", user]) full_image = image if tag: full_image = "%s:%s" % (full_image, tag) From 4bf47d08770c14010bf925c73ccba2262dfe78ee Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 8 Jan 2018 16:35:44 -0500 Subject: [PATCH 3/4] Improved home and temp directory handling. Home Directory Handling ----------------------- - For profile < 18.01 tools: - If use_shared_home="false" is set on the command block - the tool will be given a clean $HOME directory. - If ``shared_home_dir`` is set in Galaxy's config or the job destination configuration, Galaxy will set $HOME in the tool's job environment to point at this directory. - For profile >= 18.01 tools, jobs will be given a clean home directory by default unless ``use_shared_home="true"`` is set. If that is set, the profile < 18.01 behavior is used. In addition to these changes for Galaxy tools, the tool framework itself has been updated to allow other potential behaviors including the CWL defaults. Integration tests for each of these cases has been added. Upgrading to 18.01 - we recommend dropping overriding ``HOME`` in any Galaxy environment configuration (e.g. env directives in job_conf.xml). If any such configuration is found, we recommend instead setting "shared_home_dir" for destinations to the that cluster destination's shared HOME directory. If all destinations share a single HOME directory, this can be set in galaxy.ini instead of job_conf.xml. Temp Directory Handling ----------------------- There were serious disagreements with how to proceed here. I felt we should aggressively isolate tool TMP directories and provide deployers options to tweak these through structured arguments. Nicola felt we should defer to the environment variables already being set job_conf.xml - but tweak their meanings by default. Nate fell somewhere in between. I think all sides have merit and could make sense depending on what you want to improve (easing cognative load, reducing out-of-box errors, easing advanced deployer configs, structured reasonsing of paths by Galaxy, etc...). I've navigated a path here that doesn't particularly reduce out of the box errors as I wanted by setting up a per-job temp directory by default but does make it possible and shouldn't break any existing configurations and doesn't make it anything more difficult to manage these things via environment variables. The new approach doesn't change any thing by default for Galaxy tools (regardless of tool profile version). The only way to achieve new behaviors is for the deployer to set a job_conf parameter called "tmp_dir". If this is set, it can be set to "True" (to default to a new, clean directory below the job directory) or to a shell expression to allow setting this dynamically at runtime to paths or paths beneath directories as needed on a per-destination basis. If a temp directory is set this way all three variables TMP, TMPDIR, and TEMP are set. Docker Environment Handling --------------------------- - Mirror the CWL behavior of mounting an external /tmp by default. - For newer tools, pass through the HOME, TMP, TMPDIR, and TEMP environment variables into the Docker container in addition to GALAXY_SLOTS. - Fix the pass through of environment variables into Docker containers. --- config/job_conf.xml.sample_advanced | 17 ++ lib/galaxy/config.py | 2 + lib/galaxy/jobs/__init__.py | 56 ++++++ lib/galaxy/jobs/rule_helper.py | 2 +- lib/galaxy/jobs/runners/__init__.py | 9 +- lib/galaxy/jobs/runners/pulsar.py | 10 ++ .../job_script/DEFAULT_JOB_FILE_TEMPLATE.sh | 5 + .../jobs/runners/util/job_script/__init__.py | 1 + lib/galaxy/tools/__init__.py | 15 ++ lib/galaxy/tools/deps/containers.py | 17 +- lib/galaxy/tools/deps/docker_util.py | 2 +- lib/galaxy/tools/evaluation.py | 9 + lib/galaxy/tools/parser/interface.py | 17 ++ lib/galaxy/tools/parser/xml.py | 21 +++ lib/galaxy/tools/xsd/galaxy.xsd | 5 + test/base/integration_util.py | 11 ++ .../tools/job_environment_default.xml | 23 +++ .../tools/job_environment_default_legacy.xml | 23 +++ .../job_environment_explicit_shared_home.xml | 23 +++ test/functional/tools/samples_tool_conf.xml | 3 + .../sets_tmp_dir_expression_job_conf.xml | 17 ++ .../sets_tmp_dir_to_true_job_conf.xml | 17 ++ test/integration/simple_job_conf.xml | 16 ++ test/integration/test_dockerized_jobs.py | 35 +++- test/integration/test_job_environments.py | 165 ++++++++++++++++++ test/unit/jobs/test_runner_local.py | 7 + test/unit/tools/test_evaluation.py | 14 ++ 27 files changed, 534 insertions(+), 8 deletions(-) create mode 100644 test/functional/tools/job_environment_default.xml create mode 100644 test/functional/tools/job_environment_default_legacy.xml create mode 100644 test/functional/tools/job_environment_explicit_shared_home.xml create mode 100644 test/integration/sets_tmp_dir_expression_job_conf.xml create mode 100644 test/integration/sets_tmp_dir_to_true_job_conf.xml create mode 100644 test/integration/simple_job_conf.xml create mode 100644 test/integration/test_job_environments.py diff --git a/config/job_conf.xml.sample_advanced b/config/job_conf.xml.sample_advanced index 89ae94aba71..9a26eaa1427 100644 --- a/config/job_conf.xml.sample_advanced +++ b/config/job_conf.xml.sample_advanced @@ -542,6 +542,23 @@ itself. --> + + + True + + + "$DRM_SET_VARIABLES_FOR_THIS_JOB" + + + $(mktemp -d /mnt/scratch/fastest/gxyjobXXXXXXXXXXX) + diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py index 7ff94d69a95..0a34136983d 100644 --- a/lib/galaxy/config.py +++ b/lib/galaxy/config.py @@ -187,10 +187,12 @@ class Configuration(object): # Where dataset files are stored self.file_path = resolve_path(kwargs.get("file_path", "database/files"), self.root) + # new_file_path and legacy_home_dir can be overridden per destination in job_conf. self.new_file_path = resolve_path(kwargs.get("new_file_path", "database/tmp"), self.root) override_tempdir = string_as_bool(kwargs.get("override_tempdir", "True")) if override_tempdir: tempfile.tempdir = self.new_file_path + self.shared_home_dir = kwargs.get("shared_home_dir", None) self.openid_consumer_cache_path = resolve_path(kwargs.get("openid_consumer_cache_path", "database/openid_consumer_cache"), self.root) self.cookie_path = kwargs.get("cookie_path", "/") # Galaxy OpenID settings diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index de1c32538bc..703361cd9df 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -1110,6 +1110,15 @@ class JobWrapper(object, HasResourceParameters): if flush: self.sa_session.flush() + @property + def home_target(self): + home_target = self.tool.home_target + return home_target + + @property + def tmp_target(self): + return self.tool.tmp_target + def get_destination_configuration(self, key, default=None): """ Get a destination parameter that can be defaulted back in app.config if it needs to be applied globally. @@ -1603,6 +1612,39 @@ class JobWrapper(object, HasResourceParameters): return dp.dataset_id return None + @property + def tmp_dir_creation_statement(self): + tmp_dir = self.get_destination_configuration("tmp_dir", None) + if not tmp_dir or tmp_dir.lower() == "true": + working_directory = self.working_directory + return '$(mktemp -d "%s/tmp.XXXXXXXXX")' % working_directory + else: + return tmp_dir + + def home_directory(self): + home_target = self.home_target + return self._target_to_directory(home_target) + + def tmp_directory(self): + tmp_target = self.tmp_target + return self._target_to_directory(tmp_target) + + def _target_to_directory(self, target): + working_directory = self.working_directory + tmp_dir = self.get_destination_configuration("tmp_dir", None) + if target is None or (target == "job_tmp_if_explicit" and tmp_dir is None): + return None + elif target in ["job_tmp", "job_tmp_if_explicit"]: + return "$_GALAXY_JOB_TMP_DIR" + elif target == "shared_home": + return self.get_destination_configuration("shared_home_dir", None) + elif target == "job_home": + return "$_GALAXY_JOB_HOME_DIR" + elif target == "pwd": + return os.path.join(working_directory, "working") + else: + raise Exception("Unknown target type [%s]" % target) + def get_tool_provided_job_metadata(self): if self.tool_provided_job_metadata is not None: return self.tool_provided_job_metadata @@ -2027,6 +2069,14 @@ class ComputeEnvironment(object): be rewritten.) """ + @abstractmethod + def home_directory(self): + """Home directory of target job - none if HOME should not be set.""" + + @abstractmethod + def tmp_directory(self): + """Temp directory of target job - none if HOME should not be set.""" + class SimpleComputeEnvironment(object): @@ -2072,6 +2122,12 @@ class SharedComputeEnvironment(SimpleComputeEnvironment): def tool_directory(self): return os.path.abspath(self.job_wrapper.tool.tool_dir) + def home_directory(self): + return self.job_wrapper.home_directory() + + def tmp_directory(self): + return self.job_wrapper.tmp_directory() + class NoopQueue(object): """ diff --git a/lib/galaxy/jobs/rule_helper.py b/lib/galaxy/jobs/rule_helper.py index 3261d636703..b8664aa12fc 100644 --- a/lib/galaxy/jobs/rule_helper.py +++ b/lib/galaxy/jobs/rule_helper.py @@ -44,7 +44,7 @@ class RuleHelper(object): tool = self.app.toolbox.get_tool(job_or_tool.tool_id, tool_version=job_or_tool.tool_version) # Can't import at top because circular import between galaxy.tools and galaxy.jobs. import galaxy.tools.deps.containers - tool_info = galaxy.tools.deps.containers.ToolInfo(tool.containers, tool.requirements, tool.requires_galaxy_python_environment) + tool_info = galaxy.tools.deps.containers.ToolInfo(tool.containers, tool.requirements, tool.requires_galaxy_python_environment, tool.docker_env_pass_through) container_description = self.app.container_finder.find_best_container_description(["docker"], tool_info) return container_description is not None diff --git a/lib/galaxy/jobs/runners/__init__.py b/lib/galaxy/jobs/runners/__init__.py index cf568c4d323..ff9953b6304 100644 --- a/lib/galaxy/jobs/runners/__init__.py +++ b/lib/galaxy/jobs/runners/__init__.py @@ -327,7 +327,9 @@ class BaseJobRunner(object): for env in envs: env_setup_commands.append(env_to_statement(env)) command_line = job_wrapper.runner_command_line + tmp_dir_creation_statement = job_wrapper.tmp_dir_creation_statement options = dict( + tmp_dir_creation_statement=tmp_dir_creation_statement, job_instrumenter=job_instrumenter, galaxy_lib=job_wrapper.galaxy_lib_dir, galaxy_virtual_env=job_wrapper.galaxy_virtual_env, @@ -353,6 +355,7 @@ class BaseJobRunner(object): compute_working_directory=None, compute_tool_directory=None, compute_job_directory=None, + compute_tmp_directory=None, ): job_directory_type = "galaxy" if compute_working_directory is None else "pulsar" if not compute_working_directory: @@ -364,13 +367,17 @@ class BaseJobRunner(object): if not compute_tool_directory: compute_tool_directory = job_wrapper.tool.tool_dir + if not compute_tmp_directory: + compute_tmp_directory = job_wrapper.tmp_directory() + tool = job_wrapper.tool from galaxy.tools.deps import containers - tool_info = containers.ToolInfo(tool.containers, tool.requirements, tool.requires_galaxy_python_environment) + tool_info = containers.ToolInfo(tool.containers, tool.requirements, tool.requires_galaxy_python_environment, tool.docker_env_pass_through) job_info = containers.JobInfo( compute_working_directory, compute_tool_directory, compute_job_directory, + compute_tmp_directory, job_directory_type, ) diff --git a/lib/galaxy/jobs/runners/pulsar.py b/lib/galaxy/jobs/runners/pulsar.py index bc201e74626..5fa23cc1a16 100644 --- a/lib/galaxy/jobs/runners/pulsar.py +++ b/lib/galaxy/jobs/runners/pulsar.py @@ -871,6 +871,16 @@ class PulsarComputeEnvironment(ComputeEnvironment): def tool_directory(self): return self._tool_dir + def home_directory(self): + # TODO: revisit and implement this, won't break anything working in the + # meantime. + return None + + def tmp_directory(self): + # TODO: revisit and implement this, won't break anything working in the + # meantime. + return None + class UnsupportedPulsarException(Exception): diff --git a/lib/galaxy/jobs/runners/util/job_script/DEFAULT_JOB_FILE_TEMPLATE.sh b/lib/galaxy/jobs/runners/util/job_script/DEFAULT_JOB_FILE_TEMPLATE.sh index f817610559b..e475ade6067 100644 --- a/lib/galaxy/jobs/runners/util/job_script/DEFAULT_JOB_FILE_TEMPLATE.sh +++ b/lib/galaxy/jobs/runners/util/job_script/DEFAULT_JOB_FILE_TEMPLATE.sh @@ -12,7 +12,12 @@ _galaxy_setup_environment() { fi export PYTHONPATH fi + _GALAXY_JOB_HOME_DIR="$working_directory/home" + _GALAXY_JOB_TMP_DIR=$tmp_dir_creation_statement $env_setup_commands + # These don't get cleaned on a re-run but may in the future. + [ -z "$_GALAXY_JOB_TMP_DIR" -a ! -f "$_GALAXY_JOB_TMP_DIR" ] || mkdir -p "$_GALAXY_JOB_TMP_DIR" + [ -z "$_GALAXY_JOB_HOME_DIR" -a ! -f "$_GALAXY_JOB_HOME_DIR" ] || mkdir -p "$_GALAXY_JOB_HOME_DIR" if [ "$GALAXY_VIRTUAL_ENV" != "None" -a -f "$GALAXY_VIRTUAL_ENV/bin/activate" \ -a "`command -v python`" != "$GALAXY_VIRTUAL_ENV/bin/python" ]; then . "$GALAXY_VIRTUAL_ENV/bin/activate" diff --git a/lib/galaxy/jobs/runners/util/job_script/__init__.py b/lib/galaxy/jobs/runners/util/job_script/__init__.py index e58daa57bf3..73bfeb64852 100644 --- a/lib/galaxy/jobs/runners/util/job_script/__init__.py +++ b/lib/galaxy/jobs/runners/util/job_script/__init__.py @@ -52,6 +52,7 @@ OPTIONAL_TEMPLATE_PARAMS = { 'integrity_injection': INTEGRITY_INJECTION, 'shell': DEFAULT_SHELL, 'preserve_python_environment': True, + 'tmp_dir_creation_statement': '""', } diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index d9b3dc74dfc..6bbe8eefd7c 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -623,6 +623,21 @@ class Tool(object, Dictifiable): self.parse_command(tool_source) self.environment_variables = self.parse_environment_variables(tool_source) + self.tmp_directories = tool_source.parse_tmp_directories() + + home_target = tool_source.parse_home_target() + tmp_target = tool_source.parse_tmp_target() + # If a tool explicitly sets one of these variables just respect that and turn off + # explicit processing by Galaxy. + for environment_variable in self.environment_variables: + if environment_variable.get("name") == "HOME": + home_target = None + for tmp_directory in self.tmp_directories: + if environment_variable.get("name") == tmp_directory: + tmp_target = None + self.home_target = home_target + self.tmp_target = tmp_target + self.docker_env_pass_through = tool_source.parse_docker_env_pass_through() # Parameters used to build URL for redirection to external app redirect_url_params = tool_source.parse_redirect_url_params_elem() diff --git a/lib/galaxy/tools/deps/containers.py b/lib/galaxy/tools/deps/containers.py index ab00a1b0b1e..3e4998edfb2 100644 --- a/lib/galaxy/tools/deps/containers.py +++ b/lib/galaxy/tools/deps/containers.py @@ -283,21 +283,24 @@ class ToolInfo(object): # variables they can consume (e.g. JVM options, license keys, etc..) # and add these to env_path_through - def __init__(self, container_descriptions=[], requirements=[], requires_galaxy_python_environment=False): + def __init__(self, container_descriptions=[], requirements=[], requires_galaxy_python_environment=False, env_pass_through=["GALAXY_SLOTS"]): self.container_descriptions = container_descriptions self.requirements = requirements self.requires_galaxy_python_environment = requires_galaxy_python_environment - self.env_pass_through = ["GALAXY_SLOTS"] + self.env_pass_through = env_pass_through class JobInfo(object): - def __init__(self, working_directory, tool_directory, job_directory, job_directory_type): + def __init__( + self, working_directory, tool_directory, job_directory, tmp_directory, job_directory_type + ): self.working_directory = working_directory self.job_directory = job_directory # Tool files may be remote staged - so this is unintuitively a property # of the job not of the tool. self.tool_directory = tool_directory + self.tmp_directory = tmp_directory self.job_directory_type = job_directory_type # "galaxy" or "pulsar" @@ -394,6 +397,7 @@ class HasDockerLikeVolumes: variables[name] = os.path.abspath(value) add_var("working_directory", self.job_info.working_directory) + add_var("tmp_directory", self.job_info.tmp_directory) add_var("job_directory", self.job_info.job_directory) add_var("tool_directory", self.job_info.tool_directory) add_var("galaxy_root", self.app_info.galaxy_root_dir) @@ -408,6 +412,8 @@ class HasDockerLikeVolumes: defaults = "$galaxy_root:default_ro,$tool_directory:default_ro" if self.job_info.job_directory: defaults += ",$job_directory:default_ro" + if self.job_info.tmp_directory is not None: + defaults += ",$tmp_directory:rw" if self.app_info.outputs_to_working_directory: # Should need default_file_path (which is of course an estimate given # object stores anyway). @@ -454,6 +460,11 @@ class DockerContainer(Container, HasDockerLikeVolumes): preprocessed_volumes_str = preprocess_volumes(volumes_raw, self.container_type) # TODO: Remove redundant volumes... volumes = docker_util.DockerVolume.volumes_from_str(preprocessed_volumes_str) + # If a tool definitely has a temp directory available set it to /tmp in container for compat. + # with CWL. This is part of that spec and should make it easier to share containers between CWL + # and Galaxy. + if self.job_info.tmp_directory is not None: + volumes.append(docker_util.DockerVolume.volume_from_str("%s:/tmp:rw" % self.job_info.tmp_directory)) volumes_from = self.destination_info.get("docker_volumes_from", docker_util.DEFAULT_VOLUMES_FROM) docker_host_props = dict( diff --git a/lib/galaxy/tools/deps/docker_util.py b/lib/galaxy/tools/deps/docker_util.py index a28b2103e71..e97da9c737b 100644 --- a/lib/galaxy/tools/deps/docker_util.py +++ b/lib/galaxy/tools/deps/docker_util.py @@ -155,7 +155,7 @@ def build_docker_run_command( if terminal: command_parts.append("-t") for env_directive in env_directives: - command_parts.extend(["-e", shlex_quote(env_directive)]) + command_parts.extend(["-e", env_directive]) for volume in volumes: command_parts.extend(["-v", shlex_quote(str(volume))]) if volumes_from: diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index b4f3a44f4f4..1c190bc5880 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -521,6 +521,15 @@ class ToolEvaluator(object): environment_variable["raw"] = True environment_variables.append(environment_variable) + home_dir = self.compute_environment.home_directory() + tmp_dir = self.compute_environment.tmp_directory() + if home_dir: + environment_variable = dict(name="HOME", value='"%s"' % home_dir, raw=True) + environment_variables.append(environment_variable) + if tmp_dir: + for tmp_directory in self.tool.tmp_directories: + environment_variable = dict(name=tmp_directory, value='"%s"' % tmp_dir, raw=True) + environment_variables.append(environment_variable) self.environment_variables = environment_variables return environment_variables diff --git a/lib/galaxy/tools/parser/interface.py b/lib/galaxy/tools/parser/interface.py index 75e77a5b1bc..c7d809c8579 100644 --- a/lib/galaxy/tools/parser/interface.py +++ b/lib/galaxy/tools/parser/interface.py @@ -91,6 +91,23 @@ class ToolSource(object): """ Return environment variable templates to expose. """ + def parse_home_target(self): + """Should be "job_home", "shared_home", "job_tmp", "pwd", or None. + """ + return "pwd" + + def parse_tmp_target(self): + """Should be "pwd", "shared_home", "job_tmp", "job_tmp_if_explicit", or None. + """ + return "job_tmp" + + def parse_tmp_directories(self): + """Directories to override if a tmp_target is not None.""" + return ["TMPDIR", "TMP", "TEMP"] + + def parse_docker_env_pass_through(self): + return ["GALAXY_SLOTS", "HOME"] + self.parse_tmp_directories() + @abstractmethod def parse_interpreter(self): """ Return string containing the interpreter to prepend to the command diff --git a/lib/galaxy/tools/parser/xml.py b/lib/galaxy/tools/parser/xml.py index d6de65c88af..86f932b2318 100644 --- a/lib/galaxy/tools/parser/xml.py +++ b/lib/galaxy/tools/parser/xml.py @@ -126,6 +126,27 @@ class XmlToolSource(ToolSource): ) return environment_variables + def parse_home_target(self): + target = "job_home" if self.parse_profile() >= "18.01" else "shared_home" + command_el = self._command_el + command_legacy = (command_el is not None) and command_el.get("use_shared_home", None) + if command_legacy is not None: + target = "shared_home" if string_as_bool(command_legacy) else "job_home" + return target + + def parse_tmp_target(self): + # Default to not touching TMPDIR et. al. but if job_tmp is set + # in job_conf then do. This is a very conservative approach that shouldn't + # 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/lib/galaxy/tools/xsd/galaxy.xsd b/lib/galaxy/tools/xsd/galaxy.xsd index 56b24d6a584..92eacf6f422 100644 --- a/lib/galaxy/tools/xsd/galaxy.xsd +++ b/lib/galaxy/tools/xsd/galaxy.xsd @@ -2706,6 +2706,11 @@ deprecated and using the ``$__tool_directory__`` variable is superior. Only used if ``detect_errors="exit_code", tells Galaxy the specified exit code indicates an out of memory error. Galaxy instances may be configured to retry such jobs on resources with more memory. + + + When running a job for this tool, do not isolate its $HOME directory within the job's directory - use either the shared_home_dir setting in Galaxy or the default $HOME specified in the job's default environment. + + This attribute defines the programming language in which the tool's executable file is written. Any language can be used (tools can be written in Python, C, Perl, Java, etc.). The executable file must be in the same directory of the XML file. If instead this attribute is not specified, the tag content should be a Bash command calling executable(s) available in the $PATH. diff --git a/test/base/integration_util.py b/test/base/integration_util.py index 4339436a9b4..363b19a32bf 100644 --- a/test/base/integration_util.py +++ b/test/base/integration_util.py @@ -7,6 +7,7 @@ tessting configuration. import os from unittest import skip, TestCase +from galaxy.tools.deps.commands import which from .api import UsesApiTestCaseMixin from .driver_util import GalaxyTestDriver @@ -21,6 +22,16 @@ def skip_if_jenkins(cls): return cls +def skip_unless_executable(executable): + if which(executable): + return lambda func: func + return skip("PATH doesn't contain executable %s" % executable) + + +def skip_unless_docker(): + return skip_unless_executable("docker") + + class IntegrationTestCase(TestCase, UsesApiTestCaseMixin): """Unit test case with utilities for spinning up Galaxy.""" diff --git a/test/functional/tools/job_environment_default.xml b/test/functional/tools/job_environment_default.xml new file mode 100644 index 00000000000..4b0d1cf8dc9 --- /dev/null +++ b/test/functional/tools/job_environment_default.xml @@ -0,0 +1,23 @@ + + + busybox:ubuntu-14.04 + + '$user_id'; + echo `id -g` > '$group_id'; + echo `pwd` > '$pwd'; + echo "\$HOME" > '$home'; + echo "\$TMP" > '$tmp'; + ]]> + + + + + + + + + + + + diff --git a/test/functional/tools/job_environment_default_legacy.xml b/test/functional/tools/job_environment_default_legacy.xml new file mode 100644 index 00000000000..62be15999b6 --- /dev/null +++ b/test/functional/tools/job_environment_default_legacy.xml @@ -0,0 +1,23 @@ + + + busybox:ubuntu-14.04 + + '$user_id'; + echo `id -g` > '$group_id'; + echo `pwd` > '$pwd'; + echo "\$HOME" > '$home'; + echo "\$TMP" > '$tmp'; + ]]> + + + + + + + + + + + + diff --git a/test/functional/tools/job_environment_explicit_shared_home.xml b/test/functional/tools/job_environment_explicit_shared_home.xml new file mode 100644 index 00000000000..45c168c00cb --- /dev/null +++ b/test/functional/tools/job_environment_explicit_shared_home.xml @@ -0,0 +1,23 @@ + + + busybox:ubuntu-14.04 + + '$user_id'; + echo `id -g` > '$group_id'; + echo `pwd` > '$pwd'; + echo "\$HOME" > '$home'; + echo "\$TMP" > '$tmp'; + ]]> + + + + + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index 512e2925aaa..859e93af5f6 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -51,6 +51,9 @@ --> + + + diff --git a/test/integration/sets_tmp_dir_expression_job_conf.xml b/test/integration/sets_tmp_dir_expression_job_conf.xml new file mode 100644 index 00000000000..ac7e5b1b233 --- /dev/null +++ b/test/integration/sets_tmp_dir_expression_job_conf.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + $(mktemp cooltmpXXXXXXXXXXXX) + + + + diff --git a/test/integration/sets_tmp_dir_to_true_job_conf.xml b/test/integration/sets_tmp_dir_to_true_job_conf.xml new file mode 100644 index 00000000000..e976483f389 --- /dev/null +++ b/test/integration/sets_tmp_dir_to_true_job_conf.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + True + + + + diff --git a/test/integration/simple_job_conf.xml b/test/integration/simple_job_conf.xml new file mode 100644 index 00000000000..abcb0768234 --- /dev/null +++ b/test/integration/simple_job_conf.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + + diff --git a/test/integration/test_dockerized_jobs.py b/test/integration/test_dockerized_jobs.py index 88aa843938c..e868d13c24d 100644 --- a/test/integration/test_dockerized_jobs.py +++ b/test/integration/test_dockerized_jobs.py @@ -1,23 +1,28 @@ """Integration tests for running tools in Docker containers.""" import os +import tempfile from base import integration_util from base.populators import ( DatasetPopulator, ) +from .test_job_environments import RunsEnvironmentJobs + SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) DOCKERIZED_JOB_CONFIG_FILE = os.path.join(SCRIPT_DIRECTORY, "dockerized_job_conf.xml") -# DOCKERIZED_JOB_DEPENDENCY_RESOLVERS_CONF = os.path.join(SCRIPT_DIRECTORY, "dockerzied_dependency_resolvers_conf.xml") -class DockerizedJobsIntegrationTestCase(integration_util.IntegrationTestCase): +@integration_util.skip_unless_docker() +class DockerizedJobsIntegrationTestCase(integration_util.IntegrationTestCase, RunsEnvironmentJobs): framework_tool_and_types = True @classmethod def handle_galaxy_config_kwds(cls, config): + cls.jobs_directory = tempfile.mkdtemp() + config["jobs_directory"] = cls.jobs_directory config["job_config_file"] = DOCKERIZED_JOB_CONFIG_FILE # Disable tool dependency resolution. config["tool_dependency_dir"] = "none" @@ -39,3 +44,29 @@ class DockerizedJobsIntegrationTestCase(integration_util.IntegrationTestCase): self.dataset_populator.wait_for_history(self.history_id, assert_ok=True) output = self.dataset_populator.get_history_dataset_content(self.history_id) assert "0.7.15-r1140" in output + + def test_docker_job_enviornment(self): + job_env = self._run_and_get_environment_properties("job_environment_default") + + euid = os.geteuid() + egid = os.getgid() + + assert job_env.user_id == str(euid), job_env.user_id + 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") + assert job_env.home == job_env.pwd, job_env.home + + def test_docker_job_environment_legacy(self): + job_env = self._run_and_get_environment_properties("job_environment_default_legacy") + + euid = os.geteuid() + egid = os.getgid() + + assert job_env.user_id == str(euid), job_env.user_id + 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 job_env.home == "/", job_env.home diff --git a/test/integration/test_job_environments.py b/test/integration/test_job_environments.py new file mode 100644 index 00000000000..46def1a1114 --- /dev/null +++ b/test/integration/test_job_environments.py @@ -0,0 +1,165 @@ +"""Integration tests for the Pulsar embedded runner.""" + +import collections +import os +import tempfile + +from base import integration_util +from base.populators import ( + DatasetPopulator, + skip_without_tool, +) + +SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) +SIMPLE_JOB_CONFIG_FILE = os.path.join(SCRIPT_DIRECTORY, "simple_job_conf.xml") +SETS_TMP_DIR_TO_TRUE_JOB_CONFIG = os.path.join(SCRIPT_DIRECTORY, "sets_tmp_dir_to_true_job_conf.xml") +SETS_TMP_DIR_AS_EXPRESSION_JOB_CONFIG = os.path.join(SCRIPT_DIRECTORY, "sets_tmp_dir_expression_job_conf.xml") + +JobEnviromentProperties = collections.namedtuple("JobEnvironmentProperties", [ + "user_id", + "group_id", + "pwd", + "home", + "tmp", +]) + + +class RunsEnvironmentJobs: + + def _run_and_get_environment_properties(self, tool_id="job_environment_default"): + with self.dataset_populator.test_history() as history_id: + self.dataset_populator.run_tool(tool_id, {}, history_id) + self.dataset_populator.wait_for_history(history_id, assert_ok=True) + return self._environment_properties(history_id) + + def _environment_properties(self, history_id): + user_id = self.dataset_populator.get_history_dataset_content(history_id, hid=1).strip() + group_id = self.dataset_populator.get_history_dataset_content(history_id, hid=2).strip() + pwd = self.dataset_populator.get_history_dataset_content(history_id, hid=3).strip() + home = self.dataset_populator.get_history_dataset_content(history_id, hid=4).strip() + tmp = self.dataset_populator.get_history_dataset_content(history_id, hid=5).strip() + + return JobEnviromentProperties(user_id, group_id, pwd, home, tmp) + + +class BaseJobEnvironmentIntegrationTestCase(integration_util.IntegrationTestCase, RunsEnvironmentJobs): + + framework_tool_and_types = True + + def setUp(self): + super(BaseJobEnvironmentIntegrationTestCase, self).setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + + +class DefaultJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase): + + @classmethod + def handle_galaxy_config_kwds(cls, config): + cls.jobs_directory = tempfile.mkdtemp() + config["jobs_directory"] = cls.jobs_directory + config["job_config_file"] = SIMPLE_JOB_CONFIG_FILE # Ensure no Docker for these tests + + @skip_without_tool("job_environment_default") + def test_default_environment_1801(self): + job_env = self._run_and_get_environment_properties() + + euid = os.geteuid() + egid = os.getgid() + + assert job_env.user_id == str(euid), job_env.user_id + 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") + + # Newer tools have 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 + + # Since job_conf doesn't set tmp_dir parameter - temp isn't in job_directory + assert not job_env.tmp.startswith(job_directory) + + @skip_without_tool("job_environment_default_legacy") + def test_default_environment_legacy(self): + job_env = self._run_and_get_environment_properties("job_environment_default_legacy") + + euid = os.geteuid() + egid = os.getgid() + home = os.getenv("HOME") + + assert job_env.user_id == str(euid), job_env.user_id + assert job_env.group_id == str(egid), job_env.group_id + assert job_env.home == home, job_env.home + + @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. + 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 + + +class TmpDirToTrueJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase): + + @classmethod + def handle_galaxy_config_kwds(cls, config): + cls.jobs_directory = tempfile.mkdtemp() + config["jobs_directory"] = cls.jobs_directory + config["job_config_file"] = SETS_TMP_DIR_TO_TRUE_JOB_CONFIG + + @skip_without_tool("job_environment_default") + def test_default_environment_1801(self): + job_env = self._run_and_get_environment_properties() + + job_directory = os.path.dirname(job_env.pwd) + + # Since job_conf sets tmp_dir parameter to True - temp is in job_directory + assert job_env.tmp.startswith(job_directory) + + +class TmpDirAsShellCommandJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase): + + @classmethod + def handle_galaxy_config_kwds(cls, config): + cls.jobs_directory = tempfile.mkdtemp() + config["jobs_directory"] = cls.jobs_directory + config["job_config_file"] = SETS_TMP_DIR_AS_EXPRESSION_JOB_CONFIG + + @skip_without_tool("job_environment_default") + def test_default_environment_1801(self): + job_env = self._run_and_get_environment_properties() + + # Since job_conf sets tmp_dir parameter to $(mktemp cooltmpXXXXXXXXXXXX) should + # start with cooltmp. + basename = os.path.basename(job_env.tmp) + assert basename.startswith("cooltmp"), job_env.tmp + + +class SharedHomeJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase): + + @classmethod + def handle_galaxy_config_kwds(cls, config): + cls.jobs_directory = tempfile.mkdtemp() + cls.shared_home_directory = tempfile.mkdtemp() + config["jobs_directory"] = cls.jobs_directory + config["job_config_file"] = SIMPLE_JOB_CONFIG_FILE # Ensure no Docker for these tests + config["shared_home_dir"] = cls.shared_home_directory + + @skip_without_tool("job_environment_default") + def test_default_environment(self): + # Test shared_home_dir ignored for newer tools by default + job_env = self._run_and_get_environment_properties() + job_directory = os.path.dirname(job_env.pwd) + assert job_env.home == os.path.join(job_directory, "home"), job_env.home + + @skip_without_tool("job_environment_default_legacy") + def test_default_environment_legacy(self): + # shared_home_dir used by default for older tools + job_env = self._run_and_get_environment_properties("job_environment_default_legacy") + assert job_env.home == self.shared_home_directory, job_env.home + + @skip_without_tool("job_environment_explicit_shared_home") + def test_default_environment_force_legacy_home(self): + # shared_home_dir used for newer tools if forced in tool XML + job_env = self._run_and_get_environment_properties("job_environment_explicit_shared_home") + assert job_env.home == self.shared_home_directory, job_env.home diff --git a/test/unit/jobs/test_runner_local.py b/test/unit/jobs/test_runner_local.py index 6b3c26cc9ce..2a5955d2389 100644 --- a/test/unit/jobs/test_runner_local.py +++ b/test/unit/jobs/test_runner_local.py @@ -147,6 +147,7 @@ class MockJobWrapper(object): self.galaxy_virtual_env = None self.shell = "/bin/bash" self.cleanup_job = "never" + self.tmp_dir_creation_statement = "" # Cruft for setting metadata externally, axe at some point. self.external_output_metadata = bunch.Bunch( @@ -210,3 +211,9 @@ class MockJobWrapper(object): self.stdout = stdout self.stderr = stderr self.exit_code = exit_code + + def tmp_directory(self): + return None + + def home_directory(self): + return None diff --git a/test/unit/tools/test_evaluation.py b/test/unit/tools/test_evaluation.py index 08db6eb8cb2..e40242e9227 100644 --- a/test/unit/tools/test_evaluation.py +++ b/test/unit/tools/test_evaluation.py @@ -243,6 +243,12 @@ class TestComputeEnviornment(SimpleComputeEnvironment): def working_directory(self): return self._working_directory + def home_directory(self): + return self._working_directory + + def tmp_directory(self): + return self._working_directory + def new_file_path(self): return self._new_file_path @@ -293,6 +299,14 @@ class MockTool(object): output1=ToolOutput("output1"), ) + @property + def config_file(self): + return self._config_files[0] + + @property + def tmp_directories(self): + return ["TMP"] + @property def config_files(self): return self._config_files From db0633eb8965306f4a7a8cca36e882cffc73ad55 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 9 Jan 2018 09:27:21 -0500 Subject: [PATCH 4/4] Follow up on comments for #5193. --- lib/galaxy/jobs/__init__.py | 2 +- lib/galaxy/tools/__init__.py | 8 +++++--- lib/galaxy/tools/deps/docker_util.py | 2 ++ lib/galaxy/tools/evaluation.py | 4 ++-- lib/galaxy/tools/parser/interface.py | 4 ++-- test/integration/test_dockerized_jobs.py | 3 ++- test/integration/test_job_environments.py | 2 +- test/unit/tools/test_evaluation.py | 2 +- 8 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 703361cd9df..559dbcb6c7d 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -1617,7 +1617,7 @@ class JobWrapper(object, HasResourceParameters): tmp_dir = self.get_destination_configuration("tmp_dir", None) if not tmp_dir or tmp_dir.lower() == "true": working_directory = self.working_directory - return '$(mktemp -d "%s/tmp.XXXXXXXXX")' % working_directory + return '''$([ ! -e '{0}/tmp' ] || mv '{0}/tmp' '{0}'/tmp.$(date +%Y%m%d-%H%M%S) ; mkdir '{0}/tmp'; echo '{0}/tmp')'''.format(working_directory) else: return tmp_dir diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 6bbe8eefd7c..f7837be5110 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -623,7 +623,7 @@ class Tool(object, Dictifiable): self.parse_command(tool_source) self.environment_variables = self.parse_environment_variables(tool_source) - self.tmp_directories = tool_source.parse_tmp_directories() + self.tmp_directory_vars = tool_source.parse_tmp_directory_vars() home_target = tool_source.parse_home_target() tmp_target = tool_source.parse_tmp_target() @@ -632,9 +632,11 @@ class Tool(object, Dictifiable): for environment_variable in self.environment_variables: if environment_variable.get("name") == "HOME": home_target = None - for tmp_directory in self.tmp_directories: - if environment_variable.get("name") == tmp_directory: + continue + for tmp_directory_var in self.tmp_directory_vars: + if environment_variable.get("name") == tmp_directory_var: tmp_target = None + break self.home_target = home_target self.tmp_target = tmp_target self.docker_env_pass_through = tool_source.parse_docker_env_pass_through() diff --git a/lib/galaxy/tools/deps/docker_util.py b/lib/galaxy/tools/deps/docker_util.py index e97da9c737b..5e3e83416f6 100644 --- a/lib/galaxy/tools/deps/docker_util.py +++ b/lib/galaxy/tools/deps/docker_util.py @@ -155,6 +155,8 @@ def build_docker_run_command( if terminal: command_parts.append("-t") for env_directive in env_directives: + # e.g. -e "GALAXY_SLOTS=$GALAXY_SLOTS" + # These are environment variable expansions so we don't quote these. command_parts.extend(["-e", env_directive]) for volume in volumes: command_parts.extend(["-v", shlex_quote(str(volume))]) diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 1c190bc5880..25f5804a50b 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -527,8 +527,8 @@ class ToolEvaluator(object): environment_variable = dict(name="HOME", value='"%s"' % home_dir, raw=True) environment_variables.append(environment_variable) if tmp_dir: - for tmp_directory in self.tool.tmp_directories: - environment_variable = dict(name=tmp_directory, value='"%s"' % tmp_dir, raw=True) + for tmp_directory_var in self.tool.tmp_directory_vars: + environment_variable = dict(name=tmp_directory_var, value='"%s"' % tmp_dir, raw=True) environment_variables.append(environment_variable) self.environment_variables = environment_variables return environment_variables diff --git a/lib/galaxy/tools/parser/interface.py b/lib/galaxy/tools/parser/interface.py index c7d809c8579..626a8f2114c 100644 --- a/lib/galaxy/tools/parser/interface.py +++ b/lib/galaxy/tools/parser/interface.py @@ -101,12 +101,12 @@ class ToolSource(object): """ return "job_tmp" - def parse_tmp_directories(self): + def parse_tmp_directory_vars(self): """Directories to override if a tmp_target is not None.""" return ["TMPDIR", "TMP", "TEMP"] def parse_docker_env_pass_through(self): - return ["GALAXY_SLOTS", "HOME"] + self.parse_tmp_directories() + return ["GALAXY_SLOTS", "HOME"] + self.parse_tmp_directory_vars() @abstractmethod def parse_interpreter(self): diff --git a/test/integration/test_dockerized_jobs.py b/test/integration/test_dockerized_jobs.py index e868d13c24d..a9bb909b09e 100644 --- a/test/integration/test_dockerized_jobs.py +++ b/test/integration/test_dockerized_jobs.py @@ -55,7 +55,8 @@ 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") - assert job_env.home == job_env.pwd, job_env.home + assert job_env.home.startswith(self.jobs_directory) + assert job_env.home.endswith("/home") def test_docker_job_environment_legacy(self): job_env = self._run_and_get_environment_properties("job_environment_default_legacy") diff --git a/test/integration/test_job_environments.py b/test/integration/test_job_environments.py index 46def1a1114..7d2ef8cffa2 100644 --- a/test/integration/test_job_environments.py +++ b/test/integration/test_job_environments.py @@ -114,7 +114,7 @@ class TmpDirToTrueJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegratio job_directory = os.path.dirname(job_env.pwd) # Since job_conf sets tmp_dir parameter to True - temp is in job_directory - assert job_env.tmp.startswith(job_directory) + assert job_env.tmp.startswith(job_directory), job_env class TmpDirAsShellCommandJobEnvironmentIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase): diff --git a/test/unit/tools/test_evaluation.py b/test/unit/tools/test_evaluation.py index e40242e9227..0aaa3aeb038 100644 --- a/test/unit/tools/test_evaluation.py +++ b/test/unit/tools/test_evaluation.py @@ -304,7 +304,7 @@ class MockTool(object): return self._config_files[0] @property - def tmp_directories(self): + def tmp_directory_vars(self): return ["TMP"] @property