From df5589ec75a101bd71fcf37b9d1fa76930d0196d Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 9 Oct 2022 14:25:25 +0100 Subject: [PATCH 01/11] Removed the `--view` option of `shpc install` as per https://github.com/singularityhub/singularity-hpc/pull/590#issuecomment-1272536735 --- .github/workflows/test.yml | 7 ++++--- shpc/client/__init__.py | 6 ------ shpc/client/install.py | 9 +-------- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d8a6db1db..424cfae06 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -109,13 +109,14 @@ jobs: cat test_output grep --quiet 'Python 3.9.5' test_output rm test_output - shpc uninstall --force python:3.9.5-alpine - # Try creating views install mkdir -p tmp-modules shpc config set views_base:tmp-modules shpc view create noodles - shpc install --view noodles python:3.9.5-alpine + shpc view install noodles python:3.9.5-alpine + + shpc uninstall --force python:3.9.5-alpine + shpc view --force delete noodles - name: Run python module tests (tcsh) shell: tcsh -e {0} diff --git a/shpc/client/__init__.py b/shpc/client/__init__.py index d0e71e021..18eac8e32 100644 --- a/shpc/client/__init__.py +++ b/shpc/client/__init__.py @@ -101,12 +101,6 @@ def get_parser(): "install_recipe", help="recipe to install\nshpc install python\nshpc install python:3.9.5-alpine", ) - install.add_argument( - "--view", - dest="view", - help="install module to a named view (must be installed to shpc first).", - default=None, - ) install.add_argument( "--no-view", dest="no_view", diff --git a/shpc/client/install.py b/shpc/client/install.py index c67572893..9e6e08ffe 100644 --- a/shpc/client/install.py +++ b/shpc/client/install.py @@ -3,7 +3,6 @@ __license__ = "MPL 2.0" import shpc.utils -from shpc.logger import logger def main(args, parser, extra, subparser): @@ -25,11 +24,5 @@ def main(args, parser, extra, subparser): # Update config settings on the fly cli.settings.update_params(args.config_params) - # It doesn't make sense to define view and no view - if args.view and args.no_view: - logger.exit("Conflicting arguments --view and --no-view, choose one.") - # And do the install - cli.install( - args.install_recipe, view=args.view, disable_view=args.no_view, force=args.force - ) + cli.install(args.install_recipe, disable_view=args.no_view, force=args.force) From 5ba70a2a54e951f75b72786f5a22aeffe811811f Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 9 Oct 2022 14:29:06 +0100 Subject: [PATCH 02/11] bugfix: always install in the default view, alongside additionally requested views Renamed the variables to make it clear what is a view and what is a view name. --- shpc/client/install.py | 4 +++- shpc/client/view.py | 14 ++++++++++++-- shpc/main/modules/base.py | 29 ++++++++++++++++++++--------- shpc/tests/test_views.py | 2 +- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/shpc/client/install.py b/shpc/client/install.py index 9e6e08ffe..11cc949d7 100644 --- a/shpc/client/install.py +++ b/shpc/client/install.py @@ -25,4 +25,6 @@ def main(args, parser, extra, subparser): cli.settings.update_params(args.config_params) # And do the install - cli.install(args.install_recipe, disable_view=args.no_view, force=args.force) + cli.install( + args.install_recipe, disable_default_view=args.no_view, force=args.force + ) diff --git a/shpc/client/view.py b/shpc/client/view.py index d8d6be779..549da500e 100644 --- a/shpc/client/view.py +++ b/shpc/client/view.py @@ -52,7 +52,12 @@ def create_from_file( # Extra modules to install for install_module in install_modules: - cli.install(install_module, view=view_name, disable_view=False, force=force) + cli.install( + install_module, + extra_view=view_name, + disable_default_view=False, + force=force, + ) def main(args, parser, extra, subparser): @@ -168,7 +173,12 @@ def main(args, parser, extra, subparser): # We don't make it hard to require them to install to the root first module_name = args.params.pop(0) if command == "install": - cli.install(module_name, view=view_name, disable_view=False, force=args.force) + cli.install( + module_name, + extra_view=view_name, + disable_default_view=False, + force=args.force, + ) if command == "uninstall": cli.uninstall(module_name, view=view_name, force=args.force) diff --git a/shpc/main/modules/base.py b/shpc/main/modules/base.py index fc5f921c3..9cd77ae5c 100644 --- a/shpc/main/modules/base.py +++ b/shpc/main/modules/base.py @@ -348,7 +348,13 @@ def check(self, module_name): return self.container.check(module_name, config) def install( - self, name, tag=None, view=None, disable_view=False, force=False, **kwargs + self, + name, + tag=None, + extra_view=None, + disable_default_view=False, + force=False, + **kwargs ): """ Given a unique resource identifier, install a recipe. @@ -387,22 +393,27 @@ def install( container_dir = self.container.container_dir(subfolder) # Are we also installing to a named view? - if view is None and not disable_view: - view = self.settings.default_view + view_names = [] + if self.settings.default_view and not disable_default_view: + view_names.append(self.settings.default_view) + if extra_view: + view_names.append(extra_view) # We only want to load over-rides for a tag at install time config.load_override_file(tag.name) # A view is a symlink under views_base/$view/$module - if view: - if view not in self.views: + for view_name in view_names: + if view_name not in self.views: logger.exit( - "View %s does not exist, shpc view create %s." % (view, view) + "View %s does not exist, shpc view create %s." + % (view_name, view_name) ) - # Update view from name to be View to interact with - view = self.views[view] + # Update view from name to be View to interact with + views = [self.views[view_name] for view_name in view_names] + for view in views: # Don't continue if it exists, unless force is True view.confirm_install(module_dir, force=force) @@ -476,7 +487,7 @@ def install( logger.info("Module %s was created." % name) # Install the module (symlink) to the view and create version file - if view: + for view in views: view.install(module_dir) return container_path diff --git a/shpc/tests/test_views.py b/shpc/tests/test_views.py index 002086d37..f29267af0 100644 --- a/shpc/tests/test_views.py +++ b/shpc/tests/test_views.py @@ -76,7 +76,7 @@ def test_views(tmp_path, module_sys, module_file, container_tech, remote): assert not view._config["view"]["system_modules"] # Now install to it via the client - client.install("ghcr.io/autamus/emacs:27.2", view=view_name) + client.install("ghcr.io/autamus/emacs:27.2", extra_view=view_name) # Ensure it was created and as a symlink assert view._config["view"]["modules"] From 7e93b13d91f0fa1a6010ba9a4f5a7980f4781395 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Mon, 10 Oct 2022 02:17:17 +0100 Subject: [PATCH 03/11] Print what bash is running --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 424cfae06..2dbfcd554 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -98,6 +98,7 @@ jobs: printf "\n\nmodule help ============================================\n" module help python/3.9.5-alpine + set -x python-exec echo donuts >test_output cat test_output grep --quiet donuts test_output From 7677670710e3034c89b45e7fb8225af2f93d1c19 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Mon, 10 Oct 2022 01:57:26 +0100 Subject: [PATCH 04/11] Test views on tcsh too --- .github/workflows/test.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2dbfcd554..92eea3834 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -166,4 +166,11 @@ jobs: cat test_output grep --quiet 'Python 3.9.5' test_output rm test_output + + mkdir -p tmp-modules + shpc config set views_base:tmp-modules + shpc view create noodles + shpc view install noodles python:3.9.5-alpine + shpc uninstall --force python:3.9.5-alpine + shpc view --force delete noodles From 05697dfb81c0f47ee0f76bfdd203ce38a1fefa43 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Mon, 10 Oct 2022 04:02:36 +0100 Subject: [PATCH 05/11] Moved the code to install a module into a view to a separate function --- shpc/client/install.py | 8 ++-- shpc/client/view.py | 16 ++------ shpc/main/modules/base.py | 79 ++++++++++++++++++++++----------------- shpc/tests/test_views.py | 3 +- 4 files changed, 55 insertions(+), 51 deletions(-) diff --git a/shpc/client/install.py b/shpc/client/install.py index 11cc949d7..f0a30d680 100644 --- a/shpc/client/install.py +++ b/shpc/client/install.py @@ -25,6 +25,8 @@ def main(args, parser, extra, subparser): cli.settings.update_params(args.config_params) # And do the install - cli.install( - args.install_recipe, disable_default_view=args.no_view, force=args.force - ) + cli.install(args.install_recipe, force=args.force) + if cli.settings.default_view and not args.no_view: + cli.view_install( + cli.settings.default_view, args.install_recipe, force=args.force + ) diff --git a/shpc/client/view.py b/shpc/client/view.py index 549da500e..bf6757ee7 100644 --- a/shpc/client/view.py +++ b/shpc/client/view.py @@ -52,12 +52,8 @@ def create_from_file( # Extra modules to install for install_module in install_modules: - cli.install( - install_module, - extra_view=view_name, - disable_default_view=False, - force=force, - ) + cli.install(install_module, force=force) + cli.view_install(view_name, install_module, force=force) def main(args, parser, extra, subparser): @@ -173,12 +169,8 @@ def main(args, parser, extra, subparser): # We don't make it hard to require them to install to the root first module_name = args.params.pop(0) if command == "install": - cli.install( - module_name, - extra_view=view_name, - disable_default_view=False, - force=args.force, - ) + cli.install(module_name, force=args.force) + cli.view_install(view_name, module_name, force=args.force) if command == "uninstall": cli.uninstall(module_name, view=view_name, force=args.force) diff --git a/shpc/main/modules/base.py b/shpc/main/modules/base.py index 9cd77ae5c..0bd14e6cd 100644 --- a/shpc/main/modules/base.py +++ b/shpc/main/modules/base.py @@ -347,15 +347,7 @@ def check(self, module_name): config = self._load_container(module_name.rsplit(":", 1)[0]) return self.container.check(module_name, config) - def install( - self, - name, - tag=None, - extra_view=None, - disable_default_view=False, - force=False, - **kwargs - ): + def install(self, name, tag=None, force=False, **kwargs): """ Given a unique resource identifier, install a recipe. @@ -392,31 +384,9 @@ def install( subfolder = os.path.join(uri, tag.name) container_dir = self.container.container_dir(subfolder) - # Are we also installing to a named view? - view_names = [] - if self.settings.default_view and not disable_default_view: - view_names.append(self.settings.default_view) - if extra_view: - view_names.append(extra_view) - # We only want to load over-rides for a tag at install time config.load_override_file(tag.name) - # A view is a symlink under views_base/$view/$module - for view_name in view_names: - if view_name not in self.views: - logger.exit( - "View %s does not exist, shpc view create %s." - % (view_name, view_name) - ) - - # Update view from name to be View to interact with - views = [self.views[view_name] for view_name in view_names] - - for view in views: - # Don't continue if it exists, unless force is True - view.confirm_install(module_dir, force=force) - # Create the module and container directory utils.mkdirp([module_dir, container_dir]) @@ -486,8 +456,47 @@ def install( name = "%s:%s" % (name, tag.name) logger.info("Module %s was created." % name) - # Install the module (symlink) to the view and create version file - for view in views: - view.install(module_dir) - return container_path + + def view_install(self, view_name, name, tag=None, force=False): + """ + Install a module in a view. + The module must already be installed. + """ + name = self.add_namespace(name) + + # If the module has a version, overrides provided tag + if ":" in name: + name, tag = name.split(":", 1) + config = self._load_container(name, tag) + + # The chosen tag is set for the config (or defaults to latest) + if not config.tag: + logger.exit( + "%s is not a known identifier. Choices 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 = name.split(":", 1)[0] + + # This is a tag object with name and digest + tag = config.tag + module_dir = os.path.join(self.settings.module_base, uri, tag.name) + + # A view is a symlink under views_base/$view/$module + if view_name not in self.views: + logger.exit( + "View %s does not exist, shpc view create %s." % (view_name, view_name) + ) + + # Update view from name to be View to interact with + view = self.views[view_name] + + # Don't continue if it exists, unless force is True + view.confirm_install(module_dir, force=force) + view.install(module_dir) diff --git a/shpc/tests/test_views.py b/shpc/tests/test_views.py index f29267af0..cbce599c4 100644 --- a/shpc/tests/test_views.py +++ b/shpc/tests/test_views.py @@ -76,7 +76,8 @@ def test_views(tmp_path, module_sys, module_file, container_tech, remote): assert not view._config["view"]["system_modules"] # Now install to it via the client - client.install("ghcr.io/autamus/emacs:27.2", extra_view=view_name) + client.install("ghcr.io/autamus/emacs:27.2") + client.view_install(view_name, "ghcr.io/autamus/emacs:27.2") # Ensure it was created and as a symlink assert view._config["view"]["modules"] From 048f677e7bd2d24b4d9f49b8b668cbff618aca52 Mon Sep 17 00:00:00 2001 From: vsoch Date: Mon, 10 Oct 2022 05:47:32 -0600 Subject: [PATCH 06/11] addition of intermediate module class This will help to carry around some common data attributes and functions, and reduce redundancy within the ModuleBase class. It also allows for providing fewer variables to the rendered templates since they can come from the module. Signed-off-by: vsoch --- shpc/client/view.py | 2 + shpc/main/__init__.py | 8 +- shpc/main/container/docker.py | 42 ++-- shpc/main/container/singularity.py | 40 +--- shpc/main/modules/base.py | 200 ++++++-------------- shpc/main/modules/module.py | 142 ++++++++++++++ shpc/main/modules/templates/docker.lua | 16 +- shpc/main/modules/templates/docker.tcl | 28 +-- shpc/main/modules/templates/singularity.lua | 16 +- shpc/main/modules/templates/singularity.tcl | 28 +-- 10 files changed, 275 insertions(+), 247 deletions(-) create mode 100644 shpc/main/modules/module.py diff --git a/shpc/client/view.py b/shpc/client/view.py index bf6757ee7..eddaa600e 100644 --- a/shpc/client/view.py +++ b/shpc/client/view.py @@ -52,6 +52,8 @@ def create_from_file( # Extra modules to install for install_module in install_modules: + + # TODO: can we cut out early if already installed? cli.install(install_module, force=force) cli.view_install(view_name, install_module, force=force) diff --git a/shpc/main/__init__.py b/shpc/main/__init__.py index 73de4fff8..6175bd02b 100644 --- a/shpc/main/__init__.py +++ b/shpc/main/__init__.py @@ -41,17 +41,17 @@ def get_client(quiet=False, **kwargs): # Add the container operator if container == "singularity": - from .container import SingularityContainer + from shpc.main.container import SingularityContainer Client.container = SingularityContainer() elif container == "podman": - from .container import PodmanContainer + from shpc.main.container import PodmanContainer Client.container = PodmanContainer() elif container == "docker": - from .container import DockerContainer + from shpc.main.container import DockerContainer Client.container = DockerContainer() @@ -64,6 +64,8 @@ def get_client(quiet=False, **kwargs): logger.warning( "%s is not installed, functionality might be limited." % container.upper() ) + + # Pass on settings and container to module too Client.quiet = quiet Client.settings = settings return Client() diff --git a/shpc/main/container/docker.py b/shpc/main/container/docker.py index 7ead987ee..6783a3908 100644 --- a/shpc/main/container/docker.py +++ b/shpc/main/container/docker.py @@ -185,21 +185,7 @@ def test_script(self, image, test_script): # Return code return result["return_code"] - def install( - self, - module_path, - container_path, - name, - template, - parsed_name, - aliases=None, - url=None, - description=None, - version=None, - config_features=None, - features=None, - config=None, - ): + def install(self, module_path, template, module, features=None): """Install a general container path to a module The module_dir should be created by the calling function, and @@ -210,50 +196,44 @@ def install( # Container features are defined in container.yaml and the settings # and specific values are determined by the container technology features = self.get_features( - config_features, self.settings.container_features, features + module.config.features, self.settings.container_features, features ) # Ensure that the container exists # Do we want to clean up other versions here too? - manifest = self.inspect(container_path) + manifest = self.inspect(module.container_path) if not manifest: - sys.exit("Container %s was not found. Was it pulled?" % container_path) + sys.exit( + "Container %s was not found. Was it pulled?" % module.container_path + ) labels = manifest[0].get("Labels", {}) - # If there's a tag in the name, don't use it - name = name.split(":", 1)[0] - # Option to create wrapper scripts for commands - module_dir = os.path.dirname(module_path) + aliases = module.config.get_aliases() wrapper_scripts = [] # Wrapper scripts can be global (for aliases) or container specific if self.settings.wrapper_scripts["enabled"] is True: wrapper_scripts = shpc.main.wrappers.generate( aliases=aliases, - module_dir=module_dir, + module_dir=module.module_dir, features=features, container=self, - image=container_path, - config=config, + image=module.container_path, + config=module.config, ) # Make sure to render all values! out = template.render( settings=self.settings, shell=self.shell_path, - image=container_path, - description=description, aliases=aliases, - url=url, features=features, - version=version, labels=labels, creation_date=datetime.now(), - name=name, - parsed_name=parsed_name, command=self.command, + module=module, wrapper_scripts=wrapper_scripts, ) shpc.utils.write_file(module_path, out) diff --git a/shpc/main/container/singularity.py b/shpc/main/container/singularity.py index 4048df64d..ce086f93f 100644 --- a/shpc/main/container/singularity.py +++ b/shpc/main/container/singularity.py @@ -165,22 +165,7 @@ def _add_docker_image(self, name, tag, image, config, container_yaml, **kwargs): config.add_tag(tag, tags[tag]) return config - def install( - self, - module_path, - container_path, - name, - template, - parsed_name, - aliases=None, - template_name=None, - url=None, - description=None, - config_features=None, - features=None, - version=None, - config=None, - ): + def install(self, module_path, template, module, features=None): """Install a general container path to a module The module_dir should be created by the calling function, and @@ -191,19 +176,19 @@ def install( # Container features are defined in container.yaml and the settings # and specific values are determined by the container technology features = self.get_features( - config_features, self.settings.container_features, features + module.config.features, self.settings.container_features, features ) # Remove any previous containers - container_dir = os.path.dirname(container_path) + container_dir = os.path.dirname(module.container_path) for older in glob("%s%s*.sif" % (container_dir, os.sep)): - if older == container_path: + if older == module.container_path: continue os.remove(older) # Get inspect metadata from the container (only if singularity installed try: - metadata = self.inspect(container_path) + metadata = self.inspect(module.container_path) # Add labels, and deffile labels = metadata.get("attributes", {}).get("labels") @@ -216,34 +201,29 @@ def install( labels = {} # Option to create wrapper scripts for commands - module_dir = os.path.dirname(module_path) + aliases = module.config.get_aliases() # Wrapper scripts can be global (for aliases) or container specific wrapper_scripts = [] if self.settings.wrapper_scripts["enabled"] is True: wrapper_scripts = shpc.main.wrappers.generate( aliases=aliases, - module_dir=module_dir, + module_dir=module.module_dir, features=features, container=self, - image=container_path, - config=config, + image=module.container_path, + config=module.config, ) # Make sure to render all values! out = template.render( settings=self.settings, - container_sif=container_path, - description=description, aliases=aliases, - url=url, features=features, - version=version, labels=labels, deffile=deffile, creation_date=datetime.now(), - name=name, - parsed_name=parsed_name, + module=module, wrapper_scripts=wrapper_scripts, ) utils.write_file(module_path, out) diff --git a/shpc/main/modules/base.py b/shpc/main/modules/base.py index 0bd14e6cd..995515352 100644 --- a/shpc/main/modules/base.py +++ b/shpc/main/modules/base.py @@ -5,7 +5,6 @@ import inspect import json import os -import shutil import subprocess import sys from datetime import datetime @@ -19,6 +18,7 @@ import shpc.utils as utils from shpc.logger import logger from shpc.main.client import Client as BaseClient +from .module import Module class ModuleBase(BaseClient): @@ -72,13 +72,7 @@ def uninstall(self, name, view=None, force=False): Given a unique resource identifier, uninstall a module. If a view name is provided, assume we only want to uninstall from the view """ - # The name can either be a folder or an install directory - name = self.add_namespace(name) - - # folder paths do not have : - name = name.replace(":", os.sep) - module_dir = os.path.join(self.settings.module_base, name) - container_dir = self.container.container_dir(name) + module = self.new_module(name) # We need to look for the module in all views and show to the user first views_with_module = set() @@ -88,7 +82,7 @@ def uninstall(self, name, view=None, force=False): # If uninstalling the entire module, clean up symbolic links in all views for view_name, entry in self.views.items(): - if entry.exists(module_dir): + if entry.exists(module.module_dir): views_with_module.add(view_name) # Ask before deleting anything! @@ -106,29 +100,35 @@ def uninstall(self, name, view=None, force=False): if view: if view not in self.views: logger.exit("View %s does not exist, cannot uninstall." % view) - return self.views[view].uninstall(module_dir) + return self.views[view].uninstall(module.module_dir) # Podman needs image deletion - self.container.delete(name) + self.container.delete(module.name) - if container_dir != module_dir: + if module.container_dir != module.module_dir: self._uninstall( - container_dir, self.container_base, "$container_base/%s" % name + module.container_dir, + self.container_base, + "$container_base/%s" % module.name, ) self._uninstall( - module_dir, self.settings.module_base, "$module_base/%s" % name + module.module_dir, + self.settings.module_base, + "$module_base/%s" % module.name, ) else: self._uninstall( - module_dir, self.settings.module_base, "$module_base/%s" % name + module.module_dir, + self.settings.module_base, + "$module_base/%s" % module.name, ) # If uninstalling the entire module, clean up symbolic links in all views for view_name, view in self.views.items(): - view.uninstall(module_dir) + view.uninstall(module.module_dir) # parent of versioned directory has module .version - module_dir = os.path.dirname(module_dir) + module_dir = os.path.dirname(module.module_dir) # update the default version file, if other versions still present if os.path.exists(module_dir): @@ -331,162 +331,84 @@ def check(self, module_name): at updates for entire tags. If a specific folder is provided with a container, check the digest. """ - module_name = self.add_namespace(module_name) - - # If a tag is provided, convert to directory - module_dir = module_name.replace(":", os.sep) - - # We derive the current version installed from the container - # We assume the user has provided the correct prefix - module_dir = os.path.join(self.settings.module_base, module_dir) - if not os.path.exists(module_dir): + module = self.new_module(module_name) + if not os.path.exists(module.module_dir): logger.exit( - "%s does not exist. Is this a known registry entry?" % module_dir + "%s does not exist. Is this a known registry entry?" % module.module_dir ) - config = self._load_container(module_name.rsplit(":", 1)[0]) - return self.container.check(module_name, config) + return module.check() - def install(self, name, tag=None, force=False, **kwargs): + def new_module(self, name, tag=None, tag_exists=True): """ - Given a unique resource identifier, install a recipe. - - For lmod, this means creating a subfolder in modules, pulling the - container to it, and writing a module file there. We've already - grabbed the name from docker (which is currently the only supported). + Create a new module """ name = self.add_namespace(name) # If the module has a version, overrides provided tag if ":" in name: name, tag = name.split(":", 1) - config = self._load_container(name, tag) - # The chosen tag is set for the config (or defaults to latest) - if not config.tag: - logger.exit( - "%s is not a known identifier. Choices are:\n%s" - % (name, "\n".join(config.tags.keys())) - ) + module = Module(name) + module.config = self._load_container(module.name, tag) - # We currently support gh, docker, path, or oras - uri = config.get_uri() + # Ensure the tag exists, if required, uses config.tag + if tag_exists: + module.validate_tag_exists() - # If we have a path, the URI comes from the name - if ".sif" in uri: - uri = name.split(":", 1)[0] + # Pass on container and settings + module.container = self.container + module.settings = self.settings + return module - # This is a tag object with name and digest - tag = config.tag + def install(self, name, tag=None, force=False, **kwargs): + """ + Given a unique resource identifier, install a recipe. - # Pull the container to the module directory OR container base - module_dir = os.path.join(self.settings.module_base, uri, tag.name) - subfolder = os.path.join(uri, tag.name) - container_dir = self.container.container_dir(subfolder) + For lmod, this means creating a subfolder in modules, pulling the + container to it, and writing a module file there. We've already + grabbed the name from docker (which is currently the only supported). + """ + # Create a new module + module = self.new_module(name, tag=tag, tag_exists=True) - # We only want to load over-rides for a tag at install time - config.load_override_file(tag.name) + # We always load overrides for an install + module.load_override_file() # Create the module and container directory - utils.mkdirp([module_dir, container_dir]) - - # If we have a sif URI provided by path, the container needs to exist - container_path = None - if config.path: - container_path = os.path.join(config.entry.dirname, config.path) - if not os.path.exists(container_path): - logger.exit( - "Expected container defined by path %s not found in %s." - % (config.path, config.entry.dirname) - ) - container_dest = os.path.join(container_dir, config.path) - - # Note that here we are *duplicating* the container, assuming we - # cannot use a link, and the registry won't be deleted but the - # module container might! - if not os.path.exists(container_dest): - shutil.copyfile(container_path, container_dest) - container_path = container_dest + utils.mkdirp([module.module_dir, module.container_dir]) # Add a .version file to indicate the level of versioning - self.versionfile.write(os.path.join(self.settings.module_base, uri), tag.name) - - # For Singularity this is a path, podman is a uri. If None is returned - # there was an error and we cleanup - if not container_path: - container_path = self.container.registry_pull( - module_dir, container_dir, config, tag - ) - if not container_path: - utils.remove_to_base(container_dir, self.container_base) - logger.exit("There was an issue pulling %s" % container_path) + self.versionfile.write( + os.path.join(self.settings.module_base, module.uri), module.tag.name + ) + if not module.container_path: + utils.remove_to_base(module.container_dir, self.container_base) + logger.exit("There was an issue pulling the container for %s" % module.name) # Get the template based on the module and container type template = self.template.load(self.templatefile) - module_path = os.path.join(module_dir, self.modulefile) + module_path = os.path.join(module.module_dir, self.modulefile) # Install the container - self.container.install( - module_path, - container_path, - name, - template, - aliases=config.get_aliases(), - config=config, - parsed_name=config.name, - url=config.url, - description=config.description, - version=tag.name, - config_features=config.features, - features=kwargs.get("features"), - ) + # This could be simplified to take the module + self.container.install(module_path, template, module, kwargs.get("features")) # If the container tech does not need storage, clean up - if not os.listdir(container_dir): - utils.remove_to_base(container_dir, self.container_base) + if not os.listdir(module.container_dir): + utils.remove_to_base(module.container_dir, self.container_base) # Write the environment file to be bound to the container - self.container.add_environment( - module_dir, - envars=config.get_envars(), - environment_file=self.settings.environment_file, - ) - - if ":" not in name: - name = "%s:%s" % (name, tag.name) - logger.info("Module %s was created." % name) - - return container_path + module.add_environment() + logger.info("Module %s was created." % module.tagged_name) + return module.container_path def view_install(self, view_name, name, tag=None, force=False): """ Install a module in a view. The module must already be installed. """ - name = self.add_namespace(name) - - # If the module has a version, overrides provided tag - if ":" in name: - name, tag = name.split(":", 1) - config = self._load_container(name, tag) - - # The chosen tag is set for the config (or defaults to latest) - if not config.tag: - logger.exit( - "%s is not a known identifier. Choices 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 = name.split(":", 1)[0] - - # This is a tag object with name and digest - tag = config.tag - module_dir = os.path.join(self.settings.module_base, uri, tag.name) + module = self.new_module(name, tag=tag, tag_exists=True) # A view is a symlink under views_base/$view/$module if view_name not in self.views: @@ -498,5 +420,5 @@ def view_install(self, view_name, name, tag=None, force=False): view = self.views[view_name] # Don't continue if it exists, unless force is True - view.confirm_install(module_dir, force=force) - view.install(module_dir) + view.confirm_install(module.module_dir, force=force) + view.install(module.module_dir) diff --git a/shpc/main/modules/module.py b/shpc/main/modules/module.py new file mode 100644 index 000000000..fc777b4a0 --- /dev/null +++ b/shpc/main/modules/module.py @@ -0,0 +1,142 @@ +__author__ = "Vanessa Sochat" +__copyright__ = "Copyright 2022, Vanessa Sochat" +__license__ = "MPL 2.0" + +import os +import shutil +from shpc.logger import logger + + +class Module: + def __init__(self, name): + """ + New module metadata and shared functions. + + This should be created by base.py new_module to ensure the same + container and settings are carried forward here. + """ + self.name = name + + # Cache variable properties + self._uri = None + self._container_dir = None + self._container_path = None + + @property + def tagged_name(self): + name = self.name + if ":" not in name: + name = "%s:%s" % (name, self.tag.name) + return name + + def add_environment(self): + """ + Write the environment to the module directory. + """ + self.container.add_environment( + self.module_dir, + envars=self.config.get_envars(), + environment_file=self.settings.environment_file, + ) + + def validate_tag_exists(self): + """ + Ensure that a provided module name (and tag) exists. + """ + if not self.config.tag: + logger.exit( + "%s is not a known identifier. Choices are:\n%s" + % (self.name, "\n".join(self.config.tags.keys())) + ) + + def load_override_file(self): + self.config.load_override_file(self.tag.name) + + @property + def container_dir(self): + """ + Derive the module container directory. + """ + if not self._container_dir: + # Pull the container to the module directory OR container base + self._container_dir = self.container.container_dir(self.module_basepath) + return self._container_dir + + @property + def container_path(self): + """ + Derive the container path, if possible. + """ + if self._container_path: + return self._container_path + + # If we have a sif URI provided by path, the container needs to exist + if self.config.path: + self._container_path = os.path.join( + self.config.entry.dirname, self.config.path + ) + if not os.path.exists(self._container_path): + logger.exit( + "Expected container defined by path %s not found in %s." + % (self.config.path, self.config.entry.dirname) + ) + container_dest = os.path.join(self.container_dir, self.config.path) + + # Note that here we are *duplicating* the container, assuming we + # cannot use a link, and the registry won't be deleted but the + # module container might! + if not os.path.exists(container_dest): + shutil.copyfile(self._container_path, container_dest) + self._container_path = container_dest + + # For Singularity this is a path, podman is a uri. If None is returned + # there was an error and we cleanup + if not self._container_path: + self._container_path = self.container.registry_pull( + self.module_dir, self.container_dir, self.config, self.tag + ) + return self._container_path + + @property + def tag(self): + """ + Pass forward the tag defined in the config. + """ + return self.config.tag + + def check(self): + """ + Check to see if the module version installed is up to date. + """ + return self.container.check(self.name, self.config) + + @property + 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 + + @property + def module_dir(self): + """ + Full path to the module directory. + """ + return os.path.join(self.settings.module_base, self.module_basepath) + + @property + def module_basepath(self): + """ + Path of only the module name and tag. + """ + return os.path.join(self.uri, self.tag.name) diff --git a/shpc/main/modules/templates/docker.lua b/shpc/main/modules/templates/docker.lua index 435a6a0eb..d72a3ddf5 100644 --- a/shpc/main/modules/templates/docker.lua +++ b/shpc/main/modules/templates/docker.lua @@ -1,17 +1,17 @@ -- Lmod Module -- Created by singularity-hpc (https://github.com/singularityhub/singularity-hpc) -- ## --- {{ name }} on {{ creation_date }} +-- {{ module.name }} on {{ creation_date }} -- help( [[ -This module is a {{ command }} container wrapper for {{ name }} v{{ version }} -{% if description %}{{ description }}{% endif %} +This module is a {{ command }} container wrapper for {{ module.name }} v{{ module.tag.name }} +{% if description %}{{ module.config.description }}{% endif %} Container: - - {{ image }} + - {{ module.container_path }} Commands include: @@ -49,7 +49,7 @@ if not os.getenv("PODMAN_COMMAND_OPTS") then setenv ("PODMAN_COMMAND_OPTS", "") local moduleDir = subprocess("realpath " .. myFileName()):match("(.*[/])") or "." -- interactive shell to any container, plus exec for aliases -local containerPath = '{{ image }}' +local containerPath = '{{ module.container_path }}' -- service environment variable to access docker URI setenv("PODMAN_CONTAINER", containerPath) @@ -61,7 +61,7 @@ local runCmd = "{{ command }} ${PODMAN_OPTS} run ${PODMAN_COMMAND_OPTS} -i{% if 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 %},"{{ name }}"{% endif %}{% if aliases %}{% for alias in aliases %}{% if alias.name != parsed_name.tool %},"{{ alias.name }}"{% endif %}{% endfor %}{% endif %}) +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 %} @@ -92,7 +92,7 @@ set_shell_function("{|module_name|}-inspect", inspectCmd, inspectCmd){% endif % whatis("Name : " .. myModuleName()) whatis("Version : " .. myModuleVersion()) -{% if description %}whatis("Description : {{ description }}"){% endif %} -{% if url %}whatis("Url : {{ url }}"){% endif %} +{% if description %}whatis("Description : {{ module.config.description }}"){% endif %} +{% if url %}whatis("Url : {{ module.config.url }}"){% endif %} {% if labels %}{% for key, value in labels.items() %}whatis("{{ key }} : {{ value }}") {% endfor %}{% endif %} diff --git a/shpc/main/modules/templates/docker.tcl b/shpc/main/modules/templates/docker.tcl index ea2960d57..65274103a 100644 --- a/shpc/main/modules/templates/docker.tcl +++ b/shpc/main/modules/templates/docker.tcl @@ -3,16 +3,16 @@ #===== # Created by singularity-hpc (https://github.com/singularityhub/singularity-hpc) # ## -# {{ name }} on {{ creation_date }} +# {{ module.name }} on {{ creation_date }} #===== proc ModulesHelp { } { - puts stderr "This module is a {{ command }} container wrapper for {{ name }} v{{ version }}" - {% if description %}puts stderr "{{ description }}"{% endif %} + puts stderr "This module is a {{ command }} container wrapper for {{ module.name }} v{{ module.tag.name }}" + {% if description %}puts stderr "{{ module.config.description }}"{% endif %} puts stderr "" puts stderr "Container:" - puts stderr " - {{ image }}" + puts stderr " - {{ module.container_path }}" puts stderr "Commands include:" puts stderr " - {|module_name|}-run:" puts stderr " {{ command }} run -i{% if settings.enable_tty %}t{% endif %} -u `id -u`:`id -g` --rm {% if settings.environment_file %}--env-file /{{ settings.environment_file }} {% endif %} {% if settings.bindpaths %}-v {{ settings.bindpaths }} {% endif %}{% if features.home %}-v {{ features.home }} {% endif %} -v . -w . \"\$@\"" @@ -48,14 +48,14 @@ if { ![info exists ::env(PODMAN_COMMAND_OPTS)] } { # Variables -set name "{{ name }}" -set version "{{ version }}" +set name "{{ module.name }}" +set version "{{ module.tag.name }}" set description "$name - $version" -set containerPath "{{ image }}" +set containerPath "{{ module.container_path }}" set workdir [pwd] -{% if description %}set notes "{{ description }}"{% endif %} -{% if url %}set homepage "{{ url }}"{% endif %} -set helpcommand "This module is a {{ docker }} container wrapper for {{ name }} v{{ version }}. {% if description %}{{ description }}{% endif %}" +{% if description %}set notes "{{ module.config.description }}"{% endif %} +{% if url %}set homepage "{{ module.config.url }}"{% endif %} +set helpcommand "This module is a {{ docker }} container wrapper for {{ module.name }} v{{ module.tag.name }}. {% if description %}{{ module.config.description }}{% endif %}" {% if labels %}{% for key, value in labels.items() %}set {{ key }} "{{ value }}" {% endfor %}{% endif %} @@ -64,7 +64,7 @@ set moduleDir [file dirname [expr { [string equal [file type ${ModulesCurrentM # conflict with modules with the same alias name conflict {{ parsed_name.tool }} -{% if name != parsed_name.tool %}conflict {{ name }}{% endif %} +{% if name != parsed_name.tool %}conflict {{ module.name }}{% endif %} {% if aliases %}{% for alias in aliases %}{% if alias.name != parsed_name.tool %}conflict {{ alias.name }}{% endif %} {% endfor %}{% endif %} @@ -122,10 +122,10 @@ set-alias {|module_name|}-inspect "${inspectCmd} ${containerPath}"{% endif %} #===== # Module options #===== -module-whatis " Name: {{ name }}" -module-whatis " Version: {{ version }}" +module-whatis " Name: {{ module.name }}" +module-whatis " Version: {{ module.tag.name }}" {% if description %}module-whatis " Description: ${description}"{% endif %} -{% if url %}module-whatis " Url: {{ url }}"{% endif %} +{% if url %}module-whatis " Url: {{ module.config.url }}"{% endif %} {% if labels %}{% for key, value in labels.items() %}module-whatis " {{ key }}: {{ value }}" {% endfor %}{% endif %} {% if settings.podman_module %}module load {{ settings.podman_module }}{% endif %} diff --git a/shpc/main/modules/templates/singularity.lua b/shpc/main/modules/templates/singularity.lua index 6ad9c0cc3..b29984cc6 100644 --- a/shpc/main/modules/templates/singularity.lua +++ b/shpc/main/modules/templates/singularity.lua @@ -1,17 +1,17 @@ -- Lmod Module -- Created by singularity-hpc (https://github.com/singularityhub/singularity-hpc) -- ## --- {{ name }} on {{ creation_date }} +-- {{ module.name }} on {{ creation_date }} -- help( [[ -This module is a singularity container wrapper for {{ name }} v{{ version }} -{% if description %}{{ description }}{% endif %} +This module is a singularity container wrapper for {{ module.name }} v{{ module.tag.name }} +{% if description %}{{ module.config.description }}{% endif %} Container (available through variable SINGULARITY_CONTAINER): - - {{ container_sif }} + - {{ module.container_path }} Commands include: @@ -53,7 +53,7 @@ setenv("SINGULARITY_SHELL", "{{ settings.singularity_shell }}") if not os.getenv("SINGULARITY_OPTS") then setenv ("SINGULARITY_OPTS", "") end if not os.getenv("SINGULARITY_COMMAND_OPTS") then setenv ("SINGULARITY_COMMAND_OPTS", "") end -local containerPath = '{{ container_sif }}' +local containerPath = '{{ module.container_path }}' -- service environment variable to access full SIF image path setenv("SINGULARITY_CONTAINER", containerPath) @@ -64,7 +64,7 @@ local runCmd = "singularity ${SINGULARITY_OPTS} run ${SINGULARITY_COMMAND_OPTS} local inspectCmd = "singularity ${SINGULARITY_OPTS} inspect ${SINGULARITY_COMMAND_OPTS} " -- conflict with modules with the same name -conflict("{{ parsed_name.tool }}"{% if name != parsed_name.tool %},"{{ name }}"{% endif %}{% if aliases %}{% for alias in aliases %}{% if alias.name != parsed_name.tool %},"{{ alias.name }}"{% endif %}{% endfor %}{% endif %}) +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 bin to path {% if wrapper_scripts %}prepend_path("PATH", pathJoin(moduleDir, "bin")){% endif %} @@ -98,7 +98,7 @@ set_shell_function("{|module_name|}-inspect-deffile", inspectCmd .. " -d " .. c whatis("Name : " .. myModuleName()) whatis("Version : " .. myModuleVersion()) -{% if description %}whatis("Description : {{ description }}"){% endif %} -{% if url %}whatis("Url : {{ url }}"){% endif %} +{% if description %}whatis("Description : {{ module.config.description }}"){% endif %} +{% if url %}whatis("Url : {{ module.config.url }}"){% endif %} {% if labels %}{% for key, value in labels.items() %}whatis("{{ key }} : {{ value }}") {% endfor %}{% endif %} diff --git a/shpc/main/modules/templates/singularity.tcl b/shpc/main/modules/templates/singularity.tcl index 452f3d758..fad9a2b1b 100644 --- a/shpc/main/modules/templates/singularity.tcl +++ b/shpc/main/modules/templates/singularity.tcl @@ -3,17 +3,17 @@ #===== # Created by singularity-hpc (https://github.com/singularityhub/singularity-hpc) # ## -# {{ name }} on {{ creation_date }} +# {{ module.name }} on {{ creation_date }} #===== proc ModulesHelp { } { - puts stderr "This module is a singularity container wrapper for {{ name }} v{{ version }}" - {% if description %}puts stderr "{{ description }}"{% endif %} + puts stderr "This module is a singularity container wrapper for {{ module.name }} v{{ module.tag.name }}" + {% if description %}puts stderr "{{ module.config.description }}"{% endif %} puts stderr "" puts stderr "Container (available through variable SINGULARITY_CONTAINER):" puts stderr "" - puts stderr " - {{ container_sif }}" + puts stderr " - {{ module.container_path }}" puts stderr "" puts stderr "Commands include:" puts stderr "" @@ -54,13 +54,13 @@ if { ![info exists ::env(SINGULARITY_COMMAND_OPTS)] } { # Variables -set name {{ name }} -set version {{ version }} +set name {{ module.name }} +set version {{ module.tag.name }} set description "$name - $version" -set containerPath {{ container_sif }} -{% if description %}set notes "{{ description }}"{% endif %} -{% if url %}set homepage "{{ url }}"{% endif %} -set helpcommand "This module is a singularity container wrapper for {{ name }} v{{ version }}. {% if description %}{{ description }}{% endif %}" +set containerPath {{ module.container_path }} +{% if description %}set notes "{{ module.config.description }}"{% endif %} +{% if url %}set homepage "{{ module.config.url }}"{% endif %} +set helpcommand "This module is a singularity container wrapper for {{ module.name }} v{{ module.tag.name }}. {% if description %}{{ module.config.description }}{% endif %}" {% if labels %}{% for key, value in labels.items() %}set {{ key }} "{{ value }}" {% endfor %}{% endif %} @@ -69,7 +69,7 @@ set moduleDir [file dirname [expr { [string equal [file type ${ModulesCurrentM # conflict with modules with the same alias name conflict {{ parsed_name.tool }} -{% if name != parsed_name.tool %}conflict {{ name }}{% endif %} +{% if name != parsed_name.tool %}conflict {{ module.name }}{% endif %} {% if aliases %}{% for alias in aliases %}{% if alias.name != parsed_name.tool %}conflict {{ alias.name }}{% endif %} {% endfor %}{% endif %} @@ -128,10 +128,10 @@ set-alias {|module_name|}-inspect-deffile "${inspectCmd} -d ${containerPath}"{% #===== # Module options #===== -module-whatis " Name: {{ name }}" -module-whatis " Version: {{ version }}" +module-whatis " Name: {{ module.name }}" +module-whatis " Version: {{ module.tag.name }}" {% if description %}module-whatis " Description: ${description}"{% endif %} -{% if url %}module-whatis " Url: {{ url }}"{% endif %} +{% if url %}module-whatis " Url: {{ module.config.url }}"{% endif %} {% if labels %}{% for key, value in labels.items() %}module-whatis " {{ key }}: {{ value }}" {% endfor %}{% endif %} {% if settings.singularity_module %}module load {{ settings.singularity_module }}{% endif %} From 2a1331a1e1dd8704ea54cb7e90b78e8171bd207d Mon Sep 17 00:00:00 2001 From: vsoch Date: Mon, 10 Oct 2022 05:56:11 -0600 Subject: [PATCH 07/11] ensure we maintain parsed name Signed-off-by: vsoch --- shpc/main/container/docker.py | 1 + shpc/main/container/singularity.py | 1 + shpc/main/modules/template.py | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/shpc/main/container/docker.py b/shpc/main/container/docker.py index 6783a3908..a455b84d1 100644 --- a/shpc/main/container/docker.py +++ b/shpc/main/container/docker.py @@ -234,6 +234,7 @@ def install(self, module_path, template, module, features=None): creation_date=datetime.now(), command=self.command, module=module, + parsed_name=module.config.name, wrapper_scripts=wrapper_scripts, ) shpc.utils.write_file(module_path, out) diff --git a/shpc/main/container/singularity.py b/shpc/main/container/singularity.py index ce086f93f..40c24e53d 100644 --- a/shpc/main/container/singularity.py +++ b/shpc/main/container/singularity.py @@ -224,6 +224,7 @@ def install(self, module_path, template, module, features=None): deffile=deffile, creation_date=datetime.now(), module=module, + parsed_name=module.config.name, wrapper_scripts=wrapper_scripts, ) utils.write_file(module_path, out) diff --git a/shpc/main/modules/template.py b/shpc/main/modules/template.py index 0ebb6bd6a..9fd767435 100644 --- a/shpc/main/modules/template.py +++ b/shpc/main/modules/template.py @@ -46,7 +46,7 @@ def substitute(self, template): For all known identifiers, substitute user specified format strings. """ subs = { - "{|module_name|}": self.settings.module_name or "{{ parsed_name.tool }}" + "{|module_name|}": self.settings.module_name or "{{ config.name.tool }}" } for key, replacewith in subs.items(): template = template.replace(key, replacewith) From b3121e9a7b8d78f3fca1928c0a87b5d509763e99 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Mon, 10 Oct 2022 04:13:23 +0100 Subject: [PATCH 08/11] Moved the view uninstallation to a separate function to mirror view_install --- shpc/client/view.py | 2 +- shpc/main/modules/base.py | 43 ++++++++++++++++++++++----------------- shpc/tests/test_views.py | 2 +- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/shpc/client/view.py b/shpc/client/view.py index eddaa600e..400796b83 100644 --- a/shpc/client/view.py +++ b/shpc/client/view.py @@ -175,4 +175,4 @@ def main(args, parser, extra, subparser): cli.view_install(view_name, module_name, force=args.force) if command == "uninstall": - cli.uninstall(module_name, view=view_name, force=args.force) + cli.view_uninstall(view_name, module_name, force=args.force) diff --git a/shpc/main/modules/base.py b/shpc/main/modules/base.py index 995515352..04f00facc 100644 --- a/shpc/main/modules/base.py +++ b/shpc/main/modules/base.py @@ -67,23 +67,34 @@ def modulefile(self): def templatefile(self): return "%s.%s" % (self.container.templatefile, self.module_extension) - def uninstall(self, name, view=None, force=False): + def view_uninstall(self, view, name, force=False): """ - Given a unique resource identifier, uninstall a module. If a view - name is provided, assume we only want to uninstall from the view + Uninstall a module from a view. """ module = self.new_module(name) - # We need to look for the module in all views and show to the user first - views_with_module = set() + # Ask before deleting anything! + if not force: + msg = name + "?" + if not utils.confirm_uninstall(msg, force): + return + + # Only uninstall from the view + if view not in self.views: + logger.exit("View %s does not exist, cannot uninstall." % view) + return self.views[view].uninstall(module.module_dir) - # Only populate if the command is not directed to a view - if not view: + def uninstall(self, name, force=False): + """ + Given a unique resource identifier, uninstall a module. + """ + module = self.new_module(name) - # If uninstalling the entire module, clean up symbolic links in all views - for view_name, entry in self.views.items(): - if entry.exists(module.module_dir): - views_with_module.add(view_name) + # We need to look for the module in all views and show to the user first + views_with_module = set() + for view_name, entry in self.views.items(): + if entry.exists(module.module_dir): + views_with_module.add(view_name) # Ask before deleting anything! if not force: @@ -96,12 +107,6 @@ def uninstall(self, name, view=None, force=False): if not utils.confirm_uninstall(msg, force): return - # Only uninstall from the view - if view: - if view not in self.views: - logger.exit("View %s does not exist, cannot uninstall." % view) - return self.views[view].uninstall(module.module_dir) - # Podman needs image deletion self.container.delete(module.name) @@ -124,8 +129,8 @@ def uninstall(self, name, view=None, force=False): ) # If uninstalling the entire module, clean up symbolic links in all views - for view_name, view in self.views.items(): - view.uninstall(module.module_dir) + for view_name in views_with_module: + self.views[view_name].uninstall(module.module_dir) # parent of versioned directory has module .version module_dir = os.path.dirname(module.module_dir) diff --git a/shpc/tests/test_views.py b/shpc/tests/test_views.py index cbce599c4..3c2302a4d 100644 --- a/shpc/tests/test_views.py +++ b/shpc/tests/test_views.py @@ -95,7 +95,7 @@ def test_views(tmp_path, module_sys, module_file, container_tech, remote): module_file = os.path.join(module_path, module_file[0]) assert os.path.islink(module_file) - client.uninstall("ghcr.io/autamus/emacs:27.2", view=view_name, force=True) + client.view_uninstall(view_name, "ghcr.io/autamus/emacs:27.2", force=True) # The view should be removed assert "emacs" not in os.listdir(os.path.join(view.path)) From b89b823858a581fb0c96d0c875646c2953135457 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Tue, 11 Oct 2022 22:22:05 +0100 Subject: [PATCH 09/11] No need to make this local variable Co-authored-by: Vanessasaurus <814322+vsoch@users.noreply.github.com> --- shpc/main/modules/base.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shpc/main/modules/base.py b/shpc/main/modules/base.py index 04f00facc..9e013cf9f 100644 --- a/shpc/main/modules/base.py +++ b/shpc/main/modules/base.py @@ -75,8 +75,7 @@ def view_uninstall(self, view, name, force=False): # Ask before deleting anything! if not force: - msg = name + "?" - if not utils.confirm_uninstall(msg, force): + if not utils.confirm_uninstall(name + "?", force): return # Only uninstall from the view From eec370c6465a11357787d40f5ec2f2d604887e29 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Tue, 11 Oct 2022 22:23:45 +0100 Subject: [PATCH 10/11] Use parsed_name for consistency config.name was not wrong Co-authored-by: Vanessasaurus <814322+vsoch@users.noreply.github.com> --- shpc/main/modules/template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shpc/main/modules/template.py b/shpc/main/modules/template.py index 9fd767435..0ebb6bd6a 100644 --- a/shpc/main/modules/template.py +++ b/shpc/main/modules/template.py @@ -46,7 +46,7 @@ def substitute(self, template): For all known identifiers, substitute user specified format strings. """ subs = { - "{|module_name|}": self.settings.module_name or "{{ config.name.tool }}" + "{|module_name|}": self.settings.module_name or "{{ parsed_name.tool }}" } for key, replacewith in subs.items(): template = template.replace(key, replacewith) From 7e76b6badaf4ea2ce19d7807c3a529769a10c311 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Tue, 11 Oct 2022 22:21:12 +0100 Subject: [PATCH 11/11] Document the "force" flags --- shpc/main/modules/base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/shpc/main/modules/base.py b/shpc/main/modules/base.py index 9e013cf9f..c9da29f99 100644 --- a/shpc/main/modules/base.py +++ b/shpc/main/modules/base.py @@ -70,6 +70,7 @@ def templatefile(self): def view_uninstall(self, view, name, force=False): """ Uninstall a module from a view. + Set "force" to True to bypass the confirmation prompt. """ module = self.new_module(name) @@ -86,6 +87,7 @@ def view_uninstall(self, view, name, force=False): def uninstall(self, name, force=False): """ Given a unique resource identifier, uninstall a module. + Set "force" to True to bypass the confirmation prompt. """ module = self.new_module(name) @@ -372,6 +374,7 @@ def install(self, name, tag=None, force=False, **kwargs): For lmod, this means creating a subfolder in modules, pulling the container to it, and writing a module file there. We've already grabbed the name from docker (which is currently the only supported). + "force" is currently not used. """ # Create a new module module = self.new_module(name, tag=tag, tag_exists=True) @@ -409,8 +412,8 @@ def install(self, name, tag=None, force=False, **kwargs): def view_install(self, view_name, name, tag=None, force=False): """ - Install a module in a view. - The module must already be installed. + 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)