Skip to content

Commit e6aa81a

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. Increase the default /validate forward budget to 10s 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 fbdb435 commit e6aa81a

4 files changed

Lines changed: 187 additions & 83 deletions

File tree

cmd/serverless-init/lifecycle/forwarder.go

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,21 @@ const defaultMaxResponseBodyBytes int64 = 1 << 20
2727
const (
2828
defaultForwardTimeout = 1 * time.Second
2929
defaultReadyTimeout = 60 * time.Second
30-
defaultValidateTimeout = 1 * time.Second
30+
defaultValidateTimeout = 10 * time.Second
3131
)
3232

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

@@ -76,61 +88,58 @@ func NewForwarder(port int, forwardTimeout, readyTimeout, validateTimeout time.D
7688
}
7789
}
7890

79-
// PassThroughWaiting waits for the user app to accept TCP connections bounded
80-
// by timeout, then forwards the request and mirrors the response. Used for:
91+
// PassThroughWaiting checks user-app reachability with a single fast TCP dial
92+
// (bounded by dialCheckTimeout), then forwards the request bounded by timeout
93+
// and mirrors the response. Used for:
8194
// - /ready ("I am booted and ready to be snapshotted"): the platform
82-
// retries on non-200, so the TCP wait absorbs the startup race.
95+
// retries on non-200 until its own configured timeout, so the hook
96+
// answers fast rather than blocking on the startup race.
8397
// - /validate ("I was resumed from a snapshot and everything is good"): the
84-
// TCP wait handles a crash-then-restart between resume and this
85-
// call.
98+
// same fast-answer contract applies to the crash-then-restart window
99+
// between resume and this call.
86100
//
87-
// Body is buffered before the TCP wait so the bytes survive waitForUserApp.
88-
// Deadline exceeded maps to 504. Body Close contract documented on PassThrough.
101+
// Per the /ready and /validate hook contract, only 200 and 503 are meaningful
102+
// responses — the platform retries on 503 until its own configured timeout,
103+
// while any other non-200 (including 504) fails the build. So unlike
104+
// PassThrough, an unreachable app or a deadline exceeded here always maps to
105+
// 503, never 504. Body Close contract documented on PassThrough.
89106
func (f *Forwarder) PassThroughWaiting(timeout time.Duration, path string, headers http.Header, body io.Reader) *http.Response {
90107
var bodyBytes []byte
91108
if body != nil {
92109
var err error
93-
// Read the full inbound body before the TCP wait. A read error is a
94-
// server-side failure (network, OS, memory) — not a client mistake —
95-
// so return 500 rather than 400. We still forward nothing to the user
96-
// app to avoid passing a partial body (which could make it answer
97-
// /validate "healthy" off incomplete data).
110+
// Read the full inbound body before the reachability check. A read
111+
// error is a server-side failure (network, OS, memory) — not a client
112+
// mistake — so return 500 rather than 400. We still forward nothing to
113+
// the user app to avoid passing a partial body (which could make it
114+
// answer /validate "healthy" off incomplete data).
98115
if bodyBytes, err = io.ReadAll(body); err != nil {
99116
return statusOnlyResponse(http.StatusInternalServerError)
100117
}
101118
}
102-
ctx, cancel := context.WithTimeout(context.Background(), timeout)
103-
if err := f.waitForUserApp(ctx); err != nil {
104-
cancel()
105-
return statusOnlyResponse(mapErrToStatus(err))
119+
if !f.reachable(dialCheckTimeout) {
120+
return statusOnlyResponse(http.StatusServiceUnavailable)
106121
}
122+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
107123
resp, err := f.do(ctx, path, headers, bytes.NewReader(bodyBytes))
108124
if err != nil {
109125
cancel()
110-
return statusOnlyResponse(mapErrToStatus(err))
126+
return statusOnlyResponse(http.StatusServiceUnavailable)
111127
}
112128
resp.Body = wrapResponseBody(resp.Body, f.maxResponseBodyBytes, cancel)
113129
return resp
114130
}
115131

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

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

cmd/serverless-init/lifecycle/forwarder_test.go

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -135,14 +135,14 @@ func TestNewForwarder_DisableKeepAlives_OpensNewConnectionPerRequest(t *testing.
135135
}
136136

137137
// 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.
138+
// a 60s budget, /validate gets a 10s smoke-test budget, and runtime hooks
139+
// default to 1s. Changing these defaults is a deliberate behavior change — the
140+
// test failure is 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")
144144
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")
145+
assert.Equal(t, 10*time.Second, f.validateTimeout, "validateTimeout default must be 10s")
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")
@@ -239,7 +239,8 @@ func TestForwarder_PassThroughWaiting_MirrorsStatusAndBody(t *testing.T) {
239239
// PassThroughWaiting must honor the timeout it receives. The caller (server.go)
240240
// passes readyTimeout or validateTimeout — PassThroughWaiting must not apply
241241
// any other field. This test uses a 50ms timeout against a 200ms upstream: the
242-
// context expires and the call returns 504.
242+
// context expires and the call returns 503 (per the /ready and /validate hook
243+
// contract, only 200 and 503 are meaningful — never 504).
243244
func TestForwarder_PassThroughWaiting_HonorsProvidedTimeout(t *testing.T) {
244245
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
245246
time.Sleep(200 * time.Millisecond)
@@ -251,8 +252,8 @@ func TestForwarder_PassThroughWaiting_HonorsProvidedTimeout(t *testing.T) {
251252
resp := f.PassThroughWaiting(50*time.Millisecond, "/ready", nil, nil)
252253
require.NotNil(t, resp)
253254
defer resp.Body.Close()
254-
assert.Equal(t, http.StatusGatewayTimeout, resp.StatusCode,
255-
"PassThroughWaiting must time out on the provided timeout")
255+
assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode,
256+
"PassThroughWaiting must time out on the provided timeout and return 503, never 504")
256257
}
257258

258259
// On the happy path the response body is wrapped by cancelOnCloseReader.
@@ -301,29 +302,33 @@ func TestWrapResponseBody_CapZero_DisablesCap(t *testing.T) {
301302
"cap=0 must read the full body — LimitReader must NOT be applied")
302303
}
303304

304-
// PassThroughWaiting retries TCP dials until the context expires when the port
305-
// is unbound, so the response is always 504 (deadline exceeded) — never 503.
306-
// This is distinct from PassThrough which returns 503 on the very first
307-
// connect-refused error. The behavioral difference is intentional: /ready and
308-
// /validate must wait for the user app to start, not fail-fast on a transient
309-
// dial error.
310-
func TestForwarder_PassThroughWaiting_UnboundPort_RetriesUntilTimeout504(t *testing.T) {
305+
// PassThroughWaiting makes a single reachability dial attempt, bounded by
306+
// dialCheckTimeout — it must not retry/poll, and must not wait out the
307+
// timeout parameter, before answering. Per the hook contract the platform
308+
// (not the hook) owns the retry loop for /ready and /validate, so an
309+
// unreachable app must return 503 fast, the same fail-fast contract
310+
// PassThrough already applies to connect-refused errors.
311+
func TestForwarder_PassThroughWaiting_UnboundPort_FailsFastWith503(t *testing.T) {
311312
f := &Forwarder{
312-
target: "http://127.0.0.1:1", // unbound — every dial attempt is refused
313+
target: "http://127.0.0.1:1", // unbound — dial attempt is refused
313314
client: &http.Client{},
314315
}
316+
start := time.Now()
315317
resp := f.PassThroughWaiting(150*time.Millisecond, "/ready", nil, nil)
318+
elapsed := time.Since(start)
316319
require.NotNil(t, resp)
317320
defer resp.Body.Close()
318-
assert.Equal(t, http.StatusGatewayTimeout, resp.StatusCode,
319-
"unbound port must retry until timeout (504), not immediately 503 like PassThrough")
321+
assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode,
322+
"unreachable app must return 503, not 504")
323+
assert.Less(t, elapsed, 100*time.Millisecond,
324+
"an unreachable app must fail the single dial attempt fast, not retry until the timeout parameter (150ms) or dialCheckTimeout (200ms) elapse")
320325
}
321326

322-
// passThroughWaiting buffers the request body before the TCP wait so it is
323-
// still available after waitForUserApp returns. Without buffering, the body
324-
// reader would be exhausted during the wait loop and f.do would send an empty
325-
// body to the user app. This pins the buffer-then-forward contract.
326-
func TestForwarder_PassThroughWaiting_ForwardsBodyAfterTCPWait(t *testing.T) {
327+
// passThroughWaiting buffers the request body before the reachability check
328+
// so it is still available once that check returns. Without buffering, the
329+
// body reader would be exhausted during the dial attempt and f.do would send
330+
// an empty body to the user app. This pins the buffer-then-forward contract.
331+
func TestForwarder_PassThroughWaiting_ForwardsBodyAfterReachabilityCheck(t *testing.T) {
327332
received := make(chan []byte, 1)
328333
srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
329334
b, _ := io.ReadAll(r.Body)
@@ -353,7 +358,8 @@ func (errReader) Read([]byte) (int, error) { return 0, io.ErrUnexpectedEOF }
353358

354359
// A failed read of the inbound body must NOT forward a partial body to the
355360
// user app. PassThroughWaiting returns 500 (server-side read failure) and
356-
// short-circuits before the TCP wait so no partial body reaches the user app.
361+
// short-circuits before the reachability check so no partial body reaches the
362+
// user app.
357363
func TestForwarder_PassThroughWaiting_BodyReadError_Returns500(t *testing.T) {
358364
var reached atomic.Int32
359365
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.
@@ -225,16 +227,18 @@ func NewServer(
225227
// WriteTimeout must cover the full handler wall-clock for every path:
226228
// - No forwarder: flushTimeout (flush budget + write headroom)
227229
// - /run, /resume, /suspend, /terminate: forwardTimeout (default 1s)
228-
// - /ready: readyTimeout (default 60s, matching platform /ready timeout)
229-
// - /validate: validateTimeout (default 60s, matching platform /validate timeout)
230+
// - /ready: dialCheckTimeout + readyTimeout (default 60s)
231+
// - /validate: dialCheckTimeout + validateTimeout (default 10s)
230232
// Use the largest of all applicable budgets so the HTTP server does not
231233
// close the platform-facing connection before the handler writes the
232234
// mirrored response.
233235
maxTimeout := s.flushTimeout
234236
if s.fwd != nil {
235237
// /terminate uses flushSequential: flush runs after the forward, so its
236238
// wall-clock is forwardTimeout+flushTimeout, not max of the two.
237-
maxTimeout = max(maxTimeout, s.fwd.forwardTimeout+s.flushTimeout, s.fwd.readyTimeout, s.fwd.validateTimeout)
239+
readyBudget := dialCheckTimeout + s.fwd.readyTimeout
240+
validateBudget := dialCheckTimeout + s.fwd.validateTimeout
241+
maxTimeout = max(maxTimeout, s.fwd.forwardTimeout+s.flushTimeout, readyBudget, validateBudget)
238242
}
239243
writeTimeout := maxTimeout + writeTimeoutHeadroom
240244
s.httpServer = &http.Server{
@@ -334,7 +338,8 @@ func (s *Server) handler() http.Handler {
334338
//
335339
// Dispatcher:
336340
// - If a Forwarder is configured (env-var opt-in), pass-through to the user
337-
// app with TCP-wait: dial errors map to 503, deadline to 504.
341+
// app with a fast reachability check: dial errors and deadline exceeded
342+
// both map to 503.
338343
// - Otherwise, alive-check via ChildHandle: child alive → 200, anything
339344
// else (not yet started, already exited, or nil handle) → 503. The
340345
// pre-spawn race is absorbed by the platform's /ready retry behavior;
@@ -362,9 +367,10 @@ func (s *Server) passThroughReady(w http.ResponseWriter, r *http.Request) {
362367
// lifecycle of a production MicroVM.
363368
//
364369
// When a Forwarder is configured (DD_AWS_MICROVM_USER_APP_PORT set):
365-
// pass-through to the user app with TCP-wait, mirroring the response, so the
366-
// user app's own smoke test drives the build's validity decision. The TCP-wait
367-
// absorbs the window before the app is reachable on the test run. Without a
370+
// pass-through to the user app with a fast reachability check, mirroring the
371+
// response, so the user app's own smoke test drives the build's validity
372+
// decision. A 503 while the app isn't yet reachable on the test run relies on
373+
// the platform's own /validate retry to absorb the window. Without a
368374
// forwarder the agent returns 200 directly; the user app is not required to
369375
// implement /validate in that mode.
370376
func (s *Server) handleValidate(w http.ResponseWriter, r *http.Request) {
@@ -454,7 +460,8 @@ func (s *Server) flushAll(flushCtx context.Context) {
454460
//
455461
// Used for /run and /resume (noFlush), /suspend (flushParallel), and
456462
// /terminate (flushSequential). /ready and /validate use passThroughReady
457-
// and passThroughValidate directly (TCP-wait path, not this function).
463+
// and passThroughValidate directly (fast-reachability-check path, not this
464+
// function).
458465
//
459466
// flushParallel: flush runs concurrently with the forward; wall-clock is
460467
// max(forwardTimeout, flushTimeout)+ε. Known limitation: telemetry produced
@@ -548,8 +555,13 @@ func (s *Server) handleRun(w http.ResponseWriter, r *http.Request) {
548555
// original payload to the user app. Without this, the forwarder path would
549556
// consume r.Body before the decode, losing the instance_id tag on all
550557
// subsequent lifecycle metrics.
551-
bodyBytes, _ := io.ReadAll(r.Body)
558+
bodyBytes, err := io.ReadAll(r.Body)
552559
_ = r.Body.Close()
560+
if err != nil {
561+
log.Warnf("MicroVM lifecycle: could not read run body: %v", err)
562+
w.WriteHeader(http.StatusInternalServerError)
563+
return
564+
}
553565
var body runBody
554566
if err := json.Unmarshal(bodyBytes, &body); err != nil {
555567
log.Debugf("MicroVM lifecycle: could not parse run body: %v", err)

0 commit comments

Comments
 (0)