Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs and tests for environment variable setting in containerized execution #16666

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open

Docs and tests for environment variable setting in containerized execution #16666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
98c914e
test env variable setting in containerized execution
bernt-matthias Sep 8, 2023
2a98cd8
fix setting destination env variables in containerized execution
bernt-matthias Sep 8, 2023
c202e88
test that exec env tags are possible
bernt-matthias Sep 8, 2023
085cd95
add a bit of docs
bernt-matthias Sep 8, 2023
c94ba78
consider non env
bernt-matthias Sep 8, 2023
e76272d
docs: add possibly lost subsection for admin job docs
bernt-matthias Sep 12, 2023
3aa77e2
document state of the art for setting env variables
bernt-matthias Sep 12, 2023
b8a152d
Revert "consider non env"
bernt-matthias Sep 12, 2023
51e7291
Revert "fix setting destination env variables in containerized execut…
bernt-matthias Sep 12, 2023
0cac78a
start to extend tests
bernt-matthias Sep 12, 2023
1a98c81
test env variables for containers
bernt-matthias Sep 12, 2023
43a07d6
fix lint
bernt-matthias Sep 12, 2023
7c68315
Add whitespace
mvdbeek May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/source/admin/jobs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ exec
raw
: Disable auto-quoting of values when setting up environment variables.

For containerized execution the environment setup done by ``file`` and ``exec`` ``env`` tags will
not be available in the container, but only to the pre-and-post-tool-execution job environment.
Instead, for containerized destinations variables that should only be available in the container
can be set with ``<param id="docker_env_VARIABLE">VALUE</param>`` and
``<param id="singularity_env_VARIABLE">VALUE</param>``, respectively.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe if you mention this is a known bug - it would help alleviate @mvdbeek's concerns below somewhat?


### Job resubmission

Destinations may also specify other destinations (which may be dynamic destinations) that jobs should be resubmitted to if they fail to complete at the first destination for certain reasons. This is done with the `<resubmit>` tag contained within a `<destination>`.

condition
Expand Down
4 changes: 4 additions & 0 deletions test/functional/tools/job_environment_default.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ echo \$(pwd) > '$pwd' &&
echo "\$HOME" > '$home' &&
echo "\$TMP" > '$tmp' &&
echo "\$SOME_ENV_VAR" > '$some_env_var' &&
echo "\${JOBCONF_ENV_VAR:-UNSET}" > '$jobconf_env_var' &&
Copy link
Member

@mvdbeek mvdbeek May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the JOBCONF_ENV_VAR tests ? We should fix this, not cement it with tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we leave this: It just tests the documented behavior (as per this PR :) ) and you gave good reasons for not changing it.

If at one time we allow certain env variables to be passed to the container, e.g. via ToolInfo then we can easily add another variable for this behavior.

echo "\${CONTAINER_ENV_VAR:-UNSET}" > '$container_env_var' &&
touch "\$_GALAXY_JOB_TMP_DIR/tmp_test" &&
touch "\$HOME/home_test" &&

Expand All @@ -28,6 +30,8 @@ touch "\${TMPDIR:-/tmp}/job_tmpdir"
<data name="home" format="txt" label="home" />
<data name="tmp" format="txt" label="tmp" />
<data name="some_env_var" format="txt" label="env_var" />
<data name="jobconf_env_var" format="txt" label="jobconf_env_var" />
<data name="container_env_var" format="txt" label="container_env_var" />
</outputs>
<tests>
</tests>
Expand Down
4 changes: 4 additions & 0 deletions test/functional/tools/job_environment_default_legacy.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ echo \$(pwd) > '$pwd' &&
echo "\$HOME" > '$home' &&
echo "\$TMP" > '$tmp' &&
echo "\$SOME_ENV_VAR" > '$some_env_var' &&
echo "\${JOBCONF_ENV_VAR:-UNSET}" > '$jobconf_env_var' &&
echo "\${CONTAINER_ENV_VAR:-UNSET}" > '$container_env_var' &&

touch "\${TMP:-/tmp}/job_tmp" &&
touch "\${TEMP:-/tmp}/job_temp" &&
Expand All @@ -24,6 +26,8 @@ touch "\${TMPDIR:-/tmp}/job_tmpdir"
<data name="home" format="txt" label="home" />
<data name="tmp" format="txt" label="tmp" />
<data name="some_env_var" format="txt" label="env_var" />
<data name="jobconf_env_var" format="txt" label="jobconf_env_var" />
<data name="container_env_var" format="txt" label="container_env_var" />
</outputs>
<tests>
</tests>
Expand Down
4 changes: 4 additions & 0 deletions test/functional/tools/job_environment_explicit_isolated_home.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ echo \$(pwd) > '$pwd' &&
echo "\$HOME" > '$home' &&
echo "\$TMP" > '$tmp' &&
echo "\$SOME_ENV_VAR" > '$some_env_var' &&
echo "\${JOBCONF_ENV_VAR:-UNSET}" > '$jobconf_env_var' &&
echo "\${CONTAINER_ENV_VAR:-UNSET}" > '$container_env_var' &&

touch "\${TMP:-/tmp}/job_tmp" &&
touch "\${TEMP:-/tmp}/job_temp" &&
Expand All @@ -24,6 +26,8 @@ touch "\${TMPDIR:-/tmp}/job_tmpdir"
<data name="home" format="txt" label="home" />
<data name="tmp" format="txt" label="tmp" />
<data name="some_env_var" format="txt" label="env_var" />
<data name="jobconf_env_var" format="txt" label="jobconf_env_var" />
<data name="container_env_var" format="txt" label="container_env_var" />
</outputs>
<tests>
</tests>
Expand Down
4 changes: 4 additions & 0 deletions test/functional/tools/job_environment_explicit_shared_home.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ echo \$(pwd) > '$pwd' &&
echo "\$HOME" > '$home' &&
echo "\$TMP" > '$tmp' &&
echo "\$SOME_ENV_VAR" > '$some_env_var' &&
echo "\${JOBCONF_ENV_VAR:-UNSET}" > '$jobconf_env_var' &&
echo "\${CONTAINER_ENV_VAR:-UNSET}" > '$container_env_var' &&

touch "\${TMP:-/tmp}/job_tmp" &&
touch "\${TEMP:-/tmp}/job_temp" &&
Expand All @@ -24,6 +26,8 @@ touch "\${TMPDIR:-/tmp}/job_tmpdir"
<data name="home" format="txt" label="home" />
<data name="tmp" format="txt" label="tmp" />
<data name="some_env_var" format="txt" label="env_var" />
<data name="jobconf_env_var" format="txt" label="jobconf_env_var" />
<data name="container_env_var" format="txt" label="container_env_var" />
</outputs>
<tests>
</tests>
Expand Down
8 changes: 8 additions & 0 deletions test/integration/dockerized_job_conf.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,19 @@ execution:
environments:
local_docker:
runner: local
env:
- name: 'JOBCONF_ENV_VAR'
value: 'YEAH'
- execute: 'true' # just check that this does not confuse env setup
docker_enabled: true
docker_env_CONTAINER_ENV_VAR: "CONTAINER_VAR_VALUE"
require_container: true
tmp_dir: '$(mktemp -d)'
local_docker_inline_container_resolvers:
runner: local
env:
- name: 'JOBCONF_ENV_VAR'
value: 'YEAH'
docker_enabled: true
container_resolvers:
- type: fallback
Expand Down
3 changes: 3 additions & 0 deletions test/integration/simple_job_conf.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@

<destinations>
<destination id="local_dest" runner="local">
<env id="JOBCONF_ENV_VAR">YEAH</env>
<param id="docker_env_CONTAINER_VAR">CONTAINER_VAR_VALUE</param>
<param id="singularity_env_CONTAINER_VAR">CONTAINER_VAR_VALUE</param>
</destination>
</destinations>

Expand Down
4 changes: 4 additions & 0 deletions test/integration/singularity_job_conf.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ execution:
environments:
local_singularity:
runner: local
env:
- name: 'JOBCONF_ENV_VAR'
value: 'YEAH'
singularity_enabled: true
singularity_env_CONTAINER_ENV_VAR: "CONTAINER_VAR_VALUE"
# Since tests run in /tmp/ , we apparently need to forbid the default mounting of /tmp
singularity_run_extra_arguments: '--no-mount tmp'
require_container: true
Expand Down
8 changes: 8 additions & 0 deletions test/integration/test_containerized_jobs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ def test_container_job_environment(self) -> None:
assert job_env.pwd.endswith("/working")
assert job_env.home.startswith(self.jobs_directory)
assert job_env.home.endswith("/home")
assert job_env.jobconf_env_var == "UNSET"
assert job_env.container_env_var == "CONTAINER_VAR_VALUE"

def test_container_job_environment_legacy(self) -> None:
"""
Expand All @@ -154,6 +156,8 @@ def test_container_job_environment_legacy(self) -> None:
assert job_env.pwd.endswith("/working")
assert not job_env.home.startswith(self.jobs_directory)
assert not job_env.home.endswith("/home")
assert job_env.jobconf_env_var == "UNSET"
assert job_env.container_env_var == "CONTAINER_VAR_VALUE"

def test_container_job_environment_explicit_shared_home(self) -> None:
"""
Expand All @@ -166,6 +170,8 @@ def test_container_job_environment_explicit_shared_home(self) -> None:
assert job_env.pwd.endswith("/working")
assert not job_env.home.startswith(self.jobs_directory)
assert not job_env.home.endswith("/home"), job_env.home
assert job_env.jobconf_env_var == "UNSET"
assert job_env.container_env_var == "CONTAINER_VAR_VALUE"

def test_container_job_environment_explicit_isolated_home(self) -> None:
"""
Expand All @@ -178,6 +184,8 @@ def test_container_job_environment_explicit_isolated_home(self) -> None:
assert job_env.pwd.endswith("/working")
assert job_env.home.startswith(self.jobs_directory)
assert job_env.home.endswith("/home"), job_env.home
assert job_env.jobconf_env_var == "UNSET"
assert job_env.container_env_var == "CONTAINER_VAR_VALUE"

def test_build_mulled(self) -> None:
"""
Expand Down
12 changes: 11 additions & 1 deletion test/integration/test_job_environments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
"home",
"tmp",
"some_env",
"jobconf_env_var",
"container_env_var",
],
)

Expand Down Expand Up @@ -60,7 +62,9 @@ def _environment_properties(self, history_id):
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()
some_env = self.dataset_populator.get_history_dataset_content(history_id, hid=6).strip()
return JobEnvironmentProperties(user_id, group_id, pwd, home, tmp, some_env)
jobconf_env_var = self.dataset_populator.get_history_dataset_content(history_id, hid=7).strip()
container_env_var = self.dataset_populator.get_history_dataset_content(history_id, hid=8).strip()
return JobEnvironmentProperties(user_id, group_id, pwd, home, tmp, some_env, jobconf_env_var, container_env_var)

def _check_completed_history(self, history_id):
"""Extension point that lets subclasses investigate the completed job."""
Expand Down Expand Up @@ -88,6 +92,12 @@ def test_default_environment_1801(self):
assert job_env.pwd.startswith(self.jobs_directory)
assert job_env.pwd.endswith("/working")

# check that env variables job
# - variables defined via env in job_conf are set
# - container specific vars via <param id="docker_env_.."> are not
assert job_env.jobconf_env_var == "YEAH"
assert job_env.container_env_var == "UNSET"

# 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
Expand Down
Loading