mirror of
https://github.com/langchain-ai/langgraph.git
synced 2026-10-17 05:35:31 +02:00
fix(cli): push through the active Docker context in langgraph deploy (#9321)
## Summary <!-- Write the summary yourself --> ## Notes for reviewers - Known gap: `--host` carries only the context's endpoint, not its TLS settings, so a remote context that uses client certificates still can't push. That case was broken before this change too, and it's rare for local builds. - A context with no Docker endpoint (Docker prints `<no value>`) or a missing `docker` binary falls back to today's behavior. ## Test Plan - [x] `make lint` and `make test` pass in `libs/cli` (556 tests) - [x] New tests: `login` and `push` get the active context's endpoint as `--host`; with `DOCKER_HOST` set there's no lookup and no `--host`; when the context lookup fails, or the context has no Docker endpoint, there's no `--host` and the deploy still succeeds. The first fails without the change. - [x] On a Mac running Docker Desktop with no `/var/run/docker.sock`: `docker --config <temp dir> version` fails with `failed to connect to the docker API at unix:///var/run/docker.sock`, and adding `--host <active context endpoint>` reaches the daemon - [x] Reproduced end to end on LangSmith AWS: `langgraph deploy` (0.4.35) failed at the push with that error, then passed with `DOCKER_HOST` set to the context's endpoint, which this change does automatically - [ ] After release, `langgraph deploy` pushes on Docker Desktop without `DOCKER_HOST` set ## Demo <!-- Drag a screen recording or screenshot here -->
This commit is contained in:
@@ -1209,6 +1209,33 @@ class DockerBuildCommand:
|
||||
return cls(("docker", "buildx", "build"), flags)
|
||||
|
||||
|
||||
def _active_docker_host(runner: CommandRunner) -> str | None:
|
||||
"""Return the daemon endpoint of the user's active Docker context.
|
||||
|
||||
None when `DOCKER_HOST` is set, which Docker honors with any config
|
||||
directory. Also None when the lookup fails (for example, `DOCKER_CONTEXT`
|
||||
names a context that doesn't exist) or the context has no Docker endpoint
|
||||
(Docker prints `<no value>`); that keeps Docker's own default in place.
|
||||
"""
|
||||
if os.environ.get("DOCKER_HOST"):
|
||||
return None
|
||||
try:
|
||||
stdout, _ = runner.run(
|
||||
subp_exec(
|
||||
"docker",
|
||||
"context",
|
||||
"inspect",
|
||||
"--format",
|
||||
"{{.Endpoints.docker.Host}}",
|
||||
collect=True,
|
||||
)
|
||||
)
|
||||
except (click.exceptions.Exit, OSError):
|
||||
return None
|
||||
host = (stdout or "").strip()
|
||||
return host if "://" in host else None
|
||||
|
||||
|
||||
def _docker_argv(docker_config_dir: str | None) -> tuple[str, ...]:
|
||||
if docker_config_dir is None:
|
||||
return ("docker",)
|
||||
@@ -1242,17 +1269,13 @@ def _push_image(
|
||||
runner: CommandRunner,
|
||||
image: str,
|
||||
*,
|
||||
docker_config_dir: str | None,
|
||||
docker: tuple[str, ...],
|
||||
verbose: bool,
|
||||
) -> None:
|
||||
for attempt in range(1, _PUSH_ATTEMPTS + 1):
|
||||
try:
|
||||
with Progress(message="Pushing...", elapsed=not verbose):
|
||||
runner.run(
|
||||
subp_exec(
|
||||
*_docker_argv(docker_config_dir), "push", image, verbose=verbose
|
||||
)
|
||||
)
|
||||
runner.run(subp_exec(*docker, "push", image, verbose=verbose))
|
||||
return
|
||||
except click.exceptions.Exit:
|
||||
if attempt == _PUSH_ATTEMPTS:
|
||||
@@ -1337,6 +1360,11 @@ def _run_local_build(
|
||||
registry_host = repository.split("/")[0]
|
||||
|
||||
with _docker_config_for_token(registry_host, deployment_token, username) as cfg:
|
||||
# A config directory has no current context, so Docker would fall
|
||||
# back to /var/run/docker.sock; keep using the daemon the build used.
|
||||
docker_host = _active_docker_host(runner)
|
||||
host_args = ("--host", docker_host) if docker_host else ()
|
||||
docker = (*_docker_argv(cfg), *host_args)
|
||||
_log_deploy_step(step, f"Logging into {registry_host}")
|
||||
token_input = (
|
||||
deployment_token
|
||||
@@ -1345,7 +1373,7 @@ def _run_local_build(
|
||||
)
|
||||
runner.run(
|
||||
subp_exec(
|
||||
*_docker_argv(cfg),
|
||||
*docker,
|
||||
"login",
|
||||
"-u",
|
||||
username,
|
||||
@@ -1361,7 +1389,7 @@ def _run_local_build(
|
||||
runner.run(
|
||||
subp_exec("docker", "tag", image_to_push, remote_image, verbose=verbose)
|
||||
)
|
||||
_push_image(runner, remote_image, docker_config_dir=cfg, verbose=verbose)
|
||||
_push_image(runner, remote_image, docker=docker, verbose=verbose)
|
||||
step += 1
|
||||
|
||||
resolved_image = _resolve_pushed_image_digest(
|
||||
@@ -1826,7 +1854,7 @@ class CustomerRegistrySource:
|
||||
_build_image(runner, ctx.spec, image, verbose=ctx.verbose)
|
||||
step += 1
|
||||
_log_deploy_step(step, f"Pushing image {image}")
|
||||
_push_image(runner, image, docker_config_dir=None, verbose=ctx.verbose)
|
||||
_push_image(runner, image, docker=_docker_argv(None), verbose=ctx.verbose)
|
||||
step += 1
|
||||
digest = _resolve_pushed_image_digest(
|
||||
runner, remote_image=image, docker_config_dir=None, verbose=ctx.verbose
|
||||
|
||||
@@ -52,6 +52,8 @@ ARCHIVE = ("/tmp/src.tgz", 2048, "langgraph.json")
|
||||
OBJECT_PATH = "tarballs/src.tgz"
|
||||
PLATFORM_FORMAT = "{{.Os}}/{{.Architecture}}"
|
||||
DIGESTS_FORMAT = "{{json .RepoDigests}}"
|
||||
CONTEXT_HOST_FORMAT = "{{.Endpoints.docker.Host}}"
|
||||
CONTEXT_HOST = "unix:///Users/dev/.docker/run/docker.sock"
|
||||
NOT_A_CLI_DEPLOYMENT = (
|
||||
"push token is only available for 'internal_docker' source deployments"
|
||||
)
|
||||
@@ -192,6 +194,8 @@ class DockerCommand:
|
||||
class DockerDouble:
|
||||
timeline: list[str]
|
||||
failing_pushes: int = 0
|
||||
context_lookup_fails: bool = False
|
||||
context_host: str = CONTEXT_HOST
|
||||
builds: list[dict] = field(default_factory=list)
|
||||
commands: list[DockerCommand] = field(default_factory=list)
|
||||
|
||||
@@ -235,6 +239,10 @@ class DockerDouble:
|
||||
if "push" in args and self.failing_pushes > 0:
|
||||
self.failing_pushes -= 1
|
||||
raise click.exceptions.Exit(1)
|
||||
if CONTEXT_HOST_FORMAT in args:
|
||||
if self.context_lookup_fails:
|
||||
raise click.exceptions.Exit(1)
|
||||
return f"{self.context_host}\n", None
|
||||
if PLATFORM_FORMAT in args:
|
||||
return "linux/amd64\n", None
|
||||
if DIGESTS_FORMAT in args:
|
||||
@@ -244,6 +252,8 @@ class DockerDouble:
|
||||
|
||||
@staticmethod
|
||||
def _verb(args: tuple[str, ...]) -> str:
|
||||
if CONTEXT_HOST_FORMAT in args:
|
||||
return "inspect-context"
|
||||
if PLATFORM_FORMAT in args:
|
||||
return "inspect-platform"
|
||||
if DIGESTS_FORMAT in args:
|
||||
@@ -292,7 +302,12 @@ def deploy_project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> DeployPro
|
||||
json.dumps({"graphs": {"agent": "agent.py:graph"}, "dependencies": ["."]})
|
||||
)
|
||||
monkeypatch.chdir(tmp_path)
|
||||
for name in ("LANGSMITH_TENANT_ID", "LANGSMITH_ENDPOINT", "LANGGRAPH_HOST_URL"):
|
||||
for name in (
|
||||
"LANGSMITH_TENANT_ID",
|
||||
"LANGSMITH_ENDPOINT",
|
||||
"LANGGRAPH_HOST_URL",
|
||||
"DOCKER_HOST",
|
||||
):
|
||||
monkeypatch.delenv(name, raising=False)
|
||||
|
||||
timeline: list[str] = []
|
||||
@@ -340,6 +355,7 @@ def test_first_local_deploy_creates_then_builds_pushes_and_updates_in_order(
|
||||
CREATE_DEPLOYMENT,
|
||||
"docker build",
|
||||
_push_token(CREATED_ID),
|
||||
"docker inspect-context",
|
||||
"docker login",
|
||||
"docker tag",
|
||||
"docker push",
|
||||
@@ -406,7 +422,7 @@ def test_local_deploy_logs_in_with_the_control_plane_push_token(
|
||||
|
||||
login = deploy_project.docker.command("login")
|
||||
assert login.args[:2] == ("docker", "--config")
|
||||
assert login.args[3:] == (
|
||||
assert login.args[-5:] == (
|
||||
"login",
|
||||
"-u",
|
||||
"oauth2accesstoken",
|
||||
@@ -416,6 +432,53 @@ def test_local_deploy_logs_in_with_the_control_plane_push_token(
|
||||
assert login.kwargs["input"] == f"{PUSH_TOKEN}\n"
|
||||
|
||||
|
||||
def test_local_deploy_logs_in_and_pushes_through_the_active_docker_context(
|
||||
deploy_project: DeployProject,
|
||||
) -> None:
|
||||
"""The temporary config has no current context, so without the active
|
||||
context's endpoint Docker would push through /var/run/docker.sock."""
|
||||
result = deploy_project.run("--no-remote")
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
for verb in ("login", "push"):
|
||||
args = deploy_project.docker.command(verb).args
|
||||
assert args[3:5] == ("--host", CONTEXT_HOST), verb
|
||||
|
||||
|
||||
def test_local_deploy_leaves_an_explicit_docker_host_to_docker(
|
||||
deploy_project: DeployProject, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.setenv("DOCKER_HOST", "tcp://127.0.0.1:2375")
|
||||
|
||||
result = deploy_project.run("--no-remote")
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
for verb in ("login", "push"):
|
||||
assert "--host" not in deploy_project.docker.command(verb).args, verb
|
||||
|
||||
|
||||
def test_local_deploy_keeps_dockers_default_when_the_context_lookup_fails(
|
||||
deploy_project: DeployProject,
|
||||
) -> None:
|
||||
deploy_project.docker.context_lookup_fails = True
|
||||
|
||||
result = deploy_project.run("--no-remote")
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "--host" not in deploy_project.docker.command("push").args
|
||||
|
||||
|
||||
def test_local_deploy_ignores_a_context_without_a_docker_endpoint(
|
||||
deploy_project: DeployProject,
|
||||
) -> None:
|
||||
deploy_project.docker.context_host = "<no value>"
|
||||
|
||||
result = deploy_project.run("--no-remote")
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "--host" not in deploy_project.docker.command("push").args
|
||||
|
||||
|
||||
def test_local_deploy_tells_the_control_plane_it_honors_registry_usernames(
|
||||
deploy_project: DeployProject,
|
||||
) -> None:
|
||||
@@ -438,7 +501,7 @@ def test_local_deploy_logs_in_with_the_username_the_control_plane_returns(
|
||||
deploy_project.run("--no-remote")
|
||||
|
||||
login = deploy_project.docker.command("login")
|
||||
assert login.args[3:] == (
|
||||
assert login.args[-5:] == (
|
||||
"login",
|
||||
"-u",
|
||||
"AWS",
|
||||
@@ -506,6 +569,7 @@ def test_prebuilt_image_is_validated_and_pushed_without_a_build(
|
||||
assert deploy_project.docker.builds == []
|
||||
assert deploy_project.docker.verbs() == [
|
||||
"docker inspect-platform",
|
||||
"docker inspect-context",
|
||||
"docker login",
|
||||
"docker tag",
|
||||
"docker push",
|
||||
|
||||
Reference in New Issue
Block a user