fix(cli): look up listeners only when they can change the placement

This commit is contained in:
Hugo Durand
2026-09-22 15:23:36 -04:00
parent e1156979e3
commit beb3e9c1f1
3 changed files with 99 additions and 42 deletions
+33 -19
View File
@@ -102,6 +102,7 @@ _PUSH_ATTEMPTS = 3
_LOCAL_BUILD_TAG_PREFIX = "langgraph-deploy-tmp"
_OPERATOR_DEFAULT_RESOURCE_SPEC: Mapping[str, object] = {}
_CUSTOMER_REGISTRY_SOURCE: SourceName = "external_docker"
_LISTENER_REQUIRED_MARKER = "listener_id' is required"
_TERMINAL_STATUSES = frozenset(
@@ -211,7 +212,10 @@ class RequestedPlacement:
def requested(self) -> bool:
return self.listener_id is not None or self.k8s_namespace is not None
def resolve(self, listeners: Sequence[Listener], *, required: bool) -> Placement:
def must_place(self, *, required: bool) -> bool:
return required or self.requested
def resolve(self, listeners: Sequence[Listener]) -> Placement:
if not listeners:
if self.requested:
raise click.UsageError(
@@ -219,8 +223,6 @@ class RequestedPlacement:
"--k8s-namespace do not apply."
)
return Unplaced()
if not required and not self.requested:
return Unplaced()
listener = self._listener(listeners)
return OnListener(listener.id, self._namespace(listener))
@@ -1531,25 +1533,37 @@ class CustomerRegistrySource:
existing.id, _image_revision_result(updated, "Deployment updated")
)
def _create(self, ctx: DeployContext, name: str, step: int) -> DeployOutcome:
placement = self.placement.resolve(
_available_listeners(ctx.client), required=ctx.endpoints.is_cloud
)
def _placement(self, ctx: DeployContext) -> Placement:
if not self.placement.must_place(required=ctx.endpoints.is_cloud):
return Unplaced()
placement = self.placement.resolve(_available_listeners(ctx.client))
if placement.summary:
_get_emitter().info(placement.summary)
return placement
def _create(self, ctx: DeployContext, name: str, step: int) -> DeployOutcome:
placement = self._placement(ctx)
image_uri, step = self._publish(ctx, step)
created, _ = _create_deployment(
ctx.client,
step,
name=name,
source=_CUSTOMER_REGISTRY_SOURCE,
source_config={
"resource_spec": _OPERATOR_DEFAULT_RESOURCE_SPEC,
**placement.source_config(),
},
source_revision_config={"image_uri": image_uri},
secrets=ctx.secrets,
)
try:
created, _ = _create_deployment(
ctx.client,
step,
name=name,
source=_CUSTOMER_REGISTRY_SOURCE,
source_config={
"resource_spec": _OPERATOR_DEFAULT_RESOURCE_SPEC,
**placement.source_config(),
},
source_revision_config={"image_uri": image_uri},
secrets=ctx.secrets,
)
except HostBackendError as err:
if err.status_code == 400 and _LISTENER_REQUIRED_MARKER in err.message:
raise click.UsageError(
"This workspace deploys through a listener. Re-run with "
f"--listener-id and --k8s-namespace.\n{err.message}"
) from None
raise
return DeployOutcome(
created.id, _image_revision_result(created.resource, "Deployment created")
)
@@ -512,7 +512,6 @@ def test_push_to_builds_pushes_then_creates_an_external_deployment(
assert result.exit_code == 0, result.output
assert deploy_project.timeline == [
LIST_DEPLOYMENTS,
LIST_LISTENERS,
"docker build",
"docker push",
"docker inspect-digest",
@@ -831,3 +830,30 @@ def test_a_deployment_without_a_listener_announces_nothing(
assert result.exit_code == 0, result.output
assert "listener" not in result.output
def test_a_self_hosted_create_without_flags_never_looks_up_listeners(
deploy_project: DeployProject,
) -> None:
deploy_project.control_plane.listeners = [LISTENER]
result = deploy_project.run("--push-to", PUSH_REPOSITORY)
assert result.exit_code == 0, result.output
assert LIST_LISTENERS not in deploy_project.timeline
def test_a_control_plane_that_demands_a_listener_names_the_flags(
deploy_project: DeployProject,
) -> None:
deploy_project.control_plane.create_error = (
"Source configuration error: 'source_config.listener_id' is required "
"for workspace with available listener IDs: ['listener-1']"
)
result = deploy_project.run("--push-to", PUSH_REPOSITORY)
assert result.exit_code != 0
assert "--listener-id" in result.output
assert "--k8s-namespace" in result.output
assert "listener-1" in result.output
@@ -963,57 +963,68 @@ NO_NAMESPACE = Listener("listener-3", "broken-cluster", ())
class TestRequestedPlacement:
@pytest.mark.parametrize(
("request_", "listeners", "required", "expected"),
("request_", "required", "expected"),
[
pytest.param(
RequestedPlacement(), (), True, Unplaced(), id="no_listeners_no_request"
),
pytest.param(RequestedPlacement(), True, True, id="cloud_must_place"),
pytest.param(
RequestedPlacement(),
(ONE_NAMESPACE,),
True,
OnListener("listener-1", "agents"),
id="cloud_uses_the_only_possible_answer",
),
pytest.param(
RequestedPlacement(),
(ONE_NAMESPACE,),
False,
Unplaced(),
False,
id="self_hosted_keeps_its_bundled_operator",
),
pytest.param(
RequestedPlacement(listener_id="listener-1"),
(ONE_NAMESPACE,),
False,
OnListener("listener-1", "agents"),
True,
id="self_hosted_places_when_asked",
),
pytest.param(
RequestedPlacement(k8s_namespace="agents"),
False,
True,
id="a_namespace_alone_is_still_a_request",
),
],
)
def test_must_place_decides_whether_listeners_matter(
self, request_, required, expected
):
assert request_.must_place(required=required) is expected
@pytest.mark.parametrize(
("request_", "listeners", "expected"),
[
pytest.param(
RequestedPlacement(), (), Unplaced(), id="no_listeners_no_request"
),
pytest.param(
RequestedPlacement(),
(ONE_NAMESPACE,),
OnListener("listener-1", "agents"),
id="uses_the_only_possible_answer",
),
pytest.param(
RequestedPlacement(k8s_namespace="agents-staging"),
(TWO_NAMESPACES,),
True,
OnListener("listener-2", "agents-staging"),
id="namespace_alone_picks_the_only_listener",
),
pytest.param(
RequestedPlacement(listener_id="listener-1"),
(ONE_NAMESPACE, TWO_NAMESPACES),
True,
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),
True,
OnListener("listener-2", "agents"),
id="both_given",
),
],
)
def test_resolves_to_a_placement(self, request_, listeners, required, expected):
assert request_.resolve(listeners, required=required) == expected
def test_resolves_to_a_placement(self, request_, listeners, expected):
assert request_.resolve(listeners) == expected
@pytest.mark.parametrize(
("request_", "listeners", "message"),
@@ -1036,6 +1047,12 @@ class TestRequestedPlacement:
"--listener-id",
id="namespace_alone_is_ambiguous_with_several_listeners",
),
pytest.param(
RequestedPlacement(k8s_namespace="agents"),
(),
"no listeners",
id="namespace_without_any_listener",
),
pytest.param(
RequestedPlacement(listener_id="listener-9"),
(ONE_NAMESPACE,),
@@ -1064,11 +1081,11 @@ class TestRequestedPlacement:
)
def test_refuses_and_names_the_choices(self, request_, listeners, message):
with pytest.raises(click.UsageError, match=message):
request_.resolve(listeners, required=True)
request_.resolve(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), required=True)
RequestedPlacement().resolve((ONE_NAMESPACE, TWO_NAMESPACES))
assert "listener-1" in error.value.message
assert "prod-cluster" in error.value.message