From 8d4f9955a66c94604634d436a4262cfef395b3e5 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 21 Dec 2016 14:52:59 -0500 Subject: [PATCH] Limit adjusting Galaxy's Python environment to legacy tools. Don't expose Galaxy's environment to tools unless there is evidence we need to. This should restrict future bugs by limitting what tools are exposed to Galaxy's internals and will fix bugs related to Conda dependencies masking Galaxy - such as #2994. Should fix #2994. Rebased with language and tool best practice fixes thanks to @mvdbeek. --- config/galaxy.ini.sample | 12 +++++ lib/galaxy/config.py | 5 ++ lib/galaxy/jobs/command_factory.py | 17 ++++++ lib/galaxy/jobs/runners/__init__.py | 1 + .../job_script/DEFAULT_JOB_FILE_TEMPLATE.sh | 6 +-- .../jobs/runners/util/job_script/__init__.py | 1 + lib/galaxy/tools/__init__.py | 54 +++++++++++++++++-- .../tools/python_environment_problem.py | 12 +++++ .../tools/python_environment_problem.xml | 20 +++++++ test/functional/tools/samples_tool_conf.xml | 1 + test/unit/jobs/test_command_factory.py | 4 +- test/unit/tools_support.py | 1 + 12 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 test/functional/tools/python_environment_problem.py create mode 100644 test/functional/tools/python_environment_problem.xml diff --git a/config/galaxy.ini.sample b/config/galaxy.ini.sample index 5a231a1957b..8028f9917c3 100644 --- a/config/galaxy.ini.sample +++ b/config/galaxy.ini.sample @@ -1192,6 +1192,18 @@ use_interactive = True # option (Linux) or -actimeo=0 (Solaris). #retry_job_output_collection = 0 +# In the past Galaxy would preserve its Python environment when running jobs ( +# and still does for internal tools packaged with Galaxy). This behavior exposes +# Galaxy internals to tools and could result in problems when activating +# Python environments for tools (such as with Conda packaging). The default +# legacy_only will restrict this behavior to tools identified by the Galaxy +# team as requiring this environment. Set this to "always" to restore the +# previous behavior (and potentially break Conda dependency resolution for many +# tools). Set this to legacy_and_local to preserve the environment for legacy +# tools and locally managed tools (this might be useful for instance if you are +# installing software into Galaxy's virtualenv for tool development). +#python_environment_problem = legacy_only + # Clean up various bits of jobs left on the filesystem after completion. These # bits include the job working directory, external metadata temporary files, # and DRM stdout and stderr files (if using a DRM). Possible values are: diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py index 1f62447cf10..7d6a754e772 100644 --- a/lib/galaxy/config.py +++ b/lib/galaxy/config.py @@ -175,6 +175,11 @@ class Configuration( object ): self.jobs_directory = resolve_path( kwargs.get( "jobs_directory", default_jobs_directory ), self.root ) self.default_job_shell = kwargs.get( "default_job_shell", "/bin/bash" ) self.cleanup_job = kwargs.get( "cleanup_job", "always" ) + preserve_python_environment = kwargs.get( "preserve_python_environment", "legacy_only" ) + if preserve_python_environment not in ["legacy_only", "legacy_and_local", "always"]: + log.warn("preserve_python_environment set to unknown value [%s], defaulting to legacy_only") + preserve_python_environment = "legacy_only" + self.preserve_python_environment = preserve_python_environment self.container_image_cache_path = self.resolve_path( kwargs.get( "container_image_cache_path", "database/container_images" ) ) self.outputs_to_working_directory = string_as_bool( kwargs.get( 'outputs_to_working_directory', False ) ) self.output_size_limit = int( kwargs.get( 'output_size_limit', 0 ) ) diff --git a/lib/galaxy/jobs/command_factory.py b/lib/galaxy/jobs/command_factory.py index 4f7f0aaaee0..cca621057d9 100644 --- a/lib/galaxy/jobs/command_factory.py +++ b/lib/galaxy/jobs/command_factory.py @@ -16,6 +16,21 @@ log = getLogger( __name__ ) CAPTURE_RETURN_CODE = "return_code=$?" YIELD_CAPTURED_CODE = 'sh -c "exit $return_code"' +SETUP_GALAXY_FOR_METADATA = """ +if [ "$GALAXY_LIB" != "None" ]; then + if [ -n "$PYTHONPATH" ]; then + PYTHONPATH="$GALAXY_LIB:$PYTHONPATH" + else + PYTHONPATH="$GALAXY_LIB" + fi + export PYTHONPATH +fi +if [ "$GALAXY_VIRTUAL_ENV" != "None" -a -z "$VIRTUAL_ENV" \ + -a -f "$GALAXY_VIRTUAL_ENV/bin/activate" ]; then + . "$GALAXY_VIRTUAL_ENV/bin/activate" +fi +GALAXY_PYTHON=`which python` +""" def build_command( @@ -194,6 +209,8 @@ def __handle_metadata(commands_builder, job_wrapper, runner, remote_command_para ) or '' metadata_command = metadata_command.strip() if metadata_command: + # Place Galaxy and its dependencies in environment for metadata regardless of tool. + metadata_command = "%s%s" % (SETUP_GALAXY_FOR_METADATA, metadata_command) commands_builder.capture_return_code() commands_builder.append_command(metadata_command) diff --git a/lib/galaxy/jobs/runners/__init__.py b/lib/galaxy/jobs/runners/__init__.py index e46c34a6160..ba186503144 100644 --- a/lib/galaxy/jobs/runners/__init__.py +++ b/lib/galaxy/jobs/runners/__init__.py @@ -314,6 +314,7 @@ class BaseJobRunner( object ): working_directory=os.path.abspath( job_wrapper.working_directory ), command=command_line, shell=job_wrapper.shell, + preserve_python_environment=job_wrapper.tool.requires_galaxy_python_environment, ) # Additional logging to enable if debugging from_work_dir handling, metadata # commands, etc... (or just peak in the job script.) 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 a2205509935..801df6acbc1 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 @@ -4,8 +4,9 @@ $headers $integrity_injection $slots_statement export GALAXY_SLOTS +PRESERVE_GALAXY_ENVIRONMENT="$preserve_python_environment" GALAXY_LIB="$galaxy_lib" -if [ "$GALAXY_LIB" != "None" ]; then +if [ "$GALAXY_LIB" != "None" -a "$PRESERVE_GALAXY_ENVIRONMENT" = "True" ]; then if [ -n "$PYTHONPATH" ]; then PYTHONPATH="$GALAXY_LIB:$PYTHONPATH" else @@ -16,10 +17,9 @@ fi $env_setup_commands GALAXY_VIRTUAL_ENV="$galaxy_virtual_env" if [ "$GALAXY_VIRTUAL_ENV" != "None" -a -z "$VIRTUAL_ENV" \ - -a -f "$GALAXY_VIRTUAL_ENV/bin/activate" ]; then + -a -f "$GALAXY_VIRTUAL_ENV/bin/activate" -a "$PRESERVE_GALAXY_ENVIRONMENT" = "True" ]; then . "$GALAXY_VIRTUAL_ENV/bin/activate" fi -GALAXY_PYTHON=`which python` $instrument_pre_commands cd $working_directory $command diff --git a/lib/galaxy/jobs/runners/util/job_script/__init__.py b/lib/galaxy/jobs/runners/util/job_script/__init__.py index 1bcc391fc6b..77af249e5c4 100644 --- a/lib/galaxy/jobs/runners/util/job_script/__init__.py +++ b/lib/galaxy/jobs/runners/util/job_script/__init__.py @@ -45,6 +45,7 @@ OPTIONAL_TEMPLATE_PARAMS = { 'instrument_post_commands': '', 'integrity_injection': INTEGRITY_INJECTION, 'shell': DEFAULT_SHELL, + 'preserve_python_environment': True, } diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 7eea8700528..702cecee69c 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -73,6 +73,45 @@ log = logging.getLogger( __name__ ) HELP_UNINITIALIZED = threading.Lock() MODEL_TOOLS_PATH = os.path.abspath(os.path.dirname(__file__)) +# Tools that require Galaxy's Python environment to be preserved. +GALAXY_LIB_TOOLS = [ + "upload1", + # Legacy tools bundled with Galaxy. + "vcf_to_maf_customtrack1", + "laj_1", + "meme_fimo", + "secure_hash_message_digest", + "join1", + "gff2bed1", + "gff_filter_by_feature_count", + "Extract genomic DNA 1", + "aggregate_scores_in_intervals2", + "Interval_Maf_Merged_Fasta2", + "maf_stats1", + "Interval2Maf1", + "MAF_To_Interval1", + "MAF_filter", + "MAF_To_Fasta1", + "MAF_Reverse_Complement_1", + "MAF_split_blocks_by_species1", + "maf_limit_size1", + "maf_by_block_number1", + # Tools improperly migrated to the tool shed (devteam) + "lastz_wrapper_2", + "qualityFilter", + "winSplitter", + "pileup_interval", + "count_gff_features", + "Convert characters1", + "lastz_paired_reads_wrapper", + "subRate1", + "substitutions1", + "sam_pileup", + "find_diag_hits", + "cufflinks", + # Tools improperly migrated to the tool shed (iuc) + "tabular_to_dbnsfp", +] class ToolErrorLog: @@ -404,10 +443,17 @@ class Tool( object, Dictifiable ): """Indicates this tool's runtime requires Galaxy's Python environment.""" # All special tool types (data source, history import/export, etc...) # seem to require Galaxy's Python. - return self.tool_type != "default" or self.id in [ - "__SET_METADATA__", - "upload1", - ] + if self.tool_type != "default": + return True + + config = self.app.config + preserve_python_environment = config.preserve_python_environment + if preserve_python_environment == "always": + return True + elif preserve_python_environment == "legacy_and_local" and self.repository_id is None: + return True + else: + return self.old_id in GALAXY_LIB_TOOLS def __get_job_tool_configuration(self, job_params=None): """Generalized method for getting this tool's job configuration. diff --git a/test/functional/tools/python_environment_problem.py b/test/functional/tools/python_environment_problem.py new file mode 100644 index 00000000000..19bc7305b01 --- /dev/null +++ b/test/functional/tools/python_environment_problem.py @@ -0,0 +1,12 @@ +import sys + +import galaxy.eggs # NOQA + + +def main(): + with open(sys.argv[1], "w") as f: + f.write("out_file1") + + +if __name__ == '__main__': + main() diff --git a/test/functional/tools/python_environment_problem.xml b/test/functional/tools/python_environment_problem.xml new file mode 100644 index 00000000000..dcc2961e1bc --- /dev/null +++ b/test/functional/tools/python_environment_problem.xml @@ -0,0 +1,20 @@ + + + + python $__tool_directory__/python_environment_problem.py '$out_file1' + + + + + + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index 6940f128af5..ccef907c6a1 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -97,6 +97,7 @@ + diff --git a/test/unit/jobs/test_command_factory.py b/test/unit/jobs/test_command_factory.py index 9166a489c50..cfa9be34da3 100644 --- a/test/unit/jobs/test_command_factory.py +++ b/test/unit/jobs/test_command_factory.py @@ -4,7 +4,7 @@ from os import getcwd from tempfile import mkdtemp from unittest import TestCase -from galaxy.jobs.command_factory import build_command +from galaxy.jobs.command_factory import build_command, SETUP_GALAXY_FOR_METADATA from galaxy.util.bunch import Bunch MOCK_COMMAND_LINE = "/opt/galaxy/tools/bowtie /mnt/galaxyData/files/000/input000.dat" @@ -87,7 +87,7 @@ class TestCommandFactory(TestCase): self.include_metadata = True self.include_work_dir_outputs = False self.job_wrapper.metadata_line = TEST_METADATA_LINE - expected_command = _surrond_command("%s; return_code=$?; cd '%s'; %s" % (MOCK_COMMAND_LINE, self.job_dir, TEST_METADATA_LINE)) + expected_command = _surrond_command("%s; return_code=$?; cd '%s'; %s%s" % (MOCK_COMMAND_LINE, self.job_dir, SETUP_GALAXY_FOR_METADATA, TEST_METADATA_LINE)) self.__assert_command_is( expected_command ) def test_empty_metadata(self): diff --git a/test/unit/tools_support.py b/test/unit/tools_support.py index 51f23cb4ccc..3a6fe71c00b 100644 --- a/test/unit/tools_support.py +++ b/test/unit/tools_support.py @@ -120,6 +120,7 @@ class MockApp( object ): builds_file_path=os.path.join( 'tool-data', 'shared', 'ucsc', 'builds.txt.sample' ), migrated_tools_config=os.path.join(test_directory, "migrated_tools_conf.xml"), server_name="test_server", + preserve_python_environment="always", ) # Setup some attributes for downstream extension by specific tests.