Merge pull request #71 from DeiC-HPC/singularity-env-sourcing

Handle change in Apptainer custom environment variables handling

.github/workflows/CI_pull_request.yml

@@ -22,39 +22,39 @@ jobs:
       fail-fast: false
       matrix:
         os: ["ubuntu-latest"]
-        python-version: ["3.8", "3.9", "3.10", "3.11"]
-        singularity-provider: ["singularity", "apptainer", "singularity-ce"]
-        include:
-          - singularity-provider: "singularity"
-            singularity-version: "3.8.7"
-          - singularity-provider: "apptainer"
-            singularity-version: "1.1.3"
-          - singularity-provider: "singularity-ce"
-            singularity-version: "3.9.2"
+        python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
+        singularity: [
+          {provider: "singularity", version: "3.8.7"},
+          {provider: "apptainer", version: "1.1.3"},
+          {provider: "apptainer", version: "1.3.4"},
+          {provider: "singularity-ce", version: "3.9.2"},
+          {provider: "singularity-ce", version: "4.1.3"},
+          {provider: "singularity-ce", version: "4.2.1"}
+        ]
     steps:
       - name: Checkout code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
       - name: Attempt to restore Singularity/Apptainer installer from cache
         uses: actions/cache@v3
         id: cache-singularity-installer
         with:
           path: ./singularity_installer.deb
-          key: ${{ matrix.os }}-${{ matrix.singularity-provider }}_installer-${{ matrix.singularity-version }}
-      - name: Download Singularity ${{ matrix.singularity-version }}
-        if: ${{ steps.cache-singularity-installer.outputs.cache-hit != 'true' && matrix.singularity-provider == 'singularity' }}
-        run: wget -O singularity_installer.deb https://github.com/apptainer/singularity/releases/download/v${{ matrix.singularity-version }}/singularity-container_${{ matrix.singularity-version }}_amd64.deb
-      - name: Download Apptainer ${{ matrix.apptainer-version }}
-        if: ${{ steps.cache-singularity-installer.outputs.cache-hit != 'true' && matrix.singularity-provider == 'apptainer' }}
-        run: wget -O singularity_installer.deb https://github.com/apptainer/apptainer/releases/download/v${{ matrix.singularity-version }}/apptainer_${{ matrix.singularity-version }}_amd64.deb
-      - name: Download SingularityCE ${{ matrix.singularity-version }}
-        if: ${{ steps.cache-singularity-installer.outputs.cache-hit != 'true' && matrix.singularity-provider == 'singularity-ce' }}
-        run: wget -O singularity_installer.deb https://github.com/sylabs/singularity/releases/download/v${{ matrix.singularity-version }}/singularity-ce_${{ matrix.singularity-version }}-focal_amd64.deb
+          key: ${{ matrix.os }}-${{ matrix.singularity.provider }}_installer-${{ matrix.singularity.version }}
+      - name: Download Singularity ${{ matrix.singularity.version }}
+        if: ${{ steps.cache-singularity-installer.outputs.cache-hit != 'true' && matrix.singularity.provider == 'singularity' }}
+        run: wget -O singularity_installer.deb https://github.com/apptainer/singularity/releases/download/v${{ matrix.singularity.version }}/singularity-container_${{ matrix.singularity.version }}_amd64.deb
+      - name: Download Apptainer ${{ matrix.singularity.version }}
+        if: ${{ steps.cache-singularity-installer.outputs.cache-hit != 'true' && matrix.singularity.provider == 'apptainer' }}
+        run: wget -O singularity_installer.deb https://github.com/apptainer/apptainer/releases/download/v${{ matrix.singularity.version }}/apptainer_${{ matrix.singularity.version }}_amd64.deb
+      - name: Download SingularityCE ${{ matrix.singularity.version }}
+        if: ${{ steps.cache-singularity-installer.outputs.cache-hit != 'true' && matrix.singularity.provider == 'singularity-ce' }}
+        run: wget -O singularity_installer.deb https://github.com/sylabs/singularity/releases/download/v${{ matrix.singularity.version }}/singularity-ce_${{ matrix.singularity.version }}-focal_amd64.deb
       - name: Set up Singularity/Apptainer
         run: |
           sudo apt install ./singularity_installer.deb
           singularity --version
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
           cache: pip
@@ -68,7 +68,7 @@ jobs:
       - name: Archive the test results and coverage
         uses: actions/upload-artifact@v3
         with:
-          name: test-results-and-coverage_python${{ matrix.python-version }}_${{ matrix.singularity-provider }}${{ matrix.singularity-version }}
+          name: test-results-and-coverage_python${{ matrix.python-version }}_${{ matrix.singularity.provider }}${{ matrix.singularity.version }}
           path: |
             pytest_junit_out.xml
             htmlcov/*

.github/workflows/CI_push.yml

@@ -21,12 +21,12 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ["3.9", "3.11"]
+        python-version: ["3.9", "3.12"]
     steps:
       - name: Checkout code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
           cache: pip

.github/workflows/SCHED_docs_linkcheck.yml

@@ -12,9 +12,9 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
       - name: Set up Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.x'
           cache: pip

.pre-commit-config.yaml

@@ -1,13 +1,13 @@
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.6.4
+    rev: v0.6.9
     hooks:
       - id: ruff
         args:
           - --fix
       - id: ruff-format
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
+    rev: v5.0.0
     hooks:
       - id: debug-statements
       - id: end-of-file-fixer

cotainr/container.py

@@ -151,9 +151,9 @@ def add_to_env(self, *, shell_script):
         """
         Add `shell_script` to the sourced environment in the container.
 
-        The content of `shell_script` is written as-is to the /environment file
-        in the Singularity container which is sourced on execution of the
-        container.
+        The content of `shell_script` is written as-is to the file
+        /.singularity.d/env/92-cotainr-env.sh in the Singularity container
+        which is sourced on execution of the container.
 
         Parameters
         ----------
@@ -163,7 +163,7 @@ def add_to_env(self, *, shell_script):
         """
         self._assert_within_sandbox_context()
 
-        env_file = self.sandbox_dir / "environment"
+        env_file = self.sandbox_dir / ".singularity.d/env/92-cotainr-env.sh"
         with env_file.open(mode="a") as f:
             f.write(shell_script + "\n")
 

cotainr/tests/cli/test_build.py

@@ -17,6 +17,7 @@
 from cotainr.cli import Build, CotainrCLI
 from ..container.patches import (
     patch_save_singularity_sandbox_context,
+    patch_fake_singularity_sandbox_env_folder,
     patch_disable_singularity_sandbox_subprocess_runner,
     patch_disable_add_metadata,
 )
@@ -340,6 +341,7 @@ def test_include_conda_env(
         patch_disable_singularity_sandbox_subprocess_runner,
         patch_disable_conda_install_bootstrap_conda,
         patch_disable_conda_install_download_miniforge_installer,
+        patch_fake_singularity_sandbox_env_folder,
         patch_save_singularity_sandbox_context,
         patch_disable_add_metadata,
         patch_disable_console_spinner,
@@ -350,6 +352,7 @@ def test_include_conda_env(
         base_image = "some_base_image_6021"
         conda_env = "some_conda_env_6021"
         conda_env_content = "Some conda env content 6021"
+        saved_sandbox_dir = Path(f"./{patch_save_singularity_sandbox_context}")
         Path(conda_env).write_text(conda_env_content)
         Build(
             image_path=image_path,
@@ -359,11 +362,11 @@ def test_include_conda_env(
         ).execute()
 
         # Check that conda_env file has been copied to container
-        assert Path(f"./saved_sandbox_dir/{conda_env}").read_text() == conda_env_content
+        assert (saved_sandbox_dir / f"{conda_env}").read_text() == conda_env_content
 
         # Check that the singularity environment has been updated activate conda env
         assert (
-            Path("./saved_sandbox_dir/environment")
+            (saved_sandbox_dir / ".singularity.d/env/92-cotainr-env.sh")
             .read_text()
             .strip()
             .endswith("conda activate conda_container_env")

cotainr/tests/container/patches.py

@@ -36,6 +36,36 @@ def mock_subprocess_runner(self, *, args, **kwargs):
     )
 
 
+@pytest.fixture
+def patch_fake_singularity_sandbox_env_folder(monkeypatch):
+    """
+    Fake the creation of the .singularity.d/env/ folder.
+
+    Normally, the folder is created by SingularitySandbox.__enter__() when it
+    runs Singularity to create the sandbox, but when the call to singularity
+    has been patched, this fixture may be used to create the folder anyway.
+    """
+
+    def mock_enter(self):
+        # Call "true" __enter__ for setup
+        ret_val = self._non_mocked_context_enter()
+
+        # Create fake environment folder
+        singularity_env_folder = self.sandbox_dir / ".singularity.d/env"
+        singularity_env_folder.mkdir(parents=True, exist_ok=True)
+
+        return ret_val
+
+    monkeypatch.setattr(
+        cotainr.container.SingularitySandbox,
+        "_non_mocked_context_enter",
+        cotainr.container.SingularitySandbox.__enter__,
+        raising=False,
+    )
+
+    monkeypatch.setattr(cotainr.container.SingularitySandbox, "__enter__", mock_enter)
+
+
 @pytest.fixture
 def patch_save_singularity_sandbox_context(monkeypatch):
     """
@@ -45,12 +75,14 @@ def patch_save_singularity_sandbox_context(monkeypatch):
     `saved_sandbox_dir` before being cleaned up.
     """
 
+    saved_sandbox_dir_name = "saved_sandbox_dir"
+
     def mock_exit(self, exc_type, exc_value, traceback):
         # Copy content of _tmp_dir
-        shutil.copytree(self.sandbox_dir, self._origin / "saved_sandbox_dir")
+        shutil.copytree(self.sandbox_dir, self._origin / saved_sandbox_dir_name)
 
         # Call "true" __exit__ for cleanup
-        self._non_mocked_context_exit(exc_type, exc_value, traceback)
+        return self._non_mocked_context_exit(exc_type, exc_value, traceback)
 
     monkeypatch.setattr(
         cotainr.container.SingularitySandbox,
@@ -60,6 +92,8 @@ def mock_exit(self, exc_type, exc_value, traceback):
     )
     monkeypatch.setattr(cotainr.container.SingularitySandbox, "__exit__", mock_exit)
 
+    return saved_sandbox_dir_name
+
 
 @pytest.fixture
 def patch_disable_add_metadata(monkeypatch):

cotainr/tests/container/test_singularity_sandbox.py

@@ -15,6 +15,7 @@
 from cotainr.container import SingularitySandbox
 from cotainr.tracing import LogDispatcher, LogSettings
 from .data import data_cached_alpine_sif
+from .patches import patch_fake_singularity_sandbox_env_folder
 from ..util.patches import patch_disable_stream_subprocess
 
 
@@ -72,24 +73,30 @@ def test_tmp_dir_setup_and_teardown(self, patch_disable_stream_subprocess):
 
 
 class TestAddToEnv:
-    def test_add_twice(self, patch_disable_stream_subprocess):
+    def test_add_twice(
+        self, patch_disable_stream_subprocess, patch_fake_singularity_sandbox_env_folder
+    ):
         lines = ["first script line", "second script line"]
         with SingularitySandbox(base_image="my_base_image_6021") as sandbox:
-            env_file = sandbox.sandbox_dir / "environment"
+            env_file = sandbox.sandbox_dir / ".singularity.d/env/92-cotainr-env.sh"
             for line in lines:
                 sandbox.add_to_env(shell_script=line)
             assert env_file.read_text() == lines[0] + "\n" + lines[1] + "\n"
 
-    def test_newline_encapsulation(self, patch_disable_stream_subprocess):
+    def test_newline_encapsulation(
+        self, patch_disable_stream_subprocess, patch_fake_singularity_sandbox_env_folder
+    ):
         with SingularitySandbox(base_image="my_base_image_6021") as sandbox:
-            env_file = sandbox.sandbox_dir / "environment"
+            env_file = sandbox.sandbox_dir / ".singularity.d/env/92-cotainr-env.sh"
             shell_script = "fancy shell_script\nas a double line string"
             sandbox.add_to_env(shell_script=shell_script)
             assert env_file.read_text() == shell_script + "\n"
 
-    def test_shell_script_append(self, patch_disable_stream_subprocess):
+    def test_shell_script_append(
+        self, patch_disable_stream_subprocess, patch_fake_singularity_sandbox_env_folder
+    ):
         with SingularitySandbox(base_image="my_base_image_6021") as sandbox:
-            env_file = sandbox.sandbox_dir / "environment"
+            env_file = sandbox.sandbox_dir / ".singularity.d/env/92-cotainr-env.sh"
             existing_shell_script = "some existing\nshell script"
             env_file.write_text(existing_shell_script)
             new_shell_script = "fancy_shell_script\nas_a_string"
@@ -107,6 +114,49 @@ def test_add_verbosity_arg(self, capsys, patch_disable_stream_subprocess):
         stdout_lines = capsys.readouterr().out.rstrip("\n").split("\n")
         assert "args=['singularity', '-q', " in stdout_lines[-1]
 
+    def test_environment_not_overwritten(
+        self, data_cached_alpine_sif, singularity_exec, tmp_path
+    ):
+        # Test that any custom environment variables set via
+        # SingularitySandbox.add_to_env(...) are not overwritten when the SIF
+        # image file is built.
+        # We used to write our custom environment variables to /environment
+        # which is a symlink to /.singularity.d/env/90-environment.sh However,
+        # as of some newer version of Apptainer, the content of /environment is
+        # set to its default value when building the SIF image from the
+        # sandbox, erasing our modifications. It is unclear if this is
+        # intentional. Looking at both the Apptainer documentation
+        # (https://apptainer.org/docs/user/main/environment_and_metadata.html#singularity-d-directory)
+        # as well as the Singularity documentation
+        # (https://docs.sylabs.io/guides/latest/user-guide/environment_and_metadata.html#singularity-d-directory)
+        # it is sketchy to edit this /environment file as they both state that
+        # "You should not manually modify files under /.singularity.d" and "In
+        # the longer term, metadata will be moved outside of the container, and
+        # stored only in the SIF file metadata descriptor."
+        # As a workaround, we now write our custom environment variables to a
+        # custom /.singularity.d/env/92-cotainr-env.sh file which is probably
+        # not the right solution. However, at this point, though, it is unclear
+        # how to set custom environment variables in any other way when using a
+        # Singularity sandbox.
+        build_container_path = tmp_path / "container_6021.sif"
+        with SingularitySandbox(base_image=data_cached_alpine_sif) as sandbox:
+            sandbox.add_to_env(shell_script="some shell script")
+            assert (
+                sandbox.sandbox_dir / ".singularity.d/env/92-cotainr-env.sh"
+            ).exists()
+            assert (
+                (sandbox.sandbox_dir / ".singularity.d/env/92-cotainr-env.sh")
+                .read_text()
+                .endswith("some shell script\n")
+            )
+            sandbox.build_image(path=build_container_path)
+
+        container_cat_env_process = singularity_exec(
+            f"{build_container_path} cat /.singularity.d/env/92-cotainr-env.sh"
+        )
+        env_file_contents = container_cat_env_process.stdout
+        assert env_file_contents.endswith("some shell script\n")
+
     def test_fix_perms_on_oci_docker_images(self, tmp_path):
         # Tests correct permission handling in relation to the error:
         #   FATAL:   While performing build: packer failed to pack: copy Failed: