Skip to content

Commit

Permalink
cloudinit/importer.py: print a meaningful message if import fails
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
esposem committed Jan 11, 2022
1 parent 3e64acd commit 1bfc607
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
28 changes: 21 additions & 7 deletions cloudinit/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cloudinit/sources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 1bfc607

Please sign in to comment.