From a731ddb3981168a50be51b8ad1f3c3c2e92821ee Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Thu, 9 Jul 2026 23:33:32 +0200 Subject: [PATCH 1/7] refactor(controller): take two Kubernetes writes and the cold mTLS resolve off the warm-claim path The engine's warm-claim activate is 60 ms P50, but nothing measured what surrounded it. Adding claim-path stage timing (logClaimStage, the analog of the fork path's logForkOrchStage) showed the Kubernetes round-trips cost about as much as the microVM restore, and one of them was a 300 ms cold spike paid by whichever user created the first sandbox after a controller start. Three changes, in decreasing size: * The mTLS dial config is pre-warmed once the manager cache is up. The first claim after a controller start or a leader handover used to pay 301 ms in dial_tls (the lazy Secret informer cache fill), against ~1 ms for every claim after it. Best effort: a pre-warm failure is not fatal, the reconciler still resolves on demand. * The Restoring status write is now in memory only. It was a full API round-trip on the critical path that no observer could ever see: control only reaches that line once a dormant pod has been SELECTED, so the phase was Restoring for exactly the duration of the activate. A claim that cannot place returns Pending far above it. The quota live-counter counts Pending and Restoring alike, so concurrency accounting is unchanged, and every failure path writes its own terminal phase. * The per-sandbox token Secret is written CONCURRENTLY with the activate RPC. Both inputs (the minted token, the endpoint derived from the pod IP) are known before the RPC, so it never needed to be serial. The ordering contract holds: the write is joined before the Ready status write, so the Secret is durable before the claim is servable. A failed activate leaves a Secret owner-ref'd to the claim, garbage collected with it and overwritten by the next pass. Measured on prod, mean of 9 sequential warm claims: stage before after status_write_restoring 10.40 gone dial_tls 34.42 1.04 (max 301.69 -> 2.37) token_secret (blocking) 7.42 0.00 (overlapped, span 115 ms) mark_pod_claimed 16.39 14.86 (the concurrency gate; unchanged) status_write_ready 9.44 8.39 TOTAL 190.34 139.45 (max 471.04 -> 174.53) token_secret is reported as two stages on purpose. token_secret_wait is what the critical path pays (0 ms, the activate outlasts the write); token_secret_span is the overlapped wall time. Reporting only the span would read as a 150 ms regression when the write in fact left the critical path entirely. Verified: internal/controller envtest suite green, both golangci-lint invocations clean, hosted create/run_code/fork/child-run_code and the #870 restart signal all still pass against the built controller on prod. Signed-off-by: Jannes Stubbemann --- cmd/controller/main.go | 23 +++++ .../controller/sandboxclaim_controller.go | 88 ++++++++++++++++--- 2 files changed, 99 insertions(+), 12 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index ab6ae2b2..a62553ee 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -23,6 +23,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" ctrladmission "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -467,6 +468,28 @@ func main() { return controller.HuskDialTLSConfig(dialCtx, mgr.GetClient(), discoveryNamespace, poolNamespace) } sandboxReconciler.HuskTLSFor = huskDial + + // Pre-warm the mTLS dial config once the manager cache is up, so the FIRST + // warm claim after a controller start or a leader handover does not pay the + // cold path. Measured on prod: the first claim's dial_tls stage was 301 ms + // (the lazy Secret informer cache fill for the controller namespace) against + // ~1 ms for every claim after it. That 300 ms landed squarely on one user's + // create, every rollout. + // + // Best effort by construction: a failure here is not fatal (the reconciler + // resolves the config itself on demand exactly as before), so a controller + // whose PKI is not ready yet still starts and converges. + if err := mgr.Add(manager.RunnableFunc(func(runCtx context.Context) error { + if _, werr := huskDial(runCtx, discoveryNamespace); werr != nil { + logger.Info("husk mTLS pre-warm skipped; the first claim will resolve it on demand", "detail", werr.Error()) + return nil + } + logger.Info("husk mTLS dial config pre-warmed", "namespace", discoveryNamespace) + return nil + })); err != nil { + logger.Error(err, "unable to add husk mTLS pre-warm") + os.Exit(1) + } logger.Info("PKI bootstrap complete; dialing forkd with mTLS and husk pods with per-namespace identity", "namespace", discoveryNamespace) } diff --git a/internal/controller/sandboxclaim_controller.go b/internal/controller/sandboxclaim_controller.go index cb39fb77..ba6a19fb 100644 --- a/internal/controller/sandboxclaim_controller.go +++ b/internal/controller/sandboxclaim_controller.go @@ -31,6 +31,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" + + "github.com/go-logr/logr" ) // tracer is the controller component tracer; no-op unless tracing is configured. @@ -750,6 +752,20 @@ func (r *SandboxReconciler) reconcilePoolRef(ctx context.Context, claim *v1.Sand // it pends with backpressure and an actionable message so a transient husk // (snapshot not yet materialized, stub still starting) can recover. Secret // VALUES are never logged or put in status/conditions. +// logClaimStage records one stage of the warm-claim activate, the claim-path analog of +// logForkOrchStage. Without it the only visible number was the husk-reported engine +// activate, so the Kubernetes round-trips around it (the Restoring status write, the +// pod-claim label patch, the token Secret, the Ready status write) were invisible. +// Measured end to end they cost about as much as the microVM restore itself. +func logClaimStage(logger logr.Logger, claim string, stage string, d time.Duration) { + observeForkStage("claim_"+stage, d.Seconds()) + logger.Info("claim stage timing", + "claim", claim, + "stage", stage, + "durMs", float64(d.Microseconds())/1000.0, + ) +} + func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sandbox, pool *v1.SandboxPool, template *v1.PoolTemplateSpec) (ctrl.Result, error) { logger := log.FromContext(ctx) @@ -764,7 +780,10 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa // by selectDormantHuskPod, so an evicted claim whose old pod is gone or dying // always moves on to a new dormant pod rather than re-activating the dead one. // Leader election (one active reconciler) is what bounds concurrent claiming. + claimStart := time.Now() + mark := time.Now() pod, err := r.selectDormantHuskPod(ctx, pool) + logClaimStage(logger, claim.Name, "select_pod", time.Since(mark)) if err != nil { return ctrl.Result{}, err } @@ -803,16 +822,22 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa // A dormant pod is available: mark the claim Restoring before activating it. // This is stamped here (not before pod selection) so a claim that cannot place // stays Pending and settles, never cycling Pending -> Restoring -> Pending. - if claim.Status.Phase != v1.SandboxRestoring { - claim.Status.Phase = v1.SandboxRestoring - if err := r.Status().Update(ctx, claim); err != nil { - return ctrl.Result{}, err - } - } + // Stamp Restoring IN MEMORY only. This used to be its own Status().Update, a full + // API round-trip (measured 10.4 ms mean) on the warm-claim critical path, and it + // bought nothing: control only reaches here once a dormant pod has been SELECTED, + // so the phase is Restoring for exactly as long as the activate takes (~100 ms) + // and then becomes Ready. A claim that cannot place returns Pending well above + // this line and never reaches it, so no observer ever saw a durable Restoring. + // The quota live-counter (internal/saas/controlplane/livecounter.go) counts + // Pending and Restoring alike, so concurrency accounting is unchanged, and every + // failure path below writes its own terminal phase. + claim.Status.Phase = v1.SandboxRestoring // Resolve env + secrets (same path as the forkd fork). Secret VALUES live // only in memory here and ride the mTLS control channel; never logged. + mark = time.Now() env, secretVals, err := r.resolveSecrets(ctx, claim.Namespace, claim.Labels[tenant.OrgLabelKey], claim.Spec.Env, claim.Spec.Secrets) + logClaimStage(logger, claim.Name, "resolve_secrets", time.Since(mark)) if err != nil { logger.Error(err, "secret resolution failed") recordClaimError(claim.Spec.Source.PoolRef.Name, "secret") @@ -860,7 +885,10 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa // Winning the label patch is the gate to Activate, so a pod is activated by // exactly one claim. On conflict we requeue so the next reconcile picks a // different dormant pod. - if err := r.markHuskPodClaimed(ctx, pod, claim); err != nil { + mark = time.Now() + markErr := r.markHuskPodClaimed(ctx, pod, claim) + logClaimStage(logger, claim.Name, "mark_pod_claimed", time.Since(mark)) + if err := markErr; err != nil { if apierrors.IsConflict(err) { logger.Info("husk pod claimed concurrently, requeueing to pick another", "pod", pod.Name) beforeStatus := claim.Status.DeepCopy() @@ -898,6 +926,9 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa } addr := net.JoinHostPort(pod.Status.PodIP, strconv.Itoa(controlPort)) + // Derived from the pod IP alone, not from the activate result, so it is known + // before the RPC and the token Secret write can overlap it. + endpoint := net.JoinHostPort(pod.Status.PodIP, strconv.Itoa(sandboxPort)) netCfg := huskEgressConfig(template) req := husk.ActivateRequest{ SnapshotDir: HuskSnapshotDir, @@ -913,11 +944,32 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa InboundCIDRs: netCfg.InboundCIDRs, Token: apiToken, } + // Write the per-sandbox token Secret CONCURRENTLY with the activate RPC. Both + // inputs are already known (the token was minted above; the endpoint is derived + // from the pod IP, not from the activate result), so the Secret write is a pure + // API round-trip (measured 7.4 ms) that used to sit in series after a ~100 ms + // activate for no reason. + // + // Ordering contract is preserved: the Secret is durable BEFORE the claim goes + // Ready, because we join the goroutine before the Ready status write. An activate + // that then fails leaves a Secret for a not-Ready claim; that is harmless (it is + // owner-ref'd to the claim, so it is garbage-collected with it, and the next pass + // re-mints and overwrites it before the claim can ever be served). + tokenErrCh := make(chan error, 1) + tokenStart := time.Now() + go func() { + tokenErrCh <- ensureSandboxTokenSecret(ctx, r.Client, claim, claim.Name+tokenSecretSuffix, apiToken, endpoint) + }() + + mark = time.Now() tlsConf, err := r.huskDialTLS(ctx, pod.Namespace) + logClaimStage(logger, claim.Name, "dial_tls", time.Since(mark)) var res husk.ActivateResult + mark = time.Now() if err == nil { res, err = activate(ctx, addr, tlsConf, req) } + logClaimStage(logger, claim.Name, "activate_rpc", time.Since(mark)) if err != nil || !res.OK { // FAIL CLOSED: do not go Ready. Pend so a transient husk can recover. msg := "husk activation did not complete" @@ -948,13 +1000,21 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa return ctrl.Result{RequeueAfter: capacityPendingRequeue}, nil } - endpoint := net.JoinHostPort(pod.Status.PodIP, strconv.Itoa(sandboxPort)) - // The pod was already claimed (optimistic-lock label patch) BEFORE activation, // so this VM belongs to exactly this claim. Hand the token to the claim's // consumer via an owned Secret BEFORE the Ready // write (same ordering as the forkd path). - if err := ensureSandboxTokenSecret(ctx, r.Client, claim, claim.Name+tokenSecretSuffix, apiToken, endpoint); err != nil { + // Join the concurrent token Secret write. Two numbers, because one alone lies: + // token_secret_wait is what the critical path actually PAYS (the time still + // blocked here after the activate returned, normally ~0 because the activate + // outlasts the write), while token_secret_span is the wall time of the write + // itself, overlapped. Reporting only the span would look like a 150 ms + // regression when the write in fact left the critical path entirely. + joinStart := time.Now() + tokErr := <-tokenErrCh + logClaimStage(logger, claim.Name, "token_secret_wait", time.Since(joinStart)) + logClaimStage(logger, claim.Name, "token_secret_span", time.Since(tokenStart)) + if err := tokErr; err != nil { logger.Error(err, "token secret write failed") recordClaimError(claim.Spec.Source.PoolRef.Name, "token") now := metav1.Now() @@ -992,9 +1052,13 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa Reason: "HuskActivated", Message: fmt.Sprintf("activated husk pod %s on node %s in %.2fms", pod.Name, pod.Spec.NodeName, res.LatencyMs), }) - if err := r.Status().Update(ctx, claim); err != nil { - return ctrl.Result{}, err + mark = time.Now() + readyErr := r.Status().Update(ctx, claim) + logClaimStage(logger, claim.Name, "status_write_ready", time.Since(mark)) + if readyErr != nil { + return ctrl.Result{}, readyErr } + logClaimStage(logger, claim.Name, "total", time.Since(claimStart)) logger.Info("sandbox claimed via husk activation", "sandbox", claim.Name, "pod", pod.Name, "node", pod.Spec.NodeName) return ctrl.Result{}, nil From ca8c7bacb69242d5964d80cf788d9ef90fcbf06b Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Fri, 10 Jul 2026 00:15:46 +0200 Subject: [PATCH 2/7] refactor(controller): give claim stages their own metric and the Secret write its own copy Two review findings, both real. Claim stage timings were recorded as claim_* labels on mitos_fork_stage_duration_seconds, whose help text documents a fixed fork-stage vocabulary. Any aggregation over "all fork stages" would have quietly included warm-claim work. They now go to mitos_claim_stage_duration_seconds, and a test asserts a claim observation increments that histogram and leaves the fork one untouched. The concurrent token Secret write was handed the LIVE claim object. The activate-failure path mutates claim.Status and returns without joining the goroutine, so a still-running writer shared mutable state with the reconcile path under no synchronization. It now gets claim.DeepCopy() and a precomputed Secret name, so nothing it reads can be written underneath it. The owner reference is unchanged: the copy carries the same name, namespace, and UID, so the Secret is still garbage-collected with the claim. Verified: go test -race ./internal/controller/ passes, and golangci-lint is clean for both GOOS=darwin and GOOS=linux. Signed-off-by: Jannes Stubbemann --- internal/controller/metrics.go | 15 ++++++++++++++ internal/controller/metrics_internal_test.go | 20 +++++++++++++++++++ .../controller/sandboxclaim_controller.go | 11 ++++++++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/internal/controller/metrics.go b/internal/controller/metrics.go index 7681f16f..d508f1b5 100644 --- a/internal/controller/metrics.go +++ b/internal/controller/metrics.go @@ -178,6 +178,15 @@ var ( Help: "Per-stage duration of a hosted co-location fork, by stage (controller RPC round-trips, husk sub-stages, and the end-to-end total).", Buckets: []float64{0.0005, 0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2, 5}, }, []string{"stage"}) + + // Kept separate from forkStageDurationSeconds: that histogram documents a fixed + // fork-stage vocabulary, and mixing claim labels into it would make any + // aggregation over "all fork stages" quietly include warm-claim work. + claimStageDurationSeconds = prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Name: "mitos_claim_stage_duration_seconds", + Help: "Per-stage duration of a warm-claim activate, by stage (Kubernetes round-trips around the microVM restore, the mTLS dial, the activate RPC, and the end-to-end total).", + Buckets: []float64{0.0005, 0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2, 5}, + }, []string{"stage"}) ) func init() { @@ -200,6 +209,7 @@ func init() { claimWaitForWarmSeconds, snapshotDistributionLagSeconds, forkStageDurationSeconds, + claimStageDurationSeconds, ) } @@ -303,3 +313,8 @@ func observeClaimWaitForWarm(seconds float64) { claimWaitForWarmSeconds.Observe( func observeForkStage(stage string, seconds float64) { forkStageDurationSeconds.WithLabelValues(stage).Observe(seconds) } + +// observeClaimStage records one stage of a warm-claim activate. +func observeClaimStage(stage string, seconds float64) { + claimStageDurationSeconds.WithLabelValues(stage).Observe(seconds) +} diff --git a/internal/controller/metrics_internal_test.go b/internal/controller/metrics_internal_test.go index 629dda95..f9a7086e 100644 --- a/internal/controller/metrics_internal_test.go +++ b/internal/controller/metrics_internal_test.go @@ -172,3 +172,23 @@ func TestForkStageTimingMetric(t *testing.T) { t.Errorf("total stage count = %d, want %d", got, beforeTotal+1) } } + +// TestClaimStageTimingMetricIsSeparateFromForkStages asserts that warm-claim stage +// timings land on their own histogram. mitos_fork_stage_duration_seconds documents a +// fixed fork-stage vocabulary; folding claim_* labels into it would make any +// whole-metric aggregation over fork stages silently include claim work. +func TestClaimStageTimingMetricIsSeparateFromForkStages(t *testing.T) { + const stage = "activate_rpc" + + forkBefore := histogramCountByLabel(t, "mitos_fork_stage_duration_seconds", map[string]string{"stage": "claim_" + stage}) + claimBefore := histogramCountByLabel(t, "mitos_claim_stage_duration_seconds", map[string]string{"stage": stage}) + + observeClaimStage(stage, 0.061) + + if got := histogramCountByLabel(t, "mitos_claim_stage_duration_seconds", map[string]string{"stage": stage}); got != claimBefore+1 { + t.Errorf("claim stage count = %d, want %d", got, claimBefore+1) + } + if got := histogramCountByLabel(t, "mitos_fork_stage_duration_seconds", map[string]string{"stage": "claim_" + stage}); got != forkBefore { + t.Errorf("claim timing leaked into the fork-stage histogram: count = %d, want %d", got, forkBefore) + } +} diff --git a/internal/controller/sandboxclaim_controller.go b/internal/controller/sandboxclaim_controller.go index ba6a19fb..3314e2c7 100644 --- a/internal/controller/sandboxclaim_controller.go +++ b/internal/controller/sandboxclaim_controller.go @@ -758,7 +758,7 @@ func (r *SandboxReconciler) reconcilePoolRef(ctx context.Context, claim *v1.Sand // pod-claim label patch, the token Secret, the Ready status write) were invisible. // Measured end to end they cost about as much as the microVM restore itself. func logClaimStage(logger logr.Logger, claim string, stage string, d time.Duration) { - observeForkStage("claim_"+stage, d.Seconds()) + observeClaimStage(stage, d.Seconds()) logger.Info("claim stage timing", "claim", claim, "stage", stage, @@ -955,10 +955,17 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa // that then fails leaves a Secret for a not-Ready claim; that is harmless (it is // owner-ref'd to the claim, so it is garbage-collected with it, and the next pass // re-mints and overwrites it before the claim can ever be served). + // + // The goroutine gets its OWN copy of the claim and a precomputed Secret name. The + // activate-failure path below mutates claim.Status and returns WITHOUT joining, so + // handing the live object to a still-running writer would share mutable state + // across goroutines with no synchronization. tokenErrCh := make(chan error, 1) tokenStart := time.Now() + tokenOwner := claim.DeepCopy() + tokenSecretName := claim.Name + tokenSecretSuffix go func() { - tokenErrCh <- ensureSandboxTokenSecret(ctx, r.Client, claim, claim.Name+tokenSecretSuffix, apiToken, endpoint) + tokenErrCh <- ensureSandboxTokenSecret(ctx, r.Client, tokenOwner, tokenSecretName, apiToken, endpoint) }() mark = time.Now() From 4cad94e0f2366a2117820fe38c07085e4e74d06a Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Fri, 10 Jul 2026 00:20:27 +0200 Subject: [PATCH 3/7] docs(bench): record the hosted time-to-interactive result and a reproducible peer table TTI is create() to a command having actually run inside the sandbox and returned, the definition the public ComputeSDK benchmark uses. Paced 20 iterations from a same-region client against api.mitos.run: P50 495 -> 341 ms, min 273 ms, 20/20. Both runs hit the same server build, so the whole difference is the client-side connection reuse and template caching in the previous commit. The peer numbers are computed, not copied. The vendor leaderboard renders client side and the upstream README shows only chart placeholders, so bench/peer-tti.py fetches the raw per-iteration JSON the harness commits and computes percentiles itself, per the no-unverified-claims rule. Doing that caught an error in the figure I had been carrying: Northflank is 95.9 ms P50, not 269 ms. 269 was Upstash, at 258.3. The result page states plainly what the table cannot support. Their probe is node -v against our print(1) through a Python interpreter, their run is 100 un-paced iterations against our 13 s pacing under the free tier's rate limit, and their client and region are not ours. The single clean statement is the ordering against E2B, the one peer documenting the same isolation primitive: 340.7 ms vs 365.6 ms P50. The sub-40 ms entries are below the cost of restoring a microVM at all on this hardware, so they are almost certainly not booting one per request. ComputeSDK's snapshot-fork benchmark measures object storage (S3, R2, Azure Blob, Tigris), not VM fork, so there is still no public peer number for the primitive mitos is built on. Signed-off-by: Jannes Stubbemann --- bench/peer-tti.py | 55 ++++++++++++ bench/results/2026-07-10-tti-hosted.md | 117 +++++++++++++++++++++++++ 2 files changed, 172 insertions(+) create mode 100644 bench/peer-tti.py create mode 100644 bench/results/2026-07-10-tti-hosted.md diff --git a/bench/peer-tti.py b/bench/peer-tti.py new file mode 100644 index 00000000..cca84dcc --- /dev/null +++ b/bench/peer-tti.py @@ -0,0 +1,55 @@ +#!/usr/bin/env python3 +"""Recompute the peer Time-to-Interactive table from ComputeSDK's raw data. + +The vendor leaderboard at computesdk.com renders client side, and the README shows +only chart placeholders, so the numbers there cannot be checked by reading either. +This fetches the raw per-iteration JSON the harness commits and computes the +percentiles itself, which is the only form of the number this repo is allowed to +publish (see the no-unverified-claims rule in CLAUDE.md). + +It measures THEIR harness: `node -v`, 100 sequential un-paced iterations, from their +runner. Our own row comes from bench/tti-latency.py and is not comparable without the +caveats recorded in bench/results/2026-07-10-tti-hosted.md. + +Usage: + python3 bench/peer-tti.py [date] # date like 2026-07-09, default latest +""" +import json +import math +import sys +import urllib.request + +# The repo's default branch is master, not main. +RAW = ("https://raw.githubusercontent.com/computesdk/benchmarks/master/" + "results/sequential_tti/%s.json") + + +def nearest_rank(values, pct): + values = sorted(values) + return values[max(1, math.ceil(pct / 100.0 * len(values))) - 1] + + +def main(): + which = sys.argv[1] if len(sys.argv) > 1 else "latest" + with urllib.request.urlopen(RAW % which, timeout=30) as r: + data = json.load(r) + + print("run: %s config: %s" % (data["timestamp"], data["config"])) + rows = [] + for result in data["results"]: + tti = [i["ttiMs"] for i in result.get("iterations", []) + if isinstance(i.get("ttiMs"), (int, float)) and not i.get("error")] + if not tti: + continue + rows.append((result["provider"], nearest_rank(tti, 50), + nearest_rank(tti, 90), min(tti), len(tti))) + + rows.sort(key=lambda r: r[1]) + print("%-16s%9s%9s%9s%6s" % ("provider", "P50", "P90", "min", "n")) + for provider, p50, p90, low, n in rows: + print("%-16s%9.1f%9.1f%9.1f%6d" % (provider, p50, p90, low, n)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/bench/results/2026-07-10-tti-hosted.md b/bench/results/2026-07-10-tti-hosted.md new file mode 100644 index 00000000..df2c3ddb --- /dev/null +++ b/bench/results/2026-07-10-tti-hosted.md @@ -0,0 +1,117 @@ +# Time to Interactive on the hosted API (2026-07-10) + +Source for the end-to-end latency numbers: the time from `create()` to a command +having actually RUN inside the sandbox and returned. Reproduce with +[`bench/tti-latency.py`](../tti-latency.py). + +TTI is the only figure that is apples-to-apples with the public +[ComputeSDK benchmark](https://github.com/computesdk/benchmarks), which every hosted +sandbox vendor is measured against. It is NOT the same thing as the warm-claim +activate in [`bench/husk-activate-latency.sh`](../husk-activate-latency.sh), which +times only the microVM restore and excludes the client, the API, and the exec. +Quoting the engine number against a competitor's create-API number is a category +error. + +## Setup + +| | | +|---|---| +| API | `https://api.mitos.run` (hosted, free tier) | +| client | `mitos-bench1`, Hetzner bare metal, same region as the API | +| template | `python`, guest 2 vCPU / 512 MiB | +| probe | `print(1)` via `run_code`, matching ComputeSDK's `node -v` | +| pacing | 13 s between iterations (free tier allows 5 creates/minute) | +| command | `MITOS_API_KEY=... python3 bench/tti-latency.py 20` | + +Measuring from a laptop across the public internet mostly measures the internet: an +unrouted 404 costs 250 ms or more from a random location and 6 ms from inside the +cluster. + +## Time to Interactive, ms, N=20 + +| | before | after | +|---|---|---| +| min | 460.2 | **273.4** | +| P50 | 495.0 | **340.7** | +| P90 | 555.6 | 373.3 | +| P95 | 567.8 | 438.2 | +| max | 613.1 | 448.7 | +| mean | 510.0 | 343.8 | +| success | 20/20 | 20/20 | + +Split after: create median 231.5 ms, first exec median 102.1 ms. + +"Before" and "after" ran against the SAME server build. Every millisecond of the +difference is client side, found by logging each HTTP request with the identity of +the socket it went out on: + +| defect | cost | +|---|---| +| a streaming Connect RPC never returned its connection to the pool (body not read to EOF), so every exec re-handshaked TLS | ~70 ms per exec | +| the pool was keyed by `(base URL, timeout)`, so create (60 s client) and the first exec (30 s client) never shared a socket | one extra handshake per sandbox | +| `create()` re-resolved the identical template on every call | ~50 ms per create | + +After the fixes, all four requests of an iteration (`/v1/templates`, `/v1/fork`, +`RunCodeStream`, `DELETE`) ride ONE socket for the whole process. + +## Where the remaining 341 ms goes + +| | ms | what it is | +|---|---|---| +| create | 231 | gateway ~20, control plane ~139, engine restore 60 to 76 | +| first exec | 102 | gateway, vsock to the guest agent, and the Python interpreter | + +The engine's own warm-claim activate is 60 to 76 ms P50 on this hardware +(`bench/husk-activate-latency.sh`, see +[2026-07-09-lazy-livecow-restore.md](2026-07-09-lazy-livecow-restore.md)). It is +therefore a minority of TTI: most of what a user waits for is the Kubernetes control +plane, not Firecracker. + +## Peer set + +Context, NOT a controlled comparison. The numbers below are P50s we computed from the +raw per-iteration data ComputeSDK publishes, run `2026-07-09T01:03:05Z`, 100 +iterations per provider, `results/sequential_tti/latest.json` in +`computesdk/benchmarks`. They were produced on ComputeSDK's runner against each +vendor's own hardware; we did not re-run them. + +| provider | TTI P50 | P90 | min | +|---|---|---|---| +| isorun | 12.8 | 15.0 | 11.9 | +| declaw | 37.7 | 88.8 | 28.5 | +| northflank | 95.9 | 122.0 | 66.1 | +| daytona | 136.2 | 300.7 | 97.0 | +| archil | 201.0 | 372.7 | 133.9 | +| upstash | 258.3 | 275.9 | 235.6 | +| **mitos** | **340.7** | 373.3 | 273.4 | +| e2b | 365.6 | 501.1 | 288.8 | +| beam | 377.9 | 388.6 | 175.1 | +| vercel | 392.7 | 503.3 | 241.4 | +| modal | 470.9 | 603.9 | 429.8 | +| cloudflare | 1758.7 | 2139.8 | 1027.7 | +| codesandbox | 2215.8 | 2513.5 | 1938.5 | + +Read that table with three caveats, all of which cut against us being able to claim a +win from it: + +1. **Different probe.** ComputeSDK runs `node -v`; our row runs `print(1)` through the + Python template, which pays interpreter startup inside the guest. +2. **Different pacing.** ComputeSDK runs 100 iterations back to back. Our row is paced + 13 s apart to stay under the free tier's 5 creates/minute, so it never benefits from + a hot path warmed by the previous iteration. +3. **Different clients and regions.** Their harness, their runner, their network. + +The only clean statement available today is the ordering against E2B, the one peer that +documents the same isolation primitive (Firecracker microVMs): mitos 340.7 ms P50 vs +E2B 365.6 ms. We have not verified the isolation model of the faster entries, and the +sub-40 ms rows (isorun, declaw) are far below the cost of restoring a microVM at all on +this hardware, so they are almost certainly not booting one per request. + +ComputeSDK also publishes a `snapshot-fork` benchmark, but it measures OBJECT STORAGE +(S3, R2, Azure Blob, Tigris), not VM fork, so there is still no public apples-to-apples +peer number for the primitive mitos is actually built on. + +Firecracker's own project publishes no snapshot-restore TTI number either. + +The gap to the fastest entries is control-plane cost, not engine cost, and is tracked +in #871. From ad480887ab787530f50512b26aef104659bbff31 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Fri, 10 Jul 2026 10:45:32 +0200 Subject: [PATCH 4/7] fix(controller): join the token Secret write before pending a failed claim Review found a data-integrity bug in the concurrent Secret write this branch added, and it is a real one. The warm-claim path writes the per-sandbox token Secret concurrently with the activate RPC. The activate-failure branch returned WITHOUT joining that write. The writer is still holding THIS pass's token; the next pass mints a NEW token, activates the husk with it, and rewrites the SAME Secret. Whichever write lands last wins. A stale writer landing last leaves a Ready claim whose Secret holds a token the husk never received, and every tenant call made with it is rejected. Nothing would have pointed at the cause. The write now runs under a cancellable context and EVERY path out of the function joins it. The failure branch cancels first so the join is prompt, then drains, then persists Pending. A cancelled write is not an error worth logging; any other failure is logged without the token value, and the next pass re-mints and overwrites it anyway. Proven by TestHuskClaimActivateFailureJoinsTheTokenSecretWrite, through a new ensureTokenSecret seam: the fake writer blocks and deliberately ignores its context, so the reconcile can only proceed by joining it. Without the join the test fails with "claim reached phase Pending while the token Secret write was still in flight". It also asserts the writer's context was cancelled, so a slow write can always be cut short. Verified: go test -race ./internal/controller/ passes (three consecutive runs), and golangci-lint is clean for both GOOS=darwin and GOOS=linux. Signed-off-by: Jannes Stubbemann --- internal/controller/husk_activation_test.go | 76 +++++++++++++++++++ .../controller/sandboxclaim_controller.go | 30 +++++++- internal/controller/suite_test.go | 12 ++- internal/controller/testsupport_test.go | 7 ++ 4 files changed, 123 insertions(+), 2 deletions(-) diff --git a/internal/controller/husk_activation_test.go b/internal/controller/husk_activation_test.go index 87429ec0..c01bb69b 100644 --- a/internal/controller/husk_activation_test.go +++ b/internal/controller/husk_activation_test.go @@ -21,6 +21,7 @@ import ( v1 "mitos.run/mitos/api/v1" "strings" "sync" + "sync/atomic" "testing" "time" @@ -30,6 +31,7 @@ import ( "mitos.run/mitos/internal/controller" "mitos.run/mitos/internal/husk" "mitos.run/mitos/internal/tenant" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) @@ -555,3 +557,77 @@ func TestHuskClaimActivateFailureNotReady(t *testing.T) { t.Errorf("claim eventually went Ready despite repeated activate failures: %+v", again.Status) } } + +// TestHuskClaimActivateFailureJoinsTheTokenSecretWrite is a data-integrity gate. +// +// The warm-claim path writes the per-sandbox token Secret CONCURRENTLY with the +// activate RPC. If a failed activate returned while that write was still in flight, +// the writer would still be holding THIS pass's token; the next pass mints a NEW token, +// activates the husk with it, and rewrites the SAME Secret. Whichever write lands last +// wins, so a stale writer landing last leaves a Ready claim whose Secret holds a token +// the husk never received, and every tenant call with it is rejected. +// +// The fake writer blocks until released and deliberately ignores its context, so the +// only way the reconcile can proceed is by JOINING it. If the reconcile returned early, +// the claim would reach Pending while the writer was still blocked. +func TestHuskClaimActivateFailureJoinsTheTokenSecretWrite(t *testing.T) { + makeDormantHuskPod(t, "husk-join-pool", "10.4.5.9") + + act := &fakeActivator{result: husk.ActivateResult{OK: false, Error: "load snapshot: boom"}} + setHuskTestActivator(act.activate) + t.Cleanup(func() { setHuskTestActivator(nil) }) + + started := make(chan struct{}) + release := make(chan struct{}) + var once sync.Once + var cancelSeen atomic.Bool + + setHuskTestTokenWriter(func(ctx context.Context, _ client.Client, _ client.Object, _, _, _ string) error { + // Only the FIRST pass blocks and is observed. The claim is retryable, so the + // controller reconciles it again; a later pass carries a fresh, uncancelled + // context and would otherwise clobber what the first pass recorded. + first := false + once.Do(func() { + first = true + close(started) + }) + if !first { + return nil + } + <-release // ignore ctx: the reconcile must wait for us regardless + cancelSeen.Store(ctx.Err() != nil) + return ctx.Err() + }) + t.Cleanup(func() { setHuskTestTokenWriter(nil) }) + + claim := makeHuskClaim(t, "husk-join", v1.SandboxSpec{}) + + select { + case <-started: + case <-time.After(30 * time.Second): + t.Fatal("the token Secret write never started; the claim never reached activate") + } + + // While the writer is blocked, the reconcile must NOT have persisted its outcome. + // Without the join it returns here and stamps Pending with the writer still running. + time.Sleep(750 * time.Millisecond) + var mid v1.Sandbox + if err := k8sClient.Get(ctx, types.NamespacedName{Name: claim.Name, Namespace: "default"}, &mid); err != nil { + t.Fatal(err) + } + if mid.Status.Phase == v1.SandboxPending || mid.Status.Phase == v1.SandboxFailed { + t.Fatalf("claim reached phase %q while the token Secret write was still in flight: the reconcile did not join the writer", mid.Status.Phase) + } + + close(release) + + got := waitClaimPhase(t, claim.Name, func(c *v1.Sandbox) bool { + return c.Status.Phase == v1.SandboxPending || c.Status.Phase == v1.SandboxFailed + }) + if got.Status.Phase == v1.SandboxReady { + t.Errorf("claim went Ready despite an activate failure: %+v", got.Status) + } + if !cancelSeen.Load() { + t.Error("the reconcile joined the writer but never cancelled its context, so a slow write cannot be cut short") + } +} diff --git a/internal/controller/sandboxclaim_controller.go b/internal/controller/sandboxclaim_controller.go index 3314e2c7..ba809c9f 100644 --- a/internal/controller/sandboxclaim_controller.go +++ b/internal/controller/sandboxclaim_controller.go @@ -120,6 +120,12 @@ type SandboxReconciler struct { forkSnapshot huskForkSnapshotter removeForkSnapshot huskForkSnapshotRemover + // ensureTokenSecret writes the per-sandbox token Secret. Nil defaults to + // ensureSandboxTokenSecret. Tests inject a fake to control when the concurrent + // write completes, which is the only way to assert that the warm-claim path + // JOINS that write before it returns. + ensureTokenSecret func(ctx context.Context, c client.Client, owner client.Object, name, token, endpoint string) error + // MultiVMFork routes a husk fork child into an ADDITIONAL VM spawned INSIDE the // SOURCE pod (SpawnVMOnHusk) instead of a brand-new child pod, when the source // pod is multi-VM capable. EXPERIMENTAL, DEFAULT OFF: off is byte-for-byte the @@ -960,12 +966,24 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa // activate-failure path below mutates claim.Status and returns WITHOUT joining, so // handing the live object to a still-running writer would share mutable state // across goroutines with no synchronization. + // + // The write runs under a CANCELLABLE context and every path out of this function + // JOINS it. A reconcile that returned while the write was still in flight would + // leave a writer holding an OLD token: the retry mints a NEW one and rewrites the + // SAME Secret, and whichever write lands last wins. The stale one landing last + // leaves a Ready claim whose Secret holds a token the husk never received. + writeTokenSecret := r.ensureTokenSecret + if writeTokenSecret == nil { + writeTokenSecret = ensureSandboxTokenSecret + } + tokenCtx, cancelTokenWrite := context.WithCancel(ctx) + defer cancelTokenWrite() tokenErrCh := make(chan error, 1) tokenStart := time.Now() tokenOwner := claim.DeepCopy() tokenSecretName := claim.Name + tokenSecretSuffix go func() { - tokenErrCh <- ensureSandboxTokenSecret(ctx, r.Client, tokenOwner, tokenSecretName, apiToken, endpoint) + tokenErrCh <- writeTokenSecret(tokenCtx, r.Client, tokenOwner, tokenSecretName, apiToken, endpoint) }() mark = time.Now() @@ -987,6 +1005,16 @@ func (r *SandboxReconciler) reconcileHuskClaim(ctx context.Context, claim *v1.Sa } logger.Info("husk activation failed, pending", "pod", pod.Name, "node", pod.Spec.NodeName, "detail", msg) recordClaimError(claim.Spec.Source.PoolRef.Name, "activate") + // Join the in-flight token Secret write BEFORE returning. The next pass mints a + // new token and rewrites the same Secret; a writer still holding this pass's + // token could otherwise land after it and leave the Ready claim with a token the + // husk never received. Cancelling first makes the join prompt. + cancelTokenWrite() + if tokErr := <-tokenErrCh; tokErr != nil && !errors.Is(tokErr, context.Canceled) { + // The claim is pending anyway; the next pass re-mints and overwrites. Never + // log the token VALUE. + logger.Error(tokErr, "token secret write failed while pending the claim", "secret", tokenSecretName) + } // Release the claim label stamped before activation so this pod returns to // the dormant pool and can be retried (by this claim or another). Without // this a stamped-but-not-activated pod is orphaned forever, leaking warm diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index b802f0e4..e32dd229 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -327,6 +327,16 @@ func currentHuskTestCheckpointer() func(ctx context.Context, claim *v1.Sandbox, } // setHuskTestActivator installs the husk activator the suite reconciler uses. +// setHuskTestTokenWriter installs a fake per-sandbox token Secret writer on the husk +// claim reconciler for one test, or restores the real one with nil. +// huskClaim is the husk-enabled claim reconciler the suite registers; tests reach it +// to install per-test seams. +var huskClaim *controller.SandboxReconciler + +func setHuskTestTokenWriter(fn func(ctx context.Context, c client.Client, owner client.Object, name, token, endpoint string) error) { + huskClaim.SetEnsureTokenSecretForTest(fn) +} + func setHuskTestActivator(fn func(ctx context.Context, addr string, tlsConf *tls.Config, req husk.ActivateRequest) (husk.ActivateResult, error)) { huskTestActivatorMu.Lock() defer huskTestActivatorMu.Unlock() @@ -657,7 +667,7 @@ func TestMain(m *testing.M) { // A husk-enabled claim reconciler that handles ONLY husk-test claims. Its // activator is swappable per test via setHuskTestActivator. - huskClaim := &controller.SandboxReconciler{ + huskClaim = &controller.SandboxReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), APIReader: mgr.GetAPIReader(), diff --git a/internal/controller/testsupport_test.go b/internal/controller/testsupport_test.go index a38fddcc..7de6501e 100644 --- a/internal/controller/testsupport_test.go +++ b/internal/controller/testsupport_test.go @@ -154,6 +154,13 @@ func (r *SandboxReconciler) OnlyLabels(labels ...string) { r.controllerName = "sandbox-husk" } +// SetEnsureTokenSecretForTest injects a fake per-sandbox token Secret writer, so a +// test can control exactly when that concurrent write completes. Nil restores the +// real ensureSandboxTokenSecret. +func (r *SandboxReconciler) SetEnsureTokenSecretForTest(fn func(ctx context.Context, c client.Client, owner client.Object, name, token, endpoint string) error) { + r.ensureTokenSecret = fn +} + // SetActivateForTest injects a fake husk activator (the test seam). func (r *SandboxReconciler) SetActivateForTest(fn func(ctx context.Context, addr string, tlsConf *tls.Config, req husk.ActivateRequest) (husk.ActivateResult, error)) { r.Activate = fn From c4fc27dfc45f8229829eeb612fd59b06d3a3b1a0 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Fri, 10 Jul 2026 12:35:55 +0200 Subject: [PATCH 5/7] refactor(husk): bring the tap up while the pod is dormant, not while a tenant waits A warm claim pays the whole microVM restore while a tenant waits. Measured on prod, one matched claim: the husk stub's own activate is 112.8 ms, of which egress_filter is 29.6. That stage is two things: bringing the tap up (enable forwarding, delete any leftover link, one ip -batch) and installing the netfilter policy (one atomic nft transaction). Only the policy depends on the claim. The tap does not: its name derives from the guest IP, and the in-pod guest IP is a fixed constant. applyEgressFilter is split into ensureEgressLink and applyEgressPolicy. The composed call still runs exactly the same commands in the same order, pinned by a test that records both and compares them command for command, so a pod that does not opt in is byte-for- byte what it was. Behind --husk-prepare-egress-link (controller) and --prepare-egress-link (stub), both DEFAULT OFF, a warm multi-VM pod brings its default VM's tap up while DORMANT under a default-deny policy, and the claim installs the tenant's policy on it. The hot path becomes exactly one command. Fail-closed at every step, which is most of the work: - Prepare creates the tap and its default-deny policy together and refuses to go dormant if either half fails, so the pool never offers a pod with a half-built datapath and a tap never exists without a policy. - The claim REPLACES the policy in one nft transaction, so there is no window in which a VM is unfiltered. - A rejected claim policy tears the tap down (as it always did) AND clears the prepared marker, so the retry rebuilds the link instead of installing a policy on a tap that no longer exists, which would poison the warm pod permanently. - A malformed allowlist is still rejected before any command runs; splitting the call must not start creating taps for a policy that can never be installed. Nothing is loaded behind a dormant tap today: the snapshot restore stays on the activate path. That is slice 2, and it needs this because Firecracker requires the tap to exist at restore time. The design, the measured budget, the guards, and the rollout are written down in docs/superpowers/plans/2026-07-10-prepare-time-restore.md, and the threat model records the new dormant-tap surface. Verified: go test -race ./internal/husk/, go test ./internal/controller/ ./cmd/husk-stub/ ./internal/firecracker/, and golangci-lint for both GOOS=darwin and GOOS=linux. Not yet canaried on prod; the flag is off, so merging changes nothing until an operator sets it. Signed-off-by: Jannes Stubbemann --- cmd/controller/main.go | 4 + cmd/husk-stub/main.go | 6 + .../plans/2026-07-10-prepare-time-restore.md | 137 +++++++++++++++++ docs/threat-model.md | 2 +- internal/controller/huskpod.go | 16 ++ internal/controller/huskpod_test.go | 30 ++++ .../controller/sandboxclaim_controller.go | 4 + internal/controller/sandboxpool_controller.go | 4 + internal/husk/multivm.go | 64 +++++++- internal/husk/multivm_network_test.go | 138 ++++++++++++++++++ internal/husk/netfilter.go | 64 ++++++-- internal/husk/netfilter_test.go | 115 +++++++++++++++ internal/husk/stub.go | 30 ++++ 13 files changed, 602 insertions(+), 12 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-10-prepare-time-restore.md diff --git a/cmd/controller/main.go b/cmd/controller/main.go index a62553ee..e91e05d1 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -64,6 +64,7 @@ func main() { var huskConnReuse bool var liveCowChildImport bool var prewarmChild bool + var prepareEgressLink bool var huskStubImage string var huskDNSUpstream string var huskControlPort int @@ -96,6 +97,7 @@ func main() { flag.BoolVar(&multiVMFork, "multi-vm-fork", false, "EXPERIMENTAL, DEFAULT OFF: route a hosted fork child into an ADDITIONAL VM spawned INSIDE the source husk pod (the spawn-vm control op) instead of a brand-new child pod, when the source pod is multi-VM capable (its stub runs --multi-vm). OFF is byte-for-byte the current new-pod-per-fork path, so nothing changes unless you opt in AND the source pod is multi-VM capable; a non-capable source silently falls back to a new pod. This wires the routing + status (child status.pod = source pod, status.vmId = the spawned VM); the per-pod and node memory accounting that pends or spills a fork when a pod is full is a follow-up, so co-location is capped conservatively and the remainder spills to new pods. Only used with --enable-husk-pods.") flag.BoolVar(&liveCowFork, "live-cow-fork", false, "EXPERIMENTAL, DEFAULT OFF: start warm husk pods with --live-cow-fork so a CO-LOCATED fork child shares the PARENT's resident guest memory (patched Firecracker memfd + userfaultfd write-protect) instead of restoring from the disk fork snapshot (milestone m4b), driving the hosted fork toward sub-100ms. SEPARATE from --multi-vm-fork so it can be deployed off and canaried independently. OFF is byte-for-byte the current disk co-location. The co-located child still falls back to the disk restore where the child-side memfd import is not yet complete, so turning it on never breaks a fork; it is a no-op off Linux (userfaultfd write-protect is Linux-only). Only meaningful with --enable-husk-pods.") flag.BoolVar(&liveCowChildImport, "live-cow-child-import", false, "EXPERIMENTAL, DEFAULT OFF: with --live-cow-fork on, opt warm husk pods onto the VMSTATE-ONLY fork capture (skip the ~364ms disk mem write) so a co-located child boots its guest RAM from the source shared memfd instead of the disk fork snapshot. REQUIRES a child-side-import patched Firecracker; off keeps the armed source writing the disk mem so every child restores from disk and no fork hangs. Only meaningful with --live-cow-fork and --enable-husk-pods.") + flag.BoolVar(&prepareEgressLink, "husk-prepare-egress-link", false, "EXPERIMENTAL, DEFAULT OFF: warm husk pods bring their default VM's tap up while DORMANT, under a default-deny policy, so a claim pays only the atomic nft transaction that installs the tenant's policy instead of also paying the tap create. Requires --multi-vm-fork and --enable-husk-pods. Off keeps the whole filter on the activate path byte-for-byte.") flag.BoolVar(&prewarmChild, "prewarm-child", false, "EXPERIMENTAL, DEFAULT OFF: keep one dormant generic co-located child Firecracker pre-prepared per multi-vm husk pod so a fork adopts the ready child (fc_boot=0, prepare off the hot path) instead of booting one at fork time. Requires --multi-vm-fork and --enable-husk-pods; a fresh slot re-warms async, a miss falls back to on-demand prepare byte-for-byte.") flag.BoolVar(&huskConnReuse, "husk-conn-reuse", false, "EXPERIMENTAL, DEFAULT OFF: reuse ONE authenticated mTLS husk control connection per husk pod across control-plane RPCs (activate, fork-snapshot, spawn-vm, remove-fork-snapshot) instead of opening a fresh TCP+TLS handshake per RPC. A co-located fork does fork-snapshot then spawn-vm to the SAME source pod, so reuse saves the second full handshake and cuts the per-RPC connection-setup overhead toward zero, driving the hosted fork toward sub-100ms. OFF is byte-for-byte the current one-shot dial-per-RPC path. The husk server always supports both (a one-shot client that closes after one request and a reused connection that sends several), so this flag only changes the CONTROLLER side and can be canaried + rolled back independently. mTLS identity is verified on every dial and one request is in flight per connection, so a reused connection is neither less authenticated nor frame-interleaved (see docs/threat-model.md). Only meaningful with --enable-husk-pods.") flag.StringVar(&huskStubImage, "husk-stub-image", "mitos-husk-stub:latest", "Container image that runs the dormant-VMM stub in a husk pod. Only used with --enable-husk-pods.") @@ -235,6 +237,7 @@ func main() { LiveCowFork: liveCowFork, LiveCowChildImport: liveCowChildImport, PrewarmChild: prewarmChild, + PrepareEgressLink: prepareEgressLink, HuskStubImage: huskStubImage, HuskDNSUpstream: huskDNSUpstream, KVMResourceName: "mitos.run/kvm", @@ -280,6 +283,7 @@ func main() { HuskConns: huskConnPool, LiveCowChildImport: liveCowChildImport, PrewarmChild: prewarmChild, + PrepareEgressLink: prepareEgressLink, HuskControlPort: huskControlPort, HuskStubImage: huskStubImage, HuskDNSUpstream: huskDNSUpstream, diff --git a/cmd/husk-stub/main.go b/cmd/husk-stub/main.go index 8d9fc7ef..945b0b6f 100644 --- a/cmd/husk-stub/main.go +++ b/cmd/husk-stub/main.go @@ -193,6 +193,9 @@ func run() error { enableEgress = flag.Bool("enable-egress-filter", true, "Program the in-pod nftables egress filter (default-deny + metadata block) for the activated VM. Requires NET_ADMIN in the pod netns. Default true (the husk isolation guarantee).") multiVM = flag.Bool("multi-vm", false, "EXPERIMENTAL, default false: opt into the multi-VM-per-pod execution mode (#764), running many same-tenant CoW forks inside ONE husk pod instead of one pod per VM. Increment 1 wires only a default-off scaffold; false (the default, and the only value the controller emits) keeps the single-VM behavior byte-for-byte. Not a secret.") liveCowFork = flag.Bool("live-cow-fork", false, "EXPERIMENTAL, default false: opt a CO-LOCATED fork child onto the live copy-on-write path (share the PARENT's resident guest memory via the patched Firecracker memfd + userfaultfd write-protect handler) instead of restoring from the disk fork snapshot (milestone m4b). SEPARATE from --multi-vm so it can be deployed off and canaried independently. Off (the default, and the only value the controller emits unless the operator opts in) keeps the co-located fork on the disk snapshot restore byte-for-byte. No-op off Linux (userfaultfd write-protect is Linux-only), failing closed to the disk restore. Not a secret.") + prepareEgressLink = flag.Bool("prepare-egress-link", false, "EXPERIMENTAL, DEFAULT OFF: bring the pod's default-VM tap up while the pod is DORMANT, under a default-deny policy, so a claim pays only the atomic nft transaction that installs the tenant's policy instead of also paying the tap create (measured: the link half is about two thirds of the egress_filter stage). Requires --multi-vm and --in-pod-guest-ip. Off keeps the whole filter on the activate path byte-for-byte. Not a secret.") + inPodGuestIP = flag.String("in-pod-guest-ip", "", "the fixed in-pod guest IP of this pod's default VM (the same value the controller sends in the activate request). Needed BEFORE that request arrives by --prepare-egress-link, because the tap name derives from it. Config, not a secret.") + inPodGatewayIP = flag.String("in-pod-gateway-ip", "", "the fixed in-pod gateway IP paired with --in-pod-guest-ip. Config, not a secret.") prewarmChild = flag.Bool("prewarm-child", false, "EXPERIMENTAL, DEFAULT OFF: keep ONE dormant generic co-located child Firecracker pre-prepared per multi-vm pod so a fork ADOPTS the ready child (fc_boot=0, prepare off the hot path) instead of booting one at fork time. Requires --multi-vm; a fresh slot re-warms async, a miss falls back to the on-demand prepare. Not a secret.") liveCowChildImport = flag.Bool("live-cow-child-import", false, "EXPERIMENTAL, default false: opt the live-cow fork onto the VMSTATE-ONLY capture (skip the ~364ms disk mem write). REQUIRES a shipped child-side memfd-import Firecracker patch: the co-located child boots its guest RAM from the source shared memfd instead of the disk fork snapshot. The currently shipped patched binary patches the SOURCE (restore) side only, so leave this OFF; with it off an armed --live-cow-fork source still writes the disk mem, so every co-located child restores from disk and the fork never hangs. Turn on ONLY once a child-side import binary is shipped. Not a secret.") ) @@ -437,6 +440,9 @@ func run() error { // the fork never hangs. Turn on ONLY once a child-side import binary ships. LiveCowChildImport: *liveCowChildImport, PrewarmChild: *prewarmChild, + PrepareEgressLink: *prepareEgressLink, + InPodGuestIP: *inPodGuestIP, + InPodGatewayIP: *inPodGatewayIP, }) // Publish the constructed stub to the already-serving metering source. publishedStub.Store(stub) diff --git a/docs/superpowers/plans/2026-07-10-prepare-time-restore.md b/docs/superpowers/plans/2026-07-10-prepare-time-restore.md new file mode 100644 index 00000000..266477a3 --- /dev/null +++ b/docs/superpowers/plans/2026-07-10-prepare-time-restore.md @@ -0,0 +1,137 @@ +# Prepare-time restore: move the snapshot restore off the warm-claim hot path + +Status: slice 1 implemented (default off, not yet canaried on prod); slices 2 and 3 designed. + +## Why + +A warm claim currently pays the whole microVM restore while a tenant waits. Measured on +prod (`mitos-kvm-1`), one matched claim, controller stage log against the husk stub's own: + +| stage | ms | +|---|---| +| controller `activate_rpc` | 129.7 | +| husk-reported activate total | 112.8 | +| ... `guest_ready` | 40.6 | +| ... `egress_filter` | 29.6 | +| ... `vmstate_restore` | 22.9 | +| ... `handshake` | 17.2 | +| ... `resume` | 2.4 | +| RPC + mTLS overhead between the two | ~17 | + +Under sustained load the same stages mean about 62 ms in total, so treat the numbers +above as one cold-ish sample and the ordering, not the absolute values, as the signal. + +Everything except `handshake` is work the pod could have done while it sat dormant with +no tenant attached. `guest_ready` is largely demand fault-in of guest RAM under the lazy +UFFD restore (see `bench/results/2026-07-09-lazy-livecow-restore.md`), and it is also why +the first `run_code` in a fresh sandbox costs ~105 ms against ~35 ms warm: the ipykernel +is already running in the snapshot (`warmKernel: true`), but its pages are not resident. + +Time to Interactive on the hosted API is 307.7 ms P50, of which create is ~196 ms and the +first exec ~118 ms (`bench/results/2026-07-10-tti-hosted.md`). This is the single largest +lever on both halves. + +## What is actually claim-specific + +Reading `activateInstance` (`internal/husk/multivm.go`), only three things depend on the +claim: + +1. the tenant's netfilter policy (`req.Egress`, `req.Allow`, `req.AllowCIDRs`, + `req.Inbound`, `req.BlockNetwork`); +2. the fork-correctness handshake, which carries fresh entropy, the clock step, the + guest's network re-addressing, and the tenant's secrets (`notifyReq`); +3. `onActivated` (serving the sandbox API with the claim's token). + +Everything else is a function of the pod and its pool template: + +- the tap name is `netconf.DeriveTapName(guestIP)`, and the in-pod guest IP is the FIXED + constant `10.200.0.2` (`huskGuestIP`, `internal/controller/sandboxclaim_controller.go`); +- the snapshot dir and its expected digest are already known at Prepare + (`PrepareSnapshotDir` / `PrepareExpectedDigest`), which is why Prepare already + pre-pays the content-addressed verify; +- the rootfs CoW clone is per-pod; +- the live-cow write-protect arm already happens in `prepareInstance`. + +So the restore itself (`setLiveCowMemSource` -> `LoadSnapshot` -> `PatchDrive` -> +`Resume` -> guest-ready) needs nothing from the claim except a tap to bind the baked NIC +to, and the tap name is knowable at Prepare. + +## Target + +Prepare (dormant, no tenant): + +1. ensure the tap exists and install a DEFAULT-DENY netfilter policy on it (a dormant VM + must reach nothing, fail closed); +2. `setLiveCowMemSource`, `LoadSnapshotWithOverrides` (paused), `PatchDrive` to the CoW + clone, `Resume`; +3. wait for the guest agent (this is where the demand fault-in is paid); +4. optionally prefault the run_code kernel's working set by running one inert cell, the + same trick the template build uses (`warmKernelCode`). + +Activate (claim): + +1. replace the netfilter policy with the tenant's, atomically, on the tap that already + exists (one `nft -f -`, not the tap create plus policy); +2. the fork-correctness handshake (entropy, clock, network, secrets); +3. `onActivated`. + +Expected activate: roughly `nft` + `handshake` + `serve_api`, i.e. ~30 ms against 62 to +113 ms today, plus the first `run_code` arriving warm. + +## Guards, in order of how badly they bite + +- **Snapshot identity.** If `req.SnapshotDir` is not the dir Prepare restored, the pod + has already restored the wrong image. FAIL CLOSED with a named error; never silently + reload, because a resumed VM cannot be reloaded. +- **Fork children.** A co-located fork child restores a node-local fork snapshot through + `spawnVM` -> `prepareInstance` -> `activateInstance` with `req.ForkSnapshot` set. It + must keep the current path exactly. Prepare-time restore applies ONLY to + `defaultVMID` with `PrepareSnapshotDir` set and no fork snapshot. +- **Dormant egress.** Between the restore and the claim, a live guest runs behind a tap. + It must be default-deny for that whole window, and the claim must be able to WIDEN it + atomically without a window where no policy is installed. `nft -f -` replaces the + ruleset in one transaction, which is the property we need. +- **Reseed.** The dormant guest holds the snapshot's CRNG until the claim's + `NotifyForked`. It serves no tenant before then, and the handshake still gates Ready + fail-closed. The dormancy window only makes the clock step larger, and the step is + absolute. +- **Teardown.** `Close` must remove a tap created at Prepare, not only one created at + Activate (`inst.activeTap` is already the seam). +- **Memory.** A restored, resumed, prefaulted dormant VM holds its working set resident: + about 72 MiB per sandbox measured on the lazy path, against ~0 for a dormant VMM that + never loaded. At `warm.min: 8` that is roughly 576 MiB per node. The husk pod memory + request is already sized worst-case (`guestRAM * (1 + reserved forks)`), so this fits, + but it must be stated and watched. +- **Rollout.** Default OFF behind a stub flag. Canary ONE dormant pod (delete a single + dormant husk pod, check `restartCount=0`, claim it, compare stages) before recycling + the pool. `git merge-base --is-ancestor HEAD` before building anything + for prod. + +## Slices + +1. **Tap at Prepare.** DONE, default off. `applyEgressFilter` is split into + `ensureEgressLink` (forwarding, link delete, `ip -batch`) and `applyEgressPolicy` (one + atomic `nft -f -`), and the composed call still runs exactly the same commands in the + same order (pinned by `TestApplyEgressFilterIsTheTwoHalves`). Behind + `--husk-prepare-egress-link` the pod ensures the tap with a default-deny policy while + dormant, and the claim installs the tenant policy on it: exactly one command on the hot + path (`TestPrepareBringsTheTapUpDormantAndActivateOnlyInstallsThePolicy`). A rejected + claim policy tears the tap down and clears the prepared marker, so the retry rebuilds + the link rather than installing a policy on a tap that no longer exists + (`TestAFailedClaimPolicyRebuildsTheLinkOnRetry`). Worth roughly 20 ms, and a + prerequisite for slice 2 because Firecracker requires the tap to exist at restore time. + Still to do: measure it on prod behind a one-pod canary, then a KVM gate. +2. **Restore at Prepare.** Move `setLiveCowMemSource` / `LoadSnapshot` / `PatchDrive` / + `Resume` / guest-ready into `prepareInstance`, behind the same flag, with the guards + above. Activate becomes policy + handshake + serve_api. +3. **Prefault the kernel.** Run the inert warm cell at Prepare so the first tenant + `run_code` does not fault the ipykernel's pages in. + +Each slice ships with its own KVM gate in `firecracker-test`, because none of this is +observable without `/dev/kvm` and the mitos-patched Firecracker. + +## Not in scope + +`mark_pod_claimed` (16.5 ms) is the optimistic-lock mutual exclusion that stops two +claims taking one VM. Taking it off the hot path needs a reservation protocol, not a +reordering, and is tracked separately. diff --git a/docs/threat-model.md b/docs/threat-model.md index d4dfe3cc..765dbebd 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -1219,7 +1219,7 @@ VMMs run jailed under throwaway uids. | Husk pod default ServiceAccount token automount | **mitigated (shipped)** | The husk pod spec now sets `AutomountServiceAccountToken: false` (`internal/controller/huskpod.go` `buildHuskPod`), so the kubelet does NOT mount the namespace default ServiceAccount token into a husk pod. The stub speaks vsock + mTLS and never calls the Kubernetes API (verified: no client-go, no `InClusterConfig`, no SA token read in `cmd/husk-stub` or `internal/husk`), so the token was dead weight; a guest that escapes into the stub no longer inherits a free `system:authenticated` token. The opt-out applies to BOTH warm pods and fork-child pods, which share the builder, proven in `TestBuildHuskPodDisablesSATokenAutomount`. | | forkd gRPC fails OPEN without TLS flags | **mitigated (shipped)** | `grpcServerOptions` (`cmd/forkd/main.go`) now FAILS CLOSED: with no TLS flags it returns a fatal error naming the missing `--tls-cert/--tls-key/--tls-ca` flags and the opt-in, so forkd refuses to start unauthenticated. The legacy insecure-with-loud-warning behavior is reachable only behind an explicit `--allow-insecure-grpc` opt-in (default false) for local development. A partial TLS triple is still a configuration error. The shipped DaemonSet always sets TLS, so production is unaffected; this only stops a silent-insecure misconfig. Proven in `TestGRPCServerOptionsFailClosed` (refuses with no TLS and no opt-in; allowed with the opt-in; allowed with a full TLS triple; partial is an error). | | Host-side vsock read has no deadline | **mitigated (shipped)** | The host-side `vsock.Client` now applies a per-request read deadline in `send` (`internal/vsock/client.go`), defaulting to `DefaultRequestTimeout` (60s) and overridable via `SetRequestTimeout`. The CONNECT preamble reads in `Connect`/`DialStream`/`DialStreamUnix` are likewise bounded (then cleared so the long-lived streaming reads stay ctx/`conn.Close`-cancelled). A malicious or wedged guest that connects then stalls (or dribbles a partial line under the `MaxMessageBytes` cap) now causes the one-shot call to return a timeout error rather than hang the host caller goroutine, vsock fd, and (for the husk stub) stream slot indefinitely. Proven in `TestSendReadDeadlineUnblocksOnStall` (a fake agent that connects then never responds makes `Ping` return rather than hang). | -| Husk egress enforcement | **mitigated (IMPLEMENTED and KVM-VERIFIED end to end)** | See section 0 surface 5 and section 4. The husk default path enforces an in-pod nftables default-deny egress filter in the pod's own netns (CNI-independent, the guarantee), an unconditional cloud-metadata block (`169.254.169.254`, `169.254.0.0/16`, IPv6 `fd00:ec2::254`, before any allow), and the threaded per-template allowlist with a per-pod DNS proxy (failover upstream list, recommended `1.1.1.1:53,8.8.8.8:53`, not cluster DNS); a best-effort controller-emitted NetworkPolicy adds defense in depth (CNI-dependent, cannot express name-based allows). Costs one scoped `NET_ADMIN` capability (ADR 0006) plus, on name-egress pools only, a short-lived privileged `enable-ip-forward` init container that sets `net.ipv4.ip_forward=1` in the pod netns and exits (one-shot; no node prerequisite; the long-lived husk-stub container stays unprivileged). VERIFIED: the husk-network KVM cluster e2e (`test/cluster-e2e/husk-network-e2e.sh`, `cluster-husk-network-e2e`) PASSES on the Hetzner Talos KVM cluster; the claim reaches Ready and all three in-VM enforcement assertions are green inside a real restored VM (metadata-blocked exit 1, default-deny exit 1, allowlist-works `example.com` connect exit 0), verified twice (with and without a node sysctl allowance), proving NO node prerequisite. | +| Husk egress enforcement | **mitigated (IMPLEMENTED and KVM-VERIFIED end to end)** | See section 0 surface 5 and section 4. The husk default path enforces an in-pod nftables default-deny egress filter in the pod's own netns (CNI-independent, the guarantee), an unconditional cloud-metadata block (`169.254.169.254`, `169.254.0.0/16`, IPv6 `fd00:ec2::254`, before any allow), and the threaded per-template allowlist with a per-pod DNS proxy (failover upstream list, recommended `1.1.1.1:53,8.8.8.8:53`, not cluster DNS); a best-effort controller-emitted NetworkPolicy adds defense in depth (CNI-dependent, cannot express name-based allows). Costs one scoped `NET_ADMIN` capability (ADR 0006) plus, on name-egress pools only, a short-lived privileged `enable-ip-forward` init container that sets `net.ipv4.ip_forward=1` in the pod netns and exits (one-shot; no node prerequisite; the long-lived husk-stub container stays unprivileged). VERIFIED: the husk-network KVM cluster e2e (`test/cluster-e2e/husk-network-e2e.sh`, `cluster-husk-network-e2e`) PASSES on the Hetzner Talos KVM cluster; the claim reaches Ready and all three in-VM enforcement assertions are green inside a real restored VM (metadata-blocked exit 1, default-deny exit 1, allowlist-works `example.com` connect exit 0), verified twice (with and without a node sysctl allowance), proving NO node prerequisite. PREPARE-TIME LINK (`--husk-prepare-egress-link`, controller; `--prepare-egress-link`, stub; EXPERIMENTAL, DEFAULT OFF): a warm multi-VM husk pod may bring its default VM's tap up while it is DORMANT so a claim pays only the nft transaction, not the tap create. The dormant tap is created together with a DEFAULT-DENY policy in the same Prepare, and Prepare FAILS CLOSED (the pod never reaches dormant, so the pool never offers it) if either half fails, so a tap never exists without a policy. The claim REPLACES that policy with the tenant's in ONE atomic `nft -f -` transaction, so there is no window in which a VM is unfiltered, and a rejected policy tears the tap down and clears the prepared marker so the retry rebuilds the link rather than installing a policy on a tap that no longer exists. Nothing is loaded behind a dormant tap today (the snapshot restore stays on the activate path), and the pod netns is not shared with any tenant. Off, the filter path is byte-for-byte the activate-time one. | | agents.x-k8s.io facade admission surface (v1beta1 conformance, #357) | **mitigated (bounded translation, no host surface)** | The conformance facade (`internal/facade`, opt-in deployment) watches tenant-authored upstream `agents.x-k8s.io/v1beta1` and `extensions.agents.x-k8s.io/v1beta1` custom resources (Sandbox, SandboxTemplate, SandboxWarmPool, SandboxClaim) and TRANSLATES each into our `mitos.run/v1` SandboxPool/Sandbox objects in the SAME namespace, owner-referenced to the upstream CR for GC. It creates only namespace-scoped objects and grants no cross-namespace or host access; the run-path security boundary is unchanged (the bridged Sandbox is governed by the husk-pod and sandbox-API controls above, not by the facade). Tenant-controlled upstream fields are BOUNDED: `spec.operatingMode` (Running/Suspended) drives only release/re-activate of the bridged claim; `SandboxClaim.spec.warmPoolRef.name` is a name lookup of OUR pool of that name (no path or host input); `spec.podTemplate.spec.containers[0].env` (including `env.valueFrom` references) is mirrored through by reference and never logged; all other podTemplate fields and `volumeClaimTemplates` are UNMAPPED (no tenant-driven volume provisioning or host-shaped field reaches our engine via the facade, see docs/facade-conformance.md exceptions 1, 3, 4, 5). The v0.5.0 migration to v1beta1 REDUCED the input surface: it removed the v1alpha1 `warmpool` policy resolution branch (none/default/named), so a claim now references a warm pool by name with no policy logic to abuse. Test-infra note: v1beta1 is the storage version with a `conversion.strategy: Webhook` we do not run; the kind conformance job pins the live CRD conversion to None at install time and exercises only the storage version (the vendored CRD on disk is unchanged). Residual: the facade trusts cluster RBAC that admits the upstream CRs, so whoever may create an `agents.x-k8s.io` Sandbox in a namespace can drive a pool binding there; the same-namespace tenancy boundary (section 7a) applies, and the facade adds no privilege beyond it. | | Control-plane egress lockdown (chart profile, issue #704) | **mitigated (opt-in)** | The chart ships an opt-in default-deny EGRESS profile for the control plane (`networkPolicy.controlPlane.enabled`, `templates/control-plane-netpol.yaml`): a CiliumNetworkPolicy selecting the controller, console, and gateway that enumerates every legitimate flow (DNS via kube-dns with L7 visibility, the kube-apiserver/host/remote-node entities, intra-namespace) and denies the rest, bounding what a COMPROMISED control-plane component can reach (no lateral movement into other namespaces, no arbitrary internet egress for exfiltration). When `console.onboarding.smtp.host` is set, a SECOND policy scoped to the console ALONE additionally allows that SMTP host and port; it is DERIVED from the same values that configure the sender, so the network allow cannot drift from the sender config (the drift class that silently broke hosted signups: SMTP env present, egress absent, every verification send timed out). The SMTP policy never renders without the baseline profile (a lone egress policy would itself flip the console to default-deny), and the controller and gateway gain no mail-relay egress (a compromised controller cannot exfiltrate over SMTP). `extraEgress` passes deployment-specific rules through verbatim (e.g. an in-cluster OIDC issuer namespace); rules added there widen the boundary and are the operator's responsibility. Requires the Cilium CNI (CiliumNetworkPolicy CRD), consistent with the NetworkPolicy-enforcing-CNI requirement stated for the tenancy boundary. Default OFF: a plain install is unchanged. Rendering is asserted in `deploy/charts/charttest/netpol_test.go` (opt-in absent by default, baseline flows present, SMTP allow derived and console-scoped, all-or-nothing coupling). Residual: this is an egress control only (ingress lockdown for the control plane is separate work), and on a non-Cilium CNI the profile cannot be expressed (values accepted but objects unusable; the CRD requirement is documented). | diff --git a/internal/controller/huskpod.go b/internal/controller/huskpod.go index 7c9a8dd5..05ce45ba 100644 --- a/internal/controller/huskpod.go +++ b/internal/controller/huskpod.go @@ -200,6 +200,11 @@ type HuskPodOptions struct { // one dormant generic co-located child Firecracker pre-prepared and a fork adopts // it (fc_boot off the hot path). DEFAULT OFF; requires MultiVM. PrewarmChild bool + // PrepareEgressLink starts the husk stub with --prepare-egress-link (plus the + // in-pod link addresses it needs before an activate request arrives), so a warm + // pod brings its tap up while dormant and a claim pays only the nft transaction + // that installs the tenant's policy. Requires MultiVM. Default off. + PrepareEgressLink bool // MultiVMForkVMs is the number of ADDITIONAL fork-child VMs a multi-VM pod // reserves node memory for up front (beyond the source VM), so the co-location // routing has room to co-locate that many children before a fork spills to a new @@ -563,6 +568,16 @@ func (r *SandboxPoolReconciler) buildHuskPod(pool *v1.SandboxPool, template *v1. if opts.PrewarmChild { args = append(args, "--prewarm-child") } + if opts.PrepareEgressLink { + // The stub needs the in-pod link BEFORE a claim arrives, because the tap name + // derives from the guest IP. These are the same fixed values huskNotifyNetwork + // sends in the activate request; they are config, not secrets. + args = append(args, + "--prepare-egress-link", + "--in-pod-guest-ip", huskGuestIP, + "--in-pod-gateway-ip", huskGatewayIP, + ) + } // Live-cow write-protect needs a KERNEL-MODE userfaultfd over the guest RAM: the // source Firecracker registers UFFD_WP so the KVM guest's own writes fault to the @@ -1396,6 +1411,7 @@ func (r *SandboxPoolReconciler) reconcileHuskPods(ctx context.Context, pool *v1. LiveCowFork: r.LiveCowFork, LiveCowChildImport: r.LiveCowChildImport, PrewarmChild: r.PrewarmChild, + PrepareEgressLink: r.PrepareEgressLink, MultiVMForkVMs: r.MultiVMForkVMs, StubImage: r.HuskStubImage, DNSUpstream: r.HuskDNSUpstream, diff --git a/internal/controller/huskpod_test.go b/internal/controller/huskpod_test.go index 0f2b39fa..e48e7661 100644 --- a/internal/controller/huskpod_test.go +++ b/internal/controller/huskpod_test.go @@ -1506,3 +1506,33 @@ func TestReconcileHuskPodsThreadsMultiVM(t *testing.T) { } } } + +// TestBuildHuskPodThreadsPrepareEgressLink proves the prepare-time link flag reaches the +// stub together with the in-pod addresses it cannot work without: the tap name derives +// from the guest IP, and the stub needs it BEFORE an activate request arrives. Off (the +// default) the pod is byte-for-byte the current activate-time filter. +func TestBuildHuskPodThreadsPrepareEgressLink(t *testing.T) { + r := &controller.SandboxPoolReconciler{} + pool := &v1.SandboxPool{ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "ns"}} + tmpl := &v1.PoolTemplateSpec{} + + on := r.BuildHuskPodForTest(pool, tmpl, controller.HuskPodOptions{StubImage: "img", MultiVM: true, PrepareEgressLink: true}) + args := on.Spec.Containers[0].Args + for _, want := range []string{"--prepare-egress-link", "--in-pod-guest-ip", "--in-pod-gateway-ip"} { + if !argsContain(args, want) { + t.Errorf("husk args missing %s when prepare-egress-link is enabled: %v", want, args) + } + } + // The addresses must be the ones the activate request carries, or the stub would + // bring up a tap the claim never resolves to and the skip would silently not apply. + if !argsContain(args, "10.200.0.2") || !argsContain(args, "10.200.0.1") { + t.Errorf("husk args do not carry the fixed in-pod /30: %v", args) + } + + off := r.BuildHuskPodForTest(pool, tmpl, controller.HuskPodOptions{StubImage: "img", MultiVM: true}) + for _, unwanted := range []string{"--prepare-egress-link", "--in-pod-guest-ip", "--in-pod-gateway-ip"} { + if argsContain(off.Spec.Containers[0].Args, unwanted) { + t.Errorf("husk args must omit %s by default: %v", unwanted, off.Spec.Containers[0].Args) + } + } +} diff --git a/internal/controller/sandboxclaim_controller.go b/internal/controller/sandboxclaim_controller.go index ba809c9f..42912956 100644 --- a/internal/controller/sandboxclaim_controller.go +++ b/internal/controller/sandboxclaim_controller.go @@ -155,6 +155,10 @@ type SandboxReconciler struct { // PrewarmChild passes --prewarm-child to warm husk pods so a fork adopts a // pre-warmed dormant child. DEFAULT OFF; requires MultiVMFork. PrewarmChild bool + // PrepareEgressLink passes --prepare-egress-link to warm husk pods so a dormant + // pod brings its default VM's tap up before any claim arrives, leaving only the + // atomic nft policy transaction on the warm-claim hot path. Requires MultiVMFork. + PrepareEgressLink bool // spawnVM is the controller->husk spawn-vm seam used by the MultiVMFork routing. // Nil defaults to SpawnVMOnHusk; tests inject a fake. diff --git a/internal/controller/sandboxpool_controller.go b/internal/controller/sandboxpool_controller.go index 546ddbfb..960805f8 100644 --- a/internal/controller/sandboxpool_controller.go +++ b/internal/controller/sandboxpool_controller.go @@ -58,6 +58,10 @@ type SandboxPoolReconciler struct { // PrewarmChild passes --prewarm-child to warm husk pods so a fork adopts a // pre-warmed dormant child. DEFAULT OFF; requires MultiVM. PrewarmChild bool + // PrepareEgressLink passes --prepare-egress-link to warm husk pods so a dormant + // pod brings its default VM's tap up before any claim arrives, leaving only the + // atomic nft policy transaction on the warm-claim hot path. Requires MultiVMFork. + PrepareEgressLink bool // MultiVMForkVMs is how many co-located fork VMs a multi-VM warm pod reserves // node memory for up front (beyond the source VM), so the co-location routing // has room before a fork spills to a new pod. Only consulted when MultiVM is set; diff --git a/internal/husk/multivm.go b/internal/husk/multivm.go index 42407a36..df25fe9d 100644 --- a/internal/husk/multivm.go +++ b/internal/husk/multivm.go @@ -13,6 +13,7 @@ import ( "strings" "time" + v1 "mitos.run/mitos/api/v1" "mitos.run/mitos/internal/cas" "mitos.run/mitos/internal/firecracker" "mitos.run/mitos/internal/fork" @@ -371,10 +372,53 @@ func (s *Stub) prepareInstanceOpt(ctx context.Context, id vmID, opts prepareOpts } } + // Bring THIS pod's default-VM tap up while it is dormant, under a DEFAULT-DENY + // policy, so the claim pays only the atomic nft transaction that installs the + // tenant's policy. Opt-in (Options.PrepareEgressLink), default VM only, and only + // when the in-pod link is configured; otherwise activate does the whole thing as + // before. FAIL CLOSED: ensureEgressLink tears the tap down on any error and we + // refuse to go dormant, so the pool never offers a pod whose datapath is + // half-built. A dormant tap is never a hazard: nothing is loaded behind it, and + // the policy on it denies everything. + if err := s.prepareEgressLinkFor(ctx, id, inst); err != nil { + if inst.vm != nil { + _ = inst.vm.Close() + inst.vm = nil + } + return err + } + inst.state = StateDormant return nil } +// prepareEgressLinkFor brings up the default VM's tap with a default-deny policy while +// the pod is dormant, when the stub opted in. A no-op otherwise, and never for a +// secondary (co-located fork child) VM, whose tap is derived per fork at activate. +func (s *Stub) prepareEgressLinkFor(ctx context.Context, id vmID, inst *vmInstance) error { + if !s.prepareEgressLink || id != defaultVMID || s.netRunner == nil || s.inPodGuestIP == "" { + return nil + } + tap := netconf.DeriveTapName(s.inPodGuestIP) + cfg := NetfilterConfig{ + Tap: tap, + GuestIP: net.ParseIP(s.inPodGuestIP), + HostIP: net.ParseIP(s.inPodGatewayIP), + Egress: v1.EgressDeny, + } + if err := ensureEgressLink(ctx, s.netRunner, s.enableForwarding, cfg); err != nil { + return fmt.Errorf("husk: prepare in-pod egress link for vm %q: %w", id, err) + } + if err := applyEgressPolicy(ctx, s.netRunner, cfg); err != nil { + return fmt.Errorf("husk: prepare default-deny egress policy for vm %q: %w", id, err) + } + // Record it on BOTH fields: activeTap so Close tears it down even if this pod is + // never claimed, preparedLinkTap so activate knows it can skip the link setup. + inst.activeTap = tap + inst.preparedLinkTap = tap + return nil +} + // cloneRootfsForInstance makes THIS VM's per-activation rootfs CoW clone and // records it on the instance so activate rebinds the drive to it and Close removes // it. The clone SOURCE is the pool template rootfs by default; a co-located fork @@ -508,8 +552,24 @@ func (s *Stub) activateInstance(ctx context.Context, id vmID, req ActivateReques cfg.Tap = tap cfg.GuestIP = net.ParseIP(perNet.GuestIP) cfg.HostIP = net.ParseIP(perNet.GatewayIP) - if err := applyEgressFilter(ctx, s.netRunner, s.enableForwarding, cfg); err != nil { - werr := fmt.Errorf("husk: apply in-pod egress filter for vm %q: %w", id, err) + // When this instance brought the SAME tap up while dormant, the claim pays only + // the nft transaction that REPLACES the default-deny policy with the tenant's. + // nft applies it atomically, so there is no window in which the VM is + // unfiltered. Any other tap (a fork child's, or a pod that did not opt in) takes + // the full path. Both halves fail closed by tearing the tap down, so clear the + // prepared marker first: after a failure the link no longer exists and the next + // attempt must rebuild it. + prepared := inst.preparedLinkTap != "" && inst.preparedLinkTap == tap + inst.preparedLinkTap = "" + var ferr error + if prepared { + ferr = applyEgressPolicy(ctx, s.netRunner, cfg) + } else { + ferr = applyEgressFilter(ctx, s.netRunner, s.enableForwarding, cfg) + } + if ferr != nil { + inst.activeTap = "" + werr := fmt.Errorf("husk: apply in-pod egress filter for vm %q: %w", id, ferr) return ActivateResult{OK: false, Error: werr.Error()}, werr } // Pin the baked NIC to THIS VM's tap so the restored VM's NIC is backed by diff --git a/internal/husk/multivm_network_test.go b/internal/husk/multivm_network_test.go index 3f560179..d598b7f8 100644 --- a/internal/husk/multivm_network_test.go +++ b/internal/husk/multivm_network_test.go @@ -208,3 +208,141 @@ func TestMultiVMActivateProgramsDistinctTapPerVMID(t *testing.T) { t.Errorf("the two VMs must be handed distinct guest IPs, got %v", seen) } } + +// --- prepare-time egress link ------------------------------------------------------ + +// countIPBatch counts the ip -batch (tap create) invocations. +func countIPBatch(calls []recordedCall) int { + n := 0 + for _, c := range calls { + if len(c.argv) > 0 && strings.Contains(c.argv[0], "ip") && strings.Contains(strings.Join(c.argv, " "), "-batch") { + n++ + } + } + return n +} + +func countNft(calls []recordedCall) int { + n := 0 + for _, c := range calls { + if len(c.argv) > 0 && strings.Contains(c.argv[0], "nft") { + n++ + } + } + return n +} + +// newPreparedLinkStub builds a multi-VM stub that brings its default VM's tap up while +// dormant, over a recording net runner. +func newPreparedLinkStub(t *testing.T, rr *recordingRunner) *Stub { + t.Helper() + start := func(_ firecracker.VMConfig) (vmm, error) { return &fakeVMM{}, nil } + s := New(firecracker.VMConfig{ID: "husk-test"}, Options{ + Start: start, + Ready: readyOK, + Notify: (&fakeNotifier{}).notify, + Verify: verifyOK, + MultiVM: true, + PrepareEgressLink: true, + InPodGuestIP: "10.200.0.2", + InPodGatewayIP: "10.200.0.1", + }) + s.SetNetRunner(rr.run) + return s +} + +// TestPrepareBringsTheTapUpDormantAndActivateOnlyInstallsThePolicy is the point of the +// split. A pod that opted in pays the tap create while it is DORMANT, so the claim's +// egress_filter stage is one atomic nft transaction and nothing else. The dormant tap +// carries a default-deny policy, so the VM behind it is never unfiltered. +func TestPrepareBringsTheTapUpDormantAndActivateOnlyInstallsThePolicy(t *testing.T) { + var rr recordingRunner + s := newPreparedLinkStub(t, &rr) + ctx := context.Background() + + if err := s.Prepare(ctx); err != nil { + t.Fatalf("Prepare: %v", err) + } + if got := countIPBatch(rr.calls); got != 1 { + t.Errorf("Prepare ran %d tap creates, want exactly 1", got) + } + if got := countNft(rr.calls); got != 1 { + t.Errorf("Prepare installed %d policies, want exactly 1 (the dormant default-deny)", got) + } + tap := netconf.DeriveTapName("10.200.0.2") + if !strings.Contains(strings.Join(rr.calls[0].argv, " "), tap) { + t.Errorf("Prepare did not touch the pod's tap %q: %v", tap, rr.calls[0].argv) + } + + prepared := len(rr.calls) + if _, err := s.Activate(ctx, ActivateRequest{SnapshotDir: "/snap", Network: baseNet()}); err != nil { + t.Fatalf("Activate: %v", err) + } + claim := rr.calls[prepared:] + if got := countIPBatch(claim); got != 0 { + t.Errorf("the claim re-created the tap %d times; the dormant pod already brought it up", got) + } + if got := countNft(claim); got != 1 { + t.Errorf("the claim ran %d nft transactions, want exactly 1 (replace the policy): %v", got, claim) + } + if len(claim) != 1 { + t.Errorf("the claim ran %d commands, want exactly 1: %v", len(claim), claim) + } +} + +// TestPrepareTimeLinkIsOptIn: a stub that did not opt in must behave byte-for-byte as +// before, doing all of it at activate. +func TestPrepareTimeLinkIsOptIn(t *testing.T) { + var rr recordingRunner + start := func(_ firecracker.VMConfig) (vmm, error) { return &fakeVMM{}, nil } + s := New(firecracker.VMConfig{ID: "husk-test"}, Options{ + Start: start, Ready: readyOK, Notify: (&fakeNotifier{}).notify, Verify: verifyOK, + MultiVM: true, + }) + s.SetNetRunner(rr.run) + ctx := context.Background() + + if err := s.Prepare(ctx); err != nil { + t.Fatalf("Prepare: %v", err) + } + if len(rr.calls) != 0 { + t.Fatalf("Prepare touched the network without opting in: %v", rr.calls) + } + if _, err := s.Activate(ctx, ActivateRequest{SnapshotDir: "/snap", Network: baseNet()}); err != nil { + t.Fatalf("Activate: %v", err) + } + if got := countIPBatch(rr.calls); got != 1 { + t.Errorf("activate ran %d tap creates, want 1", got) + } + if got := countNft(rr.calls); got != 1 { + t.Errorf("activate ran %d nft transactions, want 1", got) + } +} + +// TestAFailedClaimPolicyRebuildsTheLinkOnRetry: applyEgressPolicy fails closed by +// tearing the tap down, so the prepared marker must be cleared. Otherwise the retry +// would install a policy on a tap that no longer exists and the pod would be poisoned. +func TestAFailedClaimPolicyRebuildsTheLinkOnRetry(t *testing.T) { + var rr recordingRunner + s := newPreparedLinkStub(t, &rr) + ctx := context.Background() + if err := s.Prepare(ctx); err != nil { + t.Fatalf("Prepare: %v", err) + } + + inst := s.instanceFor(defaultVMID, false) + if inst == nil || inst.preparedLinkTap == "" { + t.Fatal("Prepare did not record the prepared tap") + } + + // A claim whose policy is rejected: the CIDR list cannot be parsed. + if _, err := s.Activate(ctx, ActivateRequest{SnapshotDir: "/snap", Network: baseNet(), AllowCIDRs: []string{"not-a-cidr"}}); err == nil { + t.Fatal("Activate accepted a malformed CIDR allowlist") + } + if inst.preparedLinkTap != "" { + t.Error("a failed claim left the prepared-link marker set; the retry would skip rebuilding a torn-down tap") + } + if inst.activeTap != "" { + t.Error("a failed claim left activeTap set for a tap that was torn down") + } +} diff --git a/internal/husk/netfilter.go b/internal/husk/netfilter.go index 4ee8b533..54837231 100644 --- a/internal/husk/netfilter.go +++ b/internal/husk/netfilter.go @@ -81,15 +81,37 @@ type netfilterRunner func(ctx context.Context, argv []string, stdin string) erro // `nft -f` that applies the shared table, this VM's per-tap chain, the SNAT, the // shared input table, and this tap's input chain. A malformed allowlist fails // the whole call (fail-closed: a VM never comes up with a half-applied filter). +// applyEgressFilter brings up this VM's tap and installs its policy, fail-closed. +// +// It is the composition of the two halves below, and it stays the ONLY entry point for +// callers that have no prepared link: the two are split so a warm husk pod can pay the +// link half while it is dormant (no tenant attached) and leave only the policy half on +// the claim's hot path. Measured on prod, the link half is roughly two thirds of the +// ~30 ms this call costs at activate. func applyEgressFilter(ctx context.Context, run netfilterRunner, enableForwarding func() error, cfg NetfilterConfig) (err error) { - enforceable, _, err := netconf.SplitAllowList(cfg.Allow) - if err != nil { + // Reject a malformed allowlist BEFORE creating anything, so a bad policy never + // costs a tap create and teardown, and the caller sees the parse error rather than + // a later one. applyEgressPolicy re-parses; parsing is pure and cheap. + if _, _, err := netconf.SplitAllowList(cfg.Allow); err != nil { return fmt.Errorf("husk netfilter: parse allowlist: %w", err) } - cidrV4, cidrV6, err := netconf.ParseCIDRList(cfg.AllowCIDRs) - if err != nil { + if _, _, err := netconf.ParseCIDRList(cfg.AllowCIDRs); err != nil { return fmt.Errorf("husk netfilter: parse CIDR allowlist: %w", err) } + if err := ensureEgressLink(ctx, run, enableForwarding, cfg); err != nil { + return err + } + return applyEgressPolicy(ctx, run, cfg) +} + +// ensureEgressLink enables forwarding and (re)creates this VM's tap with its host /30. +// It installs NO policy: a tap that exists without a policy carries no traffic, because +// nothing is loaded behind it yet and the shared forward table's default is a drop. +// +// FAIL CLOSED and idempotent: any error tears the tap back down before returning, and a +// pre-existing tap of the same name is removed first (the name is deterministic per +// template, so a leaked tap would otherwise poison the pod with EBUSY forever, #428). +func ensureEgressLink(ctx context.Context, run netfilterRunner, enableForwarding func() error, cfg NetfilterConfig) (err error) { // IPv4 forwarding in the pod netns: the kernel will not route the guest /30 // between the tap and the pod uplink without it, so the SNAT below would have // nothing to NAT. Nil seam (tests) skips it. Done first so a failure aborts @@ -111,11 +133,11 @@ func applyEgressFilter(ctx context.Context, run netfilterRunner, enableForwardin // ip -batch invocation may create the tap and then fail on a LATER line in the // SAME process, which would leak the tap: its name is deterministic per // template, so a leak makes the next activation fail right at tap creation - // with EBUSY (Device or resource busy), masking the real first-attempt error - // and permanently poisoning the warm pod (issue #428). Tearing down on every - // error path removes a partially created tap (link del of an absent tap is an - // ignored no-op), and the original error is preserved as the named return so - // the true root-cause step error is surfaced, not the EBUSY of a later retry. + // with EBUSY, masking the real first-attempt error and permanently poisoning + // the warm pod (issue #428). Tearing down on every error path removes a + // partially created tap (link del of an absent tap is an ignored no-op), and + // the original error is preserved as the named return so the true root-cause + // step error is surfaced, not the EBUSY of a later retry. defer func() { if err != nil { // Detached from ctx so cleanup still runs when the failure was a @@ -133,6 +155,30 @@ func applyEgressFilter(ctx context.Context, run netfilterRunner, enableForwardin if err := run(ctx, netconf.IPBatchArgs(), netconf.RenderIPBatch(cfg.Tap, cfg.HostIP, cfg.ResolverIP)); err != nil { return fmt.Errorf("husk netfilter: set up tap %s: %w", cfg.Tap, err) } + return nil +} + +// applyEgressPolicy installs (or REPLACES) this VM's netfilter policy on a tap that +// already exists. It is one nft process applying one atomic transaction, so a claim can +// widen a dormant VM's default-deny policy to its tenant posture with no window in +// which the VM is unfiltered, and a malformed allowlist installs NOTHING. +// +// FAIL CLOSED: on any error the tap is torn down, exactly as a full applyEgressFilter +// would, so a VM never comes up half-filtered. +func applyEgressPolicy(ctx context.Context, run netfilterRunner, cfg NetfilterConfig) (err error) { + enforceable, _, err := netconf.SplitAllowList(cfg.Allow) + if err != nil { + return fmt.Errorf("husk netfilter: parse allowlist: %w", err) + } + cidrV4, cidrV6, err := netconf.ParseCIDRList(cfg.AllowCIDRs) + if err != nil { + return fmt.Errorf("husk netfilter: parse CIDR allowlist: %w", err) + } + defer func() { + if err != nil { + _ = teardownEgressFilter(context.WithoutCancel(ctx), run, cfg.Tap) + } + }() // One nft process instead of ~5, applied as a SINGLE atomic transaction: // - the pod-global shared forward table (idempotent skeleton), // - this VM's per-tap egress chain (metadata drop + allowlist + counter), diff --git a/internal/husk/netfilter_test.go b/internal/husk/netfilter_test.go index 2436fcc2..4b851bec 100644 --- a/internal/husk/netfilter_test.go +++ b/internal/husk/netfilter_test.go @@ -341,3 +341,118 @@ func TestApplyEgressFilterBatchesNftIntoOneTransaction(t *testing.T) { t.Errorf("shared table must precede the per-sandbox chain in the batched payload (shared=%d chain=%d)", sharedIdx, chainIdx) } } + +// --- prepare-time link, claim-time policy ----------------------------------------- + +// isNftApply reports whether argv invokes nft. +func isNftApply(argv []string) bool { + return len(argv) > 0 && strings.Contains(argv[0], "nft") +} + +// isIPCommand reports whether argv invokes ip (tap create / link delete). +func isIPCommand(argv []string) bool { + return len(argv) > 0 && strings.Contains(argv[0], "ip") +} + +// TestApplyEgressFilterIsTheTwoHalves pins that splitting the call did not change what +// it runs: the composed call still issues exactly the link commands and then the single +// atomic nft transaction. +func TestApplyEgressFilterIsTheTwoHalves(t *testing.T) { + cfg := NetfilterConfig{Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), Egress: v1.EgressDeny} + + var whole recordingRunner + if err := applyEgressFilter(context.Background(), whole.run, func() error { return nil }, cfg); err != nil { + t.Fatalf("applyEgressFilter: %v", err) + } + + var halves recordingRunner + if err := ensureEgressLink(context.Background(), halves.run, func() error { return nil }, cfg); err != nil { + t.Fatalf("ensureEgressLink: %v", err) + } + if err := applyEgressPolicy(context.Background(), halves.run, cfg); err != nil { + t.Fatalf("applyEgressPolicy: %v", err) + } + + if len(whole.calls) != len(halves.calls) { + t.Fatalf("composed call ran %d commands, the two halves ran %d", len(whole.calls), len(halves.calls)) + } + for i := range whole.calls { + if strings.Join(whole.calls[i].argv, " ") != strings.Join(halves.calls[i].argv, " ") { + t.Errorf("command %d differs: composed %v, halves %v", i, whole.calls[i].argv, halves.calls[i].argv) + } + if whole.calls[i].stdin != halves.calls[i].stdin { + t.Errorf("command %d stdin differs", i) + } + } +} + +// TestApplyEgressPolicyTouchesOnlyNft is the point of the split: a claim that lands on a +// pod whose tap was already brought up while it was dormant must pay ONE nft transaction +// and no ip commands. The ip half is roughly two thirds of the ~30 ms this costs today. +func TestApplyEgressPolicyTouchesOnlyNft(t *testing.T) { + cfg := NetfilterConfig{Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), Egress: v1.EgressDeny} + + var rr recordingRunner + if err := applyEgressPolicy(context.Background(), rr.run, cfg); err != nil { + t.Fatalf("applyEgressPolicy: %v", err) + } + if len(rr.calls) != 1 { + t.Fatalf("applyEgressPolicy ran %d commands, want exactly 1 (the nft transaction): %v", len(rr.calls), rr.calls) + } + if !isNftApply(rr.calls[0].argv) { + t.Errorf("the one command is not nft: %v", rr.calls[0].argv) + } + for _, c := range rr.calls { + if isIPCommand(c.argv) { + t.Errorf("applyEgressPolicy must not touch the link: %v", c.argv) + } + } +} + +// TestApplyEgressPolicyFailsClosed: a rejected policy must leave no tap behind, exactly +// as the composed call does, because a VM must never run half-filtered. +func TestApplyEgressPolicyFailsClosed(t *testing.T) { + cfg := NetfilterConfig{Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), Egress: v1.EgressDeny} + rr := &failingRunner{failAt: 0, err: errors.New("nft: syntax error")} + + if err := applyEgressPolicy(context.Background(), rr.run, cfg); err == nil { + t.Fatal("applyEgressPolicy accepted a rejected nft transaction") + } + if got := countTapDelete(rr.calls, "tap0"); got == 0 { + t.Error("a rejected policy left the tap behind; it must be torn down") + } +} + +// TestEnsureEgressLinkInstallsNoPolicy: the dormant half must not install a ruleset. A +// tap with no policy carries no traffic (nothing is loaded behind it and the shared +// forward table defaults to a drop); a tap with the WRONG policy would. +func TestEnsureEgressLinkInstallsNoPolicy(t *testing.T) { + cfg := NetfilterConfig{Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), Egress: v1.EgressDeny} + + var rr recordingRunner + if err := ensureEgressLink(context.Background(), rr.run, func() error { return nil }, cfg); err != nil { + t.Fatalf("ensureEgressLink: %v", err) + } + for _, c := range rr.calls { + if isNftApply(c.argv) { + t.Errorf("ensureEgressLink applied a policy: %v", c.argv) + } + } +} + +// TestApplyEgressFilterRejectsABadAllowlistBeforeTouchingTheLink: a malformed policy +// must cost nothing. It used to be rejected before any command ran; splitting the call +// must not start creating taps for a policy that can never be installed. +func TestApplyEgressFilterRejectsABadAllowlistBeforeTouchingTheLink(t *testing.T) { + cfg := NetfilterConfig{ + Tap: "tap0", GuestIP: net.ParseIP("10.200.0.2"), HostIP: net.ParseIP("10.200.0.1"), + Egress: v1.EgressDeny, AllowCIDRs: []string{"not-a-cidr"}, + } + var rr recordingRunner + if err := applyEgressFilter(context.Background(), rr.run, func() error { return nil }, cfg); err == nil { + t.Fatal("applyEgressFilter accepted a malformed CIDR allowlist") + } + if len(rr.calls) != 0 { + t.Errorf("a malformed allowlist ran %d commands, want none: %v", len(rr.calls), rr.calls) + } +} diff --git a/internal/husk/stub.go b/internal/husk/stub.go index 20ff203c..5589e63f 100644 --- a/internal/husk/stub.go +++ b/internal/husk/stub.go @@ -99,6 +99,11 @@ type vmInstance struct { prepareVerified bool rootfsClonePath string activeTap string + // preparedLinkTap names the tap this instance brought up while DORMANT + // (Options.PrepareEgressLink). When it matches the tap the claim resolves to, + // activate installs the tenant policy on it and skips the link setup. Cleared + // whenever the tap is torn down, so a retry re-ensures it. + preparedLinkTap string dnsProxy *dnsproxy.Server // childUFFDPlan carries a co-located live-cow fork child's LAZY UFFD import @@ -605,6 +610,23 @@ type Options struct { // Independent of LiveCowFork so the source can be armed (freezer live) without // yet skipping the disk mem. LiveCowChildImport bool + // PrepareEgressLink opts a multi-VM pod into bringing up its default VM's tap + // while the pod is DORMANT (no tenant attached), so a claim pays only the atomic + // nft transaction that installs the tenant's policy instead of also paying the + // tap create. Measured on prod, the link half is roughly two thirds of the ~30 ms + // the egress_filter stage costs on the warm-claim hot path. + // + // DEFAULTS false. Requires MultiVM and InPodGuestIP; a no-op otherwise, so a pod + // that does not opt in behaves byte-for-byte as before. The dormant tap carries a + // DEFAULT-DENY policy, and the claim REPLACES it in one nft transaction, so there + // is never a window in which a VM is unfiltered. + PrepareEgressLink bool + // InPodGuestIP / InPodGatewayIP are the fixed in-pod /30 the pod's default VM + // uses. They are the same values the controller sends in the activate request; + // PrepareEgressLink needs them BEFORE that request arrives, because the tap name + // derives from the guest IP. Config, never secrets. + InPodGuestIP string + InPodGatewayIP string // PrewarmChild opts into keeping ONE dormant, generic co-located child // Firecracker pre-prepared (booted, and snapshot-verified when a template // snapshot is configured) in a multi-VM pod, so a co-located fork that does @@ -728,6 +750,11 @@ type Stub struct { // restores from the disk fork snapshot until a child-side memfd-import // Firecracker patch ships) is always restorable and the fork never hangs. liveCowChildImport bool + + // prepareEgressLink / inPodGuestIP / inPodGatewayIP back Options.PrepareEgressLink. + prepareEgressLink bool + inPodGuestIP string + inPodGatewayIP string // prewarmChild keeps ONE dormant, generic co-located child Firecracker // pre-prepared (Options.PrewarmChild) so a co-located fork that needs no // fork-specific launch env activates it instead of paying the process boot on @@ -805,6 +832,9 @@ func New(cfg firecracker.VMConfig, opts Options) *Stub { multiVM: opts.MultiVM, liveCowFork: opts.LiveCowFork, liveCowChildImport: opts.LiveCowChildImport, + prepareEgressLink: opts.PrepareEgressLink, + inPodGuestIP: opts.InPodGuestIP, + inPodGatewayIP: opts.InPodGatewayIP, prewarmChild: opts.PrewarmChild, } // Multi-VM scaffold (#764), default off: allocate the per-fork instance map From 5e40a1de72a3b52ef2e17b40109b59e57d6fce4b Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Fri, 10 Jul 2026 17:37:53 +0200 Subject: [PATCH 6/7] fix(controller): enforce the multi-vm prerequisite for the prepare-time egress link Review found that PrepareEgressLink was documented as requiring MultiVM and then trusted to the caller. With --husk-prepare-egress-link set and --multi-vm-fork unset, the pool builder received MultiVM=false while the claim reconciler still received PrepareEgressLink=true, and the stub got flags it can never act on: only the multi-VM Prepare path reaches prepareInstance, so a single-VM stub never brings its tap up, and its Activate does not consult preparedLinkTap either. The operator asked for a latency optimization and would have got a pod shape that cannot deliver one, silently. Enforced at all three layers, because each is a boundary someone can reach on its own: - cmd/controller refuses the combination at startup rather than running degraded; - the pod builder emits the flags only when MultiVM is also set; - the stub states the !multiVM guard instead of relying on the fact that only the multi-VM Prepare currently reaches it. Also from review, the threat model: the husk egress row is marked IMPLEMENTED and KVM-VERIFIED end to end, and the prepare-time paragraph sat inside it, so an experimental default-off path inherited a verification it has not had. It now says plainly that the KVM verification covered the activation-time filter, and that the prepare-time path needs a production canary and its own KVM gate before it may be called verified. TestBuildHuskPodOmitsPrepareEgressLinkWithoutMultiVM pins the pod-builder gate. Verified: go test -race ./internal/husk/, go test ./internal/controller/ ./cmd/husk-stub/, and golangci-lint clean for both GOOS=darwin and GOOS=linux. Signed-off-by: Jannes Stubbemann --- cmd/controller/main.go | 9 +++++++++ .../plans/2026-07-10-prepare-time-restore.md | 4 +++- docs/threat-model.md | 2 +- internal/controller/huskpod.go | 6 +++++- internal/controller/huskpod_test.go | 20 +++++++++++++++++++ internal/husk/multivm.go | 5 ++++- 6 files changed, 42 insertions(+), 4 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index e91e05d1..964cae26 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -140,6 +140,15 @@ func main() { // (it has no KVM), so mock implies the raw-forkd path the dev overlay uses. // forkd-the-builder runs regardless in both modes (it builds the snapshots). enableHuskPods, rawForkd := resolveRunMode(enableHuskPods, enableRawForkd, mockMode) + // FAIL FAST on a flag combination that would silently do nothing. The husk stub only + // brings its tap up on the multi-VM Prepare path, so --husk-prepare-egress-link + // without --multi-vm-fork would hand the pool builder MultiVM=false while the claim + // reconciler believed the link was prepared. An operator who asked for a latency + // optimization must not be left with a pod shape that cannot deliver it. + if prepareEgressLink && !multiVMFork { + logger.Error(nil, "--husk-prepare-egress-link requires --multi-vm-fork: the husk stub only prepares its tap on the multi-VM path") + os.Exit(1) + } if rawForkd { logger.Info("run path: raw-forkd (fork per claim); husk pods disabled", "reason-mock", mockMode, "reason-flag", enableRawForkd) } else { diff --git a/docs/superpowers/plans/2026-07-10-prepare-time-restore.md b/docs/superpowers/plans/2026-07-10-prepare-time-restore.md index 266477a3..2e181ddf 100644 --- a/docs/superpowers/plans/2026-07-10-prepare-time-restore.md +++ b/docs/superpowers/plans/2026-07-10-prepare-time-restore.md @@ -120,7 +120,9 @@ Expected activate: roughly `nft` + `handshake` + `serve_api`, i.e. ~30 ms agains the link rather than installing a policy on a tap that no longer exists (`TestAFailedClaimPolicyRebuildsTheLinkOnRetry`). Worth roughly 20 ms, and a prerequisite for slice 2 because Firecracker requires the tap to exist at restore time. - Still to do: measure it on prod behind a one-pod canary, then a KVM gate. + Requires `--multi-vm-fork`; the controller refuses the combination at startup and the pod + builder refuses it again. Still to do: measure it on prod behind a one-pod canary, then a + KVM gate. Until both, the threat model does NOT call this path verified. 2. **Restore at Prepare.** Move `setLiveCowMemSource` / `LoadSnapshot` / `PatchDrive` / `Resume` / guest-ready into `prepareInstance`, behind the same flag, with the guards above. Activate becomes policy + handshake + serve_api. diff --git a/docs/threat-model.md b/docs/threat-model.md index 765dbebd..028d9f4a 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -1219,7 +1219,7 @@ VMMs run jailed under throwaway uids. | Husk pod default ServiceAccount token automount | **mitigated (shipped)** | The husk pod spec now sets `AutomountServiceAccountToken: false` (`internal/controller/huskpod.go` `buildHuskPod`), so the kubelet does NOT mount the namespace default ServiceAccount token into a husk pod. The stub speaks vsock + mTLS and never calls the Kubernetes API (verified: no client-go, no `InClusterConfig`, no SA token read in `cmd/husk-stub` or `internal/husk`), so the token was dead weight; a guest that escapes into the stub no longer inherits a free `system:authenticated` token. The opt-out applies to BOTH warm pods and fork-child pods, which share the builder, proven in `TestBuildHuskPodDisablesSATokenAutomount`. | | forkd gRPC fails OPEN without TLS flags | **mitigated (shipped)** | `grpcServerOptions` (`cmd/forkd/main.go`) now FAILS CLOSED: with no TLS flags it returns a fatal error naming the missing `--tls-cert/--tls-key/--tls-ca` flags and the opt-in, so forkd refuses to start unauthenticated. The legacy insecure-with-loud-warning behavior is reachable only behind an explicit `--allow-insecure-grpc` opt-in (default false) for local development. A partial TLS triple is still a configuration error. The shipped DaemonSet always sets TLS, so production is unaffected; this only stops a silent-insecure misconfig. Proven in `TestGRPCServerOptionsFailClosed` (refuses with no TLS and no opt-in; allowed with the opt-in; allowed with a full TLS triple; partial is an error). | | Host-side vsock read has no deadline | **mitigated (shipped)** | The host-side `vsock.Client` now applies a per-request read deadline in `send` (`internal/vsock/client.go`), defaulting to `DefaultRequestTimeout` (60s) and overridable via `SetRequestTimeout`. The CONNECT preamble reads in `Connect`/`DialStream`/`DialStreamUnix` are likewise bounded (then cleared so the long-lived streaming reads stay ctx/`conn.Close`-cancelled). A malicious or wedged guest that connects then stalls (or dribbles a partial line under the `MaxMessageBytes` cap) now causes the one-shot call to return a timeout error rather than hang the host caller goroutine, vsock fd, and (for the husk stub) stream slot indefinitely. Proven in `TestSendReadDeadlineUnblocksOnStall` (a fake agent that connects then never responds makes `Ping` return rather than hang). | -| Husk egress enforcement | **mitigated (IMPLEMENTED and KVM-VERIFIED end to end)** | See section 0 surface 5 and section 4. The husk default path enforces an in-pod nftables default-deny egress filter in the pod's own netns (CNI-independent, the guarantee), an unconditional cloud-metadata block (`169.254.169.254`, `169.254.0.0/16`, IPv6 `fd00:ec2::254`, before any allow), and the threaded per-template allowlist with a per-pod DNS proxy (failover upstream list, recommended `1.1.1.1:53,8.8.8.8:53`, not cluster DNS); a best-effort controller-emitted NetworkPolicy adds defense in depth (CNI-dependent, cannot express name-based allows). Costs one scoped `NET_ADMIN` capability (ADR 0006) plus, on name-egress pools only, a short-lived privileged `enable-ip-forward` init container that sets `net.ipv4.ip_forward=1` in the pod netns and exits (one-shot; no node prerequisite; the long-lived husk-stub container stays unprivileged). VERIFIED: the husk-network KVM cluster e2e (`test/cluster-e2e/husk-network-e2e.sh`, `cluster-husk-network-e2e`) PASSES on the Hetzner Talos KVM cluster; the claim reaches Ready and all three in-VM enforcement assertions are green inside a real restored VM (metadata-blocked exit 1, default-deny exit 1, allowlist-works `example.com` connect exit 0), verified twice (with and without a node sysctl allowance), proving NO node prerequisite. PREPARE-TIME LINK (`--husk-prepare-egress-link`, controller; `--prepare-egress-link`, stub; EXPERIMENTAL, DEFAULT OFF): a warm multi-VM husk pod may bring its default VM's tap up while it is DORMANT so a claim pays only the nft transaction, not the tap create. The dormant tap is created together with a DEFAULT-DENY policy in the same Prepare, and Prepare FAILS CLOSED (the pod never reaches dormant, so the pool never offers it) if either half fails, so a tap never exists without a policy. The claim REPLACES that policy with the tenant's in ONE atomic `nft -f -` transaction, so there is no window in which a VM is unfiltered, and a rejected policy tears the tap down and clears the prepared marker so the retry rebuilds the link rather than installing a policy on a tap that no longer exists. Nothing is loaded behind a dormant tap today (the snapshot restore stays on the activate path), and the pod netns is not shared with any tenant. Off, the filter path is byte-for-byte the activate-time one. | +| Husk egress enforcement | **mitigated (IMPLEMENTED and KVM-VERIFIED end to end)** | See section 0 surface 5 and section 4. The husk default path enforces an in-pod nftables default-deny egress filter in the pod's own netns (CNI-independent, the guarantee), an unconditional cloud-metadata block (`169.254.169.254`, `169.254.0.0/16`, IPv6 `fd00:ec2::254`, before any allow), and the threaded per-template allowlist with a per-pod DNS proxy (failover upstream list, recommended `1.1.1.1:53,8.8.8.8:53`, not cluster DNS); a best-effort controller-emitted NetworkPolicy adds defense in depth (CNI-dependent, cannot express name-based allows). Costs one scoped `NET_ADMIN` capability (ADR 0006) plus, on name-egress pools only, a short-lived privileged `enable-ip-forward` init container that sets `net.ipv4.ip_forward=1` in the pod netns and exits (one-shot; no node prerequisite; the long-lived husk-stub container stays unprivileged). VERIFIED: the husk-network KVM cluster e2e (`test/cluster-e2e/husk-network-e2e.sh`, `cluster-husk-network-e2e`) PASSES on the Hetzner Talos KVM cluster; the claim reaches Ready and all three in-VM enforcement assertions are green inside a real restored VM (metadata-blocked exit 1, default-deny exit 1, allowlist-works `example.com` connect exit 0), verified twice (with and without a node sysctl allowance), proving NO node prerequisite. PREPARE-TIME LINK (`--husk-prepare-egress-link`, controller; `--prepare-egress-link`, stub; EXPERIMENTAL, DEFAULT OFF, and **NOT covered by the KVM verification above**: that verification exercised the activation-time filter, and the prepare-time path still needs a production canary and its own KVM gate before it may be called verified; it also requires `--multi-vm-fork`, which the controller enforces at startup and the pod builder enforces again): a warm multi-VM husk pod may bring its default VM's tap up while it is DORMANT so a claim pays only the nft transaction, not the tap create. The dormant tap is created together with a DEFAULT-DENY policy in the same Prepare, and Prepare FAILS CLOSED (the pod never reaches dormant, so the pool never offers it) if either half fails, so a tap never exists without a policy. The claim REPLACES that policy with the tenant's in ONE atomic `nft -f -` transaction, so there is no window in which a VM is unfiltered, and a rejected policy tears the tap down and clears the prepared marker so the retry rebuilds the link rather than installing a policy on a tap that no longer exists. Nothing is loaded behind a dormant tap today (the snapshot restore stays on the activate path), and the pod netns is not shared with any tenant. Off, the filter path is byte-for-byte the activate-time one. | | agents.x-k8s.io facade admission surface (v1beta1 conformance, #357) | **mitigated (bounded translation, no host surface)** | The conformance facade (`internal/facade`, opt-in deployment) watches tenant-authored upstream `agents.x-k8s.io/v1beta1` and `extensions.agents.x-k8s.io/v1beta1` custom resources (Sandbox, SandboxTemplate, SandboxWarmPool, SandboxClaim) and TRANSLATES each into our `mitos.run/v1` SandboxPool/Sandbox objects in the SAME namespace, owner-referenced to the upstream CR for GC. It creates only namespace-scoped objects and grants no cross-namespace or host access; the run-path security boundary is unchanged (the bridged Sandbox is governed by the husk-pod and sandbox-API controls above, not by the facade). Tenant-controlled upstream fields are BOUNDED: `spec.operatingMode` (Running/Suspended) drives only release/re-activate of the bridged claim; `SandboxClaim.spec.warmPoolRef.name` is a name lookup of OUR pool of that name (no path or host input); `spec.podTemplate.spec.containers[0].env` (including `env.valueFrom` references) is mirrored through by reference and never logged; all other podTemplate fields and `volumeClaimTemplates` are UNMAPPED (no tenant-driven volume provisioning or host-shaped field reaches our engine via the facade, see docs/facade-conformance.md exceptions 1, 3, 4, 5). The v0.5.0 migration to v1beta1 REDUCED the input surface: it removed the v1alpha1 `warmpool` policy resolution branch (none/default/named), so a claim now references a warm pool by name with no policy logic to abuse. Test-infra note: v1beta1 is the storage version with a `conversion.strategy: Webhook` we do not run; the kind conformance job pins the live CRD conversion to None at install time and exercises only the storage version (the vendored CRD on disk is unchanged). Residual: the facade trusts cluster RBAC that admits the upstream CRs, so whoever may create an `agents.x-k8s.io` Sandbox in a namespace can drive a pool binding there; the same-namespace tenancy boundary (section 7a) applies, and the facade adds no privilege beyond it. | | Control-plane egress lockdown (chart profile, issue #704) | **mitigated (opt-in)** | The chart ships an opt-in default-deny EGRESS profile for the control plane (`networkPolicy.controlPlane.enabled`, `templates/control-plane-netpol.yaml`): a CiliumNetworkPolicy selecting the controller, console, and gateway that enumerates every legitimate flow (DNS via kube-dns with L7 visibility, the kube-apiserver/host/remote-node entities, intra-namespace) and denies the rest, bounding what a COMPROMISED control-plane component can reach (no lateral movement into other namespaces, no arbitrary internet egress for exfiltration). When `console.onboarding.smtp.host` is set, a SECOND policy scoped to the console ALONE additionally allows that SMTP host and port; it is DERIVED from the same values that configure the sender, so the network allow cannot drift from the sender config (the drift class that silently broke hosted signups: SMTP env present, egress absent, every verification send timed out). The SMTP policy never renders without the baseline profile (a lone egress policy would itself flip the console to default-deny), and the controller and gateway gain no mail-relay egress (a compromised controller cannot exfiltrate over SMTP). `extraEgress` passes deployment-specific rules through verbatim (e.g. an in-cluster OIDC issuer namespace); rules added there widen the boundary and are the operator's responsibility. Requires the Cilium CNI (CiliumNetworkPolicy CRD), consistent with the NetworkPolicy-enforcing-CNI requirement stated for the tenancy boundary. Default OFF: a plain install is unchanged. Rendering is asserted in `deploy/charts/charttest/netpol_test.go` (opt-in absent by default, baseline flows present, SMTP allow derived and console-scoped, all-or-nothing coupling). Residual: this is an egress control only (ingress lockdown for the control plane is separate work), and on a non-Cilium CNI the profile cannot be expressed (values accepted but objects unusable; the CRD requirement is documented). | diff --git a/internal/controller/huskpod.go b/internal/controller/huskpod.go index 05ce45ba..48466d71 100644 --- a/internal/controller/huskpod.go +++ b/internal/controller/huskpod.go @@ -568,7 +568,11 @@ func (r *SandboxPoolReconciler) buildHuskPod(pool *v1.SandboxPool, template *v1. if opts.PrewarmChild { args = append(args, "--prewarm-child") } - if opts.PrepareEgressLink { + // Gated on MultiVM as documented: the stub only brings the tap up on the multi-VM + // Prepare path, so emitting these to a single-VM stub would produce an unsupported + // pod shape whose flags do nothing. main rejects the flag combination outright; this + // is the second gate, at the boundary that actually builds the pod. + if opts.PrepareEgressLink && opts.MultiVM { // The stub needs the in-pod link BEFORE a claim arrives, because the tap name // derives from the guest IP. These are the same fixed values huskNotifyNetwork // sends in the activate request; they are config, not secrets. diff --git a/internal/controller/huskpod_test.go b/internal/controller/huskpod_test.go index e48e7661..1d884b4d 100644 --- a/internal/controller/huskpod_test.go +++ b/internal/controller/huskpod_test.go @@ -1536,3 +1536,23 @@ func TestBuildHuskPodThreadsPrepareEgressLink(t *testing.T) { } } } + +// TestBuildHuskPodOmitsPrepareEgressLinkWithoutMultiVM: the option is documented as +// requiring MultiVM, and the stub only brings its tap up on the multi-VM Prepare path. +// Emitting the flags to a single-VM stub would produce an unsupported pod shape whose +// arguments do nothing. cmd/controller rejects the combination outright; this pins the +// second gate, at the boundary that actually builds the pod. +func TestBuildHuskPodOmitsPrepareEgressLinkWithoutMultiVM(t *testing.T) { + r := &controller.SandboxPoolReconciler{} + pool := &v1.SandboxPool{ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "ns"}} + tmpl := &v1.PoolTemplateSpec{} + + pod := r.BuildHuskPodForTest(pool, tmpl, controller.HuskPodOptions{ + StubImage: "img", MultiVM: false, PrepareEgressLink: true, + }) + for _, unwanted := range []string{"--prepare-egress-link", "--in-pod-guest-ip", "--in-pod-gateway-ip"} { + if argsContain(pod.Spec.Containers[0].Args, unwanted) { + t.Errorf("husk args carry %s without --multi-vm: %v", unwanted, pod.Spec.Containers[0].Args) + } + } +} diff --git a/internal/husk/multivm.go b/internal/husk/multivm.go index df25fe9d..d0e46941 100644 --- a/internal/husk/multivm.go +++ b/internal/husk/multivm.go @@ -396,7 +396,10 @@ func (s *Stub) prepareInstanceOpt(ctx context.Context, id vmID, opts prepareOpts // the pod is dormant, when the stub opted in. A no-op otherwise, and never for a // secondary (co-located fork child) VM, whose tap is derived per fork at activate. func (s *Stub) prepareEgressLinkFor(ctx context.Context, id vmID, inst *vmInstance) error { - if !s.prepareEgressLink || id != defaultVMID || s.netRunner == nil || s.inPodGuestIP == "" { + // !s.multiVM is implied today (only the multi-VM Prepare reaches prepareInstance), + // but it is stated rather than relied upon: the single-VM Activate path does not + // consult preparedLinkTap, so a dormant tap it never reuses would be dead weight. + if !s.prepareEgressLink || !s.multiVM || id != defaultVMID || s.netRunner == nil || s.inPodGuestIP == "" { return nil } tap := netconf.DeriveTapName(s.inPodGuestIP) From c4e5c07a0cffb6f4ad3fe6a5f253dc191ee30524 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Fri, 10 Jul 2026 23:07:28 +0200 Subject: [PATCH 7/7] fix(controller): reject --husk-prepare-egress-link on the raw-forkd path Review: the startup guard checked --multi-vm-fork but not the resolved run path, so --husk-prepare-egress-link --multi-vm-fork --enable-raw-forkd (or --mock) was accepted. On the raw-forkd path resolveRunMode sets enableHuskPods false, so no husk pod is ever created and the requested optimization is silently inert. The check now runs against the RESOLVED enableHuskPods and requires it, extracted into validatePrepareFlags so it is unit-tested directly (five cases, including the raw-forkd combinations) rather than only reachable through os.Exit. Signed-off-by: Jannes Stubbemann --- cmd/controller/main.go | 20 ++++++++++++++++++-- cmd/controller/main_test.go | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 964cae26..3be74a62 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "flag" + "fmt" "os" "time" @@ -38,6 +39,21 @@ func init() { utilruntime.Must(v1.AddToScheme(scheme)) } +// validatePrepareFlags rejects prepare-time flag combinations that would silently do +// nothing, so an operator asking for a latency optimization is never left with a pod +// shape that cannot deliver it. It runs AFTER resolveRunMode, so enableHuskPods already +// reflects the resolved path (false on --enable-raw-forkd or --mock). +// +// --husk-prepare-egress-link needs BOTH --multi-vm-fork (the stub prepares its tap only +// on the multi-VM Prepare path) AND the husk-pods run path (the raw-forkd path never +// creates a husk pod, so there is no pod to prepare a tap in). +func validatePrepareFlags(prepareEgressLink, multiVMFork, enableHuskPods bool) error { + if prepareEgressLink && (!multiVMFork || !enableHuskPods) { + return fmt.Errorf("--husk-prepare-egress-link requires --multi-vm-fork AND the husk-pods run path (not --enable-raw-forkd or --mock): the tap is prepared only by a multi-VM husk pod, which the raw-forkd path never creates") + } + return nil +} + // resolveRunMode picks the single active controller run path from the flags. // husk pods is the pod-native default; --enable-raw-forkd selects the // fork-per-claim fallback, and --mock forces it too (the dev mock overlay has no @@ -145,8 +161,8 @@ func main() { // without --multi-vm-fork would hand the pool builder MultiVM=false while the claim // reconciler believed the link was prepared. An operator who asked for a latency // optimization must not be left with a pod shape that cannot deliver it. - if prepareEgressLink && !multiVMFork { - logger.Error(nil, "--husk-prepare-egress-link requires --multi-vm-fork: the husk stub only prepares its tap on the multi-VM path") + if err := validatePrepareFlags(prepareEgressLink, multiVMFork, enableHuskPods); err != nil { + logger.Error(err, "invalid prepare-time flags") os.Exit(1) } if rawForkd { diff --git a/cmd/controller/main_test.go b/cmd/controller/main_test.go index c600f71d..50c5bb43 100644 --- a/cmd/controller/main_test.go +++ b/cmd/controller/main_test.go @@ -33,3 +33,24 @@ func TestResolveRunMode(t *testing.T) { }) } } + +func TestValidatePrepareFlags(t *testing.T) { + cases := []struct { + name string + egress, multiVM, huskPods, wantErr bool + }{ + {"off is fine", false, false, false, false}, + {"egress with multivm and husk pods", true, true, true, false}, + {"egress without multivm", true, false, true, true}, + {"egress without husk pods (raw-forkd path)", true, true, false, true}, + {"egress on raw-forkd and single-vm", true, false, false, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validatePrepareFlags(tc.egress, tc.multiVM, tc.huskPods) + if (err != nil) != tc.wantErr { + t.Errorf("validatePrepareFlags(%v,%v,%v) err=%v, wantErr=%v", tc.egress, tc.multiVM, tc.huskPods, err, tc.wantErr) + } + }) + } +}