test support for custom wrapper install directory (#654)

* test support for custom wrapper install directory * cleanup wrappers too * remove extra erroneous } * set wrapper base to null * restore default wrapper bin * clean up wrappers testing logic * use wrapperDir instead of wrapper_bin for wrapper templates * update environment test and wrapper templates we have more cases than I anticipated - using a custom wrapper, the default wrapper ($root/modules), havint it unset (empty string) and then entirely disabled. * try removing quotes around moduleDir for singularity lua * clean up testing logic and wrapperDir/bin with quotes * remove erroneous $ for lua module setting * fix tcl wrapperDir to have {} and be in quotes Signed-off-by: vsoch <[email protected]>
singularityhub · Sep 19, 2023 · 21370ba · 21370ba
1 parent c407e02
commit 21370ba
Show file tree

Hide file tree

Showing 29 changed files with 217 additions and 57 deletions.
diff --git a/.github/dev-requirements.txt b/.github/dev-requirements.txt
@@ -1,4 +1,4 @@
 pre-commit
-black
+black==23.3.0
 isort
 flake8
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -97,6 +97,8 @@ jobs:
         printf "\n\nmodule help ============================================\n"
         module help python/3.9.5-alpine
 
+        script_path=$(which python-exec)
+        cat $script_path
         set -x
         python-exec echo donuts >test_output
         cat test_output

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ and **Merged pull requests**. Critical items to know are:
 The versions coincide with releases on pip. Only major versions will be released as tags on Github.
 
 ## [0.0.x](https://github.com/singularityhub/singularity-hpc/tree/main) (0.0.x)
+ - Allow custom location for wrapper scripts (0.1.24)
  - Labels with newlines need additional parsing (0.1.23)
  - Do not write directly to output with shpc show (0.1.22)
  - Podman template bug (0.1.21)

diff --git a/docs/getting_started/user-guide.rst b/docs/getting_started/user-guide.rst
@@ -176,6 +176,9 @@ variable replacement. A summary table of variables is included below, and then f
    * - module_base
      - The install directory for modules
      - $root_dir/modules
+   * - wrapper_base
+     - The install directory for script wrappers
+     - $root_dir/modules
    * - container_base
      - Where to install containers. If not defined, they are installed in "containers" in the install root
      - $root_dir/containers
@@ -347,6 +350,20 @@ Singularity Registry HPC uses this simple directory structure to ensure
 a unique namespace.
 
 
+Wrapper Base
+------------
+
+By default, if you do not set a wrapper script base they will be stored alongside
+modules. However, for large installations, we recommend you customize this path
+to be somewhere else. This way, you can avoid warnings from your module software
+about having too many files.
+
+.. code-block:: console
+
+    # an absolute path
+    $ shpc config set wrapper_base /opt/lmod/wrappers
+
+
 Container Images Folder
 -----------------------
 

diff --git a/shpc/main/container/base.py b/shpc/main/container/base.py
@@ -66,13 +66,16 @@ def add(self, sif, module_name, modulefile, template, **kwargs):
         """
         logger.warning("Add is not supported for %s" % self)
 
-    def add_environment(self, module_dir, envars, environment_file):
+    def add_environment(self, env_dir, envars, environment_file):
         """
         Given one or more environment variables in a dictionary, write to file.
+
+        The environment file goes in the wrapper directory, which can default
+        to the module directory if the value uses the default or is unset.
         """
         # Podman envars are written directly to the module file
         out = Template(shpc.main.templates.environment_file).render(envars=envars)
-        env_file = os.path.join(module_dir, environment_file)
+        env_file = os.path.join(env_dir, environment_file)
         shpc.utils.write_file(env_file, out)
 
     def delete(self, image):

diff --git a/shpc/main/container/docker.py b/shpc/main/container/docker.py
@@ -218,7 +218,7 @@ def install(self, module_path, template, module, features=None):
         if self.settings.wrapper_scripts["enabled"] is True:
             wrapper_scripts = shpc.main.wrappers.generate(
                 aliases=aliases,
-                module_dir=module.module_dir,
+                wrapper_dir=module.wrapper_dir,
                 features=features,
                 container=self,
                 image=module.container_path,

diff --git a/shpc/main/container/singularity.py b/shpc/main/container/singularity.py
@@ -211,9 +211,9 @@ def install(self, module_path, template, module, features=None):
         if self.settings.wrapper_scripts["enabled"] is True:
             wrapper_scripts = shpc.main.wrappers.generate(
                 aliases=aliases,
-                module_dir=module.module_dir,
                 features=features,
                 container=self,
+                wrapper_dir=module.wrapper_dir,
                 image=module.container_path,
                 config=module.config,
             )

diff --git a/shpc/main/container/update/versions.py b/shpc/main/container/update/versions.py
@@ -155,7 +155,7 @@ def _cmp(self, other):
             this_version = self.version[i]
             other_version = other.version[i]
 
-            if type(this_version) != type(other_version):
+            if type(this_version) is not type(other_version):
                 continue
 
             if this_version == other_version:

diff --git a/shpc/main/modules/base.py b/shpc/main/modules/base.py
@@ -130,6 +130,14 @@ def uninstall(self, name, force=False):
                 "$module_base/%s" % module.name,
             )
 
+        # If we have a wrapper
+        if module.wrapper_dir != module.module_dir:
+            self._uninstall(
+                module.wrapper_dir,
+                self.settings.wrapper_base,
+                "$wrapper_base/%s" % module.name,
+            )
+
         # If uninstalling the entire module, clean up symbolic links in all views
         for view_name in views_with_module:
             self.views[view_name].uninstall(module.module_dir)

diff --git a/shpc/main/modules/module.py b/shpc/main/modules/module.py
@@ -36,7 +36,7 @@ def add_environment(self):
         Write the environment to the module directory.
         """
         self.container.add_environment(
-            self.module_dir,
+            self.wrapper_dir,
             envars=self.config.get_envars(),
             environment_file=self.settings.environment_file,
         )
@@ -165,6 +165,14 @@ def uri(self):
         """
         return self._uri
 
+    @property
+    def wrapper_dir(self):
+        """
+        Full path to the wrapper directory
+        """
+        wrapper_dir = self.settings.wrapper_base or self.settings.module_base
+        return os.path.join(wrapper_dir, self.module_basepath)
+
     @property
     def module_dir(self):
         """

diff --git a/shpc/main/modules/templates/docker.lua b/shpc/main/modules/templates/docker.lua
@@ -16,18 +16,18 @@ Container:
 Commands include:
 
  - {|module_name|}-run:
-       {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file <moduleDir>/{{ settings.environment_file }} {% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %}-v ${PWD} -w ${PWD} <container> "$@"
+       {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file <wrapperDir>/{{ settings.environment_file }} {% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %}-v ${PWD} -w ${PWD} <container> "$@"
  - {|module_name|}-shell:
-       {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file <moduleDir>/{{ settings.environment_file }} {% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %}--entrypoint {{ shell }} -v ${PWD} -w ${PWD}<container>
+       {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file <wrapperDir>/{{ settings.environment_file }} {% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %}--entrypoint {{ shell }} -v ${PWD} -w ${PWD}<container>
  - {|module_name|}-exec:
-       {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm --entrypoint "" {% if settings.environment_file %}--env-file <moduleDir>/{{ settings.environment_file }} {% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v ${PWD} -w ${PWD} <container> "$@"
+       {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm --entrypoint "" {% if settings.environment_file %}--env-file <wrapperDir>/{{ settings.environment_file }} {% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v ${PWD} -w ${PWD} <container> "$@"
  - {|module_name|}-inspect:
        {{ command }} inspect <container>
  - {|module_name|}-container:
        echo "$PODMAN_CONTAINER"
 
 {% if aliases %}{% for alias in aliases %} - {{ alias.name }}:
-       {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm --entrypoint {{ alias.entrypoint }} {% if settings.environment_file %}--env-file <moduleDir>/{{ settings.environment_file }}{% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %}{% if alias.docker_options %}{{ alias.docker_options }} {% endif %} -v ${PWD} -w ${PWD} <container> "{{ alias.args }}" "$@"
+       {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm --entrypoint {{ alias.entrypoint }} {% if settings.environment_file %}--env-file <wrapperDir>/{{ settings.environment_file }}{% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %}{% if alias.docker_options %}{{ alias.docker_options }} {% endif %} -v ${PWD} -w ${PWD} <container> "{{ alias.args }}" "$@"
 {% endfor %}{% endif %}
 
 For each of the above, you can export:
@@ -48,23 +48,26 @@ if not os.getenv("PODMAN_COMMAND_OPTS") then setenv ("PODMAN_COMMAND_OPTS", "")
 -- directory containing this modulefile, once symlinks resolved (dynamically defined)
 local moduleDir = subprocess("realpath " .. myFileName()):match("(.*[/])") or "."
 
+-- If we have wrapper base set, honor it, otherwise we use the moduleDir
+{% if settings.wrapper_base %}local wrapperDir = "{{ module.wrapper_dir }}"{% else %}local wrapperDir = moduleDir{% endif %}
+
 -- interactive shell to any container, plus exec for aliases
 local containerPath = '{{ module.container_path }}'
 
 -- service environment variable to access docker URI
 setenv("PODMAN_CONTAINER", containerPath)
 
-local shellCmd = "{{ command }} ${PODMAN_OPTS} run -i{% if settings.enable_tty %}t{% endif %} ${PODMAN_COMMAND_OPTS} -u `id -u`:`id -g` --rm --entrypoint {{ shell }} {% if settings.environment_file %}--env-file " .. moduleDir .. "/{{ settings.environment_file }}{% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v ${PWD} -w ${PWD} " .. containerPath
+local shellCmd = "{{ command }} ${PODMAN_OPTS} run -i{% if settings.enable_tty %}t{% endif %} ${PODMAN_COMMAND_OPTS} -u `id -u`:`id -g` --rm --entrypoint {{ shell }} {% if settings.environment_file %}--env-file " .. wrapperDir .. "/{{ settings.environment_file }}{% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v ${PWD} -w ${PWD} " .. containerPath
 -- execCmd needs entrypoint to be the executor
-local execCmd = "{{ command }} ${PODMAN_OPTS} run ${PODMAN_COMMAND_OPTS} -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file " .. moduleDir .. "/{{ settings.environment_file }}{% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v ${PWD} -w ${PWD} "
-local runCmd = "{{ command }} ${PODMAN_OPTS} run ${PODMAN_COMMAND_OPTS} -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file " .. moduleDir .. "/{{ settings.environment_file }}{% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v ${PWD} -w ${PWD} " .. containerPath
+local execCmd = "{{ command }} ${PODMAN_OPTS} run ${PODMAN_COMMAND_OPTS} -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file " .. wrapperDir .. "/{{ settings.environment_file }}{% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v ${PWD} -w ${PWD} "
+local runCmd = "{{ command }} ${PODMAN_OPTS} run ${PODMAN_COMMAND_OPTS} -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file " .. wrapperDir .. "/{{ settings.environment_file }}{% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v ${PWD} -w ${PWD} " .. containerPath
 local inspectCmd = "{{ command }} ${PODMAN_OPTS} inspect ${PODMAN_COMMAND_OPTS} " .. containerPath
 
 -- conflict with modules with the same name
 conflict("{{ parsed_name.tool }}"{% if name != parsed_name.tool %},"{{ module.name }}"{% endif %}{% if aliases %}{% for alias in aliases %}{% if alias.name != parsed_name.tool %},"{{ alias.name }}"{% endif %}{% endfor %}{% endif %})
 
 -- if we have any wrapper scripts, add the bin directory
-{% if wrapper_scripts %}prepend_path("PATH", pathJoin(moduleDir, "bin")){% endif %}
+{% if wrapper_scripts %}prepend_path("PATH", pathJoin(wrapperDir, "bin")){% endif %}
 
 -- "aliases" to module commands - generate only if not a wrapper script already generated
 {% if aliases %}{% for alias in aliases %}{% if alias.name not in wrapper_scripts %}set_shell_function("{{ alias.name }}", execCmd .. {% if alias.docker_options %} "{{ alias.docker_options }} " .. {% endif %} " --entrypoint {{ alias.entrypoint }} " .. containerPath .. " {{ alias.args }} \"$@\"", execCmd .. {% if alias.docker_options %} "{{ alias.docker_options }} " .. {% endif %} " --entrypoint {{ alias.entrypoint }} " .. containerPath .. " {{ alias.args }}"){% endif %}