Merge branch 'bugfix/do-not-fail-if-not-solution-iso-mounted' into tm…

…p/octopus/w/2.9/bugfix/do-not-fail-if-not-solution-iso-mounted

CHANGELOG.md

@@ -68,6 +68,10 @@
   Salt dependencies from "base" RHEL 7 repository
   (PR [#3083](https://github.com/scality/metalk8s/pull/3083))
 
+- [#3128](https://github.com/scality/metalk8s/issues/3128) - No longer assume ISOs
+  mounted under `/srv/scality` are Solutions
+  (PR [#3182](https://github.com/scality/metalk8s/pull/3182))
+
 ### Deprecations and Removals
 
 - [#3168](https://github.com/scality/metalk8s/issues/3168) - [UI] Remove the environment page

salt/_modules/metalk8s_solutions.py

@@ -156,26 +156,6 @@ def deactivate_solution(solution):
     return True
 
 
-def _is_solution_mount(mount_tuple):
-    """Return whether a mount is for a Solution archive.
-
-    Any ISO9660 mounted in `/srv/scality` that isn't for MetalK8s is considered
-    to be a Solution archive.
-    """
-    mountpoint, mount_info = mount_tuple
-
-    if not mountpoint.startswith("/srv/scality/"):
-        return False
-
-    if mountpoint.startswith("/srv/scality/metalk8s-"):
-        return False
-
-    if mount_info["fstype"] != "iso9660":
-        return False
-
-    return True
-
-
 SOLUTION_MANIFEST = "manifest.yaml"
 SOLUTION_MANIFEST_KIND = "Solution"
 SOLUTION_MANIFEST_APIVERSIONS = [
@@ -339,10 +319,29 @@ def list_available():
 
     active_mounts = __salt__["mount.active"]()
 
-    solution_mounts = filter(_is_solution_mount, active_mounts.items())
+    for mountpoint, mount_info in active_mounts.items():
+        # Skip mountpoint not in `/srv/scality`
+        if not mountpoint.startswith("/srv/scality/"):
+            continue
+
+        # Skip MetalK8s mountpoint
+        if mountpoint.startswith("/srv/scality/metalk8s-"):
+            continue
 
-    for mountpoint, mount_info in solution_mounts:
-        manifest, info = read_solution_manifest(mountpoint)
+        # Skip non ISO mountpoint
+        if mount_info["fstype"] != "iso9660":
+            continue
+
+        try:
+            manifest, info = read_solution_manifest(mountpoint)
+        except CommandExecutionError as exc:
+            log.info(
+                "Skipping %s: not a Solution (failed to read/parse %s): %s",
+                mountpoint,
+                SOLUTION_MANIFEST,
+                exc,
+            )
+            continue
 
         result[info["name"]].append(
             {

salt/tests/unit/modules/files/test_metalk8s_solutions.yaml

@@ -368,6 +368,10 @@ list_available:
   - mountpoints:
       /: {}
       /srv/scality/metalk8s-x.y.z: {}
+      # Ignored as it does not contain any manifests
+      /srv/scality/no-solution:
+        alt_device: /tmp/not-a-solution.iso
+        fstype: iso9660
   # Ok - No mountpoint
   - {}
 

salt/tests/unit/modules/test_metalk8s_solutions.py

@@ -267,29 +267,22 @@ def _cmd_run_all_mock(**_):
                 self.assertEqual(metalk8s_solutions.manifest_from_iso(path), result)
 
     @utils.parameterized_from_cases(YAML_TESTS_CASES["list_available"])
-    def test_list_available(
-        self, mountpoints=None, archive_infos=None, result=None, raises=False
-    ):
+    def test_list_available(self, mountpoints=None, archive_infos=None, result=None):
         """
         Tests the return of `list_available` function
         """
         mount_active_mock = MagicMock(return_value=mountpoints or {})
         read_solution_manifest_mock = MagicMock(return_value=(None, archive_infos))
+        if archive_infos is None:
+            read_solution_manifest_mock.side_effect = CommandExecutionError("Banana")
 
         salt_dict_patch = {
             "mount.active": mount_active_mock,
         }
         with patch.dict(metalk8s_solutions.__salt__, salt_dict_patch), patch(
             "metalk8s_solutions.read_solution_manifest", read_solution_manifest_mock
         ):
-            if raises:
-                self.assertRaisesRegex(
-                    Exception,
-                    'Path has no "product.txt"',
-                    metalk8s_solutions.list_available,
-                )
-            else:
-                self.assertEqual(metalk8s_solutions.list_available(), result or {})
+            self.assertEqual(metalk8s_solutions.list_available(), result or {})
 
     @utils.parameterized_from_cases(YAML_TESTS_CASES["operator_roles_from_manifest"])
     def test_operator_roles_from_manifest(