From 2174c8f79f6d751327aab0ed70f0b6ee0eed6fe0 Mon Sep 17 00:00:00 2001 From: Brandon Squizzato Date: Tue, 8 Feb 2022 13:19:30 -0500 Subject: [PATCH] Fix bug where optional deps are not handled for already processed component --- bonfire/processor.py | 85 +++++++++++++++++++++++++++----------------- bonfire/utils.py | 20 +++++------ 2 files changed, 62 insertions(+), 43 deletions(-) diff --git a/bonfire/processor.py b/bonfire/processor.py index 9f71d9a1..ddb24cb4 100644 --- a/bonfire/processor.py +++ b/bonfire/processor.py @@ -175,6 +175,14 @@ def process_reservation(name, requester, duration, template_path=None, local=Tru return processed_template +class ProcessedComponent: + def __init__(self, name, items, deps_handled=False, optional_deps_handled=False): + self.name = name + self.items = items + self.deps_handled = deps_handled + self.optional_deps_handled = optional_deps_handled + + class TemplateProcessor: @staticmethod def _parse_app_names(app_names): @@ -356,7 +364,7 @@ def __init__( "items": [], } - self.processed_components = set() + self.processed_components = {} def _get_app_config(self, app_name): if app_name not in self.apps_config: @@ -463,9 +471,7 @@ def _frontend_found(items): break return frontend_found - def _add_dependencies_to_config(self, component_name, app_name, new_items, in_recursion): - dependencies = set() - + def _should_fetch_optional_deps(self, app_name, component_name, in_recursion): fetch_optional_deps = False if self.optional_deps_method == "all": fetch_optional_deps = True @@ -483,60 +489,75 @@ def _add_dependencies_to_config(self, component_name, app_name, new_items, in_re # via parsing of the original app's dependencies/optionalDependencies fetch_optional_deps = True log.debug( - "parsing optionalDependencies for component '%s' (in app group '%s')", + "parsing optionalDependencies for component '%s' (a member of app group '%s')", component_name, app_name, ) if not fetch_optional_deps: log.debug( - "ignoring optionalDependencies found on component '%s'", + "ignoring optionalDependencies for component '%s'", component_name, ) - dependencies_for_app = utils_get_dependencies( - new_items, include_optional=fetch_optional_deps - ) - for _, deps in dependencies_for_app.items(): - dependencies = dependencies.union(deps) + return fetch_optional_deps - # filter out ones we've already processed before - dependencies = [d for d in dependencies if d not in self.processed_components] + def _add_dependencies_to_config(self, app_name, processed_component, in_recursion): + component_name = processed_component.name + items = processed_component.items - log.debug("dependencies not previously processed: %s", dependencies) - if dependencies: - for component_name in dependencies: - self._process_component(component_name, app_name, in_recursion=True) + all_dependencies = set() - def _handle_dependencies(self, component_name, app_name, new_items, in_recursion): - if self._frontend_found(new_items) and "frontend-configs" not in self.processed_components: + if processed_component.deps_handled: + log.debug("already handled dependencies for component '%s'", component_name) + else: + dependencies_for_app = utils_get_dependencies(items) + for _, deps in dependencies_for_app.items(): + all_dependencies = all_dependencies.union(deps) + processed_component.deps_handled = True + + if processed_component.optional_deps_handled: + log.debug("already handled optionalDependencies for component '%s'", component_name) + elif self._should_fetch_optional_deps(app_name, component_name, in_recursion): + dependencies_for_app = utils_get_dependencies(items, optional=True) + for _, deps in dependencies_for_app.items(): + all_dependencies = all_dependencies.union(deps) + processed_component.optional_deps_handled = True + + for component_name in all_dependencies: + self._process_component(component_name, app_name, in_recursion=True) + + def _handle_dependencies(self, app_name, processed_component, in_recursion): + items = processed_component.items + if self._frontend_found(items) and "frontend-configs" not in self.processed_components: log.info("found a Frontend resource, auto-adding frontend-configs as dependency") self._process_component("frontend-configs", app_name, in_recursion) - self._add_dependencies_to_config(component_name, app_name, new_items, in_recursion) + self._add_dependencies_to_config(app_name, processed_component, in_recursion) def _process_component(self, component_name, app_name, in_recursion): - if component_name not in self.processed_components: + if component_name in self.processed_components: + log.debug("template already processed for component '%s'", component_name) + processed_component = self.processed_components[component_name] + else: log.info("processing component %s", component_name) - new_items = self._get_component_items(component_name) + items = self._get_component_items(component_name) # ignore frontends if we're not supposed to deploy them - if self._frontend_found(new_items) and not self.frontends: + if self._frontend_found(items) and not self.frontends: log.info( "ignoring component %s, user opted to disable frontend deployments", component_name, ) - new_items = [] + items = [] - if new_items: - self.k8s_list["items"].extend(new_items) - self.processed_components.add(component_name) + self.k8s_list["items"].extend(items) + processed_component = ProcessedComponent(component_name, items) + self.processed_components[component_name] = processed_component - if self.get_dependencies: - # recursively process to add config for dependent apps to self.k8s_list - self._handle_dependencies(component_name, app_name, new_items, in_recursion) - else: - log.debug("component %s already processed", component_name) + if self.get_dependencies: + # recursively process to add config for dependent apps to self.k8s_list + self._handle_dependencies(app_name, processed_component, in_recursion) def _process_app(self, app_name): log.info("processing app '%s'", app_name) diff --git a/bonfire/utils.py b/bonfire/utils.py index 7ecf4924..e1e0b4ce 100644 --- a/bonfire/utils.py +++ b/bonfire/utils.py @@ -344,27 +344,24 @@ def _fetch_local(self, repo_dir=None): return commit, fp.read() -def get_dependencies(items, include_optional=True): +def get_dependencies(items, optional=False): """ Returns dict of clowdapp_name: set of dependencies found for any ClowdApps in 'items' + if optional=True, returns set of optionalDependencies + 'items' is a list of k8s resources found in a template """ + key = "optionalDependencies" if optional else "dependencies" clowdapp_items = [item for item in items if item.get("kind").lower() == "clowdapp"] deps_for_app = dict() for clowdapp in clowdapp_items: name = clowdapp["metadata"]["name"] - dependencies = {d for d in clowdapp["spec"].get("dependencies", [])} - log.debug("clowdapp '%s' has dependencies: %s", name, list(dependencies)) - - optional_deps = set() - if include_optional: - optional_deps = {d for d in clowdapp["spec"].get("optionalDependencies", [])} - log.debug("clowdapp '%s' has optionalDependencies: %s", name, list(optional_deps)) - combined = dependencies.union(optional_deps) - deps_for_app[name] = combined + dependencies = {d for d in clowdapp["spec"].get(key, [])} + log.debug("clowdapp '%s' has %s: %s", name, key, list(dependencies)) + deps_for_app[name] = dependencies return deps_for_app @@ -385,7 +382,8 @@ def find_what_depends_on(apps_config, clowdapp_name): template = yaml.safe_load(template_content) items = template.get("objects", []) - dependencies = get_dependencies(items, include_optional=True) + dependencies = get_dependencies(items) + dependencies = dependencies.union(get_dependencies(items, optional=True)) for name, deps in dependencies.items(): # check if the name of the ClowdApp is set with a parameter