Uninstall (#603)

* Alternative class to Module to allow non-Registry-backed modules
* Extended the test suite to test uninstallation
* Fold everything back into a single class: Module
* Moved the verification of the tag to the set_tag method
* config.name is a ContainerName. Need to turn it into a string
* Introduced get_module
* Reinstated module_basepath as a property
* Extracted Module.load_config and made get_module rely on that and new_module
* The `tag` parameter is almost never used, and conflicts with the convention `tool:tag`
* Moved the validation back to Module
* Moved the block to make the file closer to its original
* Made docgen work with versioned modules too
* bugfix: get rid of an unnecessary /
* Display the versioned name
* Use the requested version as an example
* Only get_module needs the container path
* Preferred style
* Cleanup: only pass the config object itself and access its properties from the template
* Use urllib to properly parse the URL
* Version bump and changelog update
* There shouldn't be a space there !

Author: muffato

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)
+ - Fix bugs uninstalling all tags of a module (0.1.16)
  - support for install using registry recipe and local image (0.1.15)
  - fix views .view_module modulefile and loading (0.1.14)
  - support for system modules, depends on, in views and editor envars (0.1.13)

shpc/main/client.py

@@ -105,11 +105,12 @@ def load_registry_config(self, name):
             logger.exit("%s is not a known recipe in any registry." % name)
         return container.ContainerConfig(result)
 
-    def _load_container(self, name, tag=None):
+    def _load_container(self, name):
         """
         Given a name and an optional tag to default to, load a package
         """
         # Split name and tag
+        tag = None
         if ":" in name:
             name, tag = name.split(":", 1)
 
@@ -175,22 +176,23 @@ def cleanup(tmpdir):
         # Test all tags (this could be subsetted)
         for tag in tags:
 
-            image = self.install(module_name, tag)
+            versioned_name = module_name + ":" + tag
+            image = self.install(versioned_name)
 
             # Do we want to test loading?
             if not skip_module and hasattr(self, "_test"):
                 result = self._test(module_name, tmpdir, tag, template)
                 if result != 0:
                     cleanup(tmpdir)
-                    logger.exit("Test of %s was not successful." % module_name)
+                    logger.exit("Test of %s was not successful." % versioned_name)
 
             # Do we want to test the test commands?
             if test_commands and config.test:
                 utils.write_file(test_file, config.test)
                 return_code = self.container.test_script(image, test_file)
                 if return_code != 0:
                     cleanup(tmpdir)
-                    logger.exit("Test of %s was not successful." % module_name)
+                    logger.exit("Test of %s was not successful." % versioned_name)
 
             # Test the commands
             if not test_exec:

shpc/main/modules/base.py

@@ -7,6 +7,7 @@
 import os
 import subprocess
 import sys
+import urllib
 from datetime import datetime
 
 import shpc.defaults as defaults
@@ -236,14 +237,14 @@ def docgen(self, module_name, registry=None, out=None, branch="main"):
         """
         Render documentation for a module within a local registry.
         """
-        config = self._load_container(module_name)
+        config = self.get_module(module_name).config
 
         out = out or sys.stdout
         aliases = config.get_aliases()
         template = self.template.load("docs.md")
         registry = registry or defaults.github_url
-        github_url = "%s/blob/%s/%s/container.yaml" % (registry, branch, module_name)
-        registry_bare = registry.split(".com")[-1]
+        github_url = "%s/blob/%s/%s/container.yaml" % (registry, branch, config.name)
+        registry_bare = urllib.parse.urlparse(registry).path.lstrip("/")
         raw = (
             "https://gitlab.com/%s/-/raw/%s/%s/container.yaml"
             if "gitlab" in registry
@@ -252,24 +253,19 @@ def docgen(self, module_name, registry=None, out=None, branch="main"):
         raw_github_url = raw % (
             registry_bare,
             branch,
-            module_name,
+            config.name,
         )
 
         # Currently one doc is rendered for all containers
         result = template.render(
             parsed_name=config.name,
+            config=config,
             settings=self.settings,
-            description=config.description,
             aliases=aliases,
-            versions=config.tags.keys(),
             github_url=github_url,
-            container_url=config.url,
             config_url=raw_github_url,
             creation_date=datetime.now(),
-            name=module_name,
-            latest=config.latest.name,
-            flatname=module_name.replace(os.sep, "-"),
-            config=json.dumps(config.entry._config),
+            config_json=json.dumps(config.entry._config),
         )
         out.write(result)
         return out
@@ -338,50 +334,48 @@ def check(self, module_name):
         at updates for entire tags. If a specific folder is provided with
         a container, check the digest.
         """
-        module = self.new_module(module_name)
+        module = self.get_module(module_name)
         if not os.path.exists(module.module_dir):
             logger.exit(
                 "%s does not exist. Is this a known registry entry?" % module.module_dir
             )
 
         return module.check()
 
-    def new_module(
-        self, name, tag=None, tag_exists=True, container_image=None, keep_path=False
-    ):
+    def new_module(self, name):
         """
-        Create a new module
+        Create a new Module just from a name, which doesn't have to exist in the registry.
+        The name may have a tag appended with a colon.
         """
         name = self.add_namespace(name)
 
-        # If the module has a version, overrides provided tag
-        if ":" in name:
-            name, tag = name.split(":", 1)
-
         module = Module(name)
-        module.config = self._load_container(module.name, tag)
-
-        # Ensure the tag exists, if required, uses config.tag
-        if tag_exists:
-            module.validate_tag_exists()
 
         # Pass on container and settings
         module.container = self.container
         module.settings = self.settings
 
+        return module
+
+    def get_module(self, name, container_image=None, keep_path=False):
+        """
+        Create a new Module from an existing registry entry, given its name.
+        The name may have a tag appended with a colon.
+        """
+        module = self.new_module(name)
+
+        config = self._load_container(module.name)
+        # Ensure the tag exists, if required, uses config.tag
+        module.load_config(config, module.name)
+
         # Do we want to use a container from the local filesystem?
         if container_image:
             module.add_local_container(container_image, keep_path=keep_path)
+
         return module
 
     def install(
-        self,
-        name,
-        tag=None,
-        force=False,
-        container_image=None,
-        keep_path=False,
-        **kwargs
+        self, name, force=False, container_image=None, keep_path=False, **kwargs
     ):
         """
         Given a unique resource identifier, install a recipe.
@@ -392,12 +386,8 @@ def install(
         "force" is currently not used.
         """
         # Create a new module
-        module = self.new_module(
-            name,
-            tag=tag,
-            tag_exists=True,
-            container_image=container_image,
-            keep_path=keep_path,
+        module = self.get_module(
+            name, container_image=container_image, keep_path=keep_path
         )
 
         # We always load overrides for an install
@@ -431,16 +421,12 @@ def install(
         logger.info("Module %s was created." % module.tagged_name)
         return module.container_path
 
-    def view_install(
-        self, view_name, name, tag=None, force=False, container_image=None
-    ):
+    def view_install(self, view_name, name, force=False, container_image=None):
         """
         Install a module in a view. The module must already be installed.
         Set "force" to True to allow overwriting existing symlinks.
         """
-        module = self.new_module(
-            name, tag=tag, tag_exists=True, container_image=container_image
-        )
+        module = self.get_module(name, container_image=container_image)
 
         # A view is a symlink under views_base/$view/$module
         if view_name not in self.views:

shpc/main/modules/module.py

@@ -41,15 +41,24 @@ def add_environment(self):
             environment_file=self.settings.environment_file,
         )
 
-    def validate_tag_exists(self):
+    def load_config(self, config, name):
         """
-        Ensure that a provided module name (and tag) exists.
+        Load a ContainerConfig into this Module
         """
-        if not self.config.tag:
+        # Ensure that the tag exists
+        if not config.tag:
             logger.exit(
-                "%s is not a known identifier. Choices are:\n%s"
-                % (self.name, "\n".join(self.config.tags.keys()))
+                "%s is not a known identifier. Valid tags are:\n%s"
+                % (name, "\n".join(config.tags.keys()))
             )
+        # We currently support gh, docker, path, or oras
+        uri = config.get_uri()
+        # If we have a path, the URI comes from the name
+        if ".sif" in uri:
+            uri = str(config.name).split(":", 1)[0]
+        self.name = uri + ":" + config.tag.name
+        self._uri = uri
+        self.config = config
 
     def load_override_file(self):
         self.config.load_override_file(self.tag.name)
@@ -154,17 +163,7 @@ def uri(self):
         """
         Get the uri for the module, docker / path / oras / gh
         """
-        if self._uri:
-            return self._uri
-
-        # We currently support gh, docker, path, or oras
-        uri = self.config.get_uri()
-
-        # If we have a path, the URI comes from the name
-        if ".sif" in uri:
-            uri = self.name.split(":", 1)[0]
-        self._uri = uri
-        return uri
+        return self._uri
 
     @property
     def module_dir(self):
@@ -178,4 +177,4 @@ def module_basepath(self):
         """
         Path of only the module name and tag.
         """
-        return os.path.join(self.uri, self.tag.name)
+        return self.name.replace(":", os.sep)

shpc/main/modules/templates/docs.md

@@ -1,33 +1,33 @@
 ---
 layout: container
-name:  "{{ name }}"
+name:  "{{ config.name }}"
 maintainer: "@vsoch"
 github: "{{ github_url }}"
 config_url: "{{ config_url }}"
 updated_at: "{{ creation_date }}"
-latest: "{{ latest }}"
-container_url: "{{ container_url }}"
+latest: "{{ config.latest.name }}"
+container_url: "{{ config.url }}"
 {% if aliases %}aliases:{% for alias in aliases %}
  - "{{ alias.name }}"{% endfor %}{% endif %}
-versions:{% for version in versions %}
+versions:{% for version in config.tags.keys() %}
  - "{{ version }}"{% endfor %}
-{% if description %}description: "{{ description }}"{% endif %}
-config: {{ config }}
+{% if config.description %}description: "{{ config.description }}"{% endif %}
+config: {{ config_json }}
 ---
 
-This module is a singularity container wrapper for {{ name }}.
-{% if description %}{{ description }}{% endif %}
+This module is a singularity container wrapper for {{ config.name }}.
+{% if config.description %}{{ config.description }}{% endif %}
 After [installing shpc](#install) you will want to install this container module:
 
 
 ```bash
-$ shpc install {{ name }}
+$ shpc install {{ config.name }}
 ```
 
 Or a specific version:
 
 ```bash
-$ shpc install {{ name }}:{{ versions.0 }}
+$ shpc install {{ config.name }}:{{ config.tag.name }}
 ```
 
 And then you can tell lmod about your modules folder:
@@ -39,8 +39,8 @@ $ module use ./modules
 And load the module, and ask for help, or similar.
 
 ```bash
-$ module load {{ name }}/{{ versions.0 }}
-$ module help {{ name }}/{{ versions.0 }}
+$ module load {{ config.name }}/{{ config.tag.name }}
+$ module help {{ config.name }}/{{ config.tag.name }}
 ```
 
 You can use tab for auto-completion of module names or commands that are provided.

shpc/tests/test_client.py

@@ -70,24 +70,37 @@ def test_features(tmp_path, module_sys, module_file, remote):
     """
     client = init_client(str(tmp_path), module_sys, "singularity", remote=remote)
 
+    module_file_392 = os.path.join(
+        client.settings.module_base, "python", "3.9.2-alpine", module_file
+    )
+    module_file_394 = os.path.join(
+        client.settings.module_base, "python", "3.9.4-alpine", module_file
+    )
+
     # Install known tag
     client.install("python:3.9.2-alpine")
 
-    module_dir = os.path.join(client.settings.module_base, "python", "3.9.2-alpine")
-    module_file = os.path.join(module_dir, module_file)
-
     # Should not have nvidia flag
-    content = shpc.utils.read_file(module_file)
+    content = shpc.utils.read_file(module_file_392)
     assert "--nv" not in content
 
+    client.install("python:3.9.4-alpine")
+    assert os.path.exists(module_file_392)
+    assert os.path.exists(module_file_394)
+
     client.uninstall("python:3.9.2-alpine", force=True)
+    assert not os.path.exists(module_file_392)
+    assert os.path.exists(module_file_394)
+
+    client.uninstall("python", force=True)
+    assert not os.path.exists(module_file_394)
 
     # Now update settings
     client.settings.set("container_features", "gpu:nvidia")
 
     # Install known tag, add extra feature of gpu
     client.install("python:3.9.2-alpine", features=["gpu"])
-    content = shpc.utils.read_file(module_file)
+    content = shpc.utils.read_file(module_file_392)
     assert "--nv" in content
 
 

shpc/version.py

@@ -2,7 +2,7 @@
 __copyright__ = "Copyright 2021-2022, Vanessa Sochat"
 __license__ = "MPL 2.0"
 
-__version__ = "0.1.15"
+__version__ = "0.1.16"
 AUTHOR = "Vanessa Sochat"
 EMAIL = "[email protected]"
 NAME = "singularity-hpc"