From 5c275aa62217ce6e7564c948cb2573cc494c1ad1 Mon Sep 17 00:00:00 2001 From: Thomas E Lackey Date: Wed, 14 Aug 2024 18:04:31 +0000 Subject: [PATCH] Defensively handle errors examining app requests. (#922) Related to https://git.vdb.to/cerc-io/webapp-deployment-status-api/issues/10 There are two issues in that. One is that the output probably changed recently, whether in the client or server, where no matching record is found by ID (Note this is specific to `laconic record get --id ` and does not seem to apply to the similar command to retrieve a record by name, `laconic name resolve `). Rather than returning `[]` it is now returning `[ null ]`. This cause us to think there *was* an application record found, and we attempt to treat the `null` entry like an Application object. That's fixed by filtering out null responses, which is a good precaution for the deployer, though I think it makes sense to ask whether this new behavior by the client/server is correct. Seems suspicious. The other issue is that all the defensive checks we had in place to deal with broken/bad AppDeploymentRequests were around the _build_. This error was coming much earlier, merely when parsing and examining the request to see if it needed to be handled at all. I have now added similar defensive error handling around that portion of the code. Reviewed-on: https://git.vdb.to/cerc-io/stack-orchestrator/pulls/922 Reviewed-by: zramsay Co-authored-by: Thomas E Lackey Co-committed-by: Thomas E Lackey --- .../webapp/deploy_webapp_from_registry.py | 65 +++++++++++-------- stack_orchestrator/deploy/webapp/util.py | 2 +- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/stack_orchestrator/deploy/webapp/deploy_webapp_from_registry.py b/stack_orchestrator/deploy/webapp/deploy_webapp_from_registry.py index 9031e798..9626bf7b 100644 --- a/stack_orchestrator/deploy/webapp/deploy_webapp_from_registry.py +++ b/stack_orchestrator/deploy/webapp/deploy_webapp_from_registry.py @@ -274,34 +274,43 @@ def command(ctx, kube_config, laconic_config, image_registry, deployment_parent_ requests_by_name = {} skipped_by_name = {} for r in requests: - if r.id in previous_requests and previous_requests[r.id].get("status", "") != "RETRY": - print(f"Skipping request {r.id}, we've already seen it.") - continue - - app = laconic.get_record(r.attributes.application) - if not app: - print("Skipping request %s, cannot locate app." % r.id) - continue - - requested_name = r.attributes.dns - if not requested_name: - requested_name = generate_hostname_for_app(app) - print("Generating name %s for request %s." % (requested_name, r.id)) - - if requested_name in skipped_by_name or requested_name in requests_by_name: - print("Ignoring request %s, it has been superseded." % r.id) - continue - - if skip_by_tag(r, include_tags, exclude_tags): - print("Skipping request %s, filtered by tag (include %s, exclude %s, present %s)" % (r.id, - include_tags, - exclude_tags, - r.attributes.tags)) - skipped_by_name[requested_name] = r - continue - - print("Found request %s to run application %s on %s." % (r.id, r.attributes.application, requested_name)) - requests_by_name[requested_name] = r + status = None + try: + if r.id in previous_requests and previous_requests[r.id].get("status", "") != "RETRY": + print(f"Skipping request {r.id}, we've already seen it.") + continue + + app = laconic.get_record(r.attributes.application) + if not app: + print(f"Skipping request {r.id}, cannot locate app.") + status = "SEEN" + continue + + requested_name = r.attributes.dns + if not requested_name: + requested_name = generate_hostname_for_app(app) + print("Generating name %s for request %s." % (requested_name, r.id)) + + if requested_name in skipped_by_name or requested_name in requests_by_name: + print("Ignoring request %s, it has been superseded." % r.id) + continue + + if skip_by_tag(r, include_tags, exclude_tags): + print("Skipping request %s, filtered by tag (include %s, exclude %s, present %s)" % (r.id, + include_tags, + exclude_tags, + r.attributes.tags)) + skipped_by_name[requested_name] = r + continue + + print("Found request %s to run application %s on %s." % (r.id, r.attributes.application, requested_name)) + requests_by_name[requested_name] = r + except Exception as e: + print(f"ERROR examining request {r.id}: " + str(e)) + status = "ERROR" + finally: + if status: + dump_known_requests(state_file, [r], status) # Find deployments. deployments = laconic.app_deployments() diff --git a/stack_orchestrator/deploy/webapp/util.py b/stack_orchestrator/deploy/webapp/util.py index cc476656..5061566e 100644 --- a/stack_orchestrator/deploy/webapp/util.py +++ b/stack_orchestrator/deploy/webapp/util.py @@ -172,7 +172,7 @@ def get_record(self, name_or_id, require=False): name_or_id, ] - parsed = [AttrDict(r) for r in json.loads(logged_cmd(self.log_file, *args))] + parsed = [AttrDict(r) for r in json.loads(logged_cmd(self.log_file, *args)) if r] if len(parsed): self._add_to_cache(parsed) return parsed[0]