Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 19 additions & 6 deletions tests/caas/test_cluster_create.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from __future__ import annotations

import contextlib
import subprocess
from pathlib import Path

from tests.core.grpc_client import GRPCClient
from tests.core.helpers import (
wait_for_cluster_deleting,
wait_for_cluster_deletion,
wait_for_cluster_grpc_deleting_or_archived,
wait_for_cluster_grpc_removal,
wait_for_cluster_order_cr,
wait_for_cluster_ready,
Expand All @@ -26,11 +30,20 @@ def test_cluster_create(
template_parameter_files={"pull_secret": pull_secret_path},
template_parameters={"ssh_public_key": Path(ssh_public_key_path).read_text().strip()},
)
co_name = wait_for_cluster_order_cr(k8s=k8s_hub_client, uuid=uuid)
assert uuid in grpc.list_cluster_ids()

wait_for_cluster_ready(k8s=k8s_hub_client, name=co_name)
try:
co_name = wait_for_cluster_order_cr(k8s=k8s_hub_client, uuid=uuid)
assert uuid in grpc.list_cluster_ids()

cli.delete_cluster(uuid=uuid)
wait_for_cluster_deletion(k8s=k8s_hub_client, name=co_name)
wait_for_cluster_grpc_removal(grpc=grpc, uuid=uuid)
wait_for_cluster_ready(k8s=k8s_hub_client, name=co_name)

cli.delete_cluster(uuid=uuid)

wait_for_cluster_deleting(k8s=k8s_hub_client, name=co_name)
wait_for_cluster_grpc_deleting_or_archived(grpc=grpc, uuid=uuid)

wait_for_cluster_deletion(k8s=k8s_hub_client, name=co_name)
wait_for_cluster_grpc_removal(grpc=grpc, uuid=uuid)
finally:
with contextlib.suppress(subprocess.CalledProcessError):
cli.delete_cluster(uuid=uuid)
Comment on lines +47 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Make fallback cleanup wait for deletion.

A failed readiness/deletion wait enters finally, but the fallback only starts deletion and lets the test exit before the ClusterOrder is gone. That can leak resources and skip wait_for_cluster_deletion’s force-cleanup path.

  • tests/caas/test_cluster_create.py#L47-L49: use the shared lifecycle cleanup context/fixture; conditionally delete and wait while the ClusterOrder exists.
  • tests/caas/test_cluster_delete_feedback_light.py#L50-L52: use the same cleanup pattern so failed assertions still complete teardown.
📍 Affects 2 files
  • tests/caas/test_cluster_create.py#L47-L49 (this comment)
  • tests/caas/test_cluster_delete_feedback_light.py#L50-L52
🤖 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/caas/test_cluster_create.py` around lines 47 - 49, Update the cleanup
blocks in tests/caas/test_cluster_create.py lines 47-49 and
tests/caas/test_cluster_delete_feedback_light.py lines 50-52 to use the shared
lifecycle cleanup context or fixture, conditionally delete while the
ClusterOrder exists, and wait for deletion to complete. Preserve cleanup on
failed assertions and ensure the wait_for_cluster_deletion force-cleanup path
can run.

Source: Learnings

52 changes: 52 additions & 0 deletions tests/caas/test_cluster_delete_feedback_light.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
from __future__ import annotations

import contextlib
import subprocess
from pathlib import Path

from tests.core.grpc_client import GRPCClient
from tests.core.helpers import (
wait_for_cluster_deleting,
wait_for_cluster_deletion,
wait_for_cluster_grpc_deleting_or_archived,
wait_for_cluster_grpc_removal,
wait_for_cluster_order_cr,
wait_for_cluster_progressing,
)
from tests.core.k8s_client import K8sClient
from tests.core.osac_cli import OsacCLI


def test_cluster_delete_reports_deleting_state_without_provisioning(
cli: OsacCLI,
grpc: GRPCClient,
k8s_hub_client: K8sClient,
cluster_template: str,
pull_secret_path: str,
ssh_public_key_path: str,
) -> None:
"""Verify that cluster deletion transitions through DELETING state
without waiting for full provisioning. Runs on kind without HyperShift
(OSAC-1586)."""

uuid = cli.create_cluster(
template=cluster_template,
template_parameter_files={"pull_secret": pull_secret_path},
template_parameters={"ssh_public_key": Path(ssh_public_key_path).read_text().strip()},
)

try:
co_name = wait_for_cluster_order_cr(k8s=k8s_hub_client, uuid=uuid)
assert uuid in grpc.list_cluster_ids()

wait_for_cluster_progressing(k8s=k8s_hub_client, name=co_name)

cli.delete_cluster(uuid=uuid)

wait_for_cluster_deleting(k8s=k8s_hub_client, name=co_name)
wait_for_cluster_grpc_deleting_or_archived(grpc=grpc, uuid=uuid)
wait_for_cluster_deletion(k8s=k8s_hub_client, name=co_name)
wait_for_cluster_grpc_removal(grpc=grpc, uuid=uuid)
finally:
with contextlib.suppress(subprocess.CalledProcessError):
cli.delete_cluster(uuid=uuid)
45 changes: 45 additions & 0 deletions tests/core/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,16 @@ def wait_for_cluster_order_cr(*, k8s: K8sClient, uuid: str) -> str:
)


def wait_for_cluster_progressing(*, k8s: K8sClient, name: str) -> None:
poll_until(
fn=lambda: k8s.get_cluster_order_phase(name=name, checked=False),
until=lambda v: v == "Progressing",
retries=30,
delay=2,
description=f"{name} ClusterOrder Progressing phase",
)


def wait_for_cluster_ready(*, k8s: K8sClient, name: str) -> None:
# Must stay safely above osac-aap's own wait_for_clusteroperators_retries
# budget (60 min) plus earlier steps in the same AAP job (create hosted
Expand Down Expand Up @@ -391,6 +401,41 @@ def _force_cleanup_machine_preterminate_hooks(*, k8s: K8sClient, name: str) -> N
run_unchecked(*base_args, "annotate", f"machines.cluster.x-k8s.io/{machine_name}", "-n", cp_ns, f"{hook}-")


def wait_for_cluster_deleting(*, k8s: K8sClient, name: str) -> None:
poll_until(
fn=lambda: k8s.get_cluster_order_phase(name=name, checked=False),
until=lambda v: v == "Deleting",
retries=30,
delay=5,
description=f"{name} ClusterOrder Deleting phase",
)


def wait_for_cluster_grpc_deleting_or_archived(*, grpc: GRPCClient, uuid: str) -> None:
"""Succeed if we catch CLUSTER_STATE_DELETING or if the cluster is already archived.

The DELETING window in the fulfillment-service is extremely short (one
Update + Signal round-trip). Polling for the exact state is racey; accepting
either DELETING or 'already gone' makes the assertion reliable.
"""

def _done() -> bool:
try:
cluster = grpc.get_cluster(cluster_id=uuid)
state = cluster.get("object", {}).get("status", {}).get("state", "")
return state == "CLUSTER_STATE_DELETING"
except subprocess.CalledProcessError:
return True

poll_until(
fn=_done,
until=lambda v: v is True,
retries=30,
delay=2,
description=f"{uuid} gRPC DELETING or already archived",
)
Comment on lines +422 to +436

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not treat every gRPC command failure as archival.

CalledProcessError also covers transient transport, authentication, and invocation failures, so this helper can report success without observing either CLUSTER_STATE_DELETING or a confirmed not-found/archive response. Distinguish the expected not-found response from other failures; retry or surface the latter.

🧰 Tools
🪛 Ruff (0.16.0)

[warning] 426-426: Consider moving this statement to an else block

(TRY300)

🤖 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/core/helpers.py` around lines 422 - 436, Update the _done helper to
treat only an explicitly confirmed not-found or archived response from
grpc.get_cluster as success; do not convert every subprocess.CalledProcessError
into True. Preserve the CLUSTER_STATE_DELETING check, while allowing transient
transport, authentication, and invocation failures to propagate or be retried by
poll_until.



Comment thread
coderabbitai[bot] marked this conversation as resolved.
def wait_for_cluster_grpc_removal(*, grpc: GRPCClient, uuid: str) -> None:
# retry_on_error=True: a flaky grpcurl call hitting a momentarily-busy
# route right after heavy cluster-deletion activity shouldn't fail the
Expand Down
Loading