fix(cli): look up an explicit listener by id instead of searching a page

This commit is contained in:
Hugo Durand
2026-09-22 17:16:36 -04:00
parent 075e85925b
commit 7ae82d40e3
4 changed files with 198 additions and 61 deletions
+35 -22
View File
@@ -105,6 +105,10 @@ _OPERATOR_DEFAULT_RESOURCE_SPEC: Mapping[str, object] = {}
_CUSTOMER_REGISTRY_SOURCE: SourceName = "external_docker"
_LISTENER_REQUIRED_MARKER = "listener_id' is required"
_LISTENERS_SHOWN = 10
_NO_LISTENERS = (
"This workspace has no listeners, so --listener-id and --k8s-namespace "
"do not apply."
)
_TERMINAL_STATUSES = frozenset(
@@ -234,32 +238,20 @@ class RequestedPlacement:
"--name."
)
def resolve(self, listeners: Sequence[Listener]) -> Placement:
if not listeners:
if self.requested:
raise click.UsageError(
"This workspace has no listeners, so --listener-id and "
"--k8s-namespace do not apply."
)
return Unplaced()
listener = self._listener(listeners)
def on(self, listener: Listener) -> Placement:
return OnListener(listener.id, self._namespace(listener))
def _listener(self, listeners: Sequence[Listener]) -> Listener:
if self.listener_id is None:
if len(listeners) == 1:
return listeners[0]
def among(self, listeners: Sequence[Listener]) -> Placement:
if not listeners:
if self.requested:
raise click.UsageError(_NO_LISTENERS)
return Unplaced()
if len(listeners) > 1:
raise click.UsageError(
"This workspace has several listeners. Choose one with "
f"--listener-id:\n{_describe_listeners(listeners)}"
)
for listener in listeners:
if listener.id == self.listener_id:
return listener
raise click.UsageError(
f"Listener {self.listener_id} was not found in this workspace. "
f"Available listeners:\n{_describe_listeners(listeners)}"
)
return self.on(listeners[0])
def _namespace(self, listener: Listener) -> str:
if not listener.namespaces:
@@ -1514,6 +1506,24 @@ def _needs_a_listener(err: HostBackendError) -> bool:
)
def _requested_listener(client: HostBackendClient, listener_id: str) -> Listener:
try:
resource = _call_host_backend_with_optional_tenant(
client, lambda c: c.get_listener(listener_id)
)
except HostBackendError as err:
if err.status_code != 404:
raise
available = _available_listeners(client)
if not available:
raise click.UsageError(_NO_LISTENERS) from None
raise click.UsageError(
f"Listener {listener_id} was not found in this workspace. "
f"Available listeners:\n{_describe_listeners(available)}"
) from None
return Listener.from_resource(resource)
def _available_listeners(client: HostBackendClient) -> tuple[Listener, ...]:
resources = _call_host_backend_with_optional_tenant(
client, lambda c: c.list_listeners()
@@ -1620,9 +1630,12 @@ class CustomerRegistrySource:
)
def _resolve_placement(self, ctx: DeployContext) -> Placement:
if not (ctx.endpoints.is_cloud or self.requested_placement.requested):
requested = self.requested_placement
if requested.listener_id is not None:
return requested.on(_requested_listener(ctx.client, requested.listener_id))
if not (ctx.endpoints.is_cloud or requested.requested):
return Unplaced()
return self.requested_placement.resolve(_available_listeners(ctx.client))
return requested.among(_available_listeners(ctx.client))
def _announce(self, placement: Placement) -> None:
if isinstance(placement, OnListener):
+3
View File
@@ -234,6 +234,9 @@ class HostBackendClient:
params = {key: value for key, value in given if value is not None}
return _resources(self._request("GET", "/v2/deployments", params=params))
def get_listener(self, listener_id: str) -> dict[str, Any]:
return self._request("GET", f"/v2/listeners/{listener_id}")
def list_listeners(self) -> list[dict[str, Any]]:
return _resources(
self._request("GET", "/v2/listeners", params={"limit": MAX_PAGE_SIZE})
@@ -78,6 +78,7 @@ class ControlPlaneDouble:
push_token_status: int = 200
create_error: str | None = None
listeners: list[dict] = field(default_factory=list)
listeners_by_id: dict[str, dict] = field(default_factory=dict)
bodies: dict[str, dict] = field(default_factory=dict)
def handle(self, request: httpx.Request) -> httpx.Response:
@@ -91,6 +92,15 @@ class ControlPlaneDouble:
method, path = request.method, request.url.path
if (method, path) == ("GET", "/v2/listeners"):
return httpx.Response(200, json={"resources": self.listeners})
if method == "GET" and path.startswith("/v2/listeners/"):
listener_id = path.rsplit("/", 1)[-1]
known = {listener["id"]: listener for listener in self.listeners}
known.update(self.listeners_by_id)
if listener_id not in known:
return httpx.Response(
404, json={"detail": f"Listener ID {listener_id} not found."}
)
return httpx.Response(200, json=known[listener_id])
if (method, path) == ("GET", "/v2/deployments"):
name = request.url.params.get("name")
return httpx.Response(
@@ -727,12 +737,6 @@ def test_push_to_places_a_new_deployment_on_the_chosen_listener(
pytest.param(
[TWO_NAMESPACE_LISTENER], (), "--k8s-namespace", id="two_namespaces"
),
pytest.param(
[LISTENER],
("--listener-id", "listener-9"),
"was not found",
id="unknown_listener",
),
pytest.param(
[LISTENER],
("--k8s-namespace", "nope"),
@@ -951,3 +955,101 @@ def test_a_listener_without_an_id_is_reported_rather_than_ignored(
assert result.exit_code != 0
assert "without an id" in result.output
assert deploy_project.docker.verbs() == []
def _listener_route(listener_id: str) -> str:
return f"GET /v2/listeners/{listener_id}"
def test_an_explicit_listener_is_fetched_by_id_not_searched(
deploy_project: DeployProject,
) -> None:
deploy_project.control_plane.listeners = [LISTENER, OTHER_LISTENER]
result = deploy_project.run(
"--push-to",
PUSH_REPOSITORY,
"--listener-id",
"listener-2",
host_url=CLOUD_CONTROL_PLANE_URL,
)
assert result.exit_code == 0, result.output
assert _listener_route("listener-2") in deploy_project.timeline
assert LIST_LISTENERS not in deploy_project.timeline
assert deploy_project.control_plane.bodies[CREATE_DEPLOYMENT]["source_config"] == {
"resource_spec": {},
"listener_id": "listener-2",
"listener_config": {"k8s_namespace": "agents"},
}
def test_an_explicit_listener_beyond_the_first_page_still_works(
deploy_project: DeployProject,
) -> None:
deploy_project.control_plane.listeners = [
{
"id": f"listener-{index}",
"compute_id": "cluster",
"compute_config": {"k8s_namespaces": ["agents"]},
}
for index in range(100)
]
deploy_project.control_plane.listeners_by_id = {
"listener-on-page-two": {
"id": "listener-on-page-two",
"compute_id": "far-cluster",
"compute_config": {"k8s_namespaces": ["agents"]},
}
}
result = deploy_project.run(
"--push-to",
PUSH_REPOSITORY,
"--listener-id",
"listener-on-page-two",
host_url=CLOUD_CONTROL_PLANE_URL,
)
assert result.exit_code == 0, result.output
assert deploy_project.control_plane.bodies[CREATE_DEPLOYMENT]["source_config"] == {
"resource_spec": {},
"listener_id": "listener-on-page-two",
"listener_config": {"k8s_namespace": "agents"},
}
def test_an_unknown_listener_names_the_ones_that_exist(
deploy_project: DeployProject,
) -> None:
deploy_project.control_plane.listeners = [LISTENER]
result = deploy_project.run(
"--push-to",
PUSH_REPOSITORY,
"--listener-id",
"listener-9",
host_url=CLOUD_CONTROL_PLANE_URL,
)
assert result.exit_code != 0
assert "was not found" in result.output
assert "listener-1" in result.output
assert "prod-cluster" in result.output
assert deploy_project.docker.verbs() == []
def test_an_explicit_listener_in_a_workspace_without_any_is_refused(
deploy_project: DeployProject,
) -> None:
result = deploy_project.run(
"--push-to",
PUSH_REPOSITORY,
"--listener-id",
"listener-1",
host_url=CLOUD_CONTROL_PLANE_URL,
)
assert result.exit_code != 0
assert "no listeners" in result.output
assert deploy_project.docker.verbs() == []
@@ -993,22 +993,10 @@ class TestRequestedPlacement:
OnListener("listener-2", "agents-staging"),
id="namespace_alone_picks_the_only_listener",
),
pytest.param(
RequestedPlacement(listener_id="listener-1"),
(ONE_NAMESPACE, TWO_NAMESPACES),
OnListener("listener-1", "agents"),
id="listener_alone_picks_its_only_namespace",
),
pytest.param(
RequestedPlacement(listener_id="listener-2", k8s_namespace="agents"),
(ONE_NAMESPACE, TWO_NAMESPACES),
OnListener("listener-2", "agents"),
id="both_given",
),
],
)
def test_resolves_to_a_placement(self, request_, listeners, expected):
assert request_.resolve(listeners) == expected
assert request_.among(listeners) == expected
@pytest.mark.parametrize(
("request_", "listeners", "message"),
@@ -1037,39 +1025,21 @@ class TestRequestedPlacement:
"no listeners",
id="namespace_without_any_listener",
),
pytest.param(
RequestedPlacement(listener_id="listener-9"),
(ONE_NAMESPACE,),
"was not found",
id="unknown_listener",
),
pytest.param(
RequestedPlacement(),
(TWO_NAMESPACES,),
"--k8s-namespace",
id="several_namespaces_need_a_choice",
),
pytest.param(
RequestedPlacement(listener_id="listener-2", k8s_namespace="nope"),
(TWO_NAMESPACES,),
"does not serve namespace",
id="unknown_namespace",
),
pytest.param(
RequestedPlacement(listener_id="listener-3"),
(NO_NAMESPACE,),
"serves no namespaces",
id="listener_without_namespaces",
),
],
)
def test_refuses_and_names_the_choices(self, request_, listeners, message):
with pytest.raises(click.UsageError, match=message):
request_.resolve(listeners)
request_.among(listeners)
def test_the_error_lists_every_listener_with_its_cluster_and_namespaces(self):
with pytest.raises(click.UsageError) as error:
RequestedPlacement().resolve((ONE_NAMESPACE, TWO_NAMESPACES))
RequestedPlacement().among((ONE_NAMESPACE, TWO_NAMESPACES))
assert "listener-1" in error.value.message
assert "prod-cluster" in error.value.message
@@ -1200,3 +1170,52 @@ def test_a_deployment_id_with_listener_flags_is_refused_without_probing_docker(
placement=RequestedPlacement(listener_id="listener-1"),
selector=ById("dep-1"),
)
class TestPlacementOnAKnownListener:
@pytest.mark.parametrize(
("request_", "listener", "expected"),
[
pytest.param(
RequestedPlacement(listener_id="listener-1"),
ONE_NAMESPACE,
OnListener("listener-1", "agents"),
id="the_only_namespace_is_used",
),
pytest.param(
RequestedPlacement(listener_id="listener-2", k8s_namespace="agents"),
TWO_NAMESPACES,
OnListener("listener-2", "agents"),
id="the_chosen_namespace_is_used",
),
],
)
def test_places_on_the_listener(self, request_, listener, expected):
assert request_.on(listener) == expected
@pytest.mark.parametrize(
("request_", "listener", "message"),
[
pytest.param(
RequestedPlacement(listener_id="listener-2"),
TWO_NAMESPACES,
"--k8s-namespace",
id="several_namespaces_need_a_choice",
),
pytest.param(
RequestedPlacement(listener_id="listener-2", k8s_namespace="nope"),
TWO_NAMESPACES,
"does not serve namespace",
id="unknown_namespace",
),
pytest.param(
RequestedPlacement(listener_id="listener-3"),
NO_NAMESPACE,
"serves no namespaces",
id="listener_without_namespaces",
),
],
)
def test_refuses_and_names_the_namespaces(self, request_, listener, message):
with pytest.raises(click.UsageError, match=message):
request_.on(listener)