chore: allow tenant cleanup script to skip control plane if tenant not found by wenxi-onyx · Pull Request #7290 · onyx-dot-app/onyx

wenxi-onyx · 2026-01-08T19:18:18Z

Description

How Has This Been Tested?

Additional Options

[Optional] Override Linear Check

Summary by cubic

Allow tenant cleanup and connector-marking scripts to skip control plane steps when the tenant is not found in the control plane. In force mode, we continue with data plane only; otherwise we prompt.

New Features
- Detect tenant not found in control plane (missing table or record) and raise TenantNotFoundInControlPlaneError.
- In --force, skip control-plane steps and proceed with data plane cleanup.
- In non-force, prompt before proceeding.
- Applied to bastion and no-bastion scripts, including mark-connectors.
Migration
- No-bastion scripts now require --data-plane-context and --control-plane-context.

^{Written for commit 3e7ce56. Summary will update on new commits.}

…t found

cubic-dev-ai

2 issues found across 4 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="backend/scripts/tenant_cleanup/no_bastion_cleanup_tenants.py">

<violation number="1" location="backend/scripts/tenant_cleanup/no_bastion_cleanup_tenants.py:494">
P1: Missing `tenant_not_found_in_control_plane = True` in interactive mode. When user confirms to continue after "tenant not found in control plane", the flag remains False, causing Step 3 to unnecessarily attempt control plane cleanup which will fail or have unexpected behavior.</violation>
</file>

<file name="backend/scripts/tenant_cleanup/cleanup_tenants.py">

<violation number="1" location="backend/scripts/tenant_cleanup/cleanup_tenants.py:476">
P2: Missing `tenant_not_found_in_control_plane = True` after user confirms to continue. In non-force mode, if the user acknowledges the tenant wasn't found and confirms to continue, the flag should also be set so Step 3 skips control plane cleanup (matching force mode behavior).</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

backend/scripts/tenant_cleanup/no_bastion_cleanup_tenants.py

backend/scripts/tenant_cleanup/cleanup_tenants.py

…ntrol plane

jmelahman

many small nits and I think they're all optional. Please do at least read them though

jmelahman · 2026-01-09T21:08:15Z

backend/scripts/tenant_cleanup/cleanup_tenants.py

+    except TenantNotFoundInControlPlaneError as e:
+        # Tenant/table not found in control plane
+        error_str = str(e)
+        print(f"⚠️  WARNING: Tenant not found in control plane: {error_str}")


this is fine, but it's probably preferred to use a logger for these so they're sent to stderr by default. slightly less likely to be missed that way

jmelahman · 2026-01-09T21:11:38Z

backend/scripts/tenant_cleanup/cleanup_utils.py

 from pathlib import Path


+class TenantNotFoundInControlPlaneError(Exception):


this is fine, but very nitpicky: it's nice to keep exceptions in a dedicated module. that's much more important with public interfaces though which this isn't really

jmelahman · 2026-01-09T21:13:08Z

backend/scripts/tenant_cleanup/cleanup_utils.py

+        error_msg = e.stderr if e.stderr else str(e)
        print(
-            f"✗ Failed to get tenant status for {tenant_id}: {e}",
+            f"✗ Failed to get tenant status for {tenant_id}: {error_msg}",


nit: while you're here wanna convert to an emoji for visibility (ideally just a colored logger imo but im boring)

jmelahman · 2026-01-09T21:18:03Z

backend/scripts/tenant_cleanup/no_bastion_mark_connectors.py

        print(
            "Usage: PYTHONPATH=. python scripts/tenant_cleanup/"
-            "no_bastion_mark_connectors.py <tenant_id> [--force] [--concurrency N]"
+            "no_bastion_mark_connectors.py <tenant_id> \\"


i know you're not introducing these prints, but the python argparser is pretty extensible and supports custom long messages. this is like a decades old pattern left over from shell scripting

jmelahman · 2026-01-09T21:19:57Z

backend/scripts/tenant_cleanup/no_bastion_cleanup_utils.py

-    cmd = ["kubectl", "get", "po"]
-    if context:
-        cmd.extend(["--context", context])
+    cmd = ["kubectl", "get", "po", "--context", context]


optional nitpick, but this would be so much better with a KubectlHelper class that stored the "kubectl" (preferably as /usr/bin/kubectl) command and context as private variables.

jmelahman · 2026-01-09T21:21:03Z

backend/scripts/tenant_cleanup/no_bastion_cleanup_tenants.py

+            "Error: --data-plane-context is required",
+            file=sys.stderr,
+        )
+        sys.exit(1)


nit: i personally prefer raise SystemExit(1)

jmelahman · 2026-01-09T21:22:04Z

backend/scripts/tenant_cleanup/no_bastion_cleanup_tenants.py

-    if confirm_step(
+    # Step 3: Clean up control plane (skip if tenant not found in control plane with --force)
+    if tenant_not_found_in_control_plane:
+        print(f"\n{'=' * 80}")


nit: I feel like this would be safer without an f-string

jmelahman · 2026-01-09T21:25:54Z

backend/scripts/tenant_cleanup/no_bastion_cleanup_tenants.py

        sys.exit(1)

-    # Parse contexts
+    # Parse contexts (required)


nitttt: these really should be required by the argparser and the corresponding types should not be optional

jmelahman · 2026-01-09T21:27:17Z

backend/scripts/tenant_cleanup/no_bastion_cleanup_tenants.py



-def setup_scripts_on_pod(pod_name: str, context: str | None = None) -> None:
+def setup_scripts_on_pod(pod_name: str, context: str) -> None:


its hard to tell if this was required for this change, but would have been really nice as a separate change so the behavioral changes related to skipping the control plane were more prominent

This was required. The intention of this script is to do actions in both planes, but the script would attempt to access control plane via data plane pods (which is impossible).

…t found (#7290)

chore: allow tenant cleanup script to skip control plane if tenant no…

f43a6e1

…t found

wenxi-onyx requested a review from a team as a code owner January 8, 2026 19:18

cubic-dev-ai bot reviewed Jan 8, 2026

View reviewed changes

backend/scripts/tenant_cleanup/no_bastion_cleanup_tenants.py Show resolved Hide resolved

backend/scripts/tenant_cleanup/cleanup_tenants.py Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

wenxi-onyx added 5 commits January 8, 2026 14:01

fix: no bastion cleanup scripts, require contexts

12d25cc

skip control plane in bastion script

8b96afb

correctly raise tenant not found error

75b9aef

update mark connectors script for contexts and tenant not found in co…

259533c

…ntrol plane

fix no tenant found error raising in bastion script

3e7ce56

jmelahman approved these changes Jan 9, 2026

View reviewed changes

wenxi-onyx added this pull request to the merge queue Jan 10, 2026

Merged via the queue into main with commit c7fc1cd Jan 10, 2026
74 of 75 checks passed

wenxi-onyx deleted the whuang/allow-tenant-cleanup-force-to-skip-control-plane branch January 10, 2026 00:21

jessicasingh7 pushed a commit that referenced this pull request Jan 12, 2026

chore: allow tenant cleanup script to skip control plane if tenant no…

08fb3d7

…t found (#7290)

		from pathlib import Path


		class TenantNotFoundInControlPlaneError(Exception):



		def setup_scripts_on_pod(pod_name: str, context: str \| None = None) -> None:
		def setup_scripts_on_pod(pod_name: str, context: str) -> None:

Conversation

wenxi-onyx commented Jan 8, 2026 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Additional Options

Summary by cubic

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

jmelahman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wenxi-onyx commented Jan 8, 2026 •

edited by cubic-dev-ai bot

Loading