From a25db6219ad9f0911cb992536c3dc7fcd12f4c5a Mon Sep 17 00:00:00 2001 From: Lars Francke Date: Tue, 14 Jul 2026 22:42:46 +0200 Subject: [PATCH] fix(runner): error-handling & robustness in the test runner - command.py: the TimeoutExpired path returned a bare string while every other path returns a list of lines; callers iterate the output, so a timeout message was logged one character per line. Return a list. - provider_ionos/provider_replicated: the "wait for state" loops polled forever. A stuck API state hung the Jenkins executor and prevented terminate_cluster() from running, leaking billable cloud resources. They now honour a timeout (30 min) and return True/False; create_cluster aborts (returns None) when a wait times out. - operator-test-runner: check clone_git_repo()'s result; on failure, terminate the already-created cluster and abort instead of proceeding into a confusing SDP-install error. - jenkins-job-builder: add a 30s timeout to the per-operator tag lookup so a slow registry can't hang the whole job. - cluster.py: log and return on an unknown provider id instead of silently returning None/no-op. - Deduplicate the identical write_cluster_info_file() into command.py. --- apps/src/jenkins-job-builder.py | 4 +- apps/src/modules/cluster.py | 4 ++ apps/src/modules/command.py | 11 +++- apps/src/modules/provider_ionos.py | 73 +++++++++++++++++++------ apps/src/modules/provider_replicated.py | 30 ++++++---- apps/src/operator-test-runner.py | 5 +- 6 files changed, 95 insertions(+), 32 deletions(-) diff --git a/apps/src/jenkins-job-builder.py b/apps/src/jenkins-job-builder.py index 6889ba3..7f02abe 100644 --- a/apps/src/jenkins-job-builder.py +++ b/apps/src/jenkins-job-builder.py @@ -218,7 +218,9 @@ def read_chart_versions() -> dict[str, list[str]]: for op_name in ops: r = requests.get( - f"https://oci.stackable.tech/v2/sdp-charts/{op_name}/tags/list", auth=("user", "pass") + f"https://oci.stackable.tech/v2/sdp-charts/{op_name}/tags/list", + auth=("user", "pass"), + timeout=30, ) if r.status_code == 200: result[op_name] = sorted([tag for tag in r.json()["tags"] if not tag.endswith(".sig")]) diff --git a/apps/src/modules/cluster.py b/apps/src/modules/cluster.py index 50b3714..a5aed55 100644 --- a/apps/src/modules/cluster.py +++ b/apps/src/modules/cluster.py @@ -27,6 +27,8 @@ def create_cluster(provider_id, id, spec, platform_version, cluster_info_file, l return replicated.create_cluster(id, spec, platform_version, cluster_info_file, logger) if provider_id == "ionos": return ionos.create_cluster(id, spec, platform_version, cluster_info_file, logger) + logger(f"Unknown provider '{provider_id}'; cannot create a cluster.") + return None def terminate_cluster(provider_id, cluster, logger): @@ -41,3 +43,5 @@ def terminate_cluster(provider_id, cluster, logger): return replicated.terminate_cluster(cluster, logger) if provider_id == "ionos": return ionos.terminate_cluster(cluster, logger) + logger(f"Unknown provider '{provider_id}'; cannot terminate the cluster.") + return False diff --git a/apps/src/modules/command.py b/apps/src/modules/command.py index aaf9103..9d49e1c 100644 --- a/apps/src/modules/command.py +++ b/apps/src/modules/command.py @@ -50,7 +50,16 @@ def _run_command(command, description, timeout=60): except TimeoutExpired: proc.kill() _, errs = proc.communicate() - return -1, f"{description} timed out after {timeout} seconds." + # Return a list, like every other path: callers iterate the output + # line by line, and a bare string would be iterated character by character. + return -1, [f"{description} timed out after {timeout} seconds."] if proc.returncode != 0: return proc.returncode, output_to_string_array(errs) return 0, output_to_string_array(output) + + +def write_cluster_info_file(cluster_info_file): + """ + Writes a file containing basic cluster information (the node list). + """ + run_command(f"kubectl get nodes > {cluster_info_file}", "kubectl get nodes") diff --git a/apps/src/modules/provider_ionos.py b/apps/src/modules/provider_ionos.py index 3085028..0f51c95 100644 --- a/apps/src/modules/provider_ionos.py +++ b/apps/src/modules/provider_ionos.py @@ -2,9 +2,14 @@ This module creates IONOS clusters using the CLI """ -from time import sleep +from time import monotonic, sleep -from modules.command import run_command +from modules.command import run_command, write_cluster_info_file + +# Upper bound for the blocking "wait for state" polls, so a stuck IONOS API +# state can never hang a Jenkins job forever (which would also prevent the +# cluster from being terminated, leaking billable resources). +WAIT_TIMEOUT_SECONDS = 1800 def query_command_for_table(command, description): @@ -98,23 +103,34 @@ def delete_datacenter(id, logger): return True -def wait_for_datacenter_state(id, target_state, logger): +def wait_for_datacenter_state(id, target_state, logger, timeout=WAIT_TIMEOUT_SECONDS): """ Waits until a datacenter is in the desired state (blocking method) - Polls the CLI every 5 seconds. + Polls the CLI every 5 seconds, up to `timeout` seconds. id ID of the DC target_state desired state logger logger (String-consuming function) + timeout max seconds to wait before giving up + + Returns True if the target state was reached, False on timeout. """ + deadline = monotonic() + timeout datacenter = get_datacenter(id, logger) state = datacenter["State"] if datacenter else None logger(f"Datacenter {id} is in state {state}") while state != target_state: + if monotonic() >= deadline: + logger( + f"Timed out after {timeout}s waiting for datacenter {id} " + f"to reach state {target_state}." + ) + return False sleep(5) datacenter = get_datacenter(id, logger) state = datacenter["State"] if datacenter else None logger(f"Datacenter {id} is in state {state}") + return True def ionosctl_create_cluster(name, k8s_version, logger): @@ -172,23 +188,34 @@ def delete_cluster(id, logger): return True -def wait_for_cluster_state(id, target_state, logger): +def wait_for_cluster_state(id, target_state, logger, timeout=WAIT_TIMEOUT_SECONDS): """ Waits until a K8s cluster is in the desired state (blocking method) - Polls the CLI every 5 seconds. + Polls the CLI every 5 seconds, up to `timeout` seconds. id ID of the cluster target_state desired state logger logger (String-consuming function) + timeout max seconds to wait before giving up + + Returns True if the target state was reached, False on timeout. """ + deadline = monotonic() + timeout cluster = get_cluster(id, logger) state = cluster["State"] if cluster else None logger(f"Cluster {id} is in state {state}") while state != target_state: + if monotonic() >= deadline: + logger( + f"Timed out after {timeout}s waiting for cluster {id} " + f"to reach state {target_state}." + ) + return False sleep(5) cluster = get_cluster(id, logger) state = cluster["State"] if cluster else None logger(f"Cluster {id} is in state {state}") + return True def create_nodepool( @@ -256,24 +283,35 @@ def delete_nodepool(id, cluster_id, logger): return True -def wait_for_nodepool_state(id, cluster_id, target_state, logger): +def wait_for_nodepool_state(id, cluster_id, target_state, logger, timeout=WAIT_TIMEOUT_SECONDS): """ Waits until a K8s nodepool is in the desired state (blocking method) - Polls the CLI every 5 seconds. + Polls the CLI every 5 seconds, up to `timeout` seconds. id ID of the nodepool cluster_id ID of the K8s cluster target_state desired state logger logger (String-consuming function) + timeout max seconds to wait before giving up + + Returns True if the target state was reached, False on timeout. """ + deadline = monotonic() + timeout nodepool = get_nodepool(id, cluster_id, logger) state = nodepool["State"] if nodepool else None logger(f"Nodepool {id} is in state {state}") while state != target_state: + if monotonic() >= deadline: + logger( + f"Timed out after {timeout}s waiting for nodepool {id} " + f"to reach state {target_state}." + ) + return False sleep(5) nodepool = get_nodepool(id, cluster_id, logger) state = nodepool["State"] if nodepool else None logger(f"Nodepool {id} is in state {state}") + return True def update_kubeconfig(cluster_id, logger): @@ -291,13 +329,6 @@ def update_kubeconfig(cluster_id, logger): return True -def write_cluster_info_file(cluster_info_file): - """ - Writes a file containing basic cluster information - """ - run_command(f"kubectl get nodes > {cluster_info_file}", "kubectl get nodes") - - def create_cluster(id, spec, platform_version, cluster_info_file, logger): """ Creates an IONOS cluster with the given spec. (Blocking operation) @@ -324,7 +355,9 @@ def create_cluster(id, spec, platform_version, cluster_info_file, logger): logger("Error creating datacenter.") return None logger(f"Created datacenter '{cluster_name}' (id={datacenter_id})") - wait_for_datacenter_state(datacenter_id, "AVAILABLE", logger) + if not wait_for_datacenter_state(datacenter_id, "AVAILABLE", logger): + logger("Datacenter did not become available in time.") + return None logger(f"Datacenter '{cluster_name}' (id={datacenter_id}) is ready for use.") logger(f"Creating K8s cluster '{cluster_name}'...") @@ -333,7 +366,9 @@ def create_cluster(id, spec, platform_version, cluster_info_file, logger): logger("Error creating K8s cluster.") return None logger(f"Created K8s cluster '{cluster_name}' (id={cluster_id})") - wait_for_cluster_state(cluster_id, "ACTIVE", logger) + if not wait_for_cluster_state(cluster_id, "ACTIVE", logger): + logger("K8s cluster did not become active in time.") + return None logger(f"K8s Cluster '{cluster_name}' (id={cluster_id}) is ready for use.") logger(f"Creating nodepool '{cluster_name}'...") @@ -352,7 +387,9 @@ def create_cluster(id, spec, platform_version, cluster_info_file, logger): logger("Error creating nodepool.") return None logger(f"Created nodepool '{cluster_name}' (id={nodepool_id})") - wait_for_nodepool_state(nodepool_id, cluster_id, "ACTIVE", logger) + if not wait_for_nodepool_state(nodepool_id, cluster_id, "ACTIVE", logger): + logger("Nodepool did not become active in time.") + return None logger(f"Nodepool '{cluster_name}' (id={nodepool_id}) is ready for use.") logger("Updating kubeconfig...") diff --git a/apps/src/modules/provider_replicated.py b/apps/src/modules/provider_replicated.py index 546dce1..0cd161b 100644 --- a/apps/src/modules/provider_replicated.py +++ b/apps/src/modules/provider_replicated.py @@ -3,9 +3,13 @@ """ import re -from time import sleep +from time import monotonic, sleep -from modules.command import run_command +from modules.command import run_command, write_cluster_info_file + +# Upper bound for the blocking "wait for running" poll, so a stuck cluster +# state can never hang a Jenkins job forever. +WAIT_TIMEOUT_SECONDS = 1800 def api_call_create_cluster(cluster_name, spec, platform_version, logger): @@ -70,16 +74,23 @@ def get_cluster(name): return clusters.get(name, None) -def wait_for_running_cluster(name, logger): +def wait_for_running_cluster(name, logger, timeout=WAIT_TIMEOUT_SECONDS): """ Wait until the named cluster is in the state 'running' (blocking operation) name name of the cluster logger logger (String-consuming function) + timeout max seconds to wait before giving up + + Returns True if the cluster became 'running', False on timeout. """ + deadline = monotonic() + timeout state = None while state != "running": logger(f"Cluster '{name}' is in state {state}") + if monotonic() >= deadline: + logger(f"Timed out after {timeout}s waiting for cluster '{name}' to be running.") + return False sleep(5) cluster = get_cluster(name) state = cluster["state"] if cluster else None @@ -87,6 +98,8 @@ def wait_for_running_cluster(name, logger): logger(f"Cluster '{name}' is in state {state}") logger("Sleeping 10 seconds, as sometimes kubeconfig could not be generated directly afterwards") sleep(10) + return True + def update_kubeconfig(cluster, logger): """ @@ -102,13 +115,6 @@ def update_kubeconfig(cluster, logger): return True -def write_cluster_info_file(cluster_info_file): - """ - Writes a file containing basic cluster information - """ - run_command(f"kubectl get nodes > {cluster_info_file}", "kubectl get nodes") - - def create_cluster(id, spec, platform_version, cluster_info_file, logger): """ Creates a replicated.com cluster with the given spec. (Blocking operation) @@ -132,7 +138,9 @@ def create_cluster(id, spec, platform_version, cluster_info_file, logger): return None logger("Polling for the cluster to be up and running...") - wait_for_running_cluster(cluster_name, logger) + if not wait_for_running_cluster(cluster_name, logger): + logger("Cluster did not become running in time.") + return None cluster = get_cluster(cluster_name) diff --git a/apps/src/operator-test-runner.py b/apps/src/operator-test-runner.py index 10bf578..9c676cb 100644 --- a/apps/src/operator-test-runner.py +++ b/apps/src/operator-test-runner.py @@ -367,7 +367,10 @@ def write_logs_html(cluster_id, timestamp_start, timestamp_stop, opensearch_dash exit(EXIT_CODE_CLUSTER_FAILED) log("Cloning git repo...") - clone_git_repo(param_operator) + if not clone_git_repo(param_operator): + log("Could not clone the git repo; terminating the cluster and aborting.") + terminate_cluster(platform["provider"], cluster, log) + exit(EXIT_CODE_CLUSTER_FAILED) log("Waiting 1 minute for the cluster to become ready...") sleep(60)