Skip to content

Commit 3747439

Browse files
fix(serverless-init): align MicroVM lifecycle hooks with AWS contract
Update MicroVM lifecycle hook handling to match AWS's documented behavior for image-build and runtime hooks. For /ready and /validate, keep the platform-owned retry loop in control by returning fast 503s when the user app is not reachable or when forwarding times out. Set the default /ready and /validate forward budgets to 30s each so build-time smoke tests are not capped by the runtime hook default, and include the reachability dial budget in the lifecycle server WriteTimeout. For /run, fail the hook when the platform request body cannot be read instead of parsing or forwarding a partial runHookPayload. Validated with dda inv test --targets=./cmd/serverless-init/lifecycle/... and dda inv test --targets=./cmd/serverless-init.
1 parent ff03f63 commit 3747439

4 files changed

Lines changed: 192 additions & 86 deletions

File tree

cmd/serverless-init/lifecycle/forwarder.go

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,25 @@ import (
2727
const defaultMaxResponseBodyBytes int64 = 1 << 20
2828

2929
const (
30+
// Per AWS, Run/Resume/Suspend/Terminate: 1 second
31+
// Ready/Validate: 30 seconds
3032
defaultForwardTimeout = 1 * time.Second
31-
defaultReadyTimeout = 60 * time.Second
32-
defaultValidateTimeout = 1 * time.Second
33+
defaultReadyTimeout = 30 * time.Second
34+
defaultValidateTimeout = 30 * time.Second
3335
)
3436

37+
// dialCheckTimeout bounds the single TCP dial attempt PassThroughWaiting uses
38+
// to check user-app reachability for /ready and /validate. Per the hook
39+
// contract, the platform — not the hook — owns the retry loop for these two
40+
// hooks, so the check must answer fast rather than block until the app is up.
41+
// A refused loopback connection fails in microseconds (the kernel sends RST
42+
// immediately), so this timeout only matters for a slow/hung connect (e.g.
43+
// host scheduling jitter on an oversubscribed MicroVM host). 200ms is
44+
// generous enough to absorb that jitter while staying well inside the "answer
45+
// fast" contract, and costs at most one extra platform retry (a 503) on the
46+
// rare call where it's hit.
47+
const dialCheckTimeout = 200 * time.Millisecond
48+
3549
// Forwarder POSTs lifecycle hooks to the user app. It is constructed only
3650
// when DD_AWS_MICROVM_USER_APP_PORT is set and we're in init-container
3751
// mode + MicroVM origin.
@@ -40,7 +54,7 @@ type Forwarder struct {
4054
client *http.Client // shared; no client-level Timeout (per-call deadlines via ctx)
4155
forwardTimeout time.Duration // default 1s, used for suspend/terminate/run/resume
4256
readyTimeout time.Duration // default 60s, used for /ready
43-
validateTimeout time.Duration // default 1s, used for /validate
57+
validateTimeout time.Duration // default 10s, used for /validate
4458
maxResponseBodyBytes int64 // default defaultMaxResponseBodyBytes; cap on user-app body surfaced to platform
4559
}
4660

@@ -78,61 +92,58 @@ func NewForwarder(port int, forwardTimeout, readyTimeout, validateTimeout time.D
7892
}
7993
}
8094

81-
// PassThroughWaiting waits for the user app to accept TCP connections bounded
82-
// by timeout, then forwards the request and mirrors the response. Used for:
95+
// PassThroughWaiting checks user-app reachability with a single fast TCP dial
96+
// (bounded by dialCheckTimeout), then forwards the request bounded by timeout
97+
// and mirrors the response. Used for:
8398
// - /ready ("I am booted and ready to be snapshotted"): the platform
84-
// retries on non-200, so the TCP wait absorbs the startup race.
99+
// retries on non-200 until its own configured timeout, so the hook
100+
// answers fast rather than blocking on the startup race.
85101
// - /validate ("I was resumed from a snapshot and everything is good"): the
86-
// TCP wait handles a crash-then-restart between resume and this
87-
// call.
102+
// same fast-answer contract applies to the crash-then-restart window
103+
// between resume and this call.
88104
//
89-
// Body is buffered before the TCP wait so the bytes survive waitForUserApp.
90-
// Deadline exceeded maps to 504. Body Close contract documented on PassThrough.
105+
// Per the /ready and /validate hook contract, only 200 and 503 are meaningful
106+
// responses — the platform retries on 503 until its own configured timeout,
107+
// while any other non-200 (including 504) fails the build. So unlike
108+
// PassThrough, an unreachable app or a deadline exceeded here always maps to
109+
// 503, never 504. Body Close contract documented on PassThrough.
91110
func (f *Forwarder) PassThroughWaiting(timeout time.Duration, path string, headers http.Header, body io.Reader) *http.Response {
92111
var bodyBytes []byte
93112
if body != nil {
94113
var err error
95-
// Read the full inbound body before the TCP wait. A read error is a
96-
// server-side failure (network, OS, memory) — not a client mistake —
97-
// so return 500 rather than 400. We still forward nothing to the user
98-
// app to avoid passing a partial body (which could make it answer
99-
// /validate "healthy" off incomplete data).
114+
// Read the full inbound body before the reachability check. A read
115+
// error is a server-side failure (network, OS, memory) — not a client
116+
// mistake — so return 500 rather than 400. We still forward nothing to
117+
// the user app to avoid passing a partial body (which could make it
118+
// answer /validate "healthy" off incomplete data).
100119
if bodyBytes, err = io.ReadAll(body); err != nil {
101120
return statusOnlyResponse(http.StatusInternalServerError)
102121
}
103122
}
104-
ctx, cancel := context.WithTimeout(context.Background(), timeout)
105-
if err := f.waitForUserApp(ctx); err != nil {
106-
cancel()
107-
return statusOnlyResponse(mapErrToStatus(err))
123+
if !f.reachable(dialCheckTimeout) {
124+
return statusOnlyResponse(http.StatusServiceUnavailable)
108125
}
126+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
109127
resp, err := f.do(ctx, path, headers, bytes.NewReader(bodyBytes))
110128
if err != nil {
111129
cancel()
112-
return statusOnlyResponse(mapErrToStatus(err))
130+
return statusOnlyResponse(http.StatusServiceUnavailable)
113131
}
114132
resp.Body = wrapResponseBody(resp.Body, f.maxResponseBodyBytes, cancel)
115133
return resp
116134
}
117135

118-
// waitForUserApp polls the user app's TCP port until a connection succeeds or
119-
// ctx is cancelled. Returns nil when the port is reachable, ctx.Err() when
120-
// the deadline is exceeded. Polls every 50ms.
121-
func (f *Forwarder) waitForUserApp(ctx context.Context) error {
136+
// reachable performs a single TCP dial attempt to the user app, bounded by
137+
// timeout. No polling/retrying: per the hook contract, the platform (not the
138+
// hook) owns the retry loop for /ready and /validate.
139+
func (f *Forwarder) reachable(timeout time.Duration) bool {
122140
addr := strings.TrimPrefix(f.target, "http://")
123-
dialer := &net.Dialer{}
124-
for {
125-
conn, err := dialer.DialContext(ctx, "tcp", addr)
126-
if err == nil {
127-
_ = conn.Close()
128-
return nil
129-
}
130-
select {
131-
case <-ctx.Done():
132-
return ctx.Err()
133-
case <-time.After(50 * time.Millisecond):
134-
}
141+
conn, err := net.DialTimeout("tcp", addr, timeout)
142+
if err != nil {
143+
return false
135144
}
145+
_ = conn.Close()
146+
return true
136147
}
137148

138149
// PassThrough forwards a per-MicroVM lifecycle hook (/run, /resume,

cmd/serverless-init/lifecycle/forwarder_test.go

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -134,15 +134,15 @@ func TestNewForwarder_DisableKeepAlives_OpensNewConnectionPerRequest(t *testing.
134134
"DisableKeepAlives must open a fresh TCP connection per request; with keep-alives enabled only 1 connection would be accepted")
135135
}
136136

137-
// NewForwarder defaults reflect the configured wire.go constants. /ready keeps
138-
// a 60s budget (matches the platform hook timeout); the remaining hooks default
139-
// to 1s. Changing these defaults is a deliberate behavior change — the test
140-
// failure is intentional.
137+
// NewForwarder defaults reflect the configured wire.go constants. /ready and
138+
// /validate each keep a 30s budget, and runtime hooks default to 1s. Changing
139+
// these defaults is a deliberate behavior change — the test failure is
140+
// intentional.
141141
func TestNewForwarder_Defaults(t *testing.T) {
142142
f := NewForwarder(8080, defaultForwardTimeout, defaultReadyTimeout, defaultValidateTimeout)
143143
assert.Equal(t, 1*time.Second, f.forwardTimeout, "forwardTimeout default must be 1s")
144-
assert.Equal(t, 60*time.Second, f.readyTimeout, "readyTimeout default must be 60s (matches platform /ready hook timeout)")
145-
assert.Equal(t, 1*time.Second, f.validateTimeout, "validateTimeout default must be 1s")
144+
assert.Equal(t, 30*time.Second, f.readyTimeout, "readyTimeout default must be 30s (matches platform /ready hook timeout)")
145+
assert.Equal(t, 30*time.Second, f.validateTimeout, "validateTimeout default must be 30s")
146146
assert.Equal(t, defaultMaxResponseBodyBytes, f.maxResponseBodyBytes, "maxResponseBodyBytes default must be 1 MiB")
147147
tr, ok := f.client.Transport.(*http.Transport)
148148
require.True(t, ok, "transport must be *http.Transport")
@@ -266,7 +266,8 @@ func TestForwarder_PassThroughWaiting_MirrorsStatusAndBody(t *testing.T) {
266266
// PassThroughWaiting must honor the timeout it receives. The caller (server.go)
267267
// passes readyTimeout or validateTimeout — PassThroughWaiting must not apply
268268
// any other field. This test uses a 50ms timeout against a 200ms upstream: the
269-
// context expires and the call returns 504.
269+
// context expires and the call returns 503 (per the /ready and /validate hook
270+
// contract, only 200 and 503 are meaningful — never 504).
270271
func TestForwarder_PassThroughWaiting_HonorsProvidedTimeout(t *testing.T) {
271272
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
272273
time.Sleep(200 * time.Millisecond)
@@ -278,8 +279,8 @@ func TestForwarder_PassThroughWaiting_HonorsProvidedTimeout(t *testing.T) {
278279
resp := f.PassThroughWaiting(50*time.Millisecond, "/ready", nil, nil)
279280
require.NotNil(t, resp)
280281
defer resp.Body.Close()
281-
assert.Equal(t, http.StatusGatewayTimeout, resp.StatusCode,
282-
"PassThroughWaiting must time out on the provided timeout")
282+
assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode,
283+
"PassThroughWaiting must time out on the provided timeout and return 503, never 504")
283284
}
284285

285286
// On the happy path the response body is wrapped by cancelOnCloseReader.
@@ -328,29 +329,33 @@ func TestWrapResponseBody_CapZero_DisablesCap(t *testing.T) {
328329
"cap=0 must read the full body — LimitReader must NOT be applied")
329330
}
330331

331-
// PassThroughWaiting retries TCP dials until the context expires when the port
332-
// is unbound, so the response is always 504 (deadline exceeded) — never 503.
333-
// This is distinct from PassThrough which returns 503 on the very first
334-
// connect-refused error. The behavioral difference is intentional: /ready and
335-
// /validate must wait for the user app to start, not fail-fast on a transient
336-
// dial error.
337-
func TestForwarder_PassThroughWaiting_UnboundPort_RetriesUntilTimeout504(t *testing.T) {
332+
// PassThroughWaiting makes a single reachability dial attempt, bounded by
333+
// dialCheckTimeout — it must not retry/poll, and must not wait out the
334+
// timeout parameter, before answering. Per the hook contract the platform
335+
// (not the hook) owns the retry loop for /ready and /validate, so an
336+
// unreachable app must return 503 fast, the same fail-fast contract
337+
// PassThrough already applies to connect-refused errors.
338+
func TestForwarder_PassThroughWaiting_UnboundPort_FailsFastWith503(t *testing.T) {
338339
f := &Forwarder{
339-
target: "http://127.0.0.1:1", // unbound — every dial attempt is refused
340+
target: "http://127.0.0.1:1", // unbound — dial attempt is refused
340341
client: &http.Client{},
341342
}
343+
start := time.Now()
342344
resp := f.PassThroughWaiting(150*time.Millisecond, "/ready", nil, nil)
345+
elapsed := time.Since(start)
343346
require.NotNil(t, resp)
344347
defer resp.Body.Close()
345-
assert.Equal(t, http.StatusGatewayTimeout, resp.StatusCode,
346-
"unbound port must retry until timeout (504), not immediately 503 like PassThrough")
348+
assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode,
349+
"unreachable app must return 503, not 504")
350+
assert.Less(t, elapsed, 100*time.Millisecond,
351+
"an unreachable app must fail the single dial attempt fast, not retry until the timeout parameter (150ms) or dialCheckTimeout (200ms) elapse")
347352
}
348353

349-
// passThroughWaiting buffers the request body before the TCP wait so it is
350-
// still available after waitForUserApp returns. Without buffering, the body
351-
// reader would be exhausted during the wait loop and f.do would send an empty
352-
// body to the user app. This pins the buffer-then-forward contract.
353-
func TestForwarder_PassThroughWaiting_ForwardsBodyAfterTCPWait(t *testing.T) {
354+
// passThroughWaiting buffers the request body before the reachability check
355+
// so it is still available once that check returns. Without buffering, the
356+
// body reader would be exhausted during the dial attempt and f.do would send
357+
// an empty body to the user app. This pins the buffer-then-forward contract.
358+
func TestForwarder_PassThroughWaiting_ForwardsBodyAfterReachabilityCheck(t *testing.T) {
354359
received := make(chan []byte, 1)
355360
srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
356361
b, _ := io.ReadAll(r.Body)
@@ -380,7 +385,8 @@ func (errReader) Read([]byte) (int, error) { return 0, io.ErrUnexpectedEOF }
380385

381386
// A failed read of the inbound body must NOT forward a partial body to the
382387
// user app. PassThroughWaiting returns 500 (server-side read failure) and
383-
// short-circuits before the TCP wait so no partial body reaches the user app.
388+
// short-circuits before the reachability check so no partial body reaches the
389+
// user app.
384390
func TestForwarder_PassThroughWaiting_BodyReadError_Returns500(t *testing.T) {
385391
var reached atomic.Int32
386392
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {

cmd/serverless-init/lifecycle/server.go

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@
3232
// - When the env var is set: the agent forwards each hook to
3333
// 127.0.0.1:<user-app-port> on the same path and mirrors the user app's
3434
// response (status, body, Content-Type) back to the platform. /ready and
35-
// /validate wait for TCP reachability before forwarding. For /run,
36-
// /resume, /suspend, and /terminate the agent's own work — metric
37-
// emission, /suspend and /terminate telemetry flush — runs in a goroutine
38-
// in parallel with the pass-through.
35+
// /validate check TCP reachability with a single fast dial before
36+
// forwarding, answering 503 immediately if the app isn't up yet rather
37+
// than blocking — the platform owns the retry loop for those two hooks.
38+
// For /run, /resume, /suspend, and /terminate the agent's own work —
39+
// metric emission, /suspend and /terminate telemetry flush — runs in a
40+
// goroutine in parallel with the pass-through.
3941
//
4042
// /terminate does NOT synthesize SIGTERM. The platform owns process
4143
// termination via OS signals delivered independently of this HTTP event.
@@ -233,16 +235,18 @@ func NewServer(
233235
// WriteTimeout must cover the full handler wall-clock for every path:
234236
// - No forwarder: flushTimeout (flush budget + write headroom)
235237
// - /run, /resume, /suspend, /terminate: forwardTimeout (default 1s)
236-
// - /ready: readyTimeout (default 60s, matching platform /ready timeout)
237-
// - /validate: validateTimeout (default 60s, matching platform /validate timeout)
238+
// - /ready: dialCheckTimeout + readyTimeout (default 60s)
239+
// - /validate: dialCheckTimeout + validateTimeout (default 10s)
238240
// Use the largest of all applicable budgets so the HTTP server does not
239241
// close the platform-facing connection before the handler writes the
240242
// mirrored response.
241243
maxTimeout := s.flushTimeout
242244
if s.fwd != nil {
243245
// /terminate uses flushSequential: flush runs after the forward, so its
244246
// wall-clock is forwardTimeout+flushTimeout, not max of the two.
245-
maxTimeout = max(maxTimeout, s.fwd.forwardTimeout+s.flushTimeout, s.fwd.readyTimeout, s.fwd.validateTimeout)
247+
readyBudget := dialCheckTimeout + s.fwd.readyTimeout
248+
validateBudget := dialCheckTimeout + s.fwd.validateTimeout
249+
maxTimeout = max(maxTimeout, s.fwd.forwardTimeout+s.flushTimeout, readyBudget, validateBudget)
246250
}
247251
writeTimeout := maxTimeout + writeTimeoutHeadroom
248252
s.httpServer = &http.Server{
@@ -342,7 +346,8 @@ func (s *Server) handler() http.Handler {
342346
//
343347
// Dispatcher:
344348
// - If a Forwarder is configured (env-var opt-in), pass-through to the user
345-
// app with TCP-wait: dial errors map to 503, deadline to 504.
349+
// app with a fast reachability check: dial errors and deadline exceeded
350+
// both map to 503.
346351
// - Otherwise, alive-check via ChildHandle: child alive → 200, anything
347352
// else (not yet started, already exited, or nil handle) → 503. The
348353
// pre-spawn race is absorbed by the platform's /ready retry behavior;
@@ -370,9 +375,10 @@ func (s *Server) passThroughReady(w http.ResponseWriter, r *http.Request) {
370375
// lifecycle of a production MicroVM.
371376
//
372377
// When a Forwarder is configured (DD_AWS_MICROVM_USER_APP_PORT set):
373-
// pass-through to the user app with TCP-wait, mirroring the response, so the
374-
// user app's own smoke test drives the build's validity decision. The TCP-wait
375-
// absorbs the window before the app is reachable on the test run. Without a
378+
// pass-through to the user app with a fast reachability check, mirroring the
379+
// response, so the user app's own smoke test drives the build's validity
380+
// decision. A 503 while the app isn't yet reachable on the test run relies on
381+
// the platform's own /validate retry to absorb the window. Without a
376382
// forwarder the agent returns 200 directly; the user app is not required to
377383
// implement /validate in that mode.
378384
func (s *Server) handleValidate(w http.ResponseWriter, r *http.Request) {
@@ -462,7 +468,8 @@ func (s *Server) flushAll(flushCtx context.Context) {
462468
//
463469
// Used for /run and /resume (noFlush), /suspend (flushParallel), and
464470
// /terminate (flushSequential). /ready and /validate use passThroughReady
465-
// and passThroughValidate directly (TCP-wait path, not this function).
471+
// and passThroughValidate directly (fast-reachability-check path, not this
472+
// function).
466473
//
467474
// flushParallel: flush runs concurrently with the forward; wall-clock is
468475
// max(forwardTimeout, flushTimeout)+ε. Known limitation: telemetry produced
@@ -556,8 +563,13 @@ func (s *Server) handleRun(w http.ResponseWriter, r *http.Request) {
556563
// original payload to the user app. Without this, the forwarder path would
557564
// consume r.Body before the decode, losing the instance_id tag on all
558565
// subsequent lifecycle metrics.
559-
bodyBytes, _ := io.ReadAll(r.Body)
566+
bodyBytes, err := io.ReadAll(r.Body)
560567
_ = r.Body.Close()
568+
if err != nil {
569+
log.Warnf("MicroVM lifecycle: could not read run body: %v", err)
570+
w.WriteHeader(http.StatusInternalServerError)
571+
return
572+
}
561573
var body runBody
562574
if err := json.Unmarshal(bodyBytes, &body); err != nil {
563575
log.Debugf("MicroVM lifecycle: could not parse run body: %v", err)

0 commit comments

Comments
 (0)