Skip to content

Commit 86fe71b

Browse files
laitingshengcv
andauthored
fix(onboard): suppress docker manifest/build noise during gateway setup (#3311)
## Summary During `nemoclaw onboard` step [2/8], the `docker manifest inspect` JSON output and the BuildKit build log (apt-get output, layer hashes, debconf warnings) are forwarded to the user's terminal alongside the curated progress lines we already log ("Pulling upstream cluster image…", "Building patched cluster image…"). This PR keeps the curated lines as the user-facing signal and routes the raw command output to internal-only handling. ## Related Issue Closes #3248 ## Changes - Add `suppressOutput?: boolean` to the `RunOpts` interface in [src/lib/cluster-image-patch.ts](src/lib/cluster-image-patch.ts) and plumb it through `defaultRun` so callers can opt into dropping captured stdio instead of forwarding it to the user. - Pass `suppressOutput: true` on the `docker manifest inspect` reachability probe. - Pass `--quiet` and `suppressOutput: true` on the `docker build` invocation so BuildKit no longer streams its full log to the user. - Add a unit test in [src/lib/cluster-image-patch.test.ts](src/lib/cluster-image-patch.test.ts) asserting both call sites carry `suppressOutput: true` and that build now includes `--quiet`. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Reduced output verbosity from Docker operations by suppressing output during manifest inspection and build steps. * **Tests** * Added test coverage for output suppression behavior in Docker operations. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3311) <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
1 parent cf3fd66 commit 86fe71b

2 files changed

Lines changed: 47 additions & 1 deletion

File tree

src/lib/cluster-image-patch.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,42 @@ describe("ensurePatchedClusterImage", () => {
331331
).toThrow(ClusterImagePatchError);
332332
});
333333

334+
it("suppresses raw docker manifest/build output so onboard does not leak internal noise (#3248)", () => {
335+
// The manifest probe dumps a multi-line JSON manifest list, and `docker build`
336+
// dumps its full BuildKit log (apt-get, layer hashes, debconf warnings) to
337+
// the user's terminal. Both must be suppressed so the install transcript
338+
// stays clean. The caller already prints its own "Pulling …"/"Building …"
339+
// progress lines.
340+
const fsImpl = createMockFs();
341+
const runCalls: { cmd: string[]; opts: { suppressOutput?: boolean } | undefined }[] = [];
342+
let upstreamInspectCount = 0;
343+
344+
ensurePatchedClusterImage({
345+
upstreamImage: UPSTREAM,
346+
runCaptureImpl: (cmd) => {
347+
if (inspectAt(cmd) === "upstream") {
348+
upstreamInspectCount += 1;
349+
return upstreamInspectCount === 1 ? "" : "sha256:9999aaaa";
350+
}
351+
return "";
352+
},
353+
runImpl: (cmd, opts) => {
354+
runCalls.push({ cmd: [...cmd], opts });
355+
return { status: 0 };
356+
},
357+
logger: () => {},
358+
fsImpl,
359+
tmpdirImpl: () => "/tmp",
360+
});
361+
362+
const manifestCall = runCalls.find((entry) => entry.cmd[1] === "manifest");
363+
expect(manifestCall?.opts).toMatchObject({ suppressOutput: true });
364+
365+
const buildCall = runCalls.find((entry) => entry.cmd[1] === "build");
366+
expect(buildCall?.cmd).toContain("--quiet");
367+
expect(buildCall?.opts).toMatchObject({ suppressOutput: true });
368+
});
369+
334370
it("throws ClusterImagePatchError on docker build failure", () => {
335371
let upstreamInspectCount = 0;
336372
expect(() =>

src/lib/cluster-image-patch.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,13 @@ export interface RunOpts {
4242
ignoreError?: boolean;
4343
/** Hard wall-clock timeout (ms). Child is killed on expiry. */
4444
timeoutMs?: number;
45+
/**
46+
* Drop captured stdout/stderr instead of forwarding them to the user's
47+
* terminal. Used for noisy commands (`docker manifest inspect`,
48+
* `docker build`) whose raw output is internal-only — the caller logs
49+
* its own progress lines.
50+
*/
51+
suppressOutput?: boolean;
4552
}
4653

4754
export interface EnsurePatchedClusterImageOpts {
@@ -260,6 +267,7 @@ export function ensurePatchedClusterImage(opts: EnsurePatchedClusterImageOpts):
260267
// and restricted-network hosts fail in seconds instead of minutes.
261268
const probeResult = runImpl(["docker", "manifest", "inspect", opts.upstreamImage], {
262269
ignoreError: true,
270+
suppressOutput: true,
263271
timeoutMs: inspectTimeoutMs,
264272
});
265273
if (probeResult.status !== 0) {
@@ -321,13 +329,14 @@ export function ensurePatchedClusterImage(opts: EnsurePatchedClusterImageOpts):
321329
[
322330
"docker",
323331
"build",
332+
"--quiet",
324333
"--build-arg",
325334
`UPSTREAM=${opts.upstreamImage}`,
326335
"-t",
327336
tag,
328337
tmpDir,
329338
],
330-
{ ignoreError: true, timeoutMs: buildTimeoutMs },
339+
{ ignoreError: true, suppressOutput: true, timeoutMs: buildTimeoutMs },
331340
);
332341

333342
if (buildResult.status !== 0) {
@@ -397,6 +406,7 @@ function defaultRunCapture(cmd: readonly string[], opts: RunOpts = {}): string {
397406
function defaultRun(cmd: readonly string[], opts: RunOpts = {}): { status: number | null } {
398407
const result = runner.run(cmd, {
399408
ignoreError: opts.ignoreError,
409+
suppressOutput: opts.suppressOutput,
400410
...(opts.timeoutMs !== undefined ? { timeout: opts.timeoutMs } : {}),
401411
});
402412
return { status: result.status ?? null };

0 commit comments

Comments
 (0)