From 590c1791455ff4547d582a808caaadb47a954866 Mon Sep 17 00:00:00 2001 From: Jacob Callahan Date: Tue, 18 Apr 2023 11:43:09 -0400 Subject: [PATCH 1/3] Add contextual awareness to host creation This change fixes an issue we're seeing where host entries, missing a hostname and ip, are being reconstructed for checkin. While that shouldn't be an issue during checkin, it would be if used for other purposes. With that in mind, this adds a check to see if the host is being reconstructed. If so, then it will just debug log instead of raising an error. --- broker/hosts.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/broker/hosts.py b/broker/hosts.py index 93a9f7c2..bdaf3ecf 100644 --- a/broker/hosts.py +++ b/broker/hosts.py @@ -16,7 +16,12 @@ def __init__(self, hostname=None, name=None, from_dict=False, **kwargs): else: self.hostname = hostname or kwargs.get("ip", None) if not self.hostname: - raise HostError("Host must be constructed with a hostname or ip") + # check to see if we're being reconstructued, likely for checkin + import inspect + if any(f.function == "reconstruct_host" for f in inspect.stack()): + logger.debug("Ignoring missing hostname and ip for checkin reconstruction.") + else: + raise HostError("Host must be constructed with a hostname or ip") self.name = name self.username = kwargs.get("username", settings.HOST_USERNAME) self.password = kwargs.get("password", settings.HOST_PASSWORD) From 0597d31d2e788bdbb51e5edb492c2e853cebfb16 Mon Sep 17 00:00:00 2001 From: Jacob Callahan Date: Tue, 18 Apr 2023 16:00:58 -0400 Subject: [PATCH 2/3] Add progress bar to inventory sync Some inventory syncs take a long time to complete. This change adds in a progress bar so users know there is actual progress and have an estimated completion time. --- broker/providers/ansible_tower.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/broker/providers/ansible_tower.py b/broker/providers/ansible_tower.py index c71f4343..1fc9ec29 100644 --- a/broker/providers/ansible_tower.py +++ b/broker/providers/ansible_tower.py @@ -549,7 +549,9 @@ def get_inventory(self, user=None): for inv in invs: inv_hosts = inv.get_related("hosts", page_size=200).results hosts.extend(inv_hosts) - return [self._compile_host_info(host) for host in hosts] + with click.progressbar(hosts, label='Compiling host information') as hosts_bar: + compiled_host_info = [self._compile_host_info(host) for host in hosts_bar] + return compiled_host_info def extend(self, target_vm, new_expire_time=None): """Run the extend workflow with defaults args From 427ea586164aec7c4b29c8854075fdf0fc1b8de4 Mon Sep 17 00:00:00 2001 From: Jacob Callahan Date: Fri, 28 Apr 2023 10:49:53 -0400 Subject: [PATCH 3/3] Add in some new workarounds and log statements We're seeing some strang behavior in CI. These are attempts to solve them and/or get more information about what's going on. --- broker/providers/__init__.py | 5 ++-- broker/providers/ansible_tower.py | 39 ++++++++++++++++++++----------- broker/providers/container.py | 2 +- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/broker/providers/__init__.py b/broker/providers/__init__.py index 604526d4..fe91e616 100644 --- a/broker/providers/__init__.py +++ b/broker/providers/__init__.py @@ -89,8 +89,9 @@ def _validate_settings(self, instance_name=None): if not inst_vals.get("override_envars"): # if a provider instance doesn't want to override envars, load them settings.execute_loaders(loaders=[dynaconf.loaders.env_loader]) - - settings.validators.extend([v for v in self._validators if v not in settings.validators]) + new_validators = [v for v in self._validators if v not in settings.validators] + logger.debug(f"Adding new validators: {[v.names[0] for v in new_validators]}") + settings.validators.extend(new_validators) # use selective validation to only validate the instance settings try: settings.validators.validate(only=section_name) diff --git a/broker/providers/ansible_tower.py b/broker/providers/ansible_tower.py index 1fc9ec29..250173cd 100644 --- a/broker/providers/ansible_tower.py +++ b/broker/providers/ansible_tower.py @@ -205,20 +205,31 @@ def _translate_inventory(self, inventory): provider="AnsibleTower", message=f"Unknown AnsibleTower inventory by id {inventory}", ) - if inventory_info := self.v2.inventory.get(search=inventory): - if inventory_info.count > 1: - raise exceptions.ProviderError( - provider="AnsibleTower", - message=f"Ambigious AnsibleTower inventory name {inventory}", - ) - elif inventory_info.count == 1: - inv_struct = inventory_info.results.pop() - return inv_struct.id - else: - raise exceptions.ProviderError( - provider="AnsibleTower", - message=f"Unknown AnsibleTower inventory {inventory}", - ) + elif isinstance(inventory, str): + if inventory_info := self.v2.inventory.get(search=inventory): + if inventory_info.count > 1: + raise exceptions.ProviderError( + provider="AnsibleTower", + message=f"Ambigious AnsibleTower inventory name {inventory}", + ) + elif inventory_info.count == 1: + inv_struct = inventory_info.results.pop() + return inv_struct.id + else: + raise exceptions.ProviderError( + provider="AnsibleTower", + message=f"Unknown AnsibleTower inventory {inventory}", + ) + elif inv_id := getattr(inventory, "id", None): + return inv_id + elif inv_name := getattr(inventory, "name", None): + return inv_name + else: + caller_context = inspect.stack()[1][0].f_locals + raise exceptions.ProviderError( + provider="AnsibleTower", + message=f"Ambiguous AnsibleTower inventory {inventory} passed from {caller_context}", + ) def _merge_artifacts(self, at_object, strategy="last", artifacts=None): """Gather and merge all artifacts associated with an object and its children diff --git a/broker/providers/container.py b/broker/providers/container.py index 8a4790f7..0471390f 100644 --- a/broker/providers/container.py +++ b/broker/providers/container.py @@ -212,7 +212,7 @@ def construct_host(self, provider_params, host_classes, **kwargs): def nick_help(self, **kwargs): """Useful information about container images""" - results_limit = kwargs.get("results_limit", settings.CONTAINER.results_limit) + results_limit = kwargs.get("results_limit", settings.container.results_limit) if image := kwargs.get("container_host"): logger.info( f"Information for {image} container-host:\n"