If the container is deleted, the module has to be deleted too. No need to ask the user (#528)

* Only ask for confirmation once
* The .version file is only in module_dir, not container_dir
* When deleting containers, also delete the parent directories as long as they are empty

Author: muffato

shpc/main/modules/base.py

@@ -58,22 +58,6 @@ def substitute(self, template):
             template = template.replace(key, replacewith)
         return template
 
-    def _cleanup(self, module_dir):
-        """
-        Given a module directory, delete up to module base
-        """
-        # Exit early if the module directory for some reason was removed
-        if not os.path.exists(module_dir):
-            return
-        shutil.rmtree(module_dir)
-
-        # If directories above it are empty, remove
-        while module_dir != self.settings.module_base:
-            if not utils.can_be_deleted(module_dir, [".version"]):
-                break
-            shutil.rmtree(module_dir)
-            module_dir = os.path.dirname(module_dir)
-
     @property
     def container_base(self):
         """
@@ -106,25 +90,22 @@ def uninstall(self, name, force=False):
         # Podman needs image deletion
         self.container.delete(name)
 
-        if container_dir != module_dir:
-            self._uninstall(container_dir, "$container_base/%s" % name, force)
-            self._uninstall(module_dir, "$module_base/%s" % name, force)
-        else:
-            self._uninstall(module_dir, "$module_base/%s" % name, force)
+        if not force:
+            msg = name + "?"
+            if not utils.confirm_uninstall(msg, force):
+                return
 
-    def _uninstall(self, module_dir, name, force=False):
-        """
-        Sub function, so we can pass more than one folder from uninstall
-        """
-        if os.path.exists(module_dir):
-            if not force:
-                msg = "%s, and all content below it? " % name
-                if not utils.confirm_uninstall(msg, force):
-                    return
-            self._cleanup(module_dir)
-            logger.info("%s and all subdirectories have been removed." % name)
+        if container_dir != module_dir:
+            self._uninstall(
+                container_dir, self.container_base, "$container_base/%s" % name
+            )
+            self._uninstall(
+                module_dir, self.settings.module_base, "$module_base/%s" % name
+            )
         else:
-            logger.warning("%s does not exist." % name)
+            self._uninstall(
+                module_dir, self.settings.module_base, "$module_base/%s" % name
+            )
 
         # parent of versioned directory has module .version
         module_dir = os.path.dirname(module_dir)
@@ -133,6 +114,16 @@ def _uninstall(self, module_dir, name, force=False):
         if os.path.exists(module_dir):
             self.write_version_file(module_dir)
 
+    def _uninstall(self, path, base_path, name):
+        """
+        Sub function, so we can pass more than one folder from uninstall
+        """
+        if os.path.exists(path):
+            utils.rmdir_to_base(path, base_path)
+            logger.info("%s and all subdirectories have been removed." % name)
+        else:
+            logger.warning("%s does not exist." % name)
+
     def _test_setup(self, tmpdir):
         """
         Setup tests, including changes to settings or test directory
@@ -371,7 +362,7 @@ def install(self, name, tag=None, **kwargs):
                 module_dir, container_dir, config, tag
             )
         if not container_path:
-            self._cleanup(container_dir)
+            utils.rmdir_to_base(container_dir, self.container_base)
             logger.exit("There was an issue pulling %s" % container_path)
 
         # Get the template based on the module and container type
@@ -401,7 +392,7 @@ def install(self, name, tag=None, **kwargs):
 
         # If the container tech does not need storage, clean up
         if not os.listdir(container_dir):
-            self._cleanup(container_dir)
+            utils.rmdir_to_base(container_dir, self.container_base)
 
         # Write the environment file to be bound to the container
         self.container.add_environment(

shpc/utils/__init__.py

@@ -15,6 +15,7 @@
     get_tmpfile,
     mkdir_p,
     mkdirp,
+    rmdir_to_base,
     print_json,
     read_file,
     read_json,

shpc/utils/fileio.py

@@ -61,6 +61,28 @@ def mkdir_p(path):
             logger.exit("Error creating path %s, exiting." % path)
 
 
+def rmdir_to_base(path, base_path):
+    """
+    Delete the tree under $path and all the parents
+    up to $base_path as long as they are empty
+    """
+    if not os.path.isdir(base_path):
+        logger.exit("Error: %s is not a directory" % base_path)
+    if not path.startswith(base_path):
+        logger.exit("Error: %s is not a parent of %s" % (base_path, path))
+
+    if os.path.exists(path):
+        shutil.rmtree(path)
+
+    # If directories above it are empty, remove
+    while path != base_path:
+        if os.path.exists(path):
+            if not can_be_deleted(path, [".version"]):
+                break
+            shutil.rmtree(path)
+        path = os.path.dirname(path)
+
+
 def get_tmpfile(tmpdir=None, prefix=""):
     """
     Get a temporary file with an optional prefix.