Various wrapper scripts improvement (#548)

* Defines the constructor arguments just once
* Factored out the shell method as it is almost the same between both container technologies
* The DockerContainer doesn't need to know about its subclass any more
* Stop forbidding templates from being named podman.sh
Only use WrapperScript - no more subclasses.
* WrapperScript objects don't have the wrapper_template attribute anymore
* Merged the three generation methods of WrapperScript into one
* Rewrote the template search
- Don't restrict user-defined global templates to be named singularity.sh or
  docker.sh as in shpc's default template directory
- Properly defined the search path based on all available template directories
- Minor bugfix: do not search the templates in the current working directory

* Allow using a custom template for aliases

* Turned get_shell_path to a property

Co-authored-by: Matthieu Muffato <[email protected]>

* Allow defining the alternative template script in a long form option rather than hijacking the singularity_scripts section

* Ensure that the wrapper is always defined
* Extracted a function that checks the wrapper exists
and returns where to find it

* Updated the docs to reflect the current wait templates are rendered
* Not used anywhere, so no need to make it a class variable
* Reinstated wrapper_template as a (mandatory) constructor argument
* Further refactoring to split the function that establishes the search path, from the one that finds the template file
* Added a test for the two new methods of WrapperScript
* Fixed the comment
* Pass the config as a kwarg so that we don't need to pass None for the container
* Applied black

Author: muffato

docs/getting_started/developer-guide.rst

@@ -267,7 +267,7 @@ about it. For alias wrapper scripts, the following variables are passed for rend
      - Example
    * - alias
      - dictionary
-     - The entire alias in question, including subfields name, command, singularity_options or docker_options, and args
+     - The entire alias in question, including subfields name, command, singularity_options or docker_options, singularity_script or docker_script, and args
      - ``{{ alias.name }}`` 
    * - settings
      - dictionary
@@ -304,44 +304,10 @@ If you want to write a custom container.yaml script:
 1. Add either (or both) of singularity_scripts/docker_scripts in the container.yaml, including an alias command and an associated script.
 2. Write the script with the associated name into that folder.
 
-The following variables are passed for rendering.
-
-.. list-table:: Container YAML Alias Variables
-   :widths: 15 15 40 30
-   :header-rows: 1
-
-   * - Name
-     - Type
-     - Description
-     - Example
-   * - alias
-     - string
-     - The alias name defined under singularity_scripts or docker_scripts
-     - ``{{ alias }}`` 
-   * - settings
-     - dictionary
-     - Everything referenced in the user settings
-     - ``{{ settings.wrapper_shell }}``
-   * - container
-     - dictionary
-     - The container technology
-     - ``{{ container.command }}`` renders to docker, singularity, or podman
-   * - config
-     - dictionary
-     - The entire container config (container.yaml) structured the same
-     - ``{{ config.docker }}``
-   * - image
-     - string
-     - The name of the container binary (SIF) or unique resource identifier
-     - ``{{ image }}``
-   * - module_dir
-     - string
-     - The name of the module directory
-     - ``{{ module_dir }}``
-   * - features
-     - dictionary
-     - A dictionary of parsed features
-     - ``{{ features.gpu }}``
+For rendering, the same variables as for alias wrapper scripts are passed,
+**except** ``alias`` which is now a *string* (the name of the alias defined
+under singularity_scripts or docker_scripts) and should be used directly, e.g.
+``{{ alias }}``.
 
 
 Templating for both wrapper script types

shpc/main/container/docker.py

@@ -33,13 +33,20 @@ def __init__(self):
             )
         super(DockerContainer, self).__init__()
 
+    @property
+    def shell_path(self):
+        """
+        Return the path of the shell to use with this container.
+        """
+        return self.settings.docker_shell
+
     def shell(self, image):
         """
         Interactive shell into a container image.
         """
         os.system(
-            "docker run -it --rm --entrypoint %s %s"
-            % (self.settings.docker_shell, image)
+            "%s run -it --rm --entrypoint %s %s"
+            % (self.command, self.shell_path, image)
         )
 
     def add_registry(self, uri):
@@ -231,17 +238,10 @@ def install(
                 config=config,
             )
 
-        # What shell to use?
-        shell = (
-            self.settings.podman_shell
-            if self.command == "podman"
-            else self.settings.docker_shell
-        )
-
         # Make sure to render all values!
         out = template.render(
             settings=self.settings,
-            shell=shell,
+            shell=self.shell_path,
             image=container_path,
             description=description,
             aliases=aliases,

shpc/main/container/podman.py

@@ -5,8 +5,6 @@
 
 from .docker import DockerContainer
 
-import os
-
 
 class PodmanContainer(DockerContainer):
     """
@@ -17,11 +15,9 @@ class PodmanContainer(DockerContainer):
     templatefile = "docker"
     command = "podman"
 
-    def shell(self, image):
+    @property
+    def shell_path(self):
         """
-        Interactive shell into a container image.
+        Return the path of the shell to use with this container.
         """
-        os.system(
-            "podman run -it --rm --entrypoint %s %s"
-            % (self.settings.podman_shell, image)
-        )
+        return self.settings.podman_shell

shpc/main/schemas.py

@@ -52,6 +52,8 @@
             "command": {"type": "string"},
             "singularity_options": {"type": "string"},
             "docker_options": {"type": "string"},
+            "singularity_script": {"type": "string"},
+            "docker_script": {"type": "string"},
         },
     },
 }

shpc/main/wrappers/__init__.py

@@ -2,7 +2,8 @@
 __copyright__ = "Copyright 2022, Vanessa Sochat"
 __license__ = "MPL 2.0"
 
-from .scripts import get_wrapper_script
+from shpc.logger import logger
+from .base import WrapperScript
 import os
 
 
@@ -22,40 +23,56 @@ def generate(image, container, config, **kwargs):
     settings = container.settings
     aliases = kwargs.get("aliases", [])
 
-    # Global wrapper scripts are generated via aliases
-    for container_tech in ["docker", "podman", "singularity"]:
-        template_name = settings.wrapper_scripts.get(container_tech)
-        if template_name and aliases and container_tech == container.command:
-            wrapper = get_wrapper_script(template_name)(
-                settings=settings,
-                wrapper_template=template_name,
-                config=config,
-                container=container,
-                image=image,
-                **kwargs
+    constructor_kwargs = {
+        "settings": settings,
+        "config": config,
+        "container": container,
+        "image": image,
+        **kwargs,
+    }
+
+    # Default wrapper for this container technology. Will be used for all
+    # aliases (unless overriden)
+    default_wrapper = None
+    default_template_name = settings.wrapper_scripts.get(container.command)
+    if default_template_name:
+        default_wrapper = WrapperScript(default_template_name, **constructor_kwargs)
+        # include_container_dir not set -> only look in the global locations
+        default_wrapper.load_template()
+
+    # Command aliases
+    custom_wrapper_option_name = "%s_script" % container.templatefile
+    for alias in aliases:
+        # Allow overriding the template name in the script option
+        if custom_wrapper_option_name in alias:
+            wrapper = WrapperScript(
+                alias[custom_wrapper_option_name], **constructor_kwargs
             )
-            generated += wrapper.generate_aliases()
+            wrapper.load_template(include_container_dir=True)
+        elif default_wrapper:
+            wrapper = default_wrapper
+        else:
+            logger.exit(
+                "Can't generate a wrapper script for '%s' as there is no template defined for %s"
+                % (alias["name"], container.templatefile)
+            )
+
+        # NB: alias is a dictionary
+        generated += wrapper.generate(alias["name"], alias)
 
     # Container level wrapper scripts (allow eventually supporting custom podman)
     scripts = {
         "singularity": config.singularity_scripts,
         "docker": config.docker_scripts,
         "podman": config.docker_scripts,
     }
-    for command, listing in scripts.items():
-
-        # Don't look if no custom container.yaml scripts OR wrong container tech
-        if not listing or container.templatefile != command:
-            continue
-        for alias, template_name in listing.items():
-            wrapper = get_wrapper_script(template_name)(
-                settings=settings,
-                wrapper_template=template_name,
-                config=config,
-                container=container,
-                image=image,
-                **kwargs
-            )
-            generated += wrapper.generate(alias)
+    # Additional commands defined in the custom container.yaml script section
+    listing = scripts.get(container.templatefile, [])
+    for alias, template_name in listing.items():
+        wrapper = WrapperScript(template_name, **constructor_kwargs)
+        # Template wrapper scripts may live alongside container.yaml
+        wrapper.load_template(include_container_dir=True)
+        # NB: alias is a string
+        generated += wrapper.generate(alias, alias)
 
     return list(set(generated))