From 4a24b71847bba4a7c9ddd77d6b4caf4f7a9d92bb Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Fri, 3 May 2024 16:24:18 -0600 Subject: [PATCH] fixup! chore: Make exception handling more explicit --- cloudinit/config/cc_mounts.py | 7 ++----- cloudinit/config/cc_ssh_import_id.py | 10 ---------- cloudinit/config/modules.py | 8 +++++--- cloudinit/handlers/jinja_template.py | 5 ++++- cloudinit/sources/helpers/openstack.py | 14 ++++++++++++-- .../helpers/vmware/imc/config_custom_script.py | 2 +- .../sources/helpers/vmware/imc/guestcust_util.py | 2 +- cloudinit/stages.py | 14 +++++++++++--- cloudinit/util.py | 10 +++++++--- 9 files changed, 43 insertions(+), 29 deletions(-) diff --git a/cloudinit/config/cc_mounts.py b/cloudinit/config/cc_mounts.py index 17dfbce1c458..b5396f5f7e0e 100644 --- a/cloudinit/config/cc_mounts.py +++ b/cloudinit/config/cc_mounts.py @@ -406,13 +406,9 @@ def handle_swapcfg(swapcfg): LOG.warning( "swap file %s exists. Error reading /proc/swaps", fname ) - return fname except Exception as e: LOG.warning("Unhandled exception: %s", e) - LOG.warning( - "swap file %s exists. Error reading /proc/swaps", fname - ) - return fname + return fname try: if isinstance(size, str) and size != "auto": @@ -472,6 +468,7 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None: util.logexc( LOG, "Failed to parse devs from file %s", FSTAB_PATH ) + raise device_aliases = cfg.get("device_aliases", {}) diff --git a/cloudinit/config/cc_ssh_import_id.py b/cloudinit/config/cc_ssh_import_id.py index 52cba0034adc..6900845882bf 100644 --- a/cloudinit/config/cc_ssh_import_id.py +++ b/cloudinit/config/cc_ssh_import_id.py @@ -94,11 +94,6 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None: try: import_ids = util.uniq_merge(import_ids) import_ids = [str(i) for i in import_ids] - except (AttributeError, TypeError): - LOG.warning( - "User %s is not correctly configured for ssh_import_id", user - ) - continue except Exception: util.logexc(LOG, "Unhandled configuration for user %s", user) continue @@ -108,11 +103,6 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None: try: import_ssh_ids(import_ids, user) - except subp.ProcessExecutionError as exc: - util.logexc( - LOG, "ssh-import-id failed for: %s %s", user, import_ids - ) - elist.append(exc) except Exception as exc: util.logexc( LOG, "ssh-import-id failed for: %s %s", user, import_ids diff --git a/cloudinit/config/modules.py b/cloudinit/config/modules.py index f86a9ac3b17c..5b565e57b698 100644 --- a/cloudinit/config/modules.py +++ b/cloudinit/config/modules.py @@ -39,6 +39,8 @@ "cc_ubuntu_advantage": "cc_ubuntu_pro", # Renamed 24.1 } +RunOutput = Tuple[List[str], List[Tuple[str, Exception]]] + class ModuleDetails(NamedTuple): module: ModuleType @@ -248,7 +250,7 @@ def _fixup_modules(self, raw_mods) -> List[ModuleDetails]: def _run_modules( self, mostly_mods: List[ModuleDetails] - ) -> Tuple[List[str], List[Tuple[str, Exception]]]: + ) -> RunOutput: cc = self.init.cloudify() # Return which ones ran # and which ones failed + the exception of why it failed @@ -300,7 +302,7 @@ def _run_modules( def run_single( self, mod_name: str, args=None, freq=None - ) -> Tuple[List[str], List[Tuple[str, Exception]]]: + ) -> RunOutput: """Run a single module return: a tuple containing two lists: @@ -321,7 +323,7 @@ def run_single( def run_section( self, section_name: str - ) -> Tuple[List[str], List[Tuple[str, Exception]]]: + ) -> RunOutput: """Runs all modules in the given section. section_name - One of the modules lists as defined in diff --git a/cloudinit/handlers/jinja_template.py b/cloudinit/handlers/jinja_template.py index d9e3c2e453fa..b81259100ff3 100644 --- a/cloudinit/handlers/jinja_template.py +++ b/cloudinit/handlers/jinja_template.py @@ -122,7 +122,10 @@ def render_jinja_payload_from_file( " '%s'. Try sudo" % instance_data_file ) raise JinjaLoadError(msg) from e - except (OSError, ValueError) as e: + except (OSError, ValueError, TypeError) as e: + raise JinjaLoadError("Loading Jinja instance data failed") from e + except Exception as e: + LOG.warning("Unhandled exception: %s", e) raise JinjaLoadError("Loading Jinja instance data failed") from e rendered_payload = render_jinja_payload( diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index d633a7be0584..518ba6299f02 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -190,7 +190,7 @@ def _find_working_version(self): e, ) except Exception as e: - LOG.warning("Unhandled exception") + LOG.warning("Unhandled exception: %s", e) LOG.debug( "Unable to read openstack versions from %s due to: %s", self.base_path, @@ -317,6 +317,11 @@ def datafiles(version): raise BrokenMetadata( "Badly formatted metadata random_seed entry: %s" % e ) from e + except Exception as e: + LOG.warning("Unhandled exception: %s", e) + raise BrokenMetadata( + "Badly formatted metadata random_seed entry: %s" % e + ) from e # load any files that were provided files = {} @@ -403,7 +408,12 @@ def _read_ec2_metadata(self): else: try: return util.load_json(self._path_read(path)) - except OSError as e: + except (OSError, ValueError) as e: + raise BrokenMetadata( + "Failed to process path %s: %s" % (path, e) + ) from e + except Exception as e: + LOG.warning("Unhandled exception: %s", e) raise BrokenMetadata( "Failed to process path %s: %s" % (path, e) ) from e diff --git a/cloudinit/sources/helpers/vmware/imc/config_custom_script.py b/cloudinit/sources/helpers/vmware/imc/config_custom_script.py index c989f0f29c53..15c307e3fd8b 100644 --- a/cloudinit/sources/helpers/vmware/imc/config_custom_script.py +++ b/cloudinit/sources/helpers/vmware/imc/config_custom_script.py @@ -14,7 +14,7 @@ LOG = logging.getLogger(__name__) -class CustomScriptNotFound(OSError): +class CustomScriptNotFound(Exception): pass diff --git a/cloudinit/sources/helpers/vmware/imc/guestcust_util.py b/cloudinit/sources/helpers/vmware/imc/guestcust_util.py index f691607641d4..8a459dd84b9a 100644 --- a/cloudinit/sources/helpers/vmware/imc/guestcust_util.py +++ b/cloudinit/sources/helpers/vmware/imc/guestcust_util.py @@ -52,7 +52,7 @@ def send_rpc(rpc): # Remove the trailing newline in the output. if out: out = out.rstrip() - except OSError as e: + except subp.ProcessExecutionError as e: logger.debug("Failed to send RPC command") logger.exception(e) diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 855884e1a2d2..f32ace2d8ab0 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -477,7 +477,11 @@ def _reflect_cur_instance(self): try: previous_ds = util.load_text_file(ds_fn, quiet=True).strip() except OSError: - LOG.info("Couldn't load file %s", previous_ds) + pass + except UnicodeDecodeError: + LOG.warning("Invalid previous datasource in file: %s", previous_ds) + except Exception as e: + LOG.warning("Unhandled exception: %s", e) if not previous_ds: previous_ds = ds util.write_file(ds_fn, "%s\n" % ds) @@ -509,11 +513,15 @@ def previous_iid(self): dp = self.paths.get_cpath("data") iid_fn = os.path.join(dp, "instance-id") + self._previous_iid = NO_PREVIOUS_INSTANCE_ID try: self._previous_iid = util.load_text_file(iid_fn).strip() except OSError: - LOG.info("Couldn't load file %s", iid_fn) - self._previous_iid = NO_PREVIOUS_INSTANCE_ID + pass + except UnicodeDecodeError: + LOG.warning("Invalid instance id in file: %s", iid_fn) + except Exception as e: + LOG.warning("Unhandled exception: %s", e) LOG.debug("previous iid found to be %s", self._previous_iid) return self._previous_iid diff --git a/cloudinit/util.py b/cloudinit/util.py index 2abd46cf0a30..3773a9ecb7b2 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1391,10 +1391,14 @@ def search_for_mirror(candidates): LOG.debug("search for mirror in candidates: '%s'", candidates) for cand in candidates: - with suppress(ValueError): + try: if is_resolvable_url(cand): LOG.debug("found working mirror: '%s'", cand) return cand + except ValueError: + LOG.debug("Failed to parse url: %s", cand) + except Exception as e: + LOG.warning("Unhandled exception: %s", e) return None @@ -3309,11 +3313,11 @@ def read_hotplug_enabled_file(paths: "Paths") -> dict: content: dict = {"scopes": []} try: content = json.loads( - load_binary_file(paths.get_cpath("hotplug.enabled"), quiet=False) + load_text_file(paths.get_cpath("hotplug.enabled"), quiet=False) ) except FileNotFoundError: LOG.debug("File not found: %s", paths.get_cpath("hotplug.enabled")) - except json.JSONDecodeError as e: + except (json.JSONDecodeError, UnicodeDecodeError) as e: LOG.warning( "Ignoring contents of %s because it is not decodable. Error: %s", settings.HOTPLUG_ENABLED_FILE,