Skip to content

Commit

Permalink
Raise custom FatalError for cleaner error msgs (#105)
Browse files Browse the repository at this point in the history
* Raise custom FatalError for cleaner error msgs

* Use entry point that handles FatalError

* Handle FatalError on all deploy cmds
  • Loading branch information
bsquizz authored Aug 30, 2021
1 parent 712b9d4 commit 1c7db2a
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 64 deletions.
83 changes: 51 additions & 32 deletions bonfire/bonfire.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
wait_for_clowd_env_target_ns,
wait_on_cji,
)
from bonfire.utils import split_equals, find_what_depends_on
from bonfire.utils import FatalError, split_equals, find_what_depends_on
from bonfire.local import get_local_apps
from bonfire.processor import TemplateProcessor, process_clowd_env, process_iqe_cji
from bonfire.namespaces import (
Expand Down Expand Up @@ -759,15 +759,16 @@ def _cmd_config_deploy(
clowd_env = match["metadata"]["name"]
log.debug("inferred clowd_env: '%s'", clowd_env)

def _err_handler():
def _err_handler(err):
try:
if not no_release_on_fail and not requested_ns and used_ns_reservation_system:
# if we auto-reserved this ns, auto-release it on failure unless
# --no-release-on-fail was requested
log.info("releasing namespace '%s'", ns)
release_namespace(ns)
finally:
_error("deploy failed")
msg = f"deploy failed: {str(err)}"
_error(msg)

try:
log.info("processing app templates...")
Expand Down Expand Up @@ -795,29 +796,26 @@ def _err_handler():
apply_config(ns, apps_config)
log.info("waiting on resources for max of %dsec...", timeout)
_wait_on_namespace_resources(ns, timeout)
except KeyboardInterrupt:
log.error("Aborted by keyboard interrupt!")
_err_handler()
except KeyboardInterrupt as err:
log.error("aborted by keyboard interrupt!")
_err_handler(err)
except TimedOutError as err:
log.error("Hit timeout error: %s", err)
_err_handler()
except Exception:
log.error("hit timeout error: %s", err)
_err_handler(err)
except FatalError as err:
log.error("hit fatal error: %s", err)
_err_handler(err)
except Exception as err:
log.exception("hit unexpected error!")
_err_handler()
_err_handler(err)
else:
log.info("successfully deployed to namespace '%s'", ns)
click.echo(ns)


def _process_clowdenv(target_namespace, quay_user, env_name, template_file):
env_name = _get_env_name(target_namespace, env_name)

try:
clowd_env_config = process_clowd_env(target_namespace, quay_user, env_name, template_file)
except ValueError as err:
_error(str(err))

return clowd_env_config
return process_clowd_env(target_namespace, quay_user, env_name, template_file)


@main.command("process-env")
Expand Down Expand Up @@ -849,6 +847,10 @@ def _cmd_deploy_clowdenv(
"""Process ClowdEnv template and deploy to a cluster"""
_warn_if_unsafe(namespace)

def _err_handler(err):
msg = f"deploy failed: {str(err)}"
_error(msg)

try:
if import_secrets:
import_secrets_from_dir(secrets_dir)
Expand All @@ -867,15 +869,18 @@ def _cmd_deploy_clowdenv(
_wait_on_namespace_resources(namespace, timeout)

clowd_env_name = find_clowd_env_for_ns(namespace)["metadata"]["name"]
except KeyboardInterrupt:
log.error("Aborted by keyboard interrupt!")
_error("deploy failed")
except KeyboardInterrupt as err:
log.error("aborted by keyboard interrupt!")
_err_handler(err)
except TimedOutError as err:
log.error("Hit timeout error: %s", err)
_error("deploy failed")
except Exception:
log.error("hit timeout error: %s", err)
_err_handler(err)
except FatalError as err:
log.error("hit fatal error: %s", err)
_err_handler(err)
except Exception as err:
log.exception("hit unexpected error!")
_error("deploy failed")
_err_handler(err)
else:
log.info("ClowdEnvironment '%s' using ns '%s' is ready", clowd_env_name, namespace)
click.echo(namespace)
Expand Down Expand Up @@ -912,6 +917,10 @@ def _cmd_deploy_iqe_cji(
"""Process IQE CJI template, apply it, and wait for it to start running."""
_warn_if_unsafe(namespace)

def _err_handler(err):
msg = f"deploy failed: {str(err)}"
_error(msg)

try:
cji_config = process_iqe_cji(
clowd_app_name, debug, marker, filter, env, image_tag, cji_name, template_file
Expand All @@ -928,15 +937,18 @@ def _cmd_deploy_iqe_cji(

log.info("waiting on CJI '%s' for max of %dsec...", cji_name, timeout)
pod_name = wait_on_cji(namespace, cji_name, timeout)
except KeyboardInterrupt:
log.error("Aborted by keyboard interrupt!")
_error("deploy failed")
except KeyboardInterrupt as err:
log.error("aborted by keyboard interrupt!")
_err_handler(err)
except TimedOutError as err:
log.error("Hit timeout error: %s", err)
_error("deploy failed")
except Exception:
log.error("hit timeout error: %s", err)
_err_handler(err)
except FatalError as err:
log.error("hit fatal error: %s", err)
_err_handler(err)
except Exception as err:
log.exception("hit unexpected error!")
_error("deploy failed")
_err_handler(err)
else:
log.info(
"pod '%s' related to CJI '%s' in ns '%s' is running", pod_name, cji_name, namespace
Expand Down Expand Up @@ -1004,5 +1016,12 @@ def _cmd_apps_what_depends_on(
print("\n".join(found) or f"no apps depending on {component} found")


def main_with_handler():
try:
main()
except FatalError as err:
_error(str(err))


if __name__ == "__main__":
main()
main_with_handler()
4 changes: 2 additions & 2 deletions bonfire/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from dotenv import load_dotenv

from bonfire.utils import load_file
from bonfire.utils import load_file, FatalError

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -83,7 +83,7 @@ def load_config(config_path=None):
log.debug("user provided explicit config path: %s", config_path)
config_path = Path(config_path)
if not config_path.exists():
raise ValueError(f"provided config file path '{str(config_path)}' does not exist")
raise FatalError(f"provided config file path '{str(config_path)}' does not exist")
else:
# no user-provided path, check default locations
config_path = Path("config.yaml")
Expand Down
8 changes: 4 additions & 4 deletions bonfire/local.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import yaml

from bonfire.utils import RepoFile, get_dupes
from bonfire.utils import RepoFile, get_dupes, FatalError

log = logging.getLogger(__name__)

Expand All @@ -20,12 +20,12 @@ def _fetch_apps_file(config):
fetched_apps = yaml.safe_load(content)

if "apps" not in fetched_apps:
raise ValueError("fetched apps file has no 'apps' key")
raise FatalError("fetched apps file has no 'apps' key")

app_names = [a["name"] for a in fetched_apps["apps"]]
dupes = get_dupes(app_names)
if dupes:
raise ValueError("duplicate app names found in fetched apps file: {dupes}")
raise FatalError("duplicate app names found in fetched apps file: {dupes}")

return {a["name"]: a for a in fetched_apps["apps"]}

Expand All @@ -34,7 +34,7 @@ def _parse_apps_in_cfg(config):
app_names = [a["name"] for a in config["apps"]]
dupes = get_dupes(app_names)
if dupes:
raise ValueError("duplicate app names found in config: {dupes}")
raise FatalError("duplicate app names found in config: {dupes}")
return {a["name"]: a for a in config["apps"]}


Expand Down
26 changes: 13 additions & 13 deletions bonfire/processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import bonfire.config as conf
from bonfire.openshift import process_template
from bonfire.utils import RepoFile
from bonfire.utils import RepoFile, FatalError
from bonfire.utils import get_dependencies as utils_get_dependencies

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -59,7 +59,7 @@ def process_clowd_env(target_ns, quay_user, env_name, template_path):
env_template_path = Path(template_path if template_path else conf.DEFAULT_CLOWDENV_TEMPLATE)

if not env_template_path.exists():
raise ValueError("ClowdEnvironment template file does not exist: %s", env_template_path)
raise FatalError("ClowdEnvironment template file does not exist: %s", env_template_path)

with env_template_path.open() as fp:
template_data = yaml.safe_load(fp)
Expand All @@ -75,7 +75,7 @@ def process_clowd_env(target_ns, quay_user, env_name, template_path):
processed_template = process_template(template_data, params=params)

if not processed_template.get("items"):
raise ValueError("Processed ClowdEnvironment template has no items")
raise FatalError("Processed ClowdEnvironment template has no items")

return processed_template

Expand All @@ -95,7 +95,7 @@ def process_iqe_cji(
template_path = Path(template_path if template_path else conf.DEFAULT_IQE_CJI_TEMPLATE)

if not template_path.exists():
raise ValueError("CJI template file does not exist: %s", template_path)
raise FatalError("CJI template file does not exist: %s", template_path)

with template_path.open() as fp:
template_data = yaml.safe_load(fp)
Expand All @@ -112,7 +112,7 @@ def process_iqe_cji(
processed_template = process_template(template_data, params=params)

if not processed_template.get("items"):
raise ValueError("Processed CJI template has no items")
raise FatalError("Processed CJI template has no items")

return processed_template

Expand Down Expand Up @@ -140,7 +140,7 @@ def _find_dupe_components(components_for_app):
if component in other_components:
found_in.append(other_app_name)
if len(found_in) > 1:
raise ValueError(
raise FatalError(
f"component '{component}' is not unique, found in apps: {found_in}"
)

Expand All @@ -151,18 +151,18 @@ def _validate_app_config(self, apps_config):
required_keys = ["name", "components"]
missing_keys = [k for k in required_keys if k not in app_cfg]
if missing_keys:
raise ValueError(f"app '{app_name}' is missing required keys: {missing_keys}")
raise FatalError(f"app '{app_name}' is missing required keys: {missing_keys}")

app_name = app_cfg["name"]
if app_name in components_for_app:
raise ValueError(f"app with name '{app_name}' is not unique")
raise FatalError(f"app with name '{app_name}' is not unique")
components_for_app[app_name] = []

for component in app_cfg.get("components", []):
required_keys = ["name", "host", "repo", "path"]
missing_keys = [k for k in required_keys if k not in component]
if missing_keys:
raise ValueError(
raise FatalError(
f"component on app {app_name} is missing required keys: {missing_keys}"
)
comp_name = component["name"]
Expand Down Expand Up @@ -209,7 +209,7 @@ def __init__(

def _get_app_config(self, app_name):
if app_name not in self.apps_config:
raise ValueError(f"app {app_name} not found in apps config")
raise FatalError(f"app {app_name} not found in apps config")
return self.apps_config[app_name]

def _get_component_config(self, component_name):
Expand All @@ -218,7 +218,7 @@ def _get_component_config(self, component_name):
if component["name"] == component_name:
return component
else:
raise ValueError(f"component with name '{component_name}' not found")
raise FatalError(f"component with name '{component_name}' not found")

def _sub_image_tags(self, items):
content = json.dumps(items)
Expand All @@ -238,7 +238,7 @@ def _sub_ref(self, current_component_name, repo_file):
elif len(split) == 1:
component_name = split[0]
else:
raise ValueError(
raise FatalError(
f"invalid format for template ref override: {app_component}={value}"
)

Expand All @@ -259,7 +259,7 @@ def _sub_params(self, current_component_name, params):
elif len(split) == 2:
component_name, param_name = split
else:
raise ValueError(f"invalid format for parameter override: {param_path}={value}")
raise FatalError(f"invalid format for parameter override: {param_path}={value}")

if current_component_name == component_name:
log.info(
Expand Down
10 changes: 5 additions & 5 deletions bonfire/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import os

from bonfire.openshift import oc, get_json
from bonfire.utils import load_file
from bonfire.utils import load_file, FatalError


log = logging.getLogger(__name__)
Expand All @@ -30,7 +30,7 @@ def _parse_secret_file(path):
try:
secrets[item["metadata"]["name"]] = item
except KeyError:
raise ValueError("Secret at path '{}' has no metadata/name".format(path))
raise FatalError("Secret at path '{}' has no metadata/name".format(path))

return secrets

Expand Down Expand Up @@ -61,10 +61,10 @@ def _import_secret(secret_name, secret_data):

def import_secrets_from_dir(path):
if not os.path.exists(path):
raise ValueError(f"secrets directory not found: {path}")
raise FatalError(f"secrets directory not found: {path}")

if not os.path.isdir(path):
raise ValueError(f"invalid secrets directory: {path}")
raise FatalError(f"invalid secrets directory: {path}")

files = _get_files_in_dir(path)
secrets = {}
Expand All @@ -74,7 +74,7 @@ def import_secrets_from_dir(path):
log.info("loaded %d secret(s) from file '%s'", len(secrets_in_file), secret_file)
for secret_name in secrets_in_file:
if secret_name in secrets:
raise ValueError(f"secret with name '{secret_name}' defined twice in secrets dir")
raise FatalError(f"secret with name '{secret_name}' defined twice in secrets dir")
secrets.update(secrets_in_file)

for secret_name, secret_data in secrets.items():
Expand Down
Loading

0 comments on commit 1c7db2a

Please sign in to comment.