You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
feat(serverless-init): wire MicroVM CloudService into main.go and register it (#53096)
### What does this PR do?
Completes the MicroVM integration started in #53092/#53093/#53094:
- **`cloudservice/service.go`** — `GetCloudServiceType` now returns
`&MicroVM{}` when `DD_AWS_MICROVM_IMAGE_ARN` is set. This check runs
before the amd64-only arch gate, since MicroVM also supports arm64.
- **`main.go`** — `setup()` builds a `LifecycleContext` from the
telemetry dependencies it already constructs (metric/trace/log flushers,
base log tags, base trace tags) and passes it through
`TracingContext.LifecycleCtx` so `MicroVM.Init` can start the lifecycle
server. This happens on both the normal path and the no-API-key path —
MicroVM's lifecycle handshake with the platform must complete regardless
of whether trace/metric collection is enabled, unlike other cloud
services, which are intentionally not initialized on the no-API-key path
(doing so would create trace spans with tracing disabled and unset span
tags, leading to a nil-map panic on shutdown for e.g. Cloud Run Jobs).
- **`lifecycle/server.go`** — `flushAll` no longer calls
`s.logsFlusher.Flush(...)` unconditionally. The `logsAgent` passed
through as `LifecycleContext.LogsFlusher` can be nil when the logs agent
fails to start (`SetupLogAgent`'s error is discarded in
`cmd/serverless-init/log/log.go`), which would panic during MicroVM's
`/suspend` and `/terminate` handshake. Guarded with a nil check,
matching the existing precedent in `main.go`'s `lastFlush` and
`flushAll`'s own `sampleDrainer` handling.
- **`main_test.go`** — `TestRun_LocalService_SidecarMode` started the
real `RunSidecar` signal-wait path but never sent a signal or cleaned
up, leaking a goroutine that would intercept SIGTERM for the rest of the
test binary's life. It now registers its own SIGTERM listener first (to
override the default terminate disposition before signaling), sends
itself a real SIGTERM, and asserts `RunSidecar` returns.
### Motivation
This is the atomic step that turns on the feature: before this PR,
`MicroVM` (PR 2, fully tested by PR 3/PR 4) exists in the binary but is
unreachable — `GetCloudServiceType` never selects it and `main.go` never
gives it a `LifecycleContext`, so `DD_AWS_MICROVM_IMAGE_ARN` has no
effect. Registration and `main.go` wiring are deliberately kept together
in one PR (rather than splitting further) because registering `MicroVM`
without also wiring `LifecycleContext` would leave `Init` a no-op and
`MicroVM.Run` dereferencing a nil child handle — this PR is the only
point in the stack where that combination is safe to land.
The `flushAll` nil-guard and the sidecar-test signal leak were both
flagged by Codex's automated review of this PR and are fixed here rather
than as follow-ups, since both are one-line-scoped and directly touch
code this PR introduces.
### Update (amended)
This branch was originally built on top of an older, unrestacked copy of
`forwarder.go`/`wire.go`/`server.go` before being reset onto the current
`07-02` base. The reset didn't refresh the working tree, so the original
commit silently reintroduced pre-fix versions of five things (each had a
test pinning it that was deleted alongside it), none of which were
mentioned in the original PR description above:
- **`forwarder.go`** — restores the `CheckRedirect` handler on the
forwarder's `http.Client` so a 3xx from the user app is mirrored to the
platform as-is instead of silently followed (which would replay a POST
hook as a body-dropped GET). See
`TestForwarder_PassThrough_DoesNotFollowRedirects`.
- **`wire.go`** — restores the `sidecarMode` early-return before parsing
`userAppPort` and the forward/ready/validate timeouts, so a stale or
colliding value inherited from an init-mode config produces a warning
instead of failing setup in sidecar mode. See the three
`TestSetupComponents_SidecarMode_*` tests.
- **`lifecycle/server.go`** — restores `writeTimeoutHeadroom`
(`heartbeatStopTimeout + mirrorResponseTimeout`) in the `WriteTimeout`
calculation; restores unconditional response-body buffering in
`handleWithForwarder` for both `flushParallel` (`/suspend`) and
`flushSequential` (`/terminate`) — the `/suspend` path had silently lost
its buffering, risking a partial mirrored body if the parallel flush
outlives `forwardTimeout`; and restores the nil-map guard before writing
`lambda_microvm_id` into a cloned `baseTraceTags` map. See
`TestHandleSuspend_WithForwarder_BodyBufferedBeforeFlush` and
`TestHandleRun_NilBaseTraceTags_DoesNotPanic`.
- **`server_test.go`** — restores `go.uber.org/atomic` in place of
`sync/atomic`, per this repo's `codereview_guideline.md` (this had
already been fixed once, per Codex review on an earlier PR in this
stack, before being reintroduced by the same reset).
Verified via `git merge-tree` that these restorations merge cleanly with
the PRs already stacked on top of this branch (#53104, #53230, #53231).
### Describe how you validated your changes
```
dda inv test --targets=./cmd/serverless-init/...
```
279 tests, 275 passed, 4 skipped (pre-existing platform skips). New
coverage:
- `TestFlushAllNilLogsFlusherDoesNotPanic` — verifies `flushAll`
tolerates a nil `logsFlusher` and still completes promptly (asserts it
returns via the normal "all workers done" path, not the `flushTimeout`
fallback, since a panic in `flushAll`'s spawned goroutine wouldn't be
caught by `assert.NotPanics` alone).
- `TestRun_LocalService_SidecarMode` (revised) — now drives `RunSidecar`
through a real SIGTERM instead of a fixed timeout, and asserts it
returns cleanly with no leaked signal handler.
- `main_test.go` also verifies the `logTagsBase`/`baseTraceTags` values
threaded into `LifecycleContext`, that `metrics.ServerlessMetricAgent`
methods are nil-safe without a started `Demux`, and that
`CloudService.Run` correctly dispatches to both init-container and
sidecar paths.
- `service_test.go` verifies `GetCloudServiceType` selects `MicroVM` and
that it takes priority over `CloudRunJobs` when env vars for both happen
to be set.
- `TestForwarder_PassThrough_DoesNotFollowRedirects`,
`TestSetupComponents_SidecarMode_*` (×3), and
`TestHandleSuspend_WithForwarder_BodyBufferedBeforeFlush`/`TestHandleRun_NilBaseTraceTags_DoesNotPanic`
— restored, per above.
Also ran the linter:
```
dda inv linter.go --targets=./cmd/serverless-init/...
```
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
0 commit comments