From 2864b5172ead652758eab9cd17f142c064253396 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Tue, 23 Apr 2024 09:17:54 -0600 Subject: [PATCH] chore(exception): Replace errno with proper exceptions --- cloudinit/config/cc_mcollective.py | 25 +++++++--------------- cloudinit/config/cc_power_state_change.py | 14 ++++-------- cloudinit/config/cc_resizefs.py | 9 +++----- cloudinit/config/schema.py | 15 ++++++------- cloudinit/handlers/jinja_template.py | 1 - cloudinit/net/__init__.py | 4 +++- cloudinit/sources/DataSourceAltCloud.py | 11 ++++------ cloudinit/sources/DataSourceBigstep.py | 11 +++------- cloudinit/sources/DataSourceNoCloud.py | 6 ++---- cloudinit/sources/DataSourceRbxCloud.py | 6 ++---- cloudinit/sources/DataSourceSmartOS.py | 9 ++------ cloudinit/temp_utils.py | 7 ++---- tests/unittests/cmd/test_query.py | 7 +++--- tests/unittests/config/test_cc_growpart.py | 5 +---- tests/unittests/config/test_schema.py | 10 ++++----- tests/unittests/net/test_init.py | 6 ++---- tests/unittests/test_builtin_handlers.py | 3 +-- tests/unittests/test_util.py | 5 ++--- 18 files changed, 53 insertions(+), 101 deletions(-) diff --git a/cloudinit/config/cc_mcollective.py b/cloudinit/config/cc_mcollective.py index f17f1823b39..360cc0c09dc 100644 --- a/cloudinit/config/cc_mcollective.py +++ b/cloudinit/config/cc_mcollective.py @@ -9,9 +9,9 @@ """ Mcollective: Install, configure and start mcollective""" -import errno import io import logging +from contextlib import suppress from textwrap import dedent # Used since this can maintain comments @@ -100,15 +100,12 @@ def configure( try: old_contents = util.load_binary_file(server_cfg, quiet=False) mcollective_config = ConfigObj(io.BytesIO(old_contents)) - except OSError as e: - if e.errno != errno.ENOENT: - raise - else: - LOG.debug( - "Did not find file %s (starting with an empty config)", - server_cfg, - ) - mcollective_config = ConfigObj() + except FileNotFoundError: + LOG.debug( + "Did not find file %s (starting with an empty config)", + server_cfg, + ) + mcollective_config = ConfigObj() for cfg_name, cfg in config.items(): if cfg_name == "public-cert": util.write_file(pubcert_file, cfg, mode=0o644) @@ -133,16 +130,10 @@ def configure( # Otherwise just try to convert it to a string mcollective_config[cfg_name] = str(cfg) - try: + with suppress(FileNotFoundError): # We got all our config as wanted we'll copy # the previous server.cfg and overwrite the old with our new one util.copy(server_cfg, "%s.old" % (server_cfg)) - except OSError as e: - if e.errno == errno.ENOENT: - # Doesn't exist to copy... - pass - else: - raise # Now we got the whole (new) file, write to disk... contents = io.BytesIO() diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py index 401d18f1492..d72661c912b 100644 --- a/cloudinit/config/cc_power_state_change.py +++ b/cloudinit/config/cc_power_state_change.py @@ -6,7 +6,6 @@ """Power State Change: Change power state""" -import errno import logging import os import re @@ -229,8 +228,6 @@ def fatal(msg): LOG.warning(msg) doexit(EXIT_FAIL) - known_errnos = (errno.ENOENT, errno.ESRCH) - while True: if time.time() > end_time: msg = "timeout reached before %s ended" % pid @@ -241,14 +238,11 @@ def fatal(msg): if cmdline != pidcmdline: msg = "cmdline changed for %s [now: %s]" % (pid, cmdline) break - - except OSError as ioerr: - if ioerr.errno in known_errnos: - msg = "pidfile gone [%d]" % ioerr.errno - else: - fatal("OSError during wait: %s" % ioerr) + except (ProcessLookupError, FileNotFoundError): + msg = "pidfile gone" break - + except OSError as ioerr: + fatal("OSError during wait: %s" % ioerr) except Exception as e: fatal("Unexpected Exception: %s" % e) diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py index 39457946798..9f19a5360ac 100644 --- a/cloudinit/config/cc_resizefs.py +++ b/cloudinit/config/cc_resizefs.py @@ -8,7 +8,6 @@ """Resizefs: cloud-config module which resizes the filesystem""" -import errno import logging import os import re @@ -233,19 +232,17 @@ def maybe_get_writable_device_path(devpath, info): try: statret = os.stat(devpath) - except OSError as exc: - if container and exc.errno == errno.ENOENT: + except FileNotFoundError: + if container: LOG.debug( "Device '%s' did not exist in container. cannot resize: %s", devpath, info, ) - elif exc.errno == errno.ENOENT: + else: LOG.warning( "Device '%s' did not exist. cannot resize: %s", devpath, info ) - else: - raise exc return None if not stat.S_ISBLK(statret.st_mode) and not stat.S_ISCHR(statret.st_mode): diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 3a6e5651cb0..b8ae3bd372f 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -13,7 +13,6 @@ from contextlib import suppress from copy import deepcopy from enum import Enum -from errno import EACCES from functools import partial from itertools import chain from typing import ( @@ -1722,14 +1721,12 @@ def get_processed_or_fallback_path( try: paths = read_cfg_paths(fetch_existing_datasource="trust") - except OSError as e: - if e.errno == EACCES: - LOG.debug( - "Using default instance-data/user-data paths for non-root user" - ) - paths = read_cfg_paths() - else: - raise + except PermissionError: + LOG.debug( + "Using default instance-data/user-data " + "paths with insufficient permissions" + ) + paths = read_cfg_paths() except DataSourceNotFoundException: paths = read_cfg_paths() LOG.warning( diff --git a/cloudinit/handlers/jinja_template.py b/cloudinit/handlers/jinja_template.py index d0e2b3b6180..d9e3c2e453f 100644 --- a/cloudinit/handlers/jinja_template.py +++ b/cloudinit/handlers/jinja_template.py @@ -4,7 +4,6 @@ import logging import os import re -from errno import EACCES from typing import Optional, Type from cloudinit import handlers diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 5ad959db1ac..e5e41e6c736 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -78,8 +78,10 @@ def read_sys_net_safe(iface, field): iface, field, ) + except (FileNotFoundError, NotADirectoryError): + return False except OSError as e: - if e.errno in (errno.ENOENT, errno.ENOTDIR, errno.EINVAL): + if e.errno == errno.EINVAL: return False raise diff --git a/cloudinit/sources/DataSourceAltCloud.py b/cloudinit/sources/DataSourceAltCloud.py index c6cf0bfad01..c9ade29ddd2 100644 --- a/cloudinit/sources/DataSourceAltCloud.py +++ b/cloudinit/sources/DataSourceAltCloud.py @@ -12,7 +12,6 @@ instance on RHEVm and vSphere. """ -import errno import logging import os import os.path @@ -213,9 +212,8 @@ def user_data_rhevm(self): try: return_str = util.mount_cb(floppy_dev, read_user_data_callback) - except OSError as err: - if err.errno != errno.ENOENT: - raise + except FileNotFoundError: + pass except util.MountFailedError: util.logexc( LOG, @@ -253,9 +251,8 @@ def user_data_vsphere(self): if return_str: self.source = cdrom_dev break - except OSError as err: - if err.errno != errno.ENOENT: - raise + except FileNotFoundError: + pass except util.MountFailedError: util.logexc( LOG, diff --git a/cloudinit/sources/DataSourceBigstep.py b/cloudinit/sources/DataSourceBigstep.py index 043313b54d1..e28b0b29dc7 100644 --- a/cloudinit/sources/DataSourceBigstep.py +++ b/cloudinit/sources/DataSourceBigstep.py @@ -4,7 +4,6 @@ # # This file is part of cloud-init. See LICENSE file for license information. -import errno import json import os @@ -41,16 +40,12 @@ def _get_url_from_file(self): self.paths.cloud_dir, "data", "seed", "bigstep", "url" ) try: - content = util.load_text_file(url_file) - except OSError as e: + return util.load_text_file(url_file) + except FileNotFoundError: # If the file doesn't exist, then the server probably isn't a # Bigstep instance; otherwise, another problem exists which needs # investigation - if e.errno == errno.ENOENT: - return None - else: - raise - return content + return None # Used to match classes to dependencies diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py index 9cf68692430..036fdb48ca5 100644 --- a/cloudinit/sources/DataSourceNoCloud.py +++ b/cloudinit/sources/DataSourceNoCloud.py @@ -8,7 +8,6 @@ # # This file is part of cloud-init. See LICENSE file for license information. -import errno import logging import os from functools import partial @@ -142,9 +141,8 @@ def _pp2d_callback(mp, data): LOG.debug("Using data from %s", dev) found.append(dev) break - except OSError as e: - if e.errno != errno.ENOENT: - raise + except FileNotFoundError: + pass except util.MountFailedError: util.logexc( LOG, "Failed to mount %s when looking for data", dev diff --git a/cloudinit/sources/DataSourceRbxCloud.py b/cloudinit/sources/DataSourceRbxCloud.py index d3679102d23..68424491df2 100644 --- a/cloudinit/sources/DataSourceRbxCloud.py +++ b/cloudinit/sources/DataSourceRbxCloud.py @@ -9,7 +9,6 @@ This file contains code used to gather the user data passed to an instance on rootbox / hyperone cloud platforms """ -import errno import logging import os import os.path @@ -96,9 +95,8 @@ def get_md(): ) if rbx_data: return rbx_data - except OSError as err: - if err.errno != errno.ENOENT: - raise + except FileNotFoundError: + pass except util.MountFailedError: util.logexc( LOG, "Failed to mount %s when looking for user data", device diff --git a/cloudinit/sources/DataSourceSmartOS.py b/cloudinit/sources/DataSourceSmartOS.py index 7c642180cc7..f5aded12e8b 100644 --- a/cloudinit/sources/DataSourceSmartOS.py +++ b/cloudinit/sources/DataSourceSmartOS.py @@ -22,7 +22,6 @@ import base64 import binascii -import errno import fcntl import json import logging @@ -445,12 +444,8 @@ def as_ascii(): if byte == b"\n": return as_ascii() response.append(byte) - except OSError as exc: - if exc.errno == errno.EAGAIN: - raise JoyentMetadataTimeoutException( - msg % as_ascii() - ) from exc - raise + except BlockingIOError as e: + raise JoyentMetadataTimeoutException(msg % as_ascii()) from e def _write(self, msg): self.fp.write(msg.encode("ascii")) diff --git a/cloudinit/temp_utils.py b/cloudinit/temp_utils.py index 957433474aa..9481c732708 100644 --- a/cloudinit/temp_utils.py +++ b/cloudinit/temp_utils.py @@ -1,11 +1,11 @@ # This file is part of cloud-init. See LICENSE file for license information. import contextlib -import errno import logging import os import shutil import tempfile +from contextlib import suppress from cloudinit import util @@ -71,11 +71,8 @@ def ExtendedTemporaryFile(**kwargs): # file to unlink has been unlinked elsewhere.. def _unlink_if_exists(path): - try: + with suppress(FileNotFoundError): os.unlink(path) - except OSError as e: - if e.errno != errno.ENOENT: - raise e fh.unlink = _unlink_if_exists diff --git a/tests/unittests/cmd/test_query.py b/tests/unittests/cmd/test_query.py index a0836e977bb..97bee0f3920 100644 --- a/tests/unittests/cmd/test_query.py +++ b/tests/unittests/cmd/test_query.py @@ -1,6 +1,5 @@ # This file is part of cloud-init. See LICENSE file for license information. -import errno import gzip import json import os @@ -167,7 +166,7 @@ def test_handle_args_error_when_no_read_permission_instance_data( varname=None, ) with mock.patch(M_PATH + "util.load_binary_file") as m_load: - m_load.side_effect = OSError(errno.EACCES, "Not allowed") + m_load.side_effect = PermissionError("Not allowed") assert 1 == query.handle_args("anyname", args) msg = "No read permission on '%s'. Try sudo" % noread_fn assert msg in caplog.text @@ -175,8 +174,8 @@ def test_handle_args_error_when_no_read_permission_instance_data( @pytest.mark.parametrize( "exception", [ - (OSError(errno.EACCES, "Not allowed"),), - (OSError(errno.ENOENT, "Not allowed"),), + (PermissionError("Not allowed"),), + (FileNotFoundError("Not allowed"),), (OSError,), ], ) diff --git a/tests/unittests/config/test_cc_growpart.py b/tests/unittests/config/test_cc_growpart.py index 44eef5ee1ed..5e40beddd7d 100644 --- a/tests/unittests/config/test_cc_growpart.py +++ b/tests/unittests/config/test_cc_growpart.py @@ -1,7 +1,6 @@ # This file is part of cloud-init. See LICENSE file for license information. # pylint: disable=attribute-defined-outside-init -import errno import logging import os import re @@ -361,9 +360,7 @@ def mystat(path): if path in devs: return devstat_ret if path in enoent: - e = OSError("%s: does not exist" % path) - e.errno = errno.ENOENT - raise e + raise FileNotFoundError("%s: does not exist" % path) return real_stat(path) opinfo = self.distro.device_part_info diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 53574fcaec6..596c895a029 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -11,7 +11,6 @@ import unittest from collections import namedtuple from copy import deepcopy -from errno import EACCES from pathlib import Path from textwrap import dedent from types import ModuleType @@ -2589,8 +2588,11 @@ class TestHandleSchemaArgs: "failure, expected_logs", ( ( - OSError("No permissions on /var/lib/cloud/instance"), - ["Using default instance-data/user-data paths for non-root"], + PermissionError("No permissions on /var/lib/cloud/instance"), + [ + "Using default instance-data/user-data paths with " + "insufficient permissions" + ], ), ( DataSourceNotFoundException("No cached datasource found yet"), @@ -2609,8 +2611,6 @@ def test_handle_schema_unable_to_read_cfg_paths( caplog, tmpdir, ): - if isinstance(failure, OSError): - failure.errno = EACCES read_cfg_paths.side_effect = [failure, paths] user_data_fn = tmpdir.join("user-data") with open(user_data_fn, "w") as f: diff --git a/tests/unittests/net/test_init.py b/tests/unittests/net/test_init.py index 9b35a103740..6dcdeec3748 100644 --- a/tests/unittests/net/test_init.py +++ b/tests/unittests/net/test_init.py @@ -2,7 +2,6 @@ # pylint: disable=attribute-defined-outside-init import copy -import errno import ipaddress import os from pathlib import Path @@ -532,10 +531,9 @@ def setUp(self): def test_get_devicelist_raise_oserror(self): """get_devicelist raise any non-ENOENT OSerror.""" - error = OSError("Can not do it") - error.errno = errno.EPERM # Set non-ENOENT + error = PermissionError("Can not do it") self.m_sys_path.side_effect = error - with self.assertRaises(OSError) as context_manager: + with self.assertRaises(PermissionError) as context_manager: net.get_devicelist() exception = context_manager.exception self.assertEqual("Can not do it", str(exception)) diff --git a/tests/unittests/test_builtin_handlers.py b/tests/unittests/test_builtin_handlers.py index 4f3f3a8e415..fdcf41c0c1c 100644 --- a/tests/unittests/test_builtin_handlers.py +++ b/tests/unittests/test_builtin_handlers.py @@ -3,7 +3,6 @@ """Tests of the built-in user data handlers.""" import copy -import errno import os from textwrap import dedent @@ -191,7 +190,7 @@ def test_jinja_template_handle_errors_on_unreadable_instance_data(self): h = JinjaTemplatePartHandler(self.paths, sub_handlers=[script_handler]) with mock.patch(self.mpath + "load_text_file") as m_load: with self.assertRaises(JinjaLoadError) as context_manager: - m_load.side_effect = OSError(errno.EACCES, "Not allowed") + m_load.side_effect = PermissionError("Not allowed") h.handle_part( data="data", ctype="!" + handlers.CONTENT_START, diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 0fa1507dacd..0450e2bc82a 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -3,7 +3,6 @@ """Tests for cloudinit.util""" import base64 -import errno import io import json import logging @@ -542,7 +541,7 @@ def test_read_conf_with_config_invalid_jinja_syntax( @mock.patch( M_PATH + "read_conf", - side_effect=(OSError(errno.EACCES, "Not allowed"), {"0": "0"}), + side_effect=(PermissionError("Not allowed"), {"0": "0"}), ) def test_read_conf_d_no_permissions( self, m_read_conf, caplog, capsys, tmpdir @@ -577,7 +576,7 @@ def test_read_conf_d_no_permissions( @mock.patch(M_PATH + "mergemanydict") @mock.patch(M_PATH + "read_conf_d", return_value={"my_config": "foo"}) @mock.patch( - M_PATH + "read_conf", side_effect=OSError(errno.EACCES, "Not allowed") + M_PATH + "read_conf", side_effect=PermissionError("Not allowed") ) def test_read_conf_with_confd_no_permissions( self,