From beb3e9c1f13f5d77ad9af9ee337411401af32ad3 Mon Sep 17 00:00:00 2001 From: Hugo Durand Date: Tue, 22 Sep 2026 15:23:36 -0400 Subject: [PATCH] fix(cli): look up listeners only when they can change the placement --- libs/cli/langgraph_cli/deploy.py | 52 ++++++++++------ .../unit_tests/cli/test_deploy_command.py | 28 ++++++++- .../tests/unit_tests/test_deploy_helpers.py | 61 ++++++++++++------- 3 files changed, 99 insertions(+), 42 deletions(-) diff --git a/libs/cli/langgraph_cli/deploy.py b/libs/cli/langgraph_cli/deploy.py index a19125340..3ef70ea6e 100644 --- a/libs/cli/langgraph_cli/deploy.py +++ b/libs/cli/langgraph_cli/deploy.py @@ -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") ) diff --git a/libs/cli/tests/unit_tests/cli/test_deploy_command.py b/libs/cli/tests/unit_tests/cli/test_deploy_command.py index 8f94d1a73..94a13194e 100644 --- a/libs/cli/tests/unit_tests/cli/test_deploy_command.py +++ b/libs/cli/tests/unit_tests/cli/test_deploy_command.py @@ -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 diff --git a/libs/cli/tests/unit_tests/test_deploy_helpers.py b/libs/cli/tests/unit_tests/test_deploy_helpers.py index 524b9fc5a..e1e33528a 100644 --- a/libs/cli/tests/unit_tests/test_deploy_helpers.py +++ b/libs/cli/tests/unit_tests/test_deploy_helpers.py @@ -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