factory: generates gcp/vm stubs#1
Merged
sinorga merged 3 commits intoMay 26, 2026
Merged
Conversation
sinorga
added a commit
that referenced
this pull request
May 18, 2026
…try catch POST-PATCH FIX #2 on integration branch agent/gcp-workers-286265d1-… — follow-on to commit 7ed96e6 (post-patch fix #1). This commit addresses a code-review P2 concern raised against #1 that the operator chose to fix before drafting the upstream PR rather than carry forward as a known imperfection. ## Lineage - 7ed96e6 (post-patch fix #1, 2026-05-17): - trigger: post-review-finding (POST_REVIEW_REPORT_gcp_2026-05-17_1637.md P1#1) - changes: `_revoke_token_creator` return capture + `_modify_iam_policy` retry-on-5xx/429 + new URLError catch in both arms. - 7ed96e6 (this commit, 2026-05-18): - trigger: code-review-finding against post-patch fix #1 - changes: extend both URLError catch arms to also catch TimeoutError and OSError. The PR doc's §5 Human-in-loop edit log will render these as two separate rows with distinct triggers — `post-review-finding:cleanup-bool-propagation` on #1 and `code-review-finding:socket-timeout-escape` on #2. ## What this commit changes Both URLError catch arms in `_modify_iam_policy` (the getIamPolicy arm at ~line 304 and the setIamPolicy arm at ~line 349) are extended from `except urllib.error.URLError as e` to `except (urllib.error.URLError, TimeoutError, OSError) as e`. ## Why Code-reviewer flagged that on Python >= 3.10, a mid-read socket timeout surfaces as `TimeoutError` (alias for OSError with specific errno) and may NOT be wrapped in `urllib.error.URLError`. Connect-phase timeouts ARE wrapped (URLError(socket.timeout(...))), but if the TCP connection succeeds and the read times out mid-stream — which the existing 30s HTTP timeout can trigger on slow 5xx responses — the exception escapes the URLError catch and propagates to the outer `try` in `_main`, turning a retryable transient into a hard `early_failure` that exits the whole step. This may have been the root cause of today's first-attempt flake on `test-f557c7f1` (no structured error logged, consistent with the helper bailing out via the outer exception path rather than the retry path). Post-patch fix #1's URLError catch closed connect-phase timeouts; post-patch fix #2 closes mid-read timeouts and any other socket-level OSError that warrants retry (`ConnectionResetError`, `BrokenPipeError`, etc.). Error-message strings also updated to `network error ({type(e).__name__})` so logs show which class actually fired (URLError vs TimeoutError vs OSError subclass) for future post-mortem clarity. ## Validation (post-fix-#2) - static: test-8bfd1263 PASS (46/46 incl. no_internal_issue_ids 10/0/10) - live (full domain, both fixes + socket-timeout extension): test-b36f66d1 PASS — all 9 test steps + NIM + teardown. console_rbac: passed end-to-end. ConsoleRbacCheck PASSED — Console RBAC restricted for the target VM. Third consecutive full live PASS on this branch (after test-489a0efc and test-9fbc9ac7). ## Factory-side rule landing F-107 in docs/tracking/FACTORY-ISSUE-TRACKING.md will land a `modify_iam_with_retry` envelope in `common/errors.py` that covers BOTH the broad exception list (this commit) and exponential backoff (code- reviewer's P2 #1, deferred as factory-input work). The cross-NCP rule in `context/docs/existing-patterns/common.md` will explicitly enumerate the socket-level exception coverage so future regens emit the same shape on first generation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sinorga
force-pushed
the
agent/gcp-workers-286265d1-e5eb-4ffe-bf5f-c8e7a46ce147
branch
from
May 18, 2026 17:28
f4c1b98 to
4453f67
Compare
sinorga
added a commit
that referenced
this pull request
May 18, 2026
POST-PATCH FIX #3 on integration branch agent/gcp-workers-286265d1-…, addressing two final-run review iterations (test-a1edc537 → P1=1, test-d31fe561 → P1=1) that surfaced zone-walk contract violations the in-session reviewers + original 05-17 post-review adversarial pass all missed. Both findings flagged by ncp-review-contract-config; both trace to the same `select_zones` / `_list_region_zones` / `PREFERRED_ZONES` cluster. ## Three changes in one logical fix (1) `select_zones` zone-walk semantics `compute.py:251` now emits `preferred_in_region + cross_region` when the intersection is non-empty; only when no preferred zone is live in the region does it fall back to `other_in_region` (the region's nonpreferred zones). Prior code always concatenated `preferred + other + cross`, which let GCP VM launch attempt or land in nonpreferred zones BEFORE cross-region fallback. Violates the reviewed contract at `context/ncp-knowledge/gcp/vm.yaml:1162-1165` ("intersect with PREFERRED_ZONES when non-empty, walk in preferred-list order"). Docstring at lines 251-271 updated to match. (2) `_list_region_zones` structured error handling `compute.py:180-260` replaces the prior `except Exception → return []` with retry-with-classifier: * Transient (ServiceUnavailable, DeadlineExceeded, Aborted, ResourceExhausted, InternalServerError) retries 3× with linear backoff, then returns [] for offline fallback. * Terminal (NotFound, PermissionDenied, InvalidArgument, Unauthenticated) RAISES `RuntimeError` so the operator's region request is never silently substituted by a different region's zones. * Other `GoogleAPICallError` shapes also RAISE. Reviewer's `Required action`: "Do not turn region lookup failures into global fallback. Retry transient lookup errors, fail structured on invalid/permission lookup failures." (3) `PREFERRED_ZONES` reviewed-knowledge sync `compute.py:127-153` removes `us-east1-b` (L4-unavailable per `context/ncp-knowledge/gcp/common.yaml:89` — "us-east1 has L4 only in c + d"). Run 1's compute.py had the correct shape (`us-east1-c, us-east1-d` only); Run 2's foundation generator drifted by adding `us-east1-b`. Inline comment now anchors the list to the knowledge file's reviewed region. ## Iteration lineage on this branch - 7ed96e6 (post-patch fix #1, 2026-05-17): cleanup-bool capture + iam-retry envelope (5xx/429 + URLError catch). Driven by 05-17 post-review P1#1. - 0d985d6 (post-patch fix #2, 2026-05-18): broaden URLError catch arms with TimeoutError/OSError for mid-read coverage. Driven by code-review against #1. - THIS COMMIT (post-patch fix #3, 2026-05-18): zone-walk contract compliance — three sub-changes rolled into one logical fix. Driven by post-fix final-run review iterations #1 + #2. The PR doc's §5 Human-in-loop edit log will render these as three separate rows with distinct triggers — `post-review-finding` on #1, `code-review-finding` on #2, `post-fix-final-run-review-finding` on #3. ## Validation - static: test-fadac5cb / test-421be0ba / test-e3bd96f3 all PASS (46/46 incl. no_internal_issue_ids 10/0/10). - Live gate not re-run for fix #3 specifically — the happy-path output of `select_zones` is a strict subset of the prior shape (first candidate identical when intersection non-empty), and `_list_region_zones` happy-path returns the same zones; only error-class branches differ. Prior live gates test-489a0efc, test-9fbc9ac7, test-b36f66d1 PASSED on this branch lineage. ## Factory-side rule landing (separate, NOT in this commit) The post-fix review iterations exposed the same class of calibration gap captured by F-106 (lifecycle-cleanup audit grep doesn't match the helper-name shape that triggered the original P1). Zone-walk contract enforcement should land as: (a) an audit grep in `prompts/AGENT_REVIEW_PROMPT.md` against `select_zones`-shape walkers that requires intersect-then-walk-preferred semantics, and (b) a cross-NCP rule in `context/docs/existing-patterns/common.md` that names "operator-requested region preservation" as a cleanup-tier rule the reviewer must enforce. Tracker entry to land as a sibling to F-105/F-106/F-107 in a subsequent factory commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sinorga
added a commit
that referenced
this pull request
May 18, 2026
… probe POST-PATCH FIX #4 on integration branch agent/gcp-workers-286265d1-…, addressing the only iteration-5 review P1 that's a real bug worth fixing under the operator's "fix critical, skip concern-cases" criterion. ## Trigger Post-fix final-run review test-d31fe561 (P1=2) flagged `vm.yaml:112` `console_rbac` as missing `--network` flow-through from `launch_instance.vpc_id`. Reviewer's required action: thread the launched-network through to the probe so probe VMs in non-default- network projects validate against the same network as the launched instance. ## Why fix this one (and not the other iteration-5 finding) The other iteration-5 P1 (start_instance step timeout vs SSH stack) was identified as a FALSE POSITIVE — the reviewer interpreted `wait_for_ssh_stable` + `wait_for_cloud_init` as a single "SSH stability gate" (semantic grouping), but the audit rule at `test_harness/review/agents/ncp-review-contract-config.md:80-89` explicitly says "longest SINGLE wait" means one `wait_for_X(timeout=K)` call, not a sum of sequential waits forming a logical readiness check. The longest single wait in start_instance.py is `wait_for_cloud_init(timeout=600)` = 600s, equal to the step cap. NOT under-cap by the rule's literal definition. F-073 + F-103 in docs/tracking/FACTORY-ISSUE-TRACKING.md (closed/RETIRED 2026-05-15) established that step timeout is a CAP, not a SUM — operator's "we don't need to sum up all operation's timeout" guidance reaffirms this. Tracker entry for the false-positive prevention rule extension lands separately in factory inputs (common.md + ncp-review-contract-config.md clarifying language). ## What this commit changes `vm.yaml:112-122` adds two args to `console_rbac`: - "--network" - "{{steps.launch_instance.vpc_id | default(network, true)}}" mirroring the pattern used by `list_instances` (line 72 in the same file). `console_rbac.py:925` already accepts `--network` in its argparse and threads it through to `network=args.network` at `console_rbac.py:990` — the gap was solely in the provider config's wiring. ## Iteration lineage on this branch - 7ed96e6 (post-patch fix #1, 2026-05-17): cleanup-bool capture + iam-retry envelope. Driven by 05-17 post-review P1#1. - 0d985d6 (post-patch fix #2, 2026-05-18): broaden URLError catch with TimeoutError/OSError. Driven by code-review against #1. - c135fc7 (post-patch fix #3, 2026-05-18): zone-walk contract compliance — `other_in_region` semantics + structured error handling in `_list_region_zones` + PREFERRED_ZONES drift. Driven by post-fix final-run review iterations #1 + #2. - THIS COMMIT (post-patch fix #4, 2026-05-18): thread vpc_id to console_rbac. Driven by post-fix final-run review iteration #3 (the non-false-positive half of the P1=2 verdict). The PR doc's §5 Human-in-loop edit log will render these as four separate rows with distinct triggers. ## Validation - static: test-1c462ac1 PASS (46/46 incl. no_internal_issue_ids 10/0/10). - Live gate not re-run for this single yaml change — the wiring adds one arg pair the consuming script already supports. Prior live gates on this branch lineage (test-489a0efc, test-9fbc9ac7, test-b36f66d1) all PASSED with default-network setups; the new arg falls back to `network` (default) via `default(network, true)` on operator envs that don't set vpc_id. ## Factory-side rule landing (separate, NOT in this commit) The reviewer's iteration-3 finding for this class did NOT have a matching reviewer audit before today; the rule will land alongside F-105/F-106/F-107 as "operator argument flow-through to dependent probes" in a subsequent factory commit. False-positive prevention for the under-cap rule (clarify "longest SINGLE wait" definition) also lands separately as a `common.md` + `ncp-review-contract-config.md` edit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…te + new virtual_device_hardening step) Adds a complete GCP Compute Engine provider under `isvctl/configs/providers/gcp/` covering the full vm test suite — `launch_instance`, `list_instances`, `verify_tags`, `serial_console`, `console_rbac`, `virtual_device_hardening` (new step from PR NVIDIA#413), `stop_instance`, `start_instance`, `reboot_instance`, `describe_instance`, `deploy_nim` (shared) — plus teardown. AWS oracle contract shape preserved across all steps; GCE-specific divergences encoded per step where the API surface differs from EC2 (zone capacity walk, async-operation cleanup tracker semantics, verified-reuse `instance_created` ownership flag, IAM token-creator propagation budget, instance metadata fields for serial-console RBAC probes). Also adds operator setup documentation at `docs/references/gcp.md` (linked from `docs/getting-started.md`) covering authentication (ADC / service-account key), project-ID resolution order, required IAM roles, L4 GPU quota and zone list, NIM-step `NGC_API_KEY` handling, GPU-image default + how to override for Docker-based tests, and org-policy considerations operators commonly need to check before a clean run. GCP gets its own reference page rather than mixing into `references/aws.md` since AWS is treated as the canonical oracle example; future target NCPs follow the same per-NCP pattern. The default `--image-project` / `--image-family` are the public GCP Deep Learning VM Image (`deeplearning-platform-release/common-cu129-ubuntu-2204-nvidia-580`) so a clean checkout can run the VM suite without any private-image entitlement. The DLVM ships with the NVIDIA driver + CUDA toolkit; operators who need Docker (for the `deploy_nim` step) override the default by exporting `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` in their shell / `.env` (the provider config reads both via Jinja, identical pattern to the existing `GCP_VM_SKIP_TEARDOWN` precedent), or skip NIM by leaving `NGC_API_KEY` unset (the shared script short-circuits cleanly). `--set image=... --set image_project=...` is also accepted for one-off ad-hoc invocations. Mirrors two adjacent upstream changes that landed on main while this branch was in-flight: - PR NVIDIA#413 (commit 2cdf4d7, "feat: add virtual device hardening validation" by @mresvanis): the vm suite contract gained `virtual_device_hardening` as a new test step. The GCP mirror is a 302-line stub at `gcp/scripts/vm/virtual_device_hardening.py` — provider-evidence preamble naming Compute Engine (no customer-facing USB / clipboard surface on GCE tenant VMs), guest-side SSH probes using the same PROBE_SENTINEL framing and pattern lists as the AWS oracle, emitting the three REQUIRED_TESTS subtests (`usb_devices_disabled`, `clipboard_disabled`, `unnecessary_virtual_devices_absent`). Adds a sister `ssh_run` helper to `gcp/scripts/common/ssh_utils.py` returning `(exit_code, stdout, stderr)` with sentinel exit codes for TimeoutExpired (124) and OSError (255). - PR NVIDIA#424 (commit 6e1458a, "fix: handle AWS VM validation regressions" by @abegnoche): adds `IdentityAgent=none` alongside the existing `IdentitiesOnly=yes` so explicit `-i <key_file>` is the only SSH credential considered on operator hosts running a multi-identity ssh-agent. Mirrored to the GCP `_SSH_OPTS` tuple so every GCP SSH helper benefits uniformly. Verification: - **Static checks**: PASS (46/46). - **Independent post-generation review on branch HEAD**: `PASS p1=0 p2=0`. - **GCP orphans after teardown**: 0 on every run. - **Live execution end-to-end** — verified under both supported operator configurations: - canonical / custom-image (`NGC_API_KEY` set + `GCP_VM_IMAGE` + `GCP_VM_IMAGE_PROJECT` set to a Docker-equipped image): 3 consecutive full-domain PASS — 9 test steps + NIM deploy / teardown + cleanup, ~1650s per run. Post-upstream-refresh re-verification (with the new `virtual_device_hardening` step + IdentityAgent flag + `ssh_run` helper) also PASS — domain-gate live run `test-f65be511` 330.8s, all 11 vm steps + NIM deploy/teardown + cleanup, 0 orphans. - public-default (all three env vars unset → suite falls through to the DLVM): 1 expected FAIL on `host_os.ContainerRuntimeCheck` (Docker not available — DLVM ships NVIDIA driver + CUDA but not Docker; this is documented in `docs/references/gcp.md` §5 as a known portability gap of the public default). Everything else PASS: all 9 lifecycle steps, NIM steps cleanly `runtime_skip`, both `deploy_nim` and `teardown_nim` policy-skip honored end-to-end. 1113.5s. Known gaps: - `ContainerRuntimeCheck` fails on the public DLVM default image (documented above). Operators who need a fully green run override the image via `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` to a Docker-equipped image. The public default is published as "boot + lifecycle works without any private entitlement", not as "every validator passes" — see `docs/references/gcp.md` §5. Branch history before squash-for-PR: 32 commits — initial factory-generated foundation + per-step worker + domain-gate + final-review-fix commits, plus 7 operator hand-fixes addressing post-generation review findings (cleanup-bool propagation, socket-timeout retry, zone-walk contract, probe-network arg threading, factory-vocabulary scrub, warm-reuse stability gate, symmetric NIM teardown policy-skip, env-var override architecture), plus 2 docs commits (operator setup reference + ContainerRuntimeCheck clarification), plus a single squashed factory-update-mode regen against the new upstream main tip 9b79e34 bringing the two adjacent upstream mirrors above. Squashed here for a single-commit upstream PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com>
sinorga
force-pushed
the
agent/gcp-workers-286265d1-e5eb-4ffe-bf5f-c8e7a46ce147
branch
from
May 18, 2026 17:54
4453f67 to
0d8dcd7
Compare
…t review feedback) Three helper functions surfaced by CodeRabbit's docstring-coverage quality gate (78.72% < 80% threshold on PR NVIDIA#427) lacked PEP 257 docstrings: - common/compute.py: first_internal_ip (returns the nic0 internal IPv4 or None when the instance has no NICs / hasn't allocated yet). - common/compute.py: _firewall_has_isv_ownership (verified-reuse ownership marker check — distinguishes firewalls this suite created from operator-owned firewalls that happen to match the test name). - common/ssh_utils.py: _ssh_argv (canonical ssh argv builder — shared _SSH_OPTS + -i <key> + <user>@<host> + <remote_cmd> — so every subprocess SSH call in this module uses identical option semantics, per the canonical-options consistency rule). Each function gets a one-paragraph docstring (one-line summary + a second line of context where it adds value). Coverage on the gcp/scripts tree post-fix: 83.15% (74/89), above the 80% threshold. Kept as a separate commit (not amended into the squash) so the PR's "Commits" tab shows a transparent response to CodeRabbit's review feedback — easier for the upstream maintainer to see exactly what changed in response to a specific review-cycle pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com>
…MD040) CodeRabbit's review of PR NVIDIA#427 flagged the bare ``` fence around the L4-supported-zones region list as a markdownlint MD040 violation. Changing to ```text marks the block explicitly as plain text so linters don't trip while preserving the rendering (text is the de-facto fallback for un-tagged fences). One-character class change; no behavior or rendering difference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com>
sinorga
marked this pull request as ready for review
May 23, 2026 04:35
sinorga
added a commit
that referenced
this pull request
Jun 3, 2026
* factory: generates gcp/vm stubs (Compute Engine provider, full vm suite + new virtual_device_hardening step) Adds a complete GCP Compute Engine provider under `isvctl/configs/providers/gcp/` covering the full vm test suite — `launch_instance`, `list_instances`, `verify_tags`, `serial_console`, `console_rbac`, `virtual_device_hardening` (new step from PR NVIDIA#413), `stop_instance`, `start_instance`, `reboot_instance`, `describe_instance`, `deploy_nim` (shared) — plus teardown. AWS oracle contract shape preserved across all steps; GCE-specific divergences encoded per step where the API surface differs from EC2 (zone capacity walk, async-operation cleanup tracker semantics, verified-reuse `instance_created` ownership flag, IAM token-creator propagation budget, instance metadata fields for serial-console RBAC probes). Also adds operator setup documentation at `docs/references/gcp.md` (linked from `docs/getting-started.md`) covering authentication (ADC / service-account key), project-ID resolution order, required IAM roles, L4 GPU quota and zone list, NIM-step `NGC_API_KEY` handling, GPU-image default + how to override for Docker-based tests, and org-policy considerations operators commonly need to check before a clean run. GCP gets its own reference page rather than mixing into `references/aws.md` since AWS is treated as the canonical oracle example; future target NCPs follow the same per-NCP pattern. The default `--image-project` / `--image-family` are the public GCP Deep Learning VM Image (`deeplearning-platform-release/common-cu129-ubuntu-2204-nvidia-580`) so a clean checkout can run the VM suite without any private-image entitlement. The DLVM ships with the NVIDIA driver + CUDA toolkit; operators who need Docker (for the `deploy_nim` step) override the default by exporting `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` in their shell / `.env` (the provider config reads both via Jinja, identical pattern to the existing `GCP_VM_SKIP_TEARDOWN` precedent), or skip NIM by leaving `NGC_API_KEY` unset (the shared script short-circuits cleanly). `--set image=... --set image_project=...` is also accepted for one-off ad-hoc invocations. Mirrors two adjacent upstream changes that landed on main while this branch was in-flight: - PR NVIDIA#413 (commit 2cdf4d7, "feat: add virtual device hardening validation" by @mresvanis): the vm suite contract gained `virtual_device_hardening` as a new test step. The GCP mirror is a 302-line stub at `gcp/scripts/vm/virtual_device_hardening.py` — provider-evidence preamble naming Compute Engine (no customer-facing USB / clipboard surface on GCE tenant VMs), guest-side SSH probes using the same PROBE_SENTINEL framing and pattern lists as the AWS oracle, emitting the three REQUIRED_TESTS subtests (`usb_devices_disabled`, `clipboard_disabled`, `unnecessary_virtual_devices_absent`). Adds a sister `ssh_run` helper to `gcp/scripts/common/ssh_utils.py` returning `(exit_code, stdout, stderr)` with sentinel exit codes for TimeoutExpired (124) and OSError (255). - PR NVIDIA#424 (commit 6e1458a, "fix: handle AWS VM validation regressions" by @abegnoche): adds `IdentityAgent=none` alongside the existing `IdentitiesOnly=yes` so explicit `-i <key_file>` is the only SSH credential considered on operator hosts running a multi-identity ssh-agent. Mirrored to the GCP `_SSH_OPTS` tuple so every GCP SSH helper benefits uniformly. Verification: - **Static checks**: PASS (46/46). - **Independent post-generation review on branch HEAD**: `PASS p1=0 p2=0`. - **GCP orphans after teardown**: 0 on every run. - **Live execution end-to-end** — verified under both supported operator configurations: - canonical / custom-image (`NGC_API_KEY` set + `GCP_VM_IMAGE` + `GCP_VM_IMAGE_PROJECT` set to a Docker-equipped image): 3 consecutive full-domain PASS — 9 test steps + NIM deploy / teardown + cleanup, ~1650s per run. Post-upstream-refresh re-verification (with the new `virtual_device_hardening` step + IdentityAgent flag + `ssh_run` helper) also PASS — domain-gate live run `test-f65be511` 330.8s, all 11 vm steps + NIM deploy/teardown + cleanup, 0 orphans. - public-default (all three env vars unset → suite falls through to the DLVM): 1 expected FAIL on `host_os.ContainerRuntimeCheck` (Docker not available — DLVM ships NVIDIA driver + CUDA but not Docker; this is documented in `docs/references/gcp.md` §5 as a known portability gap of the public default). Everything else PASS: all 9 lifecycle steps, NIM steps cleanly `runtime_skip`, both `deploy_nim` and `teardown_nim` policy-skip honored end-to-end. 1113.5s. Known gaps: - `ContainerRuntimeCheck` fails on the public DLVM default image (documented above). Operators who need a fully green run override the image via `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` to a Docker-equipped image. The public default is published as "boot + lifecycle works without any private entitlement", not as "every validator passes" — see `docs/references/gcp.md` §5. Branch history before squash-for-PR: 32 commits — initial factory-generated foundation + per-step worker + domain-gate + final-review-fix commits, plus 7 operator hand-fixes addressing post-generation review findings (cleanup-bool propagation, socket-timeout retry, zone-walk contract, probe-network arg threading, factory-vocabulary scrub, warm-reuse stability gate, symmetric NIM teardown policy-skip, env-var override architecture), plus 2 docs commits (operator setup reference + ContainerRuntimeCheck clarification), plus a single squashed factory-update-mode regen against the new upstream main tip 9b79e34 bringing the two adjacent upstream mirrors above. Squashed here for a single-commit upstream PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> * fix(gcp/common): add PEP 257 docstrings to missing helpers (CodeRabbit review feedback) Three helper functions surfaced by CodeRabbit's docstring-coverage quality gate (78.72% < 80% threshold on PR NVIDIA#427) lacked PEP 257 docstrings: - common/compute.py: first_internal_ip (returns the nic0 internal IPv4 or None when the instance has no NICs / hasn't allocated yet). - common/compute.py: _firewall_has_isv_ownership (verified-reuse ownership marker check — distinguishes firewalls this suite created from operator-owned firewalls that happen to match the test name). - common/ssh_utils.py: _ssh_argv (canonical ssh argv builder — shared _SSH_OPTS + -i <key> + <user>@<host> + <remote_cmd> — so every subprocess SSH call in this module uses identical option semantics, per the canonical-options consistency rule). Each function gets a one-paragraph docstring (one-line summary + a second line of context where it adds value). Coverage on the gcp/scripts tree post-fix: 83.15% (74/89), above the 80% threshold. Kept as a separate commit (not amended into the squash) so the PR's "Commits" tab shows a transparent response to CodeRabbit's review feedback — easier for the upstream maintainer to see exactly what changed in response to a specific review-cycle pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> * docs(references/gcp): specify text language for L4-zones code fence (MD040) CodeRabbit's review of PR NVIDIA#427 flagged the bare ``` fence around the L4-supported-zones region list as a markdownlint MD040 violation. Changing to ```text marks the block explicitly as plain text so linters don't trip while preserving the rendering (text is the de-facto fallback for un-tagged fences). One-character class change; no behavior or rendering difference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> --------- Signed-off-by: Orga Shih <oshih@nvidia.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sinorga
added a commit
that referenced
this pull request
Jun 22, 2026
* factory: generates gcp/vm stubs (Compute Engine provider, full vm suite + new virtual_device_hardening step) Adds a complete GCP Compute Engine provider under `isvctl/configs/providers/gcp/` covering the full vm test suite — `launch_instance`, `list_instances`, `verify_tags`, `serial_console`, `console_rbac`, `virtual_device_hardening` (new step from PR NVIDIA#413), `stop_instance`, `start_instance`, `reboot_instance`, `describe_instance`, `deploy_nim` (shared) — plus teardown. AWS oracle contract shape preserved across all steps; GCE-specific divergences encoded per step where the API surface differs from EC2 (zone capacity walk, async-operation cleanup tracker semantics, verified-reuse `instance_created` ownership flag, IAM token-creator propagation budget, instance metadata fields for serial-console RBAC probes). Also adds operator setup documentation at `docs/references/gcp.md` (linked from `docs/getting-started.md`) covering authentication (ADC / service-account key), project-ID resolution order, required IAM roles, L4 GPU quota and zone list, NIM-step `NGC_API_KEY` handling, GPU-image default + how to override for Docker-based tests, and org-policy considerations operators commonly need to check before a clean run. GCP gets its own reference page rather than mixing into `references/aws.md` since AWS is treated as the canonical oracle example; future target NCPs follow the same per-NCP pattern. The default `--image-project` / `--image-family` are the public GCP Deep Learning VM Image (`deeplearning-platform-release/common-cu129-ubuntu-2204-nvidia-580`) so a clean checkout can run the VM suite without any private-image entitlement. The DLVM ships with the NVIDIA driver + CUDA toolkit; operators who need Docker (for the `deploy_nim` step) override the default by exporting `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` in their shell / `.env` (the provider config reads both via Jinja, identical pattern to the existing `GCP_VM_SKIP_TEARDOWN` precedent), or skip NIM by leaving `NGC_API_KEY` unset (the shared script short-circuits cleanly). `--set image=... --set image_project=...` is also accepted for one-off ad-hoc invocations. Mirrors two adjacent upstream changes that landed on main while this branch was in-flight: - PR NVIDIA#413 (commit 2cdf4d7, "feat: add virtual device hardening validation" by @mresvanis): the vm suite contract gained `virtual_device_hardening` as a new test step. The GCP mirror is a 302-line stub at `gcp/scripts/vm/virtual_device_hardening.py` — provider-evidence preamble naming Compute Engine (no customer-facing USB / clipboard surface on GCE tenant VMs), guest-side SSH probes using the same PROBE_SENTINEL framing and pattern lists as the AWS oracle, emitting the three REQUIRED_TESTS subtests (`usb_devices_disabled`, `clipboard_disabled`, `unnecessary_virtual_devices_absent`). Adds a sister `ssh_run` helper to `gcp/scripts/common/ssh_utils.py` returning `(exit_code, stdout, stderr)` with sentinel exit codes for TimeoutExpired (124) and OSError (255). - PR NVIDIA#424 (commit 6e1458a, "fix: handle AWS VM validation regressions" by @abegnoche): adds `IdentityAgent=none` alongside the existing `IdentitiesOnly=yes` so explicit `-i <key_file>` is the only SSH credential considered on operator hosts running a multi-identity ssh-agent. Mirrored to the GCP `_SSH_OPTS` tuple so every GCP SSH helper benefits uniformly. Verification: - **Static checks**: PASS (46/46). - **Independent post-generation review on branch HEAD**: `PASS p1=0 p2=0`. - **GCP orphans after teardown**: 0 on every run. - **Live execution end-to-end** — verified under both supported operator configurations: - canonical / custom-image (`NGC_API_KEY` set + `GCP_VM_IMAGE` + `GCP_VM_IMAGE_PROJECT` set to a Docker-equipped image): 3 consecutive full-domain PASS — 9 test steps + NIM deploy / teardown + cleanup, ~1650s per run. Post-upstream-refresh re-verification (with the new `virtual_device_hardening` step + IdentityAgent flag + `ssh_run` helper) also PASS — domain-gate live run `test-f65be511` 330.8s, all 11 vm steps + NIM deploy/teardown + cleanup, 0 orphans. - public-default (all three env vars unset → suite falls through to the DLVM): 1 expected FAIL on `host_os.ContainerRuntimeCheck` (Docker not available — DLVM ships NVIDIA driver + CUDA but not Docker; this is documented in `docs/references/gcp.md` §5 as a known portability gap of the public default). Everything else PASS: all 9 lifecycle steps, NIM steps cleanly `runtime_skip`, both `deploy_nim` and `teardown_nim` policy-skip honored end-to-end. 1113.5s. Known gaps: - `ContainerRuntimeCheck` fails on the public DLVM default image (documented above). Operators who need a fully green run override the image via `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` to a Docker-equipped image. The public default is published as "boot + lifecycle works without any private entitlement", not as "every validator passes" — see `docs/references/gcp.md` §5. Branch history before squash-for-PR: 32 commits — initial factory-generated foundation + per-step worker + domain-gate + final-review-fix commits, plus 7 operator hand-fixes addressing post-generation review findings (cleanup-bool propagation, socket-timeout retry, zone-walk contract, probe-network arg threading, factory-vocabulary scrub, warm-reuse stability gate, symmetric NIM teardown policy-skip, env-var override architecture), plus 2 docs commits (operator setup reference + ContainerRuntimeCheck clarification), plus a single squashed factory-update-mode regen against the new upstream main tip 9b79e34 bringing the two adjacent upstream mirrors above. Squashed here for a single-commit upstream PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> * fix(gcp/common): add PEP 257 docstrings to missing helpers (CodeRabbit review feedback) Three helper functions surfaced by CodeRabbit's docstring-coverage quality gate (78.72% < 80% threshold on PR NVIDIA#427) lacked PEP 257 docstrings: - common/compute.py: first_internal_ip (returns the nic0 internal IPv4 or None when the instance has no NICs / hasn't allocated yet). - common/compute.py: _firewall_has_isv_ownership (verified-reuse ownership marker check — distinguishes firewalls this suite created from operator-owned firewalls that happen to match the test name). - common/ssh_utils.py: _ssh_argv (canonical ssh argv builder — shared _SSH_OPTS + -i <key> + <user>@<host> + <remote_cmd> — so every subprocess SSH call in this module uses identical option semantics, per the canonical-options consistency rule). Each function gets a one-paragraph docstring (one-line summary + a second line of context where it adds value). Coverage on the gcp/scripts tree post-fix: 83.15% (74/89), above the 80% threshold. Kept as a separate commit (not amended into the squash) so the PR's "Commits" tab shows a transparent response to CodeRabbit's review feedback — easier for the upstream maintainer to see exactly what changed in response to a specific review-cycle pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> * docs(references/gcp): specify text language for L4-zones code fence (MD040) CodeRabbit's review of PR NVIDIA#427 flagged the bare ``` fence around the L4-supported-zones region list as a markdownlint MD040 violation. Changing to ```text marks the block explicitly as plain text so linters don't trip while preserving the rendering (text is the de-facto fallback for un-tagged fences). One-character class change; no behavior or rendering difference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> --------- Signed-off-by: Orga Shih <oshih@nvidia.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sinorga
added a commit
that referenced
this pull request
Jul 16, 2026
* factory: generates gcp/vm stubs (Compute Engine provider, full vm suite + new virtual_device_hardening step) Adds a complete GCP Compute Engine provider under `isvctl/configs/providers/gcp/` covering the full vm test suite — `launch_instance`, `list_instances`, `verify_tags`, `serial_console`, `console_rbac`, `virtual_device_hardening` (new step from PR NVIDIA#413), `stop_instance`, `start_instance`, `reboot_instance`, `describe_instance`, `deploy_nim` (shared) — plus teardown. AWS oracle contract shape preserved across all steps; GCE-specific divergences encoded per step where the API surface differs from EC2 (zone capacity walk, async-operation cleanup tracker semantics, verified-reuse `instance_created` ownership flag, IAM token-creator propagation budget, instance metadata fields for serial-console RBAC probes). Also adds operator setup documentation at `docs/references/gcp.md` (linked from `docs/getting-started.md`) covering authentication (ADC / service-account key), project-ID resolution order, required IAM roles, L4 GPU quota and zone list, NIM-step `NGC_API_KEY` handling, GPU-image default + how to override for Docker-based tests, and org-policy considerations operators commonly need to check before a clean run. GCP gets its own reference page rather than mixing into `references/aws.md` since AWS is treated as the canonical oracle example; future target NCPs follow the same per-NCP pattern. The default `--image-project` / `--image-family` are the public GCP Deep Learning VM Image (`deeplearning-platform-release/common-cu129-ubuntu-2204-nvidia-580`) so a clean checkout can run the VM suite without any private-image entitlement. The DLVM ships with the NVIDIA driver + CUDA toolkit; operators who need Docker (for the `deploy_nim` step) override the default by exporting `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` in their shell / `.env` (the provider config reads both via Jinja, identical pattern to the existing `GCP_VM_SKIP_TEARDOWN` precedent), or skip NIM by leaving `NGC_API_KEY` unset (the shared script short-circuits cleanly). `--set image=... --set image_project=...` is also accepted for one-off ad-hoc invocations. Mirrors two adjacent upstream changes that landed on main while this branch was in-flight: - PR NVIDIA#413 (commit 2cdf4d7, "feat: add virtual device hardening validation" by @mresvanis): the vm suite contract gained `virtual_device_hardening` as a new test step. The GCP mirror is a 302-line stub at `gcp/scripts/vm/virtual_device_hardening.py` — provider-evidence preamble naming Compute Engine (no customer-facing USB / clipboard surface on GCE tenant VMs), guest-side SSH probes using the same PROBE_SENTINEL framing and pattern lists as the AWS oracle, emitting the three REQUIRED_TESTS subtests (`usb_devices_disabled`, `clipboard_disabled`, `unnecessary_virtual_devices_absent`). Adds a sister `ssh_run` helper to `gcp/scripts/common/ssh_utils.py` returning `(exit_code, stdout, stderr)` with sentinel exit codes for TimeoutExpired (124) and OSError (255). - PR NVIDIA#424 (commit 6e1458a, "fix: handle AWS VM validation regressions" by @abegnoche): adds `IdentityAgent=none` alongside the existing `IdentitiesOnly=yes` so explicit `-i <key_file>` is the only SSH credential considered on operator hosts running a multi-identity ssh-agent. Mirrored to the GCP `_SSH_OPTS` tuple so every GCP SSH helper benefits uniformly. Verification: - **Static checks**: PASS (46/46). - **Independent post-generation review on branch HEAD**: `PASS p1=0 p2=0`. - **GCP orphans after teardown**: 0 on every run. - **Live execution end-to-end** — verified under both supported operator configurations: - canonical / custom-image (`NGC_API_KEY` set + `GCP_VM_IMAGE` + `GCP_VM_IMAGE_PROJECT` set to a Docker-equipped image): 3 consecutive full-domain PASS — 9 test steps + NIM deploy / teardown + cleanup, ~1650s per run. Post-upstream-refresh re-verification (with the new `virtual_device_hardening` step + IdentityAgent flag + `ssh_run` helper) also PASS — domain-gate live run `test-f65be511` 330.8s, all 11 vm steps + NIM deploy/teardown + cleanup, 0 orphans. - public-default (all three env vars unset → suite falls through to the DLVM): 1 expected FAIL on `host_os.ContainerRuntimeCheck` (Docker not available — DLVM ships NVIDIA driver + CUDA but not Docker; this is documented in `docs/references/gcp.md` §5 as a known portability gap of the public default). Everything else PASS: all 9 lifecycle steps, NIM steps cleanly `runtime_skip`, both `deploy_nim` and `teardown_nim` policy-skip honored end-to-end. 1113.5s. Known gaps: - `ContainerRuntimeCheck` fails on the public DLVM default image (documented above). Operators who need a fully green run override the image via `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` to a Docker-equipped image. The public default is published as "boot + lifecycle works without any private entitlement", not as "every validator passes" — see `docs/references/gcp.md` §5. Branch history before squash-for-PR: 32 commits — initial factory-generated foundation + per-step worker + domain-gate + final-review-fix commits, plus 7 operator hand-fixes addressing post-generation review findings (cleanup-bool propagation, socket-timeout retry, zone-walk contract, probe-network arg threading, factory-vocabulary scrub, warm-reuse stability gate, symmetric NIM teardown policy-skip, env-var override architecture), plus 2 docs commits (operator setup reference + ContainerRuntimeCheck clarification), plus a single squashed factory-update-mode regen against the new upstream main tip 9b79e34 bringing the two adjacent upstream mirrors above. Squashed here for a single-commit upstream PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> * fix(gcp/common): add PEP 257 docstrings to missing helpers (CodeRabbit review feedback) Three helper functions surfaced by CodeRabbit's docstring-coverage quality gate (78.72% < 80% threshold on PR NVIDIA#427) lacked PEP 257 docstrings: - common/compute.py: first_internal_ip (returns the nic0 internal IPv4 or None when the instance has no NICs / hasn't allocated yet). - common/compute.py: _firewall_has_isv_ownership (verified-reuse ownership marker check — distinguishes firewalls this suite created from operator-owned firewalls that happen to match the test name). - common/ssh_utils.py: _ssh_argv (canonical ssh argv builder — shared _SSH_OPTS + -i <key> + <user>@<host> + <remote_cmd> — so every subprocess SSH call in this module uses identical option semantics, per the canonical-options consistency rule). Each function gets a one-paragraph docstring (one-line summary + a second line of context where it adds value). Coverage on the gcp/scripts tree post-fix: 83.15% (74/89), above the 80% threshold. Kept as a separate commit (not amended into the squash) so the PR's "Commits" tab shows a transparent response to CodeRabbit's review feedback — easier for the upstream maintainer to see exactly what changed in response to a specific review-cycle pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> * docs(references/gcp): specify text language for L4-zones code fence (MD040) CodeRabbit's review of PR NVIDIA#427 flagged the bare ``` fence around the L4-supported-zones region list as a markdownlint MD040 violation. Changing to ```text marks the block explicitly as plain text so linters don't trip while preserving the rendering (text is the de-facto fallback for un-tagged fences). One-character class change; no behavior or rendering difference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> --------- Signed-off-by: Orga Shih <oshih@nvidia.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sinorga
added a commit
that referenced
this pull request
Jul 24, 2026
* factory: generates gcp/vm stubs (Compute Engine provider, full vm suite + new virtual_device_hardening step) Adds a complete GCP Compute Engine provider under `isvctl/configs/providers/gcp/` covering the full vm test suite — `launch_instance`, `list_instances`, `verify_tags`, `serial_console`, `console_rbac`, `virtual_device_hardening` (new step from PR NVIDIA#413), `stop_instance`, `start_instance`, `reboot_instance`, `describe_instance`, `deploy_nim` (shared) — plus teardown. AWS oracle contract shape preserved across all steps; GCE-specific divergences encoded per step where the API surface differs from EC2 (zone capacity walk, async-operation cleanup tracker semantics, verified-reuse `instance_created` ownership flag, IAM token-creator propagation budget, instance metadata fields for serial-console RBAC probes). Also adds operator setup documentation at `docs/references/gcp.md` (linked from `docs/getting-started.md`) covering authentication (ADC / service-account key), project-ID resolution order, required IAM roles, L4 GPU quota and zone list, NIM-step `NGC_API_KEY` handling, GPU-image default + how to override for Docker-based tests, and org-policy considerations operators commonly need to check before a clean run. GCP gets its own reference page rather than mixing into `references/aws.md` since AWS is treated as the canonical oracle example; future target NCPs follow the same per-NCP pattern. The default `--image-project` / `--image-family` are the public GCP Deep Learning VM Image (`deeplearning-platform-release/common-cu129-ubuntu-2204-nvidia-580`) so a clean checkout can run the VM suite without any private-image entitlement. The DLVM ships with the NVIDIA driver + CUDA toolkit; operators who need Docker (for the `deploy_nim` step) override the default by exporting `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` in their shell / `.env` (the provider config reads both via Jinja, identical pattern to the existing `GCP_VM_SKIP_TEARDOWN` precedent), or skip NIM by leaving `NGC_API_KEY` unset (the shared script short-circuits cleanly). `--set image=... --set image_project=...` is also accepted for one-off ad-hoc invocations. Mirrors two adjacent upstream changes that landed on main while this branch was in-flight: - PR NVIDIA#413 (commit 2cdf4d7, "feat: add virtual device hardening validation" by @mresvanis): the vm suite contract gained `virtual_device_hardening` as a new test step. The GCP mirror is a 302-line stub at `gcp/scripts/vm/virtual_device_hardening.py` — provider-evidence preamble naming Compute Engine (no customer-facing USB / clipboard surface on GCE tenant VMs), guest-side SSH probes using the same PROBE_SENTINEL framing and pattern lists as the AWS oracle, emitting the three REQUIRED_TESTS subtests (`usb_devices_disabled`, `clipboard_disabled`, `unnecessary_virtual_devices_absent`). Adds a sister `ssh_run` helper to `gcp/scripts/common/ssh_utils.py` returning `(exit_code, stdout, stderr)` with sentinel exit codes for TimeoutExpired (124) and OSError (255). - PR NVIDIA#424 (commit 6e1458a, "fix: handle AWS VM validation regressions" by @abegnoche): adds `IdentityAgent=none` alongside the existing `IdentitiesOnly=yes` so explicit `-i <key_file>` is the only SSH credential considered on operator hosts running a multi-identity ssh-agent. Mirrored to the GCP `_SSH_OPTS` tuple so every GCP SSH helper benefits uniformly. Verification: - **Static checks**: PASS (46/46). - **Independent post-generation review on branch HEAD**: `PASS p1=0 p2=0`. - **GCP orphans after teardown**: 0 on every run. - **Live execution end-to-end** — verified under both supported operator configurations: - canonical / custom-image (`NGC_API_KEY` set + `GCP_VM_IMAGE` + `GCP_VM_IMAGE_PROJECT` set to a Docker-equipped image): 3 consecutive full-domain PASS — 9 test steps + NIM deploy / teardown + cleanup, ~1650s per run. Post-upstream-refresh re-verification (with the new `virtual_device_hardening` step + IdentityAgent flag + `ssh_run` helper) also PASS — domain-gate live run `test-f65be511` 330.8s, all 11 vm steps + NIM deploy/teardown + cleanup, 0 orphans. - public-default (all three env vars unset → suite falls through to the DLVM): 1 expected FAIL on `host_os.ContainerRuntimeCheck` (Docker not available — DLVM ships NVIDIA driver + CUDA but not Docker; this is documented in `docs/references/gcp.md` §5 as a known portability gap of the public default). Everything else PASS: all 9 lifecycle steps, NIM steps cleanly `runtime_skip`, both `deploy_nim` and `teardown_nim` policy-skip honored end-to-end. 1113.5s. Known gaps: - `ContainerRuntimeCheck` fails on the public DLVM default image (documented above). Operators who need a fully green run override the image via `GCP_VM_IMAGE` / `GCP_VM_IMAGE_PROJECT` to a Docker-equipped image. The public default is published as "boot + lifecycle works without any private entitlement", not as "every validator passes" — see `docs/references/gcp.md` §5. Branch history before squash-for-PR: 32 commits — initial factory-generated foundation + per-step worker + domain-gate + final-review-fix commits, plus 7 operator hand-fixes addressing post-generation review findings (cleanup-bool propagation, socket-timeout retry, zone-walk contract, probe-network arg threading, factory-vocabulary scrub, warm-reuse stability gate, symmetric NIM teardown policy-skip, env-var override architecture), plus 2 docs commits (operator setup reference + ContainerRuntimeCheck clarification), plus a single squashed factory-update-mode regen against the new upstream main tip 9b79e34 bringing the two adjacent upstream mirrors above. Squashed here for a single-commit upstream PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> * fix(gcp/common): add PEP 257 docstrings to missing helpers (CodeRabbit review feedback) Three helper functions surfaced by CodeRabbit's docstring-coverage quality gate (78.72% < 80% threshold on PR NVIDIA#427) lacked PEP 257 docstrings: - common/compute.py: first_internal_ip (returns the nic0 internal IPv4 or None when the instance has no NICs / hasn't allocated yet). - common/compute.py: _firewall_has_isv_ownership (verified-reuse ownership marker check — distinguishes firewalls this suite created from operator-owned firewalls that happen to match the test name). - common/ssh_utils.py: _ssh_argv (canonical ssh argv builder — shared _SSH_OPTS + -i <key> + <user>@<host> + <remote_cmd> — so every subprocess SSH call in this module uses identical option semantics, per the canonical-options consistency rule). Each function gets a one-paragraph docstring (one-line summary + a second line of context where it adds value). Coverage on the gcp/scripts tree post-fix: 83.15% (74/89), above the 80% threshold. Kept as a separate commit (not amended into the squash) so the PR's "Commits" tab shows a transparent response to CodeRabbit's review feedback — easier for the upstream maintainer to see exactly what changed in response to a specific review-cycle pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> * docs(references/gcp): specify text language for L4-zones code fence (MD040) CodeRabbit's review of PR NVIDIA#427 flagged the bare ``` fence around the L4-supported-zones region list as a markdownlint MD040 violation. Changing to ```text marks the block explicitly as plain text so linters don't trip while preserving the rendering (text is the de-facto fallback for un-tagged fences). One-character class change; no behavior or rendering difference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Orga Shih <oshih@nvidia.com> --------- Signed-off-by: Orga Shih <oshih@nvidia.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
factory: generates/updates gcp/vm stubs
Summary
Adds GCP VM provider stubs covering the full lifecycle suite (
launch_instance/list_instances/verify_tags/serial_console/console_rbac/stop_instance/start_instance/reboot_instance/describe_instance/deploy_nim) plus teardown. AWS oracle contract shape preserved across all steps; GCE-specific divergences encoded per step where the API surface differs from EC2 (zone capacity walk, async-operation cleanup tracker semantics, verified-reuseinstance_createdownership flag, IAM token-creator propagation budget, instance metadata fields for serial-console RBAC probes).Also adds operator setup documentation at
docs/references/gcp.md(linked fromdocs/getting-started.md) covering authentication (ADC / service-account key), project-ID resolution order, required IAM roles, L4 GPU quota and zone list, NIM-stepNGC_API_KEYhandling, GPU-image default + how to override for Docker-based tests, and org-policy considerations operators commonly need to check before a clean run. GCP gets its own reference page rather than mixing intoreferences/aws.mdsince AWS is treated as the canonical oracle example; future target NCPs follow the same per-NCP pattern.The default
--image-project/--image-familyare now the public GCP Deep Learning VM Image (deeplearning-platform-release/common-cu129-ubuntu-2204-nvidia-580) so a clean checkout can run the VM suite without any private-image entitlement. The DLVM ships with the NVIDIA driver + CUDA toolkit; operators who need Docker (for thedeploy_nimstep) override the default by exportingGCP_VM_IMAGE/GCP_VM_IMAGE_PROJECTin their shell /.env(the provider config reads both via Jinja, identical pattern to the existingGCP_VM_SKIP_TEARDOWNprecedent), or skip NIM by leavingNGC_API_KEYunset (the shared script short-circuits cleanly).--set image=... --set image_project=...is also accepted for one-off ad-hoc invocations.Verification
PASS p1=0 p2=0.NGC_API_KEYset +GCP_VM_IMAGE+GCP_VM_IMAGE_PROJECTset to a Docker-equipped imagedeeplearning-platform-release/common-cu129-ubuntu-2204-nvidia-580[container_runtime] ContainerRuntimeCheck(Docker not available — documented limitation, see "Known gaps" below). Everything else PASS: all 9 lifecycle steps, NIM steps cleanlyruntime_skip, bothdeploy_nimandteardown_nimpolicy-skip honored end-to-end4453f677content (newvirtual_device_hardeningstep +IdentityAgent=noneflag +ssh_runhelper)virtual_device_hardening) + NIM deploy / teardown + cleanup. 0 orphans. Phase-2 harness review:PASS p1=0 p2=0~1650sfigure included the full step-by-step factory loop; Pass 2's gate is the same suite content)Known gaps
ContainerRuntimeCheckfails on the public DLVM default image.host_os.ContainerRuntimeCheckasserts Docker is installed and runnable on the launched VM; the GCP Deep Learning VM image ships the NVIDIA driver + CUDA but not Docker, so the check fails withDocker not availableand the run reports[FAIL] TESTregardless of whetherNGC_API_KEYis set. This is intentional and documented atdocs/references/gcp.md§5 — operators get a clean PASS by overriding the image viaGCP_VM_IMAGE/GCP_VM_IMAGE_PROJECTto a Docker-equipped image. The public default is published as "boot + lifecycle works without any private entitlement", not as "every validator passes".Provenance
The branch's content was produced in two passes, both validated end-to-end:
Pass 1 — Initial generation + post-review hand-fixes (rebased onto upstream
maintip871eea8c). Auto-generated factory output for the full vm domain, followed by 4 post-review hand-fixes and 3 pre-PR public-shipping audit fixes:577b227cisvctl/configs/providers/gcp/scripts/vm/console_rbac.py_revoke_token_creatorreturn intocleanup_errorson failure (mirrors sibling_delete_service_accountpattern; rule #9 honest-reporting compliance). Add retry envelope to_modify_iam_policycovering 5xx / 429 / 409 stale-etag plus transient network failures.70a7d11aisvctl/configs/providers/gcp/scripts/vm/console_rbac.pyTimeoutErrorandOSErrorso a mid-read socket timeout (which Python ≥3.10 does not always wrap inurllib.error.URLError) routes through retry instead of escaping the helper.f8c48b1aisvctl/configs/providers/gcp/scripts/common/compute.pyselect_zoneswalks the intersection ofPREFERRED_ZONESand the region's live zones in preferred-list order when the intersection is non-empty (noother_in_regionpromotion). Structured error handling in_list_region_zones: raise onNotFound/PermissionDenied/InvalidArgument/Unauthenticatedso an invalid region request never silently substitutes a different region's zones; retry transients (ServiceUnavailable/DeadlineExceeded/Aborted/ResourceExhausted/InternalServerError) with linear backoff. Removeus-east1-bfromPREFERRED_ZONES(no L4 capacity in that zone per published GCP GPU regions).5ec91a8fisvctl/configs/providers/gcp/config/vm.yaml--networkfromlaunch_instance.vpc_idto theconsole_rbacprobe so probe VMs validate console-access restrictions against the same network as the launched instance. Falls back to the suite-levelnetworkarg whenvpc_idisn't emitted.59bb99a6docs/references/gcp.md,docs/getting-started.mddocs/getting-started.md.c69445b2isvctl/configs/providers/gcp/**+docs/references/gcp.mddeeplearning-platform-release/common-cu129-ubuntu-2204-nvidia-580); wireimage_projectthrough as a per-run override setting; document the Docker-not-preinstalled implication for thedeploy_nimstep in the operator setup page. The DLVM is the closest portable equivalent to AWS Deep Learning AMIs and lets a clean upstream checkout run the VM suite without any private-image entitlement.225ae780isvctl/configs/providers/gcp/config/vm.yaml,docs/references/gcp.md--set.vm.yamlsettings now Jinja-readGCP_VM_IMAGE/GCP_VM_IMAGE_PROJECTwith the"none"sentinel as fallback (the stub then uses its public DLVM default), mirroring the existingGCP_VM_SKIP_TEARDOWNprecedent in this same file. Operators with a Docker-equipped custom image export the env vars once in their shell /.env; every subsequent run reuses the same pin without touching CLI flags.--set image=... --set image_project=...stays accepted for one-off invocations. Operator setup page (docs/references/gcp.md§5) updated to lead with the env-var path.9fe7a276gcp/scripts/vm/launch_instance.py,gcp/scripts/vm/console_rbac.py,gcp/scripts/common/compute.py,shared/teardown_nim.pysuite runs,default project,vendor default). (b) Warm-reuse readiness now uses consecutive-success SSH stability (wait_for_ssh_stable) regardless of whether the VM was just started or adopted-running — matches the create/start paths and prevents transient sshd flakes from passing the gate on a single connection. (c) Sharedteardown_nim.pyadds a symmetric--ngc-api-keypolicy-skip that mirrorsdeploy_nim.py's shape — when the NGC entitlement is absent, deploy is a no-op and teardown is now a matching no-op, so the documented "leave NGC_API_KEY unset to opt out of NIM coverage" path stays green end-to-end (matters on any image that doesn't ship Docker, including the new public DLVM default).9b79e340docs/references/gcp.mddocs/references/gcp.md§5 to nameContainerRuntimeCheckexplicitly — clarify that operators who opt out of NIM (leavingNGC_API_KEYunset) and run on the public DLVM default still see[FAIL] TESTbecause the host-OSContainerRuntimeCheckvalidator asserts Docker presence unconditionally. Only operators who override the image to a Docker-equipped one get a fully green run. Documentation-only edit; no stub change.Pass 2 — Upstream-tracking refresh (after upstream
mainadvanced past Pass 1's rebase base):4453f677gcp/config/vm.yaml,gcp/scripts/common/ssh_utils.py,gcp/scripts/vm/virtual_device_hardening.py(NEW),.factory_versionmaintip9b79e340. Mirrors two upstream contract changes that landed after Pass 1: (1) PR NVIDIA#413 (2cdf4d76, "feat: add virtual device hardening validation") — adds the newvirtual_device_hardeningtest step to the vm suite; this commit adds the GCP-side mirror: 302-linevirtual_device_hardening.pystub (provider-evidence preamble for Compute Engine + guest-side SSH probes for USB / clipboard / unnecessary virtual devices using the canonical PROBE_SENTINEL framing, identical to the AWS oracle's pattern lists),ssh_runhelper ingcp/scripts/common/ssh_utils.py(sister to AWS's helper, same(exit_code, stdout, stderr)return contract with sentinel exit codes for TimeoutExpired / OSError), and the matching step entry ingcp/config/vm.yaml. (2) PR NVIDIA#424 (6e1458a7, "fix: handle AWS VM validation regressions") — addsIdentityAgent=nonealongsideIdentitiesOnly=yesso explicit-i <key_file>is the only SSH credential considered on operator hosts running an ssh-agent with multiple identities; mirrored to the canonical_SSH_OPTStuple in GCP'sssh_utils.pyso every GCP SSH helper benefits uniformly..factory_versionbumped accordingly (isv_commit=9b79e340,factory_commit=21869ef).The Pass-1 fix narrative: the pre-fix
_revoke_token_creatorcleanup-error miss was the first post-generation review's only ship-blocker; the next three commits address shapes surfaced by post-fix re-review iterations; the next four commits are a pre-PR public-shipping audit pass (operator-setup docs landing, private rule-layer references in comments scrubbed, private image default replaced by the public DLVM, env-var override architecture, then final-review-driven scrub of remaining generator vocabulary + warm-reuse stability gate + symmetric NIM teardown policy-skip, then theContainerRuntimeCheckdocumentation tightening), all caught before opening the PR. Pass 2 keeps the branch aligned with subsequent upstream contract changes via the canonical/ncp-updateworkflow.