From e428e4298e3744759b04bad912f263be7ae0bbd2 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 20 Jan 2021 11:41:22 +0100 Subject: [PATCH 1/3] Fix pulling of singularity images That broke in https://github.com/galaxyproject/galaxy/pull/11134, since the singularity container resolve() inherits from docker container resolution it'd also check to see if `docker` was on path. --- .../deps/container_resolvers/mulled.py | 47 ++++++++++++------- lib/galaxy/tool_util/deps/containers.py | 2 +- .../tool_util/test_container_resolution.py | 12 ++--- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/lib/galaxy/tool_util/deps/container_resolvers/mulled.py b/lib/galaxy/tool_util/deps/container_resolvers/mulled.py index a9e9dbcb07c..ac0236c027c 100644 --- a/lib/galaxy/tool_util/deps/container_resolvers/mulled.py +++ b/lib/galaxy/tool_util/deps/container_resolvers/mulled.py @@ -323,26 +323,27 @@ def targets_to_mulled_name(targets, hash_func, namespace, resolution_cache=None, return name -class DockerContainerResolver(ContainerResolver): +class CliContainerResolver(ContainerResolver): container_type = 'docker' + cli = 'docker' def __init__(self, *args, **kwargs): - self._docker_cli_available = bool(which('docker')) + self._cli_available = bool(which(self.cli)) super().__init__(*args, **kwargs) @property - def docker_cli_available(self): - return self._docker_cli_available + def cli_available(self): + return self._cli_available - @docker_cli_available.setter - def docker_cli_available(self, value): + @cli_available.setter + def cli_available(self, value): if not value: - log.info('Docker CLI not available, cannot list or pull images in Galaxy process. Does not impact kubernetes.') - self._docker_cli_available = value + log.info('{} CLI not available, cannot list or pull images in Galaxy process. Does not impact kubernetes.'.format(self.cli)) + self._cli_available = value -class CachedMulledDockerContainerResolver(DockerContainerResolver): +class CachedMulledDockerContainerResolver(CliContainerResolver): resolver_type = "cached_mulled" shell = '/bin/bash' @@ -353,7 +354,7 @@ class CachedMulledDockerContainerResolver(DockerContainerResolver): self.hash_func = hash_func def resolve(self, enabled_container_types, tool_info, **kwds): - if not self.docker_cli_available or tool_info.requires_galaxy_python_environment or self.container_type not in enabled_container_types: + if not self.cli_available or tool_info.requires_galaxy_python_environment or self.container_type not in enabled_container_types: return None targets = mulled_targets(tool_info) @@ -364,9 +365,10 @@ class CachedMulledDockerContainerResolver(DockerContainerResolver): return "CachedMulledDockerContainerResolver[namespace=%s]" % self.namespace -class CachedMulledSingularityContainerResolver(ContainerResolver): +class CachedMulledSingularityContainerResolver(CliContainerResolver): resolver_type = "cached_mulled_singularity" + cli = "singularity" container_type = "singularity" shell = '/bin/bash' @@ -386,7 +388,7 @@ class CachedMulledSingularityContainerResolver(ContainerResolver): return "CachedMulledSingularityContainerResolver[cache_directory=%s]" % self.cache_directory -class MulledDockerContainerResolver(DockerContainerResolver): +class MulledDockerContainerResolver(CliContainerResolver): """Look for mulled images matching tool dependencies.""" resolver_type = "mulled" @@ -409,10 +411,14 @@ class MulledDockerContainerResolver(DockerContainerResolver): return None def pull(self, container): - if self.docker_cli_available: + if self.cli_available: command = container.build_pull_command() shell(command) + @property + def can_list_containers(self): + return self.cli_available + def resolve(self, enabled_container_types, tool_info, install=False, session=None, **kwds): resolution_cache = kwds.get("resolution_cache") if tool_info.requires_galaxy_python_environment or self.container_type not in enabled_container_types: @@ -432,7 +438,7 @@ class MulledDockerContainerResolver(DockerContainerResolver): type=self.container_type, shell=self.shell, ) - if self.docker_cli_available: + if self.can_list_containers: if install and not self.cached_container_description( targets, namespace=self.namespace, @@ -467,6 +473,7 @@ class MulledSingularityContainerResolver(MulledDockerContainerResolver): resolver_type = "mulled_singularity" container_type = "singularity" + cli = "singularity" protocol = 'docker://' def __init__(self, app_info=None, namespace="biocontainers", hash_func="v2", auto_install=True, **kwds): @@ -481,15 +488,21 @@ class MulledSingularityContainerResolver(MulledDockerContainerResolver): cache_directory=self.cache_directory, hash_func=hash_func) + @property + def can_list_containers(self): + # Only needs access to path, doesn't require CLI + return True + def pull(self, container): - cmds = container.build_mulled_singularity_pull_command(cache_directory=self.cache_directory, namespace=self.namespace) - shell(cmds=cmds) + if self.cli_available: + cmds = container.build_mulled_singularity_pull_command(cache_directory=self.cache_directory, namespace=self.namespace) + shell(cmds=cmds) def __str__(self): return "MulledSingularityContainerResolver[namespace=%s]" % self.namespace -class BuildMulledDockerContainerResolver(DockerContainerResolver): +class BuildMulledDockerContainerResolver(CliContainerResolver): """Build for Docker mulled images matching tool dependencies.""" resolver_type = "build_mulled" diff --git a/lib/galaxy/tool_util/deps/containers.py b/lib/galaxy/tool_util/deps/containers.py index af9778c122a..c6d3c23cfd0 100644 --- a/lib/galaxy/tool_util/deps/containers.py +++ b/lib/galaxy/tool_util/deps/containers.py @@ -226,7 +226,7 @@ class ContainerRegistry: # BuildMulledDockerContainerResolver and BuildMulledSingularityContainerResolver both need the docker daemon to build images. # If docker is not available, we don't load them. build_mulled_docker_container_resolver = BuildMulledDockerContainerResolver(self.app_info) - if build_mulled_docker_container_resolver.docker_cli_available: + if build_mulled_docker_container_resolver.cli_available: default_resolvers.extend([ build_mulled_docker_container_resolver, BuildMulledSingularityContainerResolver(self.app_info), diff --git a/test/unit/tool_util/test_container_resolution.py b/test/unit/tool_util/test_container_resolution.py index b60943afd76..7b079497b61 100644 --- a/test/unit/tool_util/test_container_resolution.py +++ b/test/unit/tool_util/test_container_resolution.py @@ -11,26 +11,26 @@ from galaxy.tool_util.deps.requirements import ToolRequirement def test_docker_container_resolver_detects_docker_cli_absent(mocker): mocker.patch('galaxy.tool_util.deps.container_resolvers.mulled.which', return_value=None) resolver = CachedMulledDockerContainerResolver() - assert resolver.docker_cli_available is False + assert resolver._cli_available is False def test_docker_container_resolver_detects_docker_cli(mocker): mocker.patch('galaxy.tool_util.deps.container_resolvers.mulled', return_value='/bin/docker') resolver = CachedMulledDockerContainerResolver() - assert resolver.docker_cli_available + assert resolver.cli_available def test_cached_docker_container_docker_cli_absent_resolve(mocker): mocker.patch('galaxy.tool_util.deps.container_resolvers.mulled.which', return_value=None) resolver = CachedMulledDockerContainerResolver() - assert resolver.docker_cli_available is False + assert resolver.cli_available is False assert resolver.resolve(enabled_container_types=[], tool_info={}) is None def test_docker_container_docker_cli_absent_resolve(mocker): mocker.patch('galaxy.tool_util.deps.container_resolvers.mulled.which', return_value=None) resolver = MulledDockerContainerResolver() - assert resolver.docker_cli_available is False + assert resolver.cli_available is False requirement = ToolRequirement(name="samtools", version="1.10", type="package") tool_info = ToolInfo(requirements=[requirement]) mocker.patch('galaxy.tool_util.deps.container_resolvers.mulled.targets_to_mulled_name', return_value='samtools:1.10--h2e538c0_3') @@ -42,12 +42,12 @@ def test_docker_container_docker_cli_absent_resolve(mocker): def test_docker_container_docker_cli_exception_resolve(mocker): mocker.patch('galaxy.tool_util.deps.container_resolvers.mulled.which', return_value='/bin/docker') resolver = MulledDockerContainerResolver() - assert resolver.docker_cli_available is True + assert resolver.cli_available is True requirement = ToolRequirement(name="samtools", version="1.10", type="package") tool_info = ToolInfo(requirements=[requirement]) mocker.patch('galaxy.tool_util.deps.container_resolvers.mulled.targets_to_mulled_name', return_value='samtools:1.10--h2e538c0_3') mocker.patch('galaxy.tool_util.deps.container_resolvers.mulled.docker_cached_container_description', side_effect=CalledProcessError(1, 'bla')) container_description = resolver.resolve(enabled_container_types=['docker'], tool_info=tool_info, install=True) - assert resolver.docker_cli_available is True + assert resolver.cli_available is True assert container_description.type == 'docker' assert container_description.identifier == 'quay.io/biocontainers/samtools:1.10--h2e538c0_3' From 44ef5a4244ed5451fb18e38547067022b52f6ea0 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 20 Jan 2021 13:40:43 +0100 Subject: [PATCH 2/3] Return uncached container description if cached container not available --- lib/galaxy/tool_util/deps/container_resolvers/mulled.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/deps/container_resolvers/mulled.py b/lib/galaxy/tool_util/deps/container_resolvers/mulled.py index ac0236c027c..bdc13f9e2ba 100644 --- a/lib/galaxy/tool_util/deps/container_resolvers/mulled.py +++ b/lib/galaxy/tool_util/deps/container_resolvers/mulled.py @@ -462,7 +462,7 @@ class MulledDockerContainerResolver(CliContainerResolver): namespace=self.namespace, hash_func=self.hash_func, resolution_cache=resolution_cache, - ) + ) or container_description return container_description def __str__(self): From 2636a28515abcb0f06b55ae28c5d9164f1a0466f Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 20 Jan 2021 14:20:08 +0100 Subject: [PATCH 3/3] Create cache dir if it doesn't exist --- .../deps/container_resolvers/mulled.py | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/lib/galaxy/tool_util/deps/container_resolvers/mulled.py b/lib/galaxy/tool_util/deps/container_resolvers/mulled.py index bdc13f9e2ba..e4a93864936 100644 --- a/lib/galaxy/tool_util/deps/container_resolvers/mulled.py +++ b/lib/galaxy/tool_util/deps/container_resolvers/mulled.py @@ -7,6 +7,7 @@ import subprocess from galaxy.util import ( + safe_makedirs, string_as_bool, unicodify, which, @@ -343,13 +344,24 @@ class CliContainerResolver(ContainerResolver): self._cli_available = value +class SingularityCliContainerResolver(CliContainerResolver): + + container_type = 'singularity' + cli = 'singularity' + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.cache_directory = kwargs.get("cache_directory", os.path.join(kwargs['app_info'].container_image_cache_path, "singularity", "mulled")) + safe_makedirs(self.cache_directory) + + class CachedMulledDockerContainerResolver(CliContainerResolver): resolver_type = "cached_mulled" shell = '/bin/bash' def __init__(self, app_info=None, namespace="biocontainers", hash_func="v2", **kwds): - super().__init__(app_info) + super().__init__(app_info=app_info, **kwds) self.namespace = namespace self.hash_func = hash_func @@ -365,16 +377,13 @@ class CachedMulledDockerContainerResolver(CliContainerResolver): return "CachedMulledDockerContainerResolver[namespace=%s]" % self.namespace -class CachedMulledSingularityContainerResolver(CliContainerResolver): +class CachedMulledSingularityContainerResolver(SingularityCliContainerResolver): resolver_type = "cached_mulled_singularity" - cli = "singularity" - container_type = "singularity" shell = '/bin/bash' def __init__(self, app_info=None, hash_func="v2", **kwds): - super().__init__(app_info) - self.cache_directory = kwds.get("cache_directory", os.path.join(app_info.container_image_cache_path, "singularity", "mulled")) + super().__init__(app_info=app_info, **kwds) self.hash_func = hash_func def resolve(self, enabled_container_types, tool_info, **kwds): @@ -396,7 +405,7 @@ class MulledDockerContainerResolver(CliContainerResolver): protocol = None def __init__(self, app_info=None, namespace="biocontainers", hash_func="v2", auto_install=True, **kwds): - super().__init__(app_info) + super().__init__(app_info=app_info, **kwds) self.namespace = namespace self.hash_func = hash_func self.auto_install = string_as_bool(auto_install) @@ -469,16 +478,13 @@ class MulledDockerContainerResolver(CliContainerResolver): return "MulledDockerContainerResolver[namespace=%s]" % self.namespace -class MulledSingularityContainerResolver(MulledDockerContainerResolver): +class MulledSingularityContainerResolver(SingularityCliContainerResolver, MulledDockerContainerResolver): resolver_type = "mulled_singularity" - container_type = "singularity" - cli = "singularity" protocol = 'docker://' def __init__(self, app_info=None, namespace="biocontainers", hash_func="v2", auto_install=True, **kwds): - super().__init__(app_info) - self.cache_directory = kwds.get("cache_directory", os.path.join(app_info.container_image_cache_path, "singularity", "mulled")) + super().__init__(app_info=app_info, **kwds) self.namespace = namespace self.hash_func = hash_func self.auto_install = string_as_bool(auto_install) @@ -510,7 +516,7 @@ class BuildMulledDockerContainerResolver(CliContainerResolver): builds_on_resolution = True def __init__(self, app_info=None, namespace="local", hash_func="v2", auto_install=True, **kwds): - super().__init__(app_info) + super().__init__(app_info=app_info, **kwds) self._involucro_context_kwds = { 'involucro_bin': self._get_config_option("involucro_path", None) } @@ -549,20 +555,18 @@ class BuildMulledDockerContainerResolver(CliContainerResolver): return "BuildDockerContainerResolver[namespace=%s]" % self.namespace -class BuildMulledSingularityContainerResolver(ContainerResolver): +class BuildMulledSingularityContainerResolver(SingularityCliContainerResolver): """Build for Singularity mulled images matching tool dependencies.""" resolver_type = "build_mulled_singularity" - container_type = "singularity" shell = '/bin/bash' builds_on_resolution = True def __init__(self, app_info=None, hash_func="v2", auto_install=True, **kwds): - super().__init__(app_info) + super().__init__(app_info=app_info, **kwds) self._involucro_context_kwds = { 'involucro_bin': self._get_config_option("involucro_path", None) } - self.cache_directory = kwds.get("cache_directory", os.path.join(app_info.container_image_cache_path, "singularity", "mulled")) self.hash_func = hash_func self.auto_install = string_as_bool(auto_install) self._mulled_kwds = {