refactor of "add" to generate a container.yaml first to close #519

Signed-off-by: vsoch <[email protected]>

.github/workflows/main.yml

@@ -34,9 +34,27 @@ jobs:
         run: |
           export PATH="/usr/share/miniconda/bin:$PATH"
           source activate black
+          pyflakes shpc/utils/fileio.py
+          pyflakes shpc/utils/terminal.py
           pyflakes shpc/main/*.py
           pyflakes shpc/main/modules
           pyflakes shpc/main/container/base.py
           pyflakes shpc/main/container/podman.py
           pyflakes shpc/main/container/docker.py
           pyflakes shpc/main/container/singularity.py
+          pyflakes shpc/tests
+          pyflakes shpc/*.py
+          pyflakes shpc/client/add.py
+          pyflakes shpc/client/check.py
+          pyflakes shpc/client/config.py
+          pyflakes shpc/client/docgen.py
+          pyflakes shpc/client/get.py
+          pyflakes shpc/client/inspect.py
+          pyflakes shpc/client/install.py
+          pyflakes shpc/client/listing.py
+          pyflakes shpc/client/namespace.py
+          pyflakes shpc/client/pull.py
+          pyflakes shpc/client/show.py
+          pyflakes shpc/client/test.py
+          pyflakes shpc/client/uninstall.py
+          pyflakes shpc/main/wrappers

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)
+ - refactor to "add" to generate a container.yaml first (0.0.49)
  - Properly cleanup empty module directories, and asking to remove a container that doesn't exist now logs a _warning_ (0.0.48)
  - wrapper script generation permissions error (0.0.47)
  - fixing but with stream command repeating output (0.0.46)

docs/getting_started/user-guide.rst

@@ -1116,17 +1116,61 @@ Add
 It might be the case that you have a container locally, and you want to
 make it available as a module (without pulling it from a registry). Although
 this is discouraged because it means you will need to manually maintain
-versions, shpc does support the "add" command to do this. You can simply provide
-the container path and the unique resource identifier:
+versions, shpc does support the "add" command to do this. The steps for adding a
+container are:
+
+1. Running ``shpc add`` to create a container.yaml in the registry namespace
+2. Customizing the container.yaml to your liking
+3. Running ``shpc install`` to formally install your new container.
+
+As an example, let's start with the container ``salad_latest.sif``. We have it
+on our local machine and cannot pull it from a registry. First, let's run ``shpc add``
+and tell shpc that we want it under the ``dinosaur/salad`` namespace.
+
+.. code-block:: console
+
+    $ shpc add salad_latest.sif dinosaur/salad:latest
+    Registry entry dinosaur/salad:latest was added! Before shpc install, edit:
+    /home/vanessa/Desktop/Code/shpc/registry/dinosaur/salad/container.yaml
+
+At this point, you should open up the container.yaml generated and edit to your liking.
+This usually means updating the description, maintainer, aliases, and possibly providing a url
+to find more information or support. Also notice we've provided the tag to be latest. If you update this registry
+entry in the future with a new version, you'll want to provide a new tag. If you provide
+an existing tag, you'll be asked to confirm before continuing. When you are happy, 
+it's time to install it, just as you would a regular container!
+
+.. code-block:: console
+
+    $ shpc install dinosaur/salad:latest
+
+
+And this will generate the expected module and container in your respective directory bases:
+
 
 .. code-block:: console
 
-    $ shpc add salad_latest.sif vanessa/salad:latest
+    $ tree modules/dinosaur/salad/
+    modules/dinosaur/salad/
+    └── latest
+        ├── 99-shpc.sh
+        └── module.lua
+
+    1 directory, 2 files
+
+    $ tree containers/dinosaur/salad/
+    containers/dinosaur/salad/
+    └── latest
+        └── sha256:77c7326e74d0e8b46d4e50d99e848fc950ed047babd60203e17449f5df8f39d4.sif
+
+    1 directory, 1 file
+
+And that's it! Note that ``add`` previously would add the container directly to the module
+directory, and as of version 0.0.49 it's been updated to generate the container.yaml first.
+Also note that ``add`` is only supported for Singularity, as Docker and Podman containers are 
+typically provided via registries. If you are looking for support for add for another
+container technology, please `open a new issue <https://github.com/singularityhub/singularity-hpc/issues>`_.
 
-If the unique resource identifier corresponds with a registry entry, you
-will not be allowed to create it, as this would create a namespace conflict.
-Since we don't have a configuration file to define custom aliases, the container
-will just be exposed as it's command to run it.
 
 Get
 ---

shpc/__init__.py

@@ -1 +1,3 @@
 from shpc.version import __version__
+
+assert __version__

shpc/main/container/__init__.py

@@ -45,6 +45,9 @@ def get(self, key, default=None):
         if digest:
             return Tag(key, digest)
 
+    def set(self, key, value):
+        self._tags[key] = value
+
 
 class Tag:
     """
@@ -67,10 +70,11 @@ class ContainerConfig:
     A ContainerConfig wraps a container.yaml file, intended for install.
     """
 
-    def __init__(self, package_file):
+    def __init__(self, package_file, validate=True):
         """Load a package file for a container."""
         self.load(package_file)
-        self.validate()
+        if validate:
+            self.validate()
 
     def __str__(self):
         return "[container:%s]" % self.name
@@ -92,7 +96,7 @@ def flatname(self):
         """
         Flatten the docker uri into a filesystem appropriate name
         """
-        name = self.docker or self.oras or self.gh
+        name = self.docker or self.oras or self.gh or self.path
         return name.replace("/", "-")
 
     @property
@@ -102,6 +106,8 @@ def name(self):
         """
         from .base import ContainerName
 
+        if hasattr(self, "path") and self.path is not None:
+            return ContainerName("/".join(self.package_dir.split("/")[-2:]))
         name = self.docker or self.oras or self.gh
         return ContainerName(name)
 
@@ -112,6 +118,17 @@ def latest(self):
         """
         return self.tags.latest
 
+    def set(self, key, value):
+        """
+        Update loaded config with keys and values
+        """
+        if key not in self._config:
+            logger.warning("%s is not already defined for this container config!" % key)
+        self._config[key] = value
+
+    def add_tag(self, key, value):
+        self._config["tags"][key] = value
+
     def set_tag(self, tag):
         """
         Set a tag to be the config default (defaults to latest otherwise)
@@ -135,25 +152,54 @@ def get_url(self):
         Given a loaded container recipe, get the registry url.
         """
         # Not in json schema, but currently required
-        if "docker" not in self._config and "gh" not in self._config:
-            logger.exit("A docker or gh field is currently required in the config.")
-        return self._config.get("docker") or self._config.get("gh")
+        if (
+            "docker" not in self._config
+            and "gh" not in self._config
+            and "path" not in self._config
+        ):
+            logger.exit(
+                "A docker, gh, or path field is currently required in the config."
+            )
+        return (
+            self._config.get("docker")
+            or self._config.get("gh")
+            or self._config.get("path")
+        )
 
     def get(self, key, default=None):
         return self._config.get(key, default)
 
+    @property
+    def package_dir(self):
+        """
+        Get the directory of the container.yaml, for finding local containers.
+        """
+        if self.package_file:
+            return os.path.dirname(self.package_file)
+
     def get_pull_type(self):
         if self.oras:
             return "oras"
         if self.gh:
             return "gh"
-        return "docker"
+        if self.docker:
+            return "docker"
+        if self.path:
+            return self.path
+        logger.exit(
+            "Cannot identify pull type: one of oras, docker, gh, or path is required."
+        )
 
     def get_uri(self):
         """
         Return the unique resource identifier
         """
-        return getattr(self, "docker") or getattr(self, "oras") or getattr(self, "gh")
+        return (
+            getattr(self, "docker")
+            or getattr(self, "oras")
+            or getattr(self, "gh")
+            or getattr(self, "path")
+        )
 
     def __getattr__(self, key):
         """
@@ -203,9 +249,18 @@ def get_aliases(self):
             seen.add(key)
         return aliases
 
-    def load(self, package_file):
-        """Load the settings file into the settings object"""
+    def save(self, package_file):
+        """
+        Save the container.yaml to file. This is usually for shpc add.
+        """
+        yaml = YAML()
+        with open(package_file, "w") as fd:
+            yaml.dump(self._config, fd)
 
+    def load(self, package_file):
+        """
+        Load the settings file into the settings object
+        """
         # Exit quickly if the package does not exist
         if not os.path.exists(package_file):
             logger.exit("%s does not exist." % package_file)

shpc/main/container/singularity.py

@@ -4,8 +4,9 @@
 
 
 from shpc.logger import logger
-import shpc.utils
-from .base import ContainerTechnology, ContainerName
+import shpc.utils as utils
+import shpc.main.wrappers
+from .base import ContainerTechnology
 
 from datetime import datetime
 from glob import glob
@@ -72,50 +73,55 @@ def get(self, module_name, env_file=False):
             logger.exit("Found more than one sif in module folder.")
         return sif[0]
 
-    def add(self, sif, module_name, modulefile, template, **kwargs):
+    def add(self, module_name, image, config, container_yaml, **kwargs):
         """
-        Manually add a registry container.
+        Manually add a registry container, e.g., generating a container.yaml
+        for an existing container file. container_yaml is the destination file.
+        If it's already exisitng, it's loaded into config. Otherwise we are
+        using a template config.
         """
-        module_dir = os.path.dirname(modulefile)
-
-        # Ensure the container exists
-        sif = os.path.abspath(sif)
-        if not os.path.exists(sif):
-            logger.exit("%s does not exist." % sif)
-
-        # First ensure that we aren't using a known namespace
-        for registry_dir, _ in self.iter_registry():
-            for subfolder in module_name.split("/"):
-                registry_dir = os.path.join(registry_dir, subfolder)
-                if os.path.exists(registry_dir):
-                    logger.exit(
-                        "%s is a known registry namespace, choose another for a custom addition."
-                        % subfolder
-                    )
-
-        # The user can have a different container directory defined
-        container_dir = self.container_dir(module_name)
-        shpc.utils.mkdirp([container_dir, module_dir])
-
-        # Name the container appropriately
-        name = module_name.replace("/", "-")
-        digest = shpc.utils.get_file_hash(sif)
-        dest = os.path.join(container_dir, "%s-sha256:%s.sif" % (name, digest))
-        shutil.copyfile(sif, dest)
-
-        # Parse the module name for the user
-        parsed_name = ContainerName(module_name)
-
-        self.install(
-            modulefile,
-            dest,
-            module_name,
-            template,
-            parsed_name=parsed_name,
-            features=kwargs.get("features"),
+        if ":" not in module_name:
+            tag = "latest"
+        else:
+            name, tag = module_name.split(":", 1)
+
+        # Ensure the sif exists
+        if not os.path.exists(image):
+            logger.exit(f"{image} does not exist.")
+
+        digest = utils.get_file_hash(image)
+
+        # Cut out early if the tag isn't latest, and we already have it
+        if tag != "latest" and tag in config.tags:
+            if not utils.confirm_action(
+                "Tag %s already is defined, are you sure you want to overwrite it? "
+                % tag
+            ):
+                return
+
+        # Destination for container in registry
+        dest_dir = os.path.dirname(container_yaml)
+
+        # The destination container in the registry folder
+        container_digest = "sha256:%s" % digest
+        container_name = "%s.sif" % container_digest
+        dest_container = os.path.join(dest_dir, container_name)
+
+        # Update the config path and latest
+        config.set("path", container_name)
+        config.set("latest", {tag: container_digest})
+        config.add_tag(tag, container_digest)
+
+        # Only copy if it's not there yet (enforces naming by hash)
+        utils.mkdir_p(dest_dir)
+        if not os.path.exists(dest_container):
+            shutil.copyfile(image, dest_container)
+
+        config.save(container_yaml)
+        logger.info(
+            "Registry entry %s was added! Before shpc install, edit:" % module_name
         )
-        self.add_environment(module_dir, {}, self.settings.environment_file)
-        logger.info("Module %s was created." % (module_name))
+        print(container_yaml)
 
     def install(
         self,
@@ -199,7 +205,7 @@ def install(
             parsed_name=parsed_name,
             wrapper_scripts=wrapper_scripts,
         )
-        shpc.utils.write_file(module_path, out)
+        utils.write_file(module_path, out)
 
     def registry_pull(self, module_dir, container_dir, config, tag):
         """
@@ -357,7 +363,7 @@ def test_script(self, image, test_script):
         Given a test file, run it and respond accordingly.
         """
         command = [self.command, "exec", image, "/bin/bash", test_script]
-        result = shpc.utils.run_command(command)
+        result = utils.run_command(command)
 
         # We can't run on incompatible hosts
         if (