Skip to content

Add request timeout to ClusterOperator API calls #5023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
OhadRevah wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add request timeout to ClusterOperator API calls #5023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
829a7c9
iuo, operator: Add request timeout to ClusterOperator API calls
OhadRevah May 27, 2026
fef672e
iuo, crypto_policy: Harden teardown against transient API and webhook…
OhadRevah May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions tests/install_upgrade_operators/crypto_policy/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,33 @@ def update_apiserver_crypto_policy(
hco_namespace=hco_namespace,
list_dependent_crs_to_check=MANAGED_CRS_LIST,
)
_wait_for_hco_webhook_ready(admin_client=admin_client, hco_namespace=hco_namespace)

Comment on lines +292 to +293

@coderabbitai coderabbitai Bot May 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

HIGH: PR template required sections are missing from the provided PR description

The PR metadata shown here does not include the required template section headers, and missing/empty What this PR does / why we need it blocks review traceability and release auditing.

As per coding guidelines "##### What this PR does / why we need it: — MUST be present AND have meaningful content."

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/install_upgrade_operators/crypto_policy/utils.py` around lines 292 -
293, The PR description is missing the required template header and content;
update the pull request description to include the header "##### What this PR
does / why we need it:" and provide a concise, meaningful summary describing the
change, its purpose, and any impact (e.g., why _wait_for_hco_webhook_ready
invocation with admin_client and hco_namespace is needed), so the PR metadata
meets the repository's template requirements and supports auditability.

@hmeir hmeir Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OhadRevah Please fix the PR description

@coderabbitai coderabbitai Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


def _get_hco_resources(admin_client: DynamicClient, namespace_name: str) -> list:
return list(HyperConverged.get(client=admin_client, namespace=namespace_name))


def _wait_for_hco_webhook_ready(admin_client: DynamicClient, hco_namespace: Resource) -> None:
"""Waits for the HCO webhook service to become reachable.

After APIServer TLS changes, the conversion webhook may briefly lose endpoints
even after cluster operators report stable. Reading the HyperConverged resource
exercises the conversion webhook, confirming it is functional before subsequent
HCO modifications.
"""
sampler = TimeoutSampler(
wait_timeout=TIMEOUT_2MIN,
sleep=10,
func=_get_hco_resources,
exceptions_dict={ApiException: []},
admin_client=admin_client,
namespace_name=hco_namespace.name,
)
Comment thread
OhadRevah marked this conversation as resolved.
for sample in sampler:
if sample:
LOGGER.info("HCO webhook service is ready.")
return


def check_service_accepts_tls_version(utility_pods: list, node: Node, service: Resource, tls_version: str) -> bool:
Expand Down
7 changes: 5 additions & 2 deletions utilities/operator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from datetime import datetime
from pprint import pformat

from kubernetes.client.exceptions import ApiException
from kubernetes.dynamic import DynamicClient
from kubernetes.dynamic.exceptions import ResourceNotFoundError
from ocp_resources.catalog_source import CatalogSource
Expand All @@ -31,6 +32,7 @@
TIMEOUT_10SEC,
TIMEOUT_15MIN,
TIMEOUT_20MIN,
TIMEOUT_30SEC,
TIMEOUT_75MIN,
)
from utilities.data_collector import collect_ocp_must_gather
Expand Down Expand Up @@ -558,10 +560,10 @@ def cluster_with_icsp():
return len(icsp_list) > 0


def get_cluster_operator_status_conditions(admin_client, operator_conditions=None):
def get_cluster_operator_status_conditions(admin_client, operator_conditions=None, request_timeout=TIMEOUT_30SEC):
operator_conditions = operator_conditions or DEFAULT_RESOURCE_CONDITIONS
cluster_operator_status = {}
for cluster_operator in list(ClusterOperator.get(client=admin_client)):
for cluster_operator in list(ClusterOperator.get(client=admin_client, _request_timeout=request_timeout)):
Comment thread
coderabbitai[bot] marked this conversation as resolved.
operator_name = cluster_operator.name
cluster_operator_status[operator_name] = {}
for condition in cluster_operator.instance.get("status", {}).get("conditions", []):
Expand Down Expand Up @@ -598,6 +600,7 @@ def wait_for_cluster_operator_stabilize(admin_client, wait_timeout=TIMEOUT_20MIN
wait_timeout=wait_timeout,
sleep=10,
func=get_failed_cluster_operator,
exceptions_dict={ApiException: []},
admin_client=admin_client,
)
consecutive_check = 0
Expand Down
Loading