Skip to content

Add --enable-nested-virtualization to setup-gcp - #1669

Merged
Benjamin Elder (BenTheElder) merged 4 commits into
agent-substrate:mainfrom
hdp617:feat/gke-nested-virtualization
Sep 18, 2026
Merged

Benjamin Elder (BenTheElder) merged 4 commits into
agent-substrate:mainfrom
hdp617:feat/gke-nested-virtualization

Conversation

@hdp617

@hdp617 Huy Pham (hdp617) commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Nested virtualization exposes /dev/kvm, which microVMs need. The new --enable-nested-virtualization flag turns it on for the node pool setup-gcp creates. It defaults to on, so pass --enable-nested-virtualization=false for a cluster that does not need it.

Also, rename the env var GVISOR_NODE_MACHINE_TYPE to NODE_MACHINE_TYPE to be consistent with other env vars and to avoid confusion for microVMs. The old env var still works with a warning.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

@hdp617

Copy link
Copy Markdown
Contributor Author

Comment thread hack/ate-dev-env.sh.example Resolved
Comment thread hack/ate-dev-env.sh.example Resolved
`setup-gcp` creates one node pool, `substrate-node-pool`, and sets no
`SandboxConfig` on it, so nothing about that pool is gVisor-specific. The
`gvisor-pool` named by `NODE_POOL_NAME` is a different pool that only
`hack/teardown.sh` touches, so the old variable named a pool this tool never
creates. Micro-VM workers need the same knob for a non-gVisor runtime, which
makes the name actively misleading.

The `--machine-type` flag and the `Config.MachineType` field already carried
neutral names, so only the environment variable and its help text change.

Developers copy `hack/ate-dev-env.sh.example` to an untracked `.ate-dev-env.sh`
once and edit it from there, so most existing setups still export the old name.
`setup-gcp` reads it when `NODE_MACHINE_TYPE` is unset and warns, which keeps
those setups on their chosen machine type instead of dropping them onto the
`c3-standard-4` default without a word. Remove the fallback once the team has
renamed the variable.

Verified with `go build ./tools/...` and `go test ./tools/setup-gcp/...`,
covering the new name, the old name, both set, and neither.
@hdp617
Huy Pham (hdp617) force-pushed the feat/gke-nested-virtualization branch from 16da292 to 67ef0e2 Compare September 16, 2026 17:15
Micro-VM workers (`ateom-microvm`) need `/dev/kvm`, which a GKE node only has
when its pool was created with nested virtualization. `setup-gcp` had no way to
ask for it, so provisioning a cluster for micro-VM work meant building the node
pool by hand with `gcloud`.

`create cluster` and `bootstrap` now accept `--enable-nested-virtualization`
(`ENABLE_NESTED_VIRTUALIZATION`), which sets
`AdvancedMachineFeatures.EnableNestedVirtualization` on the pool the tool
creates. It defaults to on, since running micro-VM workers is a reason to
create a cluster with this tool and the default `c3-standard-4` supports the
feature. Pass `--enable-nested-virtualization=false` for a cluster that does
not need it; the field then stays unset, leaving GKE its own default. A machine
type GCE excludes from the feature, such as E2 or an AMD type other than N4D,
now fails at cluster creation rather than quietly creating nodes without
`/dev/kvm`.

GKE fixes the setting at node creation and cannot update an existing pool into
it. The reconcile path therefore logs a warning naming the remedy instead of
reporting a change it did not make. With the default on, that warning reaches
every existing cluster whose pools predate the flag, which is the intended
signal: those nodes cannot place micro-VM workers. The check passes when any
pool has the feature, because keeping KVM nodes in a second pool added by hand
is a normal arrangement.

Verified with `go test ./tools/setup-gcp/...`, covering the request builder with
the flag on and off, and the pool scan across nil, empty, mixed, and disabled
clusters.
GCE excludes AMD machine types other than N4D from nested virtualization, so an
N2D instance cannot run the microVM runtime. The Linux prerequisites named
N2/N2D together as examples of KVM-capable machines, pointing readers at a
series that never works. Both N4 and N4D support nested virtualization, so name
those instead.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review I agree with (dismissed irrelevant comments):

Comment thread tools/setup-gcp/cmd/cluster.go Resolved
Comment thread tools/setup-gcp/cmd/common.go Resolved
Comment thread tools/setup-gcp/cmd/common.go Resolved
…un time

getEnv parses values with strconv.ParseBool, which rejects spellings such as
"off" and "no". It discarded those values without a word and fell back to the
default, so ENABLE_NESTED_VIRTUALIZATION=off left nested virtualization on.
Log the variable, its value, and the fallback when parsing fails. Commands
register flags for the same variable in more than one init, so the log records
each variable once rather than once per registration.

resolveMachineTypeDefault treated NODE_MACHINE_TYPE as set whenever it was
present, so exporting it empty suppressed the GVISOR_NODE_MACHINE_TYPE fallback
and left the machine type blank. GKE then picks e2-medium, which cannot run
nested virtualization. Treat an empty value as unset.

The deprecation warning for GVISOR_NODE_MACHINE_TYPE ran from a package-level
variable initializer, so it fired on every invocation, including --help and
subcommands that never read a machine type. Warn from the commands that read
the flag instead, and stay quiet when --machine-type was passed explicitly.
return ret.(T)
if err != nil {
if _, warned := warnedEnvKeys.LoadOrStore(key, struct{}{}); !warned {
slog.Warn("Ignoring unparsable environment variable", "key", key, "value", val, "using", fallback, "error", err)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 nit 🟢 – using=fallback can be wrong. This runs at flag-registration time, so an explicit CLI flag still wins afterwards. ENABLE_NESTED_VIRTUALIZATION=off ... --enable-nested-virtualization=false logs using=true while the cluster is created with nested virtualization off.

Dropping the using field, or wording it as the default rather than the effective value, would avoid the contradiction.

Comment thread tools/setup-gcp/README.md
| `--subnetwork` | VPC subnetwork name. | `SUBNETWORK` | `default` |
| `--machine-type` | Machine type for the gVisor node pool. | `GVISOR_NODE_MACHINE_TYPE` | `c3-standard-4` |
| `--machine-type` | Machine type for the node pool. | `NODE_MACHINE_TYPE` | `c3-standard-4` |
| `--enable-nested-virtualization` | Create the node pool with nested virtualization, exposing `/dev/kvm` for micro-VM workers. | `ENABLE_NESTED_VIRTUALIZATION` | `true` |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 should-fix 🟡 – Worth a note here that this applies only to the pool setup-gcp creates. GKE does not propagate nested virtualization to pools created later, and it cannot be enabled on an existing pool.

The "create additional pools" recipe further down (lines 150-154) only mentions --node-labels, so a pool made by following it has no /dev/kvm and micro-VM workers scheduled there stay Pending with no obvious cause. Adding the nested-virtualization flag to that recipe would close the gap.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Or we can consider a follow-up to support reconciling, but again, this code should really be moved out to the GKE installer repo anyhow, this tool does not belong in the main repo]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok to handle further nits in follow-up and land and iterate

@BenTheElder
Benjamin Elder (BenTheElder) added this pull request to the merge queue Sep 18, 2026
Merged via the queue into agent-substrate:main with commit 9f57d78 Sep 18, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/dev-infra area/microVM kind/feature An enhancement / feature request or implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants