mirror of
https://github.com/encounter/adk-python.git
synced 2026-07-09 18:19:28 -07:00
feat: passthrough extra args for adk deploy cloud_run as Cloud Run args
Merge https://github.com/google/adk-python/pull/2544 The command `adk deploy cloud_run` supports limited `gcloud run deploy` args 😢. Which makes the command fine for simple deployments... It should support all current and future Cloud Run deployment args for the command to be widely adopted. This can easily be done by passing through all extra args passed to `adk deploy cloud_run` to gcloud... This PR assumes any extra args/flags passed after `AGENT_PATH` are gcloud flags. ## Example ```sh # ADK flags adk deploy cloud_run \ --project=$GOOGLE_CLOUD_PROJECT \ --region=$GOOGLE_CLOUD_LOCATION \ $AGENT_PATH \ # Use the -- separator for gcloud args -- \ --min-instances=2 \ --no-allow-unauthenticated ``` This gives full Cloud Run feature support to ADK users 🤖 🚀 ## Test Plan To test you can just build locally or pip install feature branch directly: ``` uv venv uv pip install git+https://github.com/jackwotherspoon/adk-python.git ``` Deploy to Cloud Run using additional arguments following `AGENT_PATH`, such as `--min-instance=2` or `--description="Cloud Run test"`: ```sh uv run adk deploy cloud_run \ --project=$GOOGLE_CLOUD_PROJECT \ --region=$GOOGLE_CLOUD_LOCATION \ --with_ui \ $AGENT_PATH \ -- \ --labels=test-label=adk \ --min-instances=2 ``` You can click on the Cloud Run service after deployment and check the service yaml, you should see the additional label etc. <img width="1612" height="622" alt="image" src="https://github.com/user-attachments/assets/596a260a-0052-460b-9642-c18900ccf7c9" /> Fixes https://github.com/google/adk-python/issues/2351 COPYBARA_INTEGRATE_REVIEW=https://github.com/google/adk-python/pull/2544 from jackwotherspoon:main 184a4d73f8dbe6f565ff92cf1c1fe69bb163de5e PiperOrigin-RevId: 799252544
This commit is contained in:
committed by
Copybara-Service
parent
2b2f0b52d8
commit
6806deaf88
@@ -137,7 +137,7 @@ def test_adk_deploy_cloud_run():
|
||||
cloud_run_command,
|
||||
cli_deploy_cloud_run.callback,
|
||||
"deploy cloud_run",
|
||||
ignore_params={"verbose"},
|
||||
ignore_params={"verbose", "ctx"},
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -636,3 +636,76 @@ def test_to_gke_happy_path(
|
||||
|
||||
# 4. Verify cleanup
|
||||
assert str(rmtree_recorder.get_last_call_args()[0]) == str(tmp_path)
|
||||
|
||||
|
||||
# Label merging tests
|
||||
@pytest.mark.parametrize(
|
||||
"extra_gcloud_args, expected_labels",
|
||||
[
|
||||
# No user labels - should only have default ADK label
|
||||
(None, "created-by=adk"),
|
||||
([], "created-by=adk"),
|
||||
# Single user label
|
||||
(["--labels=env=test"], "created-by=adk,env=test"),
|
||||
# Multiple user labels in same argument
|
||||
(
|
||||
["--labels=env=test,team=myteam"],
|
||||
"created-by=adk,env=test,team=myteam",
|
||||
),
|
||||
# User labels mixed with other args
|
||||
(
|
||||
["--memory=1Gi", "--labels=env=test", "--cpu=1"],
|
||||
"created-by=adk,env=test",
|
||||
),
|
||||
# Multiple --labels arguments
|
||||
(
|
||||
["--labels=env=test", "--labels=team=myteam"],
|
||||
"created-by=adk,env=test,team=myteam",
|
||||
),
|
||||
# Labels with other passthrough args
|
||||
(
|
||||
["--timeout=300", "--labels=env=prod", "--max-instances=10"],
|
||||
"created-by=adk,env=prod",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_cloud_run_label_merging(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
agent_dir: Callable[[bool, bool], Path],
|
||||
tmp_path: Path,
|
||||
extra_gcloud_args: list[str] | None,
|
||||
expected_labels: str,
|
||||
) -> None:
|
||||
"""Test that user labels are properly merged with the default ADK label."""
|
||||
src_dir = agent_dir(False, False)
|
||||
run_recorder = _Recorder()
|
||||
|
||||
monkeypatch.setattr(subprocess, "run", run_recorder)
|
||||
monkeypatch.setattr(shutil, "rmtree", lambda x: None)
|
||||
|
||||
# Execute the function under test
|
||||
cli_deploy.to_cloud_run(
|
||||
agent_folder=str(src_dir),
|
||||
project="test-project",
|
||||
region="us-central1",
|
||||
service_name="test-service",
|
||||
app_name="test-app",
|
||||
temp_folder=str(tmp_path),
|
||||
port=8080,
|
||||
trace_to_cloud=False,
|
||||
with_ui=False,
|
||||
log_level="info",
|
||||
verbosity="info",
|
||||
adk_version="1.0.0",
|
||||
extra_gcloud_args=tuple(extra_gcloud_args) if extra_gcloud_args else None,
|
||||
)
|
||||
|
||||
# Verify that the gcloud command was called
|
||||
assert len(run_recorder.calls) == 1
|
||||
gcloud_args = run_recorder.get_last_call_args()[0]
|
||||
|
||||
# Find the labels argument
|
||||
labels_idx = gcloud_args.index("--labels")
|
||||
actual_labels = gcloud_args[labels_idx + 1]
|
||||
|
||||
assert actual_labels == expected_labels
|
||||
|
||||
@@ -196,6 +196,148 @@ def test_cli_deploy_cloud_run_failure(
|
||||
assert "Deploy failed: boom" in result.output
|
||||
|
||||
|
||||
def test_cli_deploy_cloud_run_passthrough_args(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Extra args after '--' should be passed through to the gcloud command."""
|
||||
rec = _Recorder()
|
||||
monkeypatch.setattr(cli_tools_click.cli_deploy, "to_cloud_run", rec)
|
||||
|
||||
agent_dir = tmp_path / "agent_passthrough"
|
||||
agent_dir.mkdir()
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(
|
||||
cli_tools_click.main,
|
||||
[
|
||||
"deploy",
|
||||
"cloud_run",
|
||||
"--project",
|
||||
"test-project",
|
||||
"--region",
|
||||
"us-central1",
|
||||
str(agent_dir),
|
||||
"--",
|
||||
"--labels=test-label=test",
|
||||
"--memory=1Gi",
|
||||
"--cpu=1",
|
||||
],
|
||||
)
|
||||
# Print debug information if the test fails
|
||||
if result.exit_code != 0:
|
||||
print(f"Exit code: {result.exit_code}")
|
||||
print(f"Output: {result.output}")
|
||||
print(f"Exception: {result.exception}")
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert rec.calls, "cli_deploy.to_cloud_run must be invoked"
|
||||
|
||||
# Check that extra_gcloud_args were passed correctly
|
||||
called_kwargs = rec.calls[0][1]
|
||||
extra_args = called_kwargs.get("extra_gcloud_args")
|
||||
assert extra_args is not None
|
||||
assert "--labels=test-label=test" in extra_args
|
||||
assert "--memory=1Gi" in extra_args
|
||||
assert "--cpu=1" in extra_args
|
||||
|
||||
|
||||
def test_cli_deploy_cloud_run_rejects_args_without_separator(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Args without '--' separator should be rejected with helpful error message."""
|
||||
rec = _Recorder()
|
||||
monkeypatch.setattr(cli_tools_click.cli_deploy, "to_cloud_run", rec)
|
||||
|
||||
agent_dir = tmp_path / "agent_no_sep"
|
||||
agent_dir.mkdir()
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(
|
||||
cli_tools_click.main,
|
||||
[
|
||||
"deploy",
|
||||
"cloud_run",
|
||||
"--project",
|
||||
"test-project",
|
||||
"--region",
|
||||
"us-central1",
|
||||
str(agent_dir),
|
||||
"--labels=test-label=test", # This should be rejected
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 2
|
||||
assert "Unexpected arguments:" in result.output
|
||||
assert "Use '--' to separate gcloud arguments" in result.output
|
||||
assert not rec.calls, "cli_deploy.to_cloud_run should not be called"
|
||||
|
||||
|
||||
def test_cli_deploy_cloud_run_rejects_args_before_separator(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Args before '--' separator should be rejected."""
|
||||
rec = _Recorder()
|
||||
monkeypatch.setattr(cli_tools_click.cli_deploy, "to_cloud_run", rec)
|
||||
|
||||
agent_dir = tmp_path / "agent_before_sep"
|
||||
agent_dir.mkdir()
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(
|
||||
cli_tools_click.main,
|
||||
[
|
||||
"deploy",
|
||||
"cloud_run",
|
||||
"--project",
|
||||
"test-project",
|
||||
"--region",
|
||||
"us-central1",
|
||||
str(agent_dir),
|
||||
"unexpected_arg", # This should be rejected
|
||||
"--",
|
||||
"--labels=test-label=test",
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 2
|
||||
assert (
|
||||
"Unexpected arguments after agent path and before '--':" in result.output
|
||||
)
|
||||
assert "unexpected_arg" in result.output
|
||||
assert not rec.calls, "cli_deploy.to_cloud_run should not be called"
|
||||
|
||||
|
||||
def test_cli_deploy_cloud_run_allows_empty_gcloud_args(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""No gcloud args after '--' should be allowed."""
|
||||
rec = _Recorder()
|
||||
monkeypatch.setattr(cli_tools_click.cli_deploy, "to_cloud_run", rec)
|
||||
|
||||
agent_dir = tmp_path / "agent_empty_gcloud"
|
||||
agent_dir.mkdir()
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(
|
||||
cli_tools_click.main,
|
||||
[
|
||||
"deploy",
|
||||
"cloud_run",
|
||||
"--project",
|
||||
"test-project",
|
||||
"--region",
|
||||
"us-central1",
|
||||
str(agent_dir),
|
||||
"--",
|
||||
# No gcloud args after --
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert rec.calls, "cli_deploy.to_cloud_run must be invoked"
|
||||
|
||||
# Check that extra_gcloud_args is empty
|
||||
called_kwargs = rec.calls[0][1]
|
||||
extra_args = called_kwargs.get("extra_gcloud_args")
|
||||
assert extra_args == ()
|
||||
|
||||
|
||||
# cli deploy agent_engine
|
||||
def test_cli_deploy_agent_engine_success(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
@@ -476,3 +618,96 @@ def test_cli_eval_with_eval_set_id(
|
||||
app_name=app_name
|
||||
)
|
||||
assert len(eval_set_results) == 2
|
||||
|
||||
|
||||
def test_cli_deploy_cloud_run_gcloud_arg_conflict(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Extra gcloud args that conflict with ADK deploy args should raise ClickException."""
|
||||
|
||||
def _mock_to_cloud_run(*_a, **kwargs):
|
||||
# Import and call the validation function
|
||||
from google.adk.cli.cli_deploy import _validate_gcloud_extra_args
|
||||
|
||||
# Build the same set of managed args as the real function would
|
||||
adk_managed_args = {"--source", "--project", "--port", "--verbosity"}
|
||||
if kwargs.get("region"):
|
||||
adk_managed_args.add("--region")
|
||||
_validate_gcloud_extra_args(
|
||||
kwargs.get("extra_gcloud_args"), adk_managed_args
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
cli_tools_click.cli_deploy, "to_cloud_run", _mock_to_cloud_run
|
||||
)
|
||||
|
||||
agent_dir = tmp_path / "agent_conflict"
|
||||
agent_dir.mkdir()
|
||||
runner = CliRunner()
|
||||
|
||||
# Test with conflicting --project arg
|
||||
result = runner.invoke(
|
||||
cli_tools_click.main,
|
||||
[
|
||||
"deploy",
|
||||
"cloud_run",
|
||||
"--project",
|
||||
"test-project",
|
||||
"--region",
|
||||
"us-central1",
|
||||
str(agent_dir),
|
||||
"--",
|
||||
"--project=conflict-project", # This should conflict
|
||||
],
|
||||
)
|
||||
|
||||
expected_msg = (
|
||||
"The argument '--project' conflicts with ADK's automatic configuration."
|
||||
" ADK will set this argument automatically, so please remove it from your"
|
||||
" command."
|
||||
)
|
||||
assert expected_msg in result.output
|
||||
|
||||
# Test with conflicting --port arg
|
||||
result = runner.invoke(
|
||||
cli_tools_click.main,
|
||||
[
|
||||
"deploy",
|
||||
"cloud_run",
|
||||
"--project",
|
||||
"test-project",
|
||||
str(agent_dir),
|
||||
"--",
|
||||
"--port=9000", # This should conflict
|
||||
],
|
||||
)
|
||||
|
||||
expected_msg = (
|
||||
"The argument '--port' conflicts with ADK's automatic configuration. ADK"
|
||||
" will set this argument automatically, so please remove it from your"
|
||||
" command."
|
||||
)
|
||||
assert expected_msg in result.output
|
||||
|
||||
# Test with conflicting --region arg
|
||||
result = runner.invoke(
|
||||
cli_tools_click.main,
|
||||
[
|
||||
"deploy",
|
||||
"cloud_run",
|
||||
"--project",
|
||||
"test-project",
|
||||
"--region",
|
||||
"us-central1",
|
||||
str(agent_dir),
|
||||
"--",
|
||||
"--region=us-west1", # This should conflict
|
||||
],
|
||||
)
|
||||
|
||||
expected_msg = (
|
||||
"The argument '--region' conflicts with ADK's automatic configuration."
|
||||
" ADK will set this argument automatically, so please remove it from your"
|
||||
" command."
|
||||
)
|
||||
assert expected_msg in result.output
|
||||
|
||||
Reference in New Issue
Block a user