From ed4c31c942041194d2bc1a66edc7a8b01c48e8ea Mon Sep 17 00:00:00 2001 From: vsoch Date: Fri, 30 Jun 2023 18:14:48 -0600 Subject: [PATCH 1/4] fix bug with container add and expose keep_path to add Signed-off-by: vsoch --- CHANGELOG.md | 1 + shpc/client/__init__.py | 16 ++++++------ shpc/client/add.py | 6 ++++- shpc/main/container/singularity.py | 41 +++++++++++++++++++++--------- shpc/main/modules/base.py | 9 +++++-- shpc/main/modules/module.py | 4 +-- shpc/version.py | 2 +- 7 files changed, 53 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b24f3334..52a6e7ad1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and **Merged pull requests**. Critical items to know are: The versions coincide with releases on pip. Only major versions will be released as tags on Github. ## [0.0.x](https://github.com/singularityhub/singularity-hpc/tree/main) (0.0.x) + - Fix bug with container add not respecting container_base (0.1.24) - Labels with newlines need additional parsing (0.1.23) - Do not write directly to output with shpc show (0.1.22) - Podman template bug (0.1.21) diff --git a/shpc/client/__init__.py b/shpc/client/__init__.py index de803e5ff..d3a95ee3b 100644 --- a/shpc/client/__init__.py +++ b/shpc/client/__init__.py @@ -103,14 +103,6 @@ def get_parser(): help="path to an existing container image for this software", nargs="?", ) - - install.add_argument( - "--keep-path", - help="if installing a local container, do not copy the container - use the provided path.", - default=False, - action="store_true", - ) - install.add_argument( "--no-view", dest="no_view", @@ -377,6 +369,14 @@ def get_parser(): action="store_true", ) + for command in install, add: + command.add_argument( + "--keep-path", + help="if using a local container, do not copy the container - use the provided path.", + default=False, + action="store_true", + ) + for command in update, sync: command.add_argument( "--dry-run", diff --git a/shpc/client/add.py b/shpc/client/add.py index fdb0ab55a..3d815ac91 100644 --- a/shpc/client/add.py +++ b/shpc/client/add.py @@ -26,4 +26,8 @@ def main(args, parser, extra, subparser): cli.reload_registry() # If we don't have a module name, we derive from container URI - cli.add(args.container_uri, args.module_id) + cli.add( + args.container_uri, + args.module_id, + keep_path=args.keep_path, + ) diff --git a/shpc/main/container/singularity.py b/shpc/main/container/singularity.py index 46fb60adb..294074eb7 100644 --- a/shpc/main/container/singularity.py +++ b/shpc/main/container/singularity.py @@ -75,7 +75,9 @@ def get(self, module_name, env_file=False): logger.exit("Found more than one sif in module folder.") return sif[0] - def add(self, module_name, image, config, container_yaml, **kwargs): + def add( + self, module_name, image, config, container_yaml, keep_path=False, **kwargs + ): """ Manually add a registry container, e.g., generating a container.yaml for an existing container file. container_yaml is the destination file. @@ -96,9 +98,11 @@ def add(self, module_name, image, config, container_yaml, **kwargs): ): return - # Destination for container in registry - dest_dir = os.path.dirname(container_yaml) - utils.mkdir_p(dest_dir) + # Destination for container in registry, either the container.yaml + # path or the container base, if a custom path is desired + if not self.settings.container_base and not keep_path: + dest_dir = os.path.dirname(container_yaml) + utils.mkdir_p(dest_dir) # Add a docker (or local image) and return the config if image.startswith("docker://"): @@ -107,7 +111,13 @@ def add(self, module_name, image, config, container_yaml, **kwargs): ) else: config = self._add_local_image( - module_name, tag, image, config, container_yaml, **kwargs + module_name, + tag, + image, + config, + container_yaml, + keep_path=keep_path, + **kwargs, ) # Final save of config, and tell the user we're done! @@ -118,7 +128,9 @@ def add(self, module_name, image, config, container_yaml, **kwargs): print(container_yaml) return container_yaml - def _add_local_image(self, name, tag, image, config, container_yaml, **kwargs): + def _add_local_image( + self, name, tag, image, config, container_yaml, keep_path=False, **kwargs + ): """ A subtype of "add" that adds a local image, e.g.,: @@ -128,14 +140,19 @@ def _add_local_image(self, name, tag, image, config, container_yaml, **kwargs): logger.exit(f"{image} does not exist.") digest = utils.get_file_hash(image) + container_digest = "sha256:%s" % digest - # Destination for container in registry - dest_dir = os.path.dirname(container_yaml) + if keep_path: + dest_container = image + container_name = os.path.basename(image) - # 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) + else: + # Destination for container in registry + dest_dir = self.settings.container_base or os.path.dirname(container_yaml) + + # The destination container in the registry folder + 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) diff --git a/shpc/main/modules/base.py b/shpc/main/modules/base.py index 8b0058c0d..4dd13de05 100644 --- a/shpc/main/modules/base.py +++ b/shpc/main/modules/base.py @@ -213,7 +213,7 @@ def remove(self, image=None, force=False): print() logger.info("Removal complete!") - def add(self, image, module_name=None, **kwargs): + def add(self, image, module_name=None, keep_path=False, **kwargs): """ Add a container to the registry to enable install. """ @@ -246,7 +246,12 @@ def add(self, image, module_name=None, **kwargs): registry.FilesystemResult(module_name, template), validate=False ) return self.container.add( - module_name, image, config, container_yaml=dest, **kwargs + module_name, + image, + config, + container_yaml=dest, + keep_path=keep_path, + **kwargs, ) def get(self, module_name, env_file=False): diff --git a/shpc/main/modules/module.py b/shpc/main/modules/module.py index d95bf9a17..efb617645 100644 --- a/shpc/main/modules/module.py +++ b/shpc/main/modules/module.py @@ -73,7 +73,7 @@ def container_dir(self): self._container_dir = self.container.container_dir(self.module_basepath) return self._container_dir - def add_container(self, container_image=None): + def add_container(self, container_image=None, keep_path=False): """ Ensure a container is pulled (or provided) @@ -82,7 +82,7 @@ def add_container(self, container_image=None): # First preference goes to provided image (actual file) # This is only allowed for Singularity containers if container_image and os.path.exists(container_image): - self.add_local_container(container_image) + self.add_local_container(container_image, keep_path=keep_path) # If we have a sif URI provided by path, the container needs to exist elif self.config.path: diff --git a/shpc/version.py b/shpc/version.py index 311dbd49a..43fac3353 100644 --- a/shpc/version.py +++ b/shpc/version.py @@ -2,7 +2,7 @@ __copyright__ = "Copyright 2021-2023, Vanessa Sochat" __license__ = "MPL 2.0" -__version__ = "0.1.23" +__version__ = "0.1.24" AUTHOR = "Vanessa Sochat" EMAIL = "vsoch@users.noreply.github.com" NAME = "singularity-hpc" From 0f271ae68c101bb909bdcd573f40bcc427d88296 Mon Sep 17 00:00:00 2001 From: vsoch Date: Fri, 30 Jun 2023 18:25:42 -0600 Subject: [PATCH 2/4] fix bug with container add and expose keep_path to add Signed-off-by: vsoch --- shpc/main/container/singularity.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/shpc/main/container/singularity.py b/shpc/main/container/singularity.py index 294074eb7..8b24d30e0 100644 --- a/shpc/main/container/singularity.py +++ b/shpc/main/container/singularity.py @@ -98,11 +98,9 @@ def add( ): return - # Destination for container in registry, either the container.yaml - # path or the container base, if a custom path is desired - if not self.settings.container_base and not keep_path: - dest_dir = os.path.dirname(container_yaml) - utils.mkdir_p(dest_dir) + # Destination for container in registry for container.yaml + dest_dir = os.path.dirname(container_yaml) + utils.mkdir_p(dest_dir) # Add a docker (or local image) and return the config if image.startswith("docker://"): From 4553cc46681ead63b34087e54fededd0cc3af72a Mon Sep 17 00:00:00 2001 From: vsoch Date: Thu, 6 Jul 2023 07:32:14 -0600 Subject: [PATCH 3/4] container dir should have module name Signed-off-by: vsoch --- shpc/main/container/singularity.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shpc/main/container/singularity.py b/shpc/main/container/singularity.py index 8b24d30e0..21d624c88 100644 --- a/shpc/main/container/singularity.py +++ b/shpc/main/container/singularity.py @@ -146,7 +146,8 @@ def _add_local_image( else: # Destination for container in registry - dest_dir = self.settings.container_base or os.path.dirname(container_yaml) + container_dir = self.container_dir(name) + dest_dir = container_dir or os.path.dirname(container_yaml) # The destination container in the registry folder container_name = "%s.sif" % container_digest From f6bc331413fbf47cb4f24678ada827ec62531c5e Mon Sep 17 00:00:00 2001 From: vsoch Date: Thu, 6 Jul 2023 07:50:03 -0600 Subject: [PATCH 4/4] ensure container directory exists Signed-off-by: vsoch --- shpc/main/container/singularity.py | 1 + 1 file changed, 1 insertion(+) diff --git a/shpc/main/container/singularity.py b/shpc/main/container/singularity.py index 21d624c88..1701497ec 100644 --- a/shpc/main/container/singularity.py +++ b/shpc/main/container/singularity.py @@ -148,6 +148,7 @@ def _add_local_image( # Destination for container in registry container_dir = self.container_dir(name) dest_dir = container_dir or os.path.dirname(container_yaml) + utils.mkdir_p(dest_dir) # The destination container in the registry folder container_name = "%s.sif" % container_digest