Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions tests/helpers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
default_labels,
default_region,
env,
run_id,
sleep,
unique_name,
wait_for_sandbox_deletion,
wait_for_sandbox_deployed,
wait_for_volume_deletion,
wait_until,
)

__all__ = [
Expand All @@ -19,9 +21,11 @@
"default_labels",
"default_region",
"env",
"run_id",
"sleep",
"unique_name",
"wait_for_sandbox_deletion",
"wait_for_sandbox_deployed",
"wait_for_volume_deletion",
"wait_until",
]
110 changes: 110 additions & 0 deletions tests/helpers/echo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
"""Self-hosted httpbin replacement for the proxy integration tests.

The proxy suite needs an external HTTPS endpoint that echoes back the headers
and body it received. It used to call ``httpbin.org``, which rate-limits the
shared Blaxel egress IPs and answers ``503`` in bursts -- taking ~30 tests down
with it on roughly half of the CI runs.

Instead we host the echo ourselves: one sandbox, one node server, one public
preview. Setup costs ~3s per pytest session and the endpoint is a real external
HTTPS host from the sandbox's point of view, so the proxy/firewall paths under
test are exercised exactly the same way.
"""

from __future__ import annotations

from urllib.parse import urlparse

from blaxel.core.sandbox import SandboxInstance

from .utils import default_image, default_labels, default_region, unique_name

# Serves the subset of httpbin routes the proxy suite actually used:
# /headers, /get, /post, /put, /delete, /redirect/N, /bytes/N.
ECHO_SERVER_SCRIPT = r"""
const http = require("http");

http.createServer((req, res) => {
let body = "";
req.on("data", (c) => (body += c));
req.on("end", () => {
if (req.url.startsWith("/redirect/")) {
const n = parseInt(req.url.split("/")[2] || "1", 10);
res.writeHead(302, { Location: n > 1 ? "/redirect/" + (n - 1) : "/get" });
res.end();
return;
}
if (req.url.startsWith("/bytes/")) {
const n = parseInt(req.url.split("/")[2] || "0", 10);
res.writeHead(200, { "Content-Type": "application/octet-stream" });
res.end(Buffer.alloc(n, "a"));
return;
}
let parsed = null;
try {
parsed = body ? JSON.parse(body) : null;
} catch (e) {}
const host = req.headers["x-forwarded-host"] || req.headers.host;
const proto = req.headers["x-forwarded-proto"] || "https";
res.writeHead(200, { "Content-Type": "application/json" });
res.end(
JSON.stringify({
headers: req.headers,
method: req.method,
url: proto + "://" + host + req.url,
data: body,
json: parsed,
args: {},
})
);
});
}).listen(3000, "0.0.0.0", () => console.log("echo listening on 3000"));
""".strip()

_ECHO_PORT = 3000

# Cached for the lifetime of the pytest process: every test class reuses the
# same endpoint instead of paying the setup again.
_echo_url: str | None = None


async def echo_url() -> str:
"""Return the base HTTPS URL of the shared echo server, creating it if needed.

The sandbox carries ``default_labels`` so the session-level cleanup in
``tests/integration/core/conftest.py`` deletes it with everything else.
"""
global _echo_url
if _echo_url is not None:
return _echo_url

sandbox = await SandboxInstance.create_if_not_exists(
{
"name": unique_name("echo"),
"image": default_image,
"region": default_region,
"labels": default_labels,
}
)
await sandbox.fs.write("/tmp/echo-server.js", ECHO_SERVER_SCRIPT)
await sandbox.process.exec(
{
"name": "echo-server",
"command": "node /tmp/echo-server.js",
"wait_for_ports": [_ECHO_PORT],
}
)
preview = await sandbox.previews.create_if_not_exists(
{
"metadata": {"name": "echo"},
"spec": {"port": _ECHO_PORT, "public": True},
}
)

_echo_url = preview.spec.url
return _echo_url
Comment thread
mendral-app[bot] marked this conversation as resolved.
Outdated


async def echo_host() -> str:
"""Hostname of the echo server, for firewall allow/deny lists."""
return urlparse(await echo_url()).hostname
22 changes: 22 additions & 0 deletions tests/helpers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,17 @@
default_region = "eu-dub-1" if env == "dev" else "us-pdx-1"
default_image = "blaxel/base-image:latest"

# Unique per pytest process. CI runs of several PRs share one workspace, so the
# end-of-session cleanup must only delete what *this* run created -- deleting by
# ``env=integration-test`` alone tears down sandboxes a concurrent run is still
# using, which is a large part of the suite's cross-run flakiness.
run_id = uuid.uuid4().hex[:12]

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.

🟡 Test cleanup deletes nothing when the suite is run in parallel

Each parallel test process generates its own random run tag (run_id = uuid.uuid4().hex[:12] at tests/helpers/utils.py:20), while the end-of-run cleanup only deletes resources tagged with the tag of the coordinating process, so nothing created by the parallel workers is ever cleaned up.
Impact: When tests run in parallel, every test sandbox is left running in the workspace instead of being deleted.

Why the master's run-id never matches a worker's

run_id is module-level state computed at import time, so with pytest-xdist each worker process imports tests.helpers separately and gets a different value. Sandboxes are created inside workers (with default_labels carrying the worker's run-id), but pytest_sessionfinish runs only on the master (tests/integration/core/conftest.py:43-44) and filters with labels.get("run-id") == run_id (tests/integration/core/conftest.py:52) using the master's own value. The fallback branch only matches resources older than 2 hours, so freshly created worker sandboxes survive. The previous implementation deleted by env=integration-test, which was worker-agnostic.

A fix is to derive the run id from a value shared across workers, e.g. propagate the master's id through an environment variable (os.environ.setdefault("BL_TEST_RUN_ID", uuid4().hex[:12])) so workers inherit it.

Suggested change
# Unique per pytest process. CI runs of several PRs share one workspace, so the
# end-of-session cleanup must only delete what *this* run created -- deleting by
# ``env=integration-test`` alone tears down sandboxes a concurrent run is still
# using, which is a large part of the suite's cross-run flakiness.
run_id = uuid.uuid4().hex[:12]
# Unique per pytest run. CI runs of several PRs share one workspace, so the
# end-of-session cleanup must only delete what *this* run created -- deleting by
# ``env=integration-test`` alone tears down sandboxes a concurrent run is still
# using, which is a large part of the suite's cross-run flakiness.
# Stored in the environment so pytest-xdist workers inherit the master's id.
run_id = os.environ.setdefault("BL_TEST_RUN_ID", uuid.uuid4().hex[:12])
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


# Default labels to identify test sandboxes in the UI
default_labels = {
"env": "integration-test",
"created-by": "pytest",
"run-id": run_id,
}


Expand Down Expand Up @@ -107,6 +114,21 @@ async def wait_for_volume_deletion(volume_name: str, max_attempts: int = 30) ->
return False


async def wait_until(predicate, timeout: float = 10.0, interval: float = 0.1) -> bool:
"""Poll ``predicate`` until it is true or ``timeout`` elapses.

Callbacks (watch events, log streams) usually fire in well under a second,
but a fixed sleep turns a slow round-trip into a test failure. Polling keeps
the fast path fast and the slow path green.
"""
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if predicate():
return True
await asyncio.sleep(interval)
return predicate()


def sleep(seconds: float) -> None:
"""Synchronous sleep helper."""
time.sleep(seconds)
Expand Down
78 changes: 48 additions & 30 deletions tests/integration/core/conftest.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,42 @@
"""Pytest configuration for core integration tests."""

import asyncio
from datetime import datetime, timedelta, timezone

# Orphans older than this were left behind by a crashed run, never by a live one.
ORPHAN_MAX_AGE = timedelta(hours=2)


def _labels(resource) -> dict:
metadata = getattr(resource, "metadata", None)
labels = getattr(metadata, "labels", None) if metadata else None
return getattr(labels, "additional_properties", {}) or {}


def _is_stale_orphan(resource) -> bool:
"""True for a pytest resource old enough that no live run still needs it."""
created_at = getattr(getattr(resource, "metadata", None), "created_at", None)
if not isinstance(created_at, str):
return False
try:
# Timestamps come back with nanosecond precision, which fromisoformat
# rejects on older Pythons -- truncate to microseconds.
head, _, tail = created_at.partition(".")
created = datetime.fromisoformat(f"{head}.{tail[:6]}+00:00" if tail else f"{head}+00:00")
except ValueError:
return False

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.

🟡 Leftover test sandboxes from crashed runs are never cleaned up for common timestamp formats

The creation time of a leftover sandbox is reformatted by blindly appending a UTC offset (datetime.fromisoformat(...) at tests/integration/core/conftest.py:25) without removing the trailing Z, so any timestamp that is not fractional-with-at-least-six-digits fails to parse and the sandbox is silently treated as not old, meaning it is never deleted.
Impact: Sandboxes left behind by crashed test runs accumulate in the workspace forever instead of being swept after two hours.

Parsing failure cases

RFC3339 timestamps from the control plane commonly look like 2025-08-18T01:58:22Z (no fraction) or 2025-08-18T01:58:22.123Z (fewer than 6 fractional digits). In the first case tail is empty and the code builds "2025-08-18T01:58:22Z+00:00"; in the second tail[:6] keeps the Z, producing "...123Z+00:00". Both raise ValueError (Python 3.10 fromisoformat also rejects Z), and the except ValueError: return False branch makes _is_stale_orphan always return False. The rest of the codebase normalizes with .replace("Z", "+00:00") (e.g. src/blaxel/core/sandbox/types.py:37).

Suggested change
try:
# Timestamps come back with nanosecond precision, which fromisoformat
# rejects on older Pythons -- truncate to microseconds.
head, _, tail = created_at.partition(".")
created = datetime.fromisoformat(f"{head}.{tail[:6]}+00:00" if tail else f"{head}+00:00")
except ValueError:
return False
try:
# Timestamps come back with nanosecond precision and a trailing ``Z``,
# which fromisoformat rejects on older Pythons -- normalise both.
stamp = created_at.strip().rstrip("Z")
head, _, tail = stamp.partition(".")
created = datetime.fromisoformat(f"{head}.{tail[:6]}+00:00" if tail else f"{head}+00:00")
except ValueError:
return False
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

return datetime.now(timezone.utc) - created > ORPHAN_MAX_AGE


def pytest_sessionfinish(session, exitstatus):
"""Clean up all test sandboxes after the test session ends.
"""Clean up the sandboxes this run created.

With pytest-xdist, this only runs on the master node after all workers finish.

Only sandboxes tagged with this run's ``run-id`` are deleted, plus stale
orphans from runs that crashed. CI runs several pull requests against the
same workspace: deleting every ``env=integration-test`` sandbox would tear
down sandboxes a concurrent run is still using.
"""
# Skip cleanup on worker nodes (pytest-xdist)
# Workers have workerinput attribute, master doesn't
Expand All @@ -15,46 +45,34 @@ def pytest_sessionfinish(session, exitstatus):

from blaxel.core.client.client import client
from blaxel.core.sandbox import SandboxInstance
from blaxel.core.volume import VolumeInstance
from tests.helpers import run_id

def is_ours(resource) -> bool:
labels = _labels(resource)
if labels.get("run-id") == run_id:
return True
return labels.get("created-by") == "pytest" and _is_stale_orphan(resource)

async def cleanup_test_resources():
"""Delete all sandboxes and volumes with test labels."""
"""Delete this run's sandboxes, plus stale orphans."""
# Reset client for cleanup
client._async_client = None

print("\n🧹 Cleaning up test resources...")

# Clean up sandboxes with test labels
# Volumes are not swept here: the list endpoint returns LiteVolumeMetadata,
# which carries no labels, so there is no way to tell ours apart without a
# GET per volume. Volume tests delete what they create in their own
# class-level fixtures.
try:
sandboxes = await SandboxInstance.list()
for sb in sandboxes:
labels = sb.metadata.labels
# Labels are stored in additional_properties of MetadataLabels object
if labels is not None:
props = getattr(labels, "additional_properties", {}) or {}
if props.get("env") == "integration-test":
try:
await sb.delete()
except Exception:
pass
except Exception as e:
print(f" Error listing sandboxes: {e}")

# Clean up volumes with test labels
try:
volumes = await VolumeInstance.list()
for vol in volumes:
labels = vol.metadata.labels if hasattr(vol, "metadata") and vol.metadata else None
# Labels are stored in additional_properties of MetadataLabels object
if labels is not None:
props = getattr(labels, "additional_properties", {}) or {}
if props.get("env") == "integration-test":
try:
await vol.delete() # type: ignore[attr-defined]
except Exception:
pass
except Exception as e:
print(f" Error listing volumes: {e}")
sandboxes = []
await asyncio.gather(
*(sb.delete() for sb in sandboxes if is_ours(sb)),
return_exceptions=True,
)

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.

🟡 Cleanup only looks at the first 50 sandboxes in the workspace

The end-of-run cleanup inspects only the first page of the workspace listing (await SandboxInstance.list() at tests/integration/core/conftest.py:69, which returns at most 50 entries), so sandboxes created by this run that fall outside that page are never deleted.
Impact: In a busy shared workspace, test sandboxes silently survive the cleanup and keep consuming resources.

Pagination detail

SandboxInstance.list() returns an AsyncPaginatedList page with limit=50 by default (src/blaxel/core/sandbox/default/sandbox.py:379-381); it is list-like, so iterating it silently yields only the first page. Since orphaned resources accumulate in the shared workspace (the volume sweep was also removed in this PR), exceeding 50 sandboxes is likely. Use auto_paging_iter() to walk every page before filtering with is_ours.

Suggested change
try:
sandboxes = await SandboxInstance.list()
for sb in sandboxes:
labels = sb.metadata.labels
# Labels are stored in additional_properties of MetadataLabels object
if labels is not None:
props = getattr(labels, "additional_properties", {}) or {}
if props.get("env") == "integration-test":
try:
await sb.delete()
except Exception:
pass
except Exception as e:
print(f" Error listing sandboxes: {e}")
# Clean up volumes with test labels
try:
volumes = await VolumeInstance.list()
for vol in volumes:
labels = vol.metadata.labels if hasattr(vol, "metadata") and vol.metadata else None
# Labels are stored in additional_properties of MetadataLabels object
if labels is not None:
props = getattr(labels, "additional_properties", {}) or {}
if props.get("env") == "integration-test":
try:
await vol.delete() # type: ignore[attr-defined]
except Exception:
pass
except Exception as e:
print(f" Error listing volumes: {e}")
sandboxes = []
await asyncio.gather(
*(sb.delete() for sb in sandboxes if is_ours(sb)),
return_exceptions=True,
)
try:
page = await SandboxInstance.list()
sandboxes = [sb async for sb in page.auto_paging_iter()]
except Exception as e:
print(f" Error listing sandboxes: {e}")
sandboxes = []
await asyncio.gather(
*(sb.delete() for sb in sandboxes if is_ours(sb)),
return_exceptions=True,
)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


# Close the client
if client._async_client is not None:
Expand Down
44 changes: 41 additions & 3 deletions tests/integration/core/sandbox/proxy/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import os

from blaxel.core.client.types import Unset
from tests.helpers.echo import echo_url

# The proxy/network routing feature is only available in specific regions, so
# these tests override ``tests.helpers.default_region`` (which points at
Expand All @@ -20,8 +21,9 @@
PROXY_HELPER_SCRIPT = r"""
const https = require("https");
const tls = require("tls");
const RESPONSE_HEADERS_MARKER = "__RESPONSE_HEADERS__";
const method = process.argv[2] || "GET";
const targetUrl = process.argv[3] || "https://httpbin.org/headers";
const targetUrl = process.argv[3];
const extraHeaders = process.argv[4] ? JSON.parse(process.argv[4]) : {};
const bodyData = process.argv[5] || null;
const proxyUrl = process.env.HTTPS_PROXY || process.env.https_proxy ||
Expand All @@ -41,7 +43,11 @@
}
const req = https.request(opts, (r) => {
let d = ""; r.on("data", c => d += c);
r.on("end", () => { process.stdout.write(d); process.exit(0); });
r.on("end", () => {
process.stdout.write(d);
process.stdout.write("\n" + RESPONSE_HEADERS_MARKER + JSON.stringify(r.headers));
process.exit(0);
});
});
req.on("error", (e) => { process.stderr.write("REQ ERR: " + e.message + "\n"); process.exit(1); });
if (bodyData) req.write(bodyData);
Expand Down Expand Up @@ -91,14 +97,28 @@
PYTHON_HELPER_SCRIPT = """
import sys, json, requests
method = sys.argv[1] if len(sys.argv) > 1 else "GET"
url = sys.argv[2] if len(sys.argv) > 2 else "https://httpbin.org/headers"
url = sys.argv[2]
headers = json.loads(sys.argv[3]) if len(sys.argv) > 3 else {}
body = sys.argv[4] if len(sys.argv) > 4 else None
resp = requests.request(method, url, headers=headers, data=body, timeout=30)
print(resp.text)
print("__RESPONSE_HEADERS__" + json.dumps(dict(resp.headers)))
""".strip()


# Each test sandbox gets the echo server base URL in this file, and the test
# commands read it back with ``$(cat /tmp/echo-url)``. Commands stay plain
# string literals this way -- no f-strings fighting with the JSON braces they
# already contain.
ECHO_URL_FILE = "/tmp/echo-url"


async def write_echo_url(sandbox) -> None:
"""Make the echo base URL readable as ``ECHO`` from commands in this sandbox."""
sandbox_echo_url = await echo_url()
await sandbox.fs.write(ECHO_URL_FILE, sandbox_echo_url)


def not_unset(val) -> bool:
"""Return True if val is a real value (not Unset/None)."""
return val is not None and not isinstance(val, Unset)
Expand Down Expand Up @@ -127,5 +147,23 @@ def parse_json_output(logs: str | None) -> dict:
return json.loads(trimmed[start:end])


RESPONSE_HEADERS_MARKER = "__RESPONSE_HEADERS__"


def parse_response_headers(logs: str | None) -> dict[str, str]:
"""Headers of the HTTP response as the client inside the sandbox saw them.

The proxy stamps ``X-Blaxel-Request-Id`` on the response it hands back, so
this is where that header is observable. It never reaches the upstream
server: the cluster gateway strips inbound ``x-blaxel-*`` headers before
forwarding, by design.
"""
marker_at = (logs or "").rfind(RESPONSE_HEADERS_MARKER)
if marker_at == -1:
raise ValueError(f"No response headers in output: {(logs or '')[:200]}")
raw = logs[marker_at + len(RESPONSE_HEADERS_MARKER) :].strip()
return lowercase_keys(json.loads(raw))


def lowercase_keys(obj: dict[str, str]) -> dict[str, str]:
return {k.lower(): v for k, v in obj.items()}
Loading