fix(cli): use _GCLOUD_CMD for gcloud calls in GKE deploy on Windows#6297
Open
anxkhn wants to merge 1 commit into
Open
fix(cli): use _GCLOUD_CMD for gcloud calls in GKE deploy on Windows#6297anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
The GKE deploy path (`adk deploy gke`) still invoked gcloud via the bare name `gcloud` in two places: the `builds submit` call and the `container clusters get-credentials` call. On Windows, gcloud is the `gcloud.cmd` batch script, and subprocess.run() without a shell does not apply PATHEXT to a bare name, so both calls raise FileNotFoundError and the deploy fails. The module already defines `_GCLOUD_CMD = 'gcloud.cmd' if _IS_WINDOWS else 'gcloud'` for exactly this reason and uses it in the Cloud Run path, but the two GKE call sites were missed. Point them at `_GCLOUD_CMD` too. No behavior change on POSIX. Add a regression test that patches `_GCLOUD_CMD` to simulate Windows and asserts both GKE gcloud invocations use `gcloud.cmd`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
No existing issue; describing the change below (per CONTRIBUTING, non-doc PRs may
describe the bug directly in the PR following the issue template structure).
2. Or, if no issue exists, describe the change:
Problem:
adk deploy gkeis broken on Windows. The GKE deploy path insrc/google/adk/cli/cli_deploy.pyinvokes gcloud via the bare name'gcloud'intwo places:
gcloud builds submitcall, andgcloud container clusters get-credentialscall.On Windows the gcloud entrypoint is the
gcloud.cmdbatch script.subprocess.run([...])is called without a shell, and Python does not applyPATHEXTto a bare command name in that case, sogcloudis not resolved togcloud.cmd. Both calls therefore raiseFileNotFoundErrorand the deploy fails.The module already defines, precisely for this reason:
and uses it correctly in the Cloud Run deploy path. The Windows fix for Cloud Run
(#1597) introduced
_GCLOUD_CMDbut only updated the Cloud Run call sites; the twoGKE call sites were left as the literal
'gcloud'. This PR is the follow-up thatfinishes the same fix for the GKE path.
Solution:
Point the two GKE gcloud invocations at the existing
_GCLOUD_CMDconstant,mirroring the Cloud Run path. This is the smallest change that resolves the issue:
two lines. There is no behavior change on POSIX, where
_GCLOUD_CMD == 'gcloud';only Windows behavior changes (bare
gcloud->gcloud.cmd).Testing Plan
Unit Tests:
Added
test_to_gke_uses_gcloud_cmd_on_windowsintests/unittests/cli/utils/test_cli_deploy.py. It mirrors the existingtest_to_gke_happy_path, patches_GCLOUD_CMDto"gcloud.cmd"to simulateWindows, and asserts that both gcloud invocations (
builds submitandcontainer clusters get-credentials) usegcloud.cmdas argv[0]. The test is redon the current code (
AssertionError: assert 'gcloud' == 'gcloud.cmd') and greenwith the fix.
pytestsummary (CLI deploy suite):Manual End-to-End (E2E) Tests:
This is a Windows-only executable-resolution fix in an external
gcloudsubprocess call, so a full GKE deploy is not reproducible in CI. The failure and
fix are demonstrated deterministically by the unit test above, which asserts the
exact argv[0] passed to
subprocess.runfor both gcloud calls. On POSIX there isno change, since
_GCLOUD_CMDalready resolves to'gcloud'.Checklist
Windows host + a real GKE project; covered by the deterministic unit test.)
Additional context
This completes the Windows gcloud-invocation fix started for the Cloud Run path,
which introduced
_GCLOUD_CMDbut did not update the GKE call sites. After thischange, every gcloud subprocess invocation in
cli_deploy.py(Cloud Run and GKE)goes through
_GCLOUD_CMD.Diff summary (for reviewer reference; not part of the PR body)
src/google/adk/cli/cli_deploy.py(+2 / -2):'gcloud'->_GCLOUD_CMDat theto_gkebuilds submitsite and thecontainer clusters get-credentialssite.tests/unittests/cli/utils/test_cli_deploy.py(+51): new regression testtest_to_gke_uses_gcloud_cmd_on_windows.Total: 2 files, +53 / -2.