From 1bfc6071d88829659e8b1f77b9a8b8adeeeb8a30 Mon Sep 17 00:00:00 2001
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Date: Tue, 11 Jan 2022 10:33:04 +0100
Subject: [PATCH] cloudinit/importer.py: print a meaningful message if import
 fails

Sometimes an import might fail for different reasons: the string
is wrongly typed, or the module has a dependency that is not
installed in python.

We should print what the error is about, otherwise it might be
really difficult to understand what is the root cause of this
issue. Currently, cloud-init just ignores the error and continues.
This can have fatal consequences, especially when used to pick
the datasource to use.

For example, we can have a missing/misspelled datasource file:
$ ls cloudinit/sources/DataSourceInexistent.py
ls: cannot access 'cloudinit/sources/DataSourceInexistent.py': No such file or directory
$ python
>>> __import__("cloudinit.sources.DataSourceInexistent")

and in this case cloud-init should also print
"No module named 'cloudinit.sources.DataSourceInexistent'"

If instead the datasource exists but uses a dependency that
is not installed (for example netifaces for DataSourceVMware):
$ ls cloudinit/sources/DataSourceVMWare.py
cloudinit/sources/DataSourceVMWare.py
$ python
>>> __import__("cloudinit.sources.DataSourceVMware")

then cloud-init should print
"No module named 'netifaces'"

In order to not add unnecessary logging and modify existing
tests, add the logging as optional and use it only in
cloudinit/sources/__init__.py, to handle the cases explained
above.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 cloudinit/importer.py         | 28 +++++++++++++++++++++-------
 cloudinit/sources/__init__.py |  2 +-
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/cloudinit/importer.py b/cloudinit/importer.py
index f84ff4dab076..7d5023d06902 100644
--- a/cloudinit/importer.py
+++ b/cloudinit/importer.py
@@ -11,6 +11,10 @@
 import sys
 import typing
 
+from cloudinit import log as logging
+
+LOG = logging.getLogger(__name__)
+
 # annotations add value for development, but don't break old versions
 # pyver: 3.5 -> 3.8
 # pylint: disable=E1101
@@ -37,8 +41,22 @@ def import_module(module_name):
     return sys.modules[module_name]
 
 
-def find_module(base_name: str, search_paths, required_attrs=None) -> tuple:
-    """Finds and imports specified modules"""
+def import_module_handle_error(module_name, log=False):
+    try:
+        return import_module(module_name)
+    except ImportError as e:
+        if log:
+            LOG.debug("Could not import module %s: %s", module_name, str(e))
+        return None
+
+
+def find_module(
+    base_name: str, search_paths, required_attrs=None, log_error=False
+) -> tuple:
+    """
+    Finds and imports specified modules.
+    Use log_error to print the error in cloud-init logs
+    """
     if not required_attrs:
         required_attrs = []
     # NOTE(harlowja): translate the search paths to include the base name.
@@ -52,11 +70,7 @@ def find_module(base_name: str, search_paths, required_attrs=None) -> tuple:
         lookup_paths.append(full_path)
     found_paths = []
     for full_path in lookup_paths:
-        mod = None
-        try:
-            mod = import_module(full_path)
-        except ImportError:
-            pass
+        mod = import_module_handle_error(full_path, log=log_error)
         if not mod:
             continue
         found_attrs = 0
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index 9083f39953a5..598a02e6dcc6 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -927,7 +927,7 @@ def list_sources(cfg_list, depends, pkg_list):
         if not ds_name.startswith(DS_PREFIX):
             ds_name = "%s%s" % (DS_PREFIX, ds_name)
         m_locs, _looked_locs = importer.find_module(
-            ds_name, pkg_list, ["get_datasource_list"]
+            ds_name, pkg_list, ["get_datasource_list"], log_error=True
         )
         for m_loc in m_locs:
             mod = importer.import_module(m_loc)