Make exception handling explicit

cloudinit/analyze/show.py

@@ -6,6 +6,7 @@
 
 import datetime
 import json
+import logging
 import os
 import sys
 import time
@@ -53,6 +54,7 @@
 FAIL_CODE = "failure"
 CONTAINER_CODE = "container"
 TIMESTAMP_UNKNOWN = (FAIL_CODE, -1, -1, -1)
+LOG = logging.getLogger(__name__)
 
 
 def format_record(msg, event):
@@ -146,7 +148,7 @@ def subp(self):
                 return err
             self.epoch = value
             return None
-        except Exception as systemctl_fail:
+        except subp.ProcessExecutionError as systemctl_fail:
             return systemctl_fail
 
     def parse_epoch_as_float(self):
@@ -217,10 +219,11 @@ def gather_timestamps_using_dmesg():
                 # systemd wont start cloud-init in this case,
                 # so we cannot get that timestamp
                 return SUCCESS_CODE, kernel_start, kernel_end, kernel_end
-
-    except Exception:
-        pass
-    return TIMESTAMP_UNKNOWN
+    except (ValueError, subp.ProcessExecutionError):
+        return TIMESTAMP_UNKNOWN
+    except Exception as e:
+        LOG.warning("Unhandled exception: %s", e)
+        return TIMESTAMP_UNKNOWN
 
 
 def gather_timestamps_using_systemd():
@@ -261,10 +264,10 @@ def gather_timestamps_using_systemd():
         cloudinit_sysd = base_time + delta_ci_s
 
     except Exception as e:
+        LOG.warning("Unhandled exception: %s", e)
         # Except ALL exceptions as Systemctl reader can throw many different
         # errors, but any failure in systemctl means that timestamps cannot be
         # obtained
-        print(e)
         return TIMESTAMP_UNKNOWN
     return status, kernel_start, kernel_end, cloudinit_sysd
 

cloudinit/apport.py

@@ -128,7 +128,7 @@ def attach_cloud_info(report, ui=None):
                 instance_data = json.load(file)
                 assert instance_data.get("v1", {}).get("cloud_name")
                 return  # Valid instance-data means generic-hooks will report
-        except (IOError, json.decoder.JSONDecodeError, AssertionError):
+        except (OSError, json.decoder.JSONDecodeError, AssertionError):
             pass
 
         # No valid /run/cloud/instance-data.json on system. Prompt for cloud.

cloudinit/cloud.py

@@ -56,7 +56,7 @@ def cfg(self):
         # Ensure that cfg is not indirectly modified
         return copy.deepcopy(self._cfg)
 
-    def run(self, name, functor, args, freq=None, clear_on_fail=False):
+    def run(self, name, functor, args, freq=None):
         """Run a function gated by a named semaphore for a desired frequency.
 
         The typical case for this method would be to limit running of the
@@ -68,7 +68,7 @@ def run(self, name, functor, args, freq=None, clear_on_fail=False):
         even if they happen to be run in different boot stages or across
         reboots.
         """
-        return self._runners.run(name, functor, args, freq, clear_on_fail)
+        return self._runners.run(name, functor, args, freq)
 
     def get_template_filename(self, name):
         fn = self.paths.template_tpl % (name)

cloudinit/cmd/clean.py

@@ -8,6 +8,7 @@
 
 import argparse
 import glob
+import logging
 import os
 import sys
 
@@ -37,6 +38,7 @@
 GEN_SSH_CONFIG_FILES = [
     "/etc/ssh/sshd_config.d/50-cloud-init.conf",
 ]
+LOG = logging.getLogger(__name__)
 
 
 def get_parser(parser=None):
@@ -149,6 +151,7 @@ def remove_artifacts(remove_logs, remove_seed=False, remove_config=None):
     try:
         runparts(settings.CLEAN_RUNPARTS_DIR)
     except Exception as e:
+        LOG.warning("Unhandled exception: %s", e)
         error(
             f"Failure during run-parts of {settings.CLEAN_RUNPARTS_DIR}: {e}"
         )

cloudinit/cmd/cloud_id.py

@@ -76,7 +76,7 @@ def handle_args(name, args):
     try:
         with open(args.instance_data) as file:
             instance_data = json.load(file)
-    except IOError:
+    except OSError:
         return error(
             "File not found '%s'. Provide a path to instance data json file"
             " using --instance-data" % args.instance_data

cloudinit/cmd/devel/hotplug_hook.py

@@ -236,6 +236,12 @@ def handle_hotplug(hotplug_init: Init, devpath, subsystem, udevaction):
             event_handler.success()
             break
         except Exception as e:
+            # The number of possible exceptions handled here is large and
+            # difficult to audit. If retries fail this exception is always
+            # re-raised. Therefore, this callsite is allowed to be an exception
+            # to the rule that handle Exception must log a warning or reraise
+            # or call util.logexc() - since it does technically do this on
+            # timeout.
             LOG.debug("Exception while processing hotplug event. %s", e)
             time.sleep(wait)
             last_exception = e

cloudinit/cmd/devel/render.py

@@ -86,7 +86,7 @@ def render_template(user_data_path, instance_data_path=None, debug=False):
     try:
         with open(user_data_path) as stream:
             user_data = stream.read()
-    except IOError:
+    except OSError:
         LOG.error("Missing user-data file: %s", user_data_path)
         return 1
     try:

cloudinit/cmd/main.py

@@ -745,8 +745,12 @@ def status_wrapper(name, args):
     else:
         try:
             status = json.loads(util.load_text_file(status_path))
-        except Exception:
-            pass
+        except OSError:
+            LOG.warning("File %s not found %s.", status_path, mode)
+        except json.JSONDecodeError as e:
+            LOG.warning("File %s not valid json %s: %s", status_path, mode, e)
+        except Exception as e:
+            LOG.warning("Unhandled exception: %s", e)
 
     nullstatus = {
         "errors": [],

cloudinit/cmd/query.py

@@ -19,7 +19,6 @@
 import logging
 import os
 import sys
-from errno import EACCES
 
 from cloudinit import atomic_helper, util
 from cloudinit.cmd.devel import read_cfg_paths
@@ -147,7 +146,7 @@ def _read_instance_data(instance_data, user_data, vendor_data) -> dict:
     Non-root users will have redacted INSTANCE_JSON_FILE content and redacted
     vendordata and userdata values.
 
-    :raise: IOError/OSError on absence of instance-data.json file or invalid
+    :raise: OSError on absence of instance-data.json file or invalid
         access perms.
     """
     uid = os.getuid()
@@ -181,19 +180,19 @@ def _read_instance_data(instance_data, user_data, vendor_data) -> dict:
 
     try:
         instance_json = util.load_text_file(instance_data_fn)
-    except (IOError, OSError) as e:
-        if e.errno == EACCES:
-            LOG.error("No read permission on '%s'. Try sudo", instance_data_fn)
-        else:
-            LOG.error("Missing instance-data file: %s", instance_data_fn)
+    except PermissionError:
+        LOG.error("No read permission on '%s'. Try sudo", instance_data_fn)
+        raise
+    except OSError:
+        LOG.error("Missing instance-data file: %s", instance_data_fn)
         raise
 
     instance_data = util.load_json(instance_json)
     try:
         combined_cloud_config = util.load_json(
             util.load_text_file(combined_cloud_config_fn)
         )
-    except (IOError, OSError):
+    except OSError:
         # File will not yet be present in init-local stage.
         # It's created in `init` when vendor-data and user-data are processed.
         combined_cloud_config = None
@@ -274,7 +273,7 @@ def handle_args(name, args):
         instance_data = _read_instance_data(
             args.instance_data, args.user_data, args.vendor_data
         )
-    except (IOError, OSError):
+    except OSError:
         return 1
     if args.format:
         payload = "## template: jinja\n{fmt}".format(fmt=args.format)

cloudinit/config/cc_apt_configure.py

@@ -268,7 +268,7 @@ def apply_apt(cfg, cloud, gpg):
 
     try:
         apply_apt_config(cfg, APT_PROXY_FN, APT_CONFIG_FN)
-    except (IOError, OSError):
+    except OSError:
         LOG.exception("Failed to apply proxy or apt config info:")
 
     # Process 'apt_source -> sources {dict}'
@@ -852,7 +852,7 @@ def add_apt_sources(
                 omode = "w"
 
             util.write_file(sourcefn, contents, omode=omode)
-        except IOError as detail:
+        except OSError as detail:
             LOG.exception("failed write to file %s: %s", sourcefn, detail)
             raise
 

cloudinit/config/cc_bootcmd.py

@@ -81,10 +81,6 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
             util.logexc(LOG, "Failed to shellify bootcmd: %s", str(e))
             raise
 
-        try:
-            iid = cloud.get_instance_id()
-            env = {"INSTANCE_ID": str(iid)} if iid else {}
-            subp.subp(["/bin/sh", tmpf.name], update_env=env, capture=False)
-        except Exception:
-            util.logexc(LOG, "Failed to run bootcmd module %s", name)
-            raise
+        iid = cloud.get_instance_id()
+        env = {"INSTANCE_ID": str(iid)} if iid else {}
+        subp.subp(["/bin/sh", tmpf.name], update_env=env, capture=False)

cloudinit/config/cc_disk_setup.py

@@ -270,7 +270,7 @@ def enumerate_disk(device, nodeps=False):
     info = None
     try:
         info, _err = subp.subp(lsblk_cmd)
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         raise RuntimeError(
             "Failed during disk check for %s\n%s" % (device, e)
         ) from e
@@ -309,7 +309,11 @@ def is_device_valid(name, partition=False):
     d_type = ""
     try:
         d_type = device_type(name)
-    except Exception:
+    except RuntimeError:
+        LOG.warning("Query against device %s failed", name)
+        return False
+    except Exception as e:
+        LOG.warning("Unhandled exception: %s", e)
         LOG.warning("Query against device %s failed", name)
         return False
 
@@ -334,7 +338,7 @@ def check_fs(device):
     blkid_cmd = [BLKID_CMD, "-c", "/dev/null", device]
     try:
         out, _err = subp.subp(blkid_cmd, rcs=[0, 2])
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         raise RuntimeError(
             "Failed during disk check for %s\n%s" % (device, e)
         ) from e
@@ -440,7 +444,7 @@ def get_hdd_size(device):
     try:
         size_in_bytes, _ = subp.subp([BLKDEV_CMD, "--getsize64", device])
         sector_size, _ = subp.subp([BLKDEV_CMD, "--getss", device])
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         raise RuntimeError("Failed to get %s size\n%s" % (device, e)) from e
 
     return int(size_in_bytes) / int(sector_size)
@@ -458,7 +462,7 @@ def check_partition_mbr_layout(device, layout):
     prt_cmd = [SFDISK_CMD, "-l", device]
     try:
         out, _err = subp.subp(prt_cmd, data="%s\n" % layout)
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         raise RuntimeError(
             "Error running partition command on %s\n%s" % (device, e)
         ) from e
@@ -489,7 +493,7 @@ def check_partition_gpt_layout(device, layout):
     prt_cmd = [SGDISK_CMD, "-p", device]
     try:
         out, _err = subp.subp(prt_cmd, update_env=LANG_C_ENV)
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         raise RuntimeError(
             "Error running partition command on %s\n%s" % (device, e)
         ) from e
@@ -680,7 +684,7 @@ def purge_disk(device):
             try:
                 LOG.info("Purging filesystem on /dev/%s", d["name"])
                 subp.subp(wipefs_cmd)
-            except Exception as e:
+            except subp.ProcessExecutionError as e:
                 raise RuntimeError(
                     "Failed FS purge of /dev/%s" % d["name"]
                 ) from e
@@ -716,7 +720,7 @@ def read_parttbl(device):
     util.udevadm_settle()
     try:
         subp.subp(probe_cmd)
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         util.logexc(LOG, "Failed reading the partition table %s" % e)
 
     util.udevadm_settle()
@@ -731,7 +735,7 @@ def exec_mkpart_mbr(device, layout):
     prt_cmd = [SFDISK_CMD, "--force", device]
     try:
         subp.subp(prt_cmd, data="%s\n" % layout)
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         raise RuntimeError(
             "Failed to partition device %s\n%s" % (device, e)
         ) from e
@@ -759,7 +763,10 @@ def exec_mkpart_gpt(device, layout):
                 subp.subp(
                     [SGDISK_CMD, "-t", "{}:{}".format(index, pinput), device]
                 )
-    except Exception:
+    except subp.ProcessExecutionError:
+        LOG.warning("Failed to partition device %s", device)
+    except Exception as e:
+        LOG.warning("Unhandled exception: %s", e)
         LOG.warning("Failed to partition device %s", device)
         raise
 
@@ -1057,5 +1064,5 @@ def mkfs(fs_cfg):
     LOG.debug("     Using cmd: %s", str(fs_cmd))
     try:
         subp.subp(fs_cmd, shell=shell)
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         raise RuntimeError("Failed to exec of '%s':\n%s" % (fs_cmd, e)) from e

cloudinit/config/cc_growpart.py

@@ -8,6 +8,7 @@
 """Growpart: Grow partitions"""
 
 import base64
+import binascii
 import copy
 import json
 import logging
@@ -374,7 +375,7 @@ def resize_encrypted(blockdev, partition) -> Tuple[str, str]:
         key = keydata["key"]
         decoded_key = base64.b64decode(key)
         slot = keydata["slot"]
-    except Exception as e:
+    except (OSError, binascii.Error, json.JSONDecodeError) as e:
         raise RuntimeError(
             "Could not load encryption key. This is expected if "
             "the volume has been previously resized."
@@ -489,7 +490,16 @@ def resize_devices(resizer, devices, distro: Distro):
                             "as it is not encrypted.",
                         )
                     )
+            except OSError as e:
+                info.append(
+                    (
+                        devent,
+                        RESIZE.FAILED,
+                        f"Resizing encrypted device ({blockdev}) failed: {e}",
+                    )
+                )
             except Exception as e:
+                LOG.warning("Unhandled exception: %s", e)
                 info.append(
                     (
                         devent,

cloudinit/config/cc_grub_dpkg.py

@@ -155,7 +155,7 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
 
     try:
         subp.subp(["debconf-set-selections"], data=dconf_sel)
-    except Exception as e:
+    except subp.ProcessExecutionError as e:
         util.logexc(
             LOG, "Failed to run debconf-set-selections for grub_dpkg: %s", e
         )

cloudinit/config/cc_keys_to_console.py

@@ -102,10 +102,6 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
         cfg, "ssh_key_console_blacklist", []
     )
 
-    try:
-        cmd = [helper_path, ",".join(fp_blacklist), ",".join(key_blacklist)]
-        (stdout, _stderr) = subp.subp(cmd)
-        util.multi_log("%s\n" % (stdout.strip()), stderr=False, console=True)
-    except Exception:
-        LOG.warning("Writing keys to the system console failed!")
-        raise
+    cmd = [helper_path, ",".join(fp_blacklist), ",".join(key_blacklist)]
+    stdout, _stderr = subp.subp(cmd)
+    util.multi_log("%s\n" % (stdout.strip()), stderr=False, console=True)