VMImage Caching Fix

Improve VMImage caching and asset handling functionality.

- Enhanced asset.py to handle direct file paths and normal asset names
- Better error handling and logging in get_metadata()
- Refactored _find_cached_image() to use Asset functionality
- Fixed multiple Pylint warnings (too-many-branches, protected-access, etc.)
- Improved network error handling in provider selection

Signed-off-by: Harvey Lynden <[email protected]>

Author: harvey0100

avocado/utils/asset.py

@@ -443,7 +443,9 @@ def fetch(self, timeout=None):
 
     def find_asset_file(self, create_metadata=False):
         """
-        Search for the asset file in each one of the cache locations
+        Search for the asset file in each one of the cache locations.
+
+        It handles both normal asset names and direct file paths.
 
         :param bool create_metadata: Should this method create the
                                      metadata in case asset file found
@@ -454,6 +456,11 @@ def find_asset_file(self, create_metadata=False):
         :raises: OSError
         """
 
+        # Try 1: Check if self.name is already a direct file path
+        if self.name and os.path.isfile(self.name):
+            return self.name
+
+        # Try 2: Search in cache directories using relative_dir
         for cache_dir in self.cache_dirs:
             cache_dir = os.path.expanduser(cache_dir)
             asset_file = os.path.join(cache_dir, self.relative_dir)
@@ -482,20 +489,28 @@ def get_metadata(self):
         """
         Returns metadata of the asset if it exists or None.
 
+        It handles both normal asset names and direct file paths.
+
         :return: metadata
         :rtype: dict or None
         """
         try:
             asset_file = self.find_asset_file()
         except OSError as exc:
+            LOG.debug("Asset file not found, metadata not available for %s", self.name)
             raise OSError("Metadata not available.") from exc
 
         basename = os.path.splitext(asset_file)[0]
         metadata_file = f"{basename}_metadata.json"
         if os.path.isfile(metadata_file):
-            with open(metadata_file, "r", encoding="utf-8") as f:
-                metadata = json.load(f)
-                return metadata
+            try:
+                with open(metadata_file, "r", encoding="utf-8") as f:
+                    metadata = json.load(f)
+                    return metadata
+            except json.JSONDecodeError as e:
+                LOG.warning("Invalid JSON in metadata file %s: %s", metadata_file, e)
+            except OSError as e:
+                LOG.warning("Cannot read metadata file %s: %s", metadata_file, e)
         return None
 
     @property

avocado/utils/vmimage.py

@@ -119,7 +119,7 @@ def _feed_html_parser(self, url, parser):
                 data = u.read()
             parser.feed(astring.to_text(data, self.HTML_ENCODING))
         except HTTPError as exc:
-            raise ImageProviderError(f"Cannot open {url}") from exc
+            raise ImageProviderError(f"Network error: Cannot open {url}") from exc
 
     @staticmethod
     def get_best_version(versions):
@@ -633,6 +633,109 @@ def _take_snapshot(self):
         process.run(cmd)
         return new_image
 
+    @classmethod
+    def _find_cached_image(  # pylint: disable=too-many-locals
+        cls,
+        cache_dirs,
+        name=None,
+        version=None,
+        build=None,
+        arch=None,
+        checksum=None,
+        algorithm=None,
+        snapshot_dir=None,
+    ):
+        """
+        Find a cached image using asset.py enhanced built-in functionality.
+
+        This version uses the enhanced Asset.get_metadata() method which supports
+        both normal asset names and direct file paths, maximizing utilization of
+        existing asset.py caching features.
+        """
+
+        # pylint: disable-next=invalid-name
+        ARCH_COMPATIBILITY = {
+            "x86_64": ["x86_64", "amd64", "64"],
+            "amd64": ["x86_64", "amd64", "64"],
+            "aarch64": ["aarch64", "arm64"],
+            "arm64": ["aarch64", "arm64"],
+        }
+        compatible_arches = ARCH_COMPATIBILITY.get(arch, [arch]) if arch else []
+
+        def matches_image_criteria(metadata, name, version, build, compatible_arches):
+            """
+            Check if metadata matches the specified image criteria.
+
+            :param metadata: Asset metadata dictionary
+            :param name: Expected image name
+            :param version: Expected image version
+            :param build: Expected image build
+            :param compatible_arches: List of compatible architectures
+            :return: True if metadata matches criteria, False otherwise
+            """
+
+            if metadata.get("type") != "vmimage":
+                return False
+
+            if name and metadata.get("name", "").lower() != name.lower():
+                return False
+
+            if version and str(metadata.get("version", "")) != str(version):
+                return False
+
+            if build and str(metadata.get("build", "")) != str(build):
+                return False
+
+            if compatible_arches and metadata.get("arch") not in compatible_arches:
+                return False
+
+            return True
+
+        # Use Asset.get_all_assets() to find cached assets
+        for asset_path in asset.Asset.get_all_assets(cache_dirs, sort=False):
+            try:
+                temp_asset = asset.Asset(
+                    name=asset_path,
+                    asset_hash=checksum,
+                    algorithm=algorithm,
+                    cache_dirs=cache_dirs,
+                )
+
+                try:
+                    metadata = temp_asset.get_metadata()
+                    if not metadata:
+                        continue
+                except OSError:
+                    continue
+
+                if matches_image_criteria(
+                    metadata, name, version, build, compatible_arches
+                ):
+                    # pylint: disable-next=W0212
+                    if checksum and not temp_asset._verify_hash(asset_path):
+                        LOG.debug("Hash mismatch for cached image: %s", asset_path)
+                        continue
+
+                    LOG.info("Found matching cached image: %s", asset_path)
+                    return cls(
+                        name=metadata.get("name", name),
+                        url=asset_path,
+                        version=metadata.get("version", version),
+                        arch=metadata.get("arch", arch),
+                        build=metadata.get("build", build),
+                        checksum=checksum,
+                        algorithm=algorithm,
+                        cache_dir=cache_dirs[0],
+                        snapshot_dir=snapshot_dir,
+                    )
+
+            except (OSError, ValueError, KeyError, AttributeError) as e:
+                # Skip assets that can't be processed (common errors during metadata processing)
+                LOG.debug("Skipping asset %s due to error: %s", asset_path, e)
+                continue
+
+        return None
+
     @classmethod
     # pylint: disable=R0913
     def from_parameters(
@@ -739,14 +842,25 @@ def get_best_provider(name=None, version=None, build=None, arch=None):
         if name == "fedora" and arch in ("ppc64", "ppc64le", "s390x"):
             name = "fedorasecondary"
 
+    provider_errors = []
     for provider in IMAGE_PROVIDERS:
         if name is None or name == provider.name.lower():
             try:
                 return provider(**provider_args)
             except ImageProviderError as e:
                 LOG.debug(e)
+                provider_errors.append(f"{provider.name}: {e}")
+            except (HTTPError, OSError) as e:
+                LOG.debug(
+                    "Network error while creating provider %s: %s", provider.name, e
+                )
+                # Network errors should raise immediately
+                raise ImageProviderError(f"Network error: {e}") from e
 
     LOG.debug("Provider for %s not available", name)
+    if provider_errors:
+        error_details = "; ".join(provider_errors)
+        raise ImageProviderError(f"Providers failed - {error_details}")
     raise AttributeError("Provider not available")