Skip to content

Commit

Permalink
runner.singularity: Avoid pre-exec evaluation of entrypoint command l…
Browse files Browse the repository at this point in the history
…ine…

…when possible, which is only on Singularity 3.10.0 or newer.

Many uses will pass thru this too-early evaluation unscathed, so we
don't require it as 3.10.0 is too new a minimum version.  Without it,
however, some uses, like our reporting of component versions with a
`bash -c` invocation, will get maimed resulting in bugs that seem very
strange at first.
  • Loading branch information
tsibley committed May 25, 2023
1 parent 15a83a4 commit 1dc3575
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions nextstrain/cli/runner/singularity.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"SINGULARITYENV_PROMPT_COMMAND": "unset PROMPT_COMMAND; #",
}

SINGULARITY_EXEC_ARGS = [
SINGULARITY_EXEC_ARGS = lambda: [
# Increase isolation.
#
# In the future, we may find we want to use additional related flags to
Expand Down Expand Up @@ -130,6 +130,11 @@
# Don't copy entire host environment. We forward our own hostenv.
"--cleanenv",

# Don't evaluate the entrypoint command line (e.g. arguments passed via
# `nextstrain build`) before exec-ing the entrypoint. It leads to unwanted
# substitutions that happen too early.
*(["--no-eval"] if singularity_version_at_least("3.10.0") else []),

# Since we use --no-home above, avoid warnings about not being able to cd
# to $HOME (the default behaviour). run() will override this by specifying
# --pwd again.
Expand Down Expand Up @@ -183,7 +188,7 @@ def run(opts, argv, working_volume = None, extra_env = {}, cpus: int = None, mem
}

return exec_or_return([
"singularity", "run", *SINGULARITY_EXEC_ARGS,
"singularity", "run", *SINGULARITY_EXEC_ARGS(),

# Map directories to bind mount into the container.
*flatten(("--bind", "%s:%s:%s" % (v.src.resolve(strict = True), docker.mount_point(v), "rw" if v.writable else "ro"))
Expand Down Expand Up @@ -249,7 +254,7 @@ def test_setup() -> RunnerTestResults:
def test_run():
try:
capture_output([
"singularity", "exec", *SINGULARITY_EXEC_ARGS,
"singularity", "exec", *SINGULARITY_EXEC_ARGS(),

# XXX TODO: We should test --bind, as that's maybe most likely
# to be adminstratively disabled, but it's a bit more ceremony
Expand Down Expand Up @@ -459,7 +464,7 @@ def run_bash(script: str, image: str = DEFAULT_IMAGE) -> List[str]:
Returns the output of the script as a list of strings.
"""
return capture_output([
"singularity", "run", *SINGULARITY_EXEC_ARGS, image_path(image),
"singularity", "run", *SINGULARITY_EXEC_ARGS(), image_path(image),
"bash", "-c", script
])

Expand Down

0 comments on commit 1dc3575

Please sign in to comment.