mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-19 05:32:19 +03:00
fix(ops): judge the canary install by the SHA on disk, not npm's exit code (#10699)
`npm install -g <tarball>` on the .17 gateway writes the whole package and then fails renaming the old tree into its staging directory (ENOTEMPTY, exit 217). The canary read that non-zero exit as "install failed", aborted before the restart, and discarded npm's stderr through execFileSync throwing — so on 2026-08-18 the deploy stopped half-done twice, each time leaving new files on disk under an old running process, with no clue in the log. The exit code is not trustworthy in either direction: the 2026-08-14 outage installed a package built from the wrong branch and exited 0. classifyInstallOutcome() therefore decides on the BUILD_SHA read back from the installed package, and fails closed when it is absent or does not match — a zero exit with the wrong artifact is still a failure. npm reuses the same staging directory name, so the orphan blocks the next install with the same error; orphanStagingDirFromStderr() surfaces the exact path. It is not removed automatically — that is an rm -rf under /usr/lib, not something a deploy script should decide on its own. Refs #10429 Co-authored-by: Xiangzhe <bakryun0718@proton.me>
This commit is contained in:
committed by
GitHub
parent
d7368e243d
commit
86fc1aade2
@@ -28,11 +28,16 @@
|
||||
* OMNIROUTE_SMOKE_API_KEY sent as Authorization: Bearer when the gateway requires auth
|
||||
*/
|
||||
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { execFileSync, spawnSync } from "node:child_process";
|
||||
import path from "node:path";
|
||||
import process from "node:process";
|
||||
|
||||
import { buildRemoteSteps, evaluateSmoke, planCanaryDeploy } from "./deployCanary.ts";
|
||||
import {
|
||||
buildRemoteSteps,
|
||||
classifyInstallOutcome,
|
||||
evaluateSmoke,
|
||||
planCanaryDeploy,
|
||||
} from "./deployCanary.ts";
|
||||
import { makeGitAncestryProbe, readBuildSha } from "../build/buildProvenance.ts";
|
||||
|
||||
function parseArgs(argv) {
|
||||
@@ -61,6 +66,22 @@ function run(step) {
|
||||
return execFileSync(command, rest, { encoding: "utf8" }).trim();
|
||||
}
|
||||
|
||||
/**
|
||||
* Like `run`, but never throws: returns the exit code plus both streams. Used for the
|
||||
* install, whose exit code does not decide the outcome (see classifyInstallOutcome) and
|
||||
* whose stderr must reach the log — it used to be swallowed by execFileSync throwing.
|
||||
*/
|
||||
function runCapturing(step) {
|
||||
console.log(`\n▶ ${step.name}: ${step.description}`);
|
||||
const [command, ...rest] = step.argv;
|
||||
const result = spawnSync(command, rest, { encoding: "utf8" });
|
||||
return {
|
||||
exitCode: result.status ?? 1,
|
||||
stdout: (result.stdout || "").trim(),
|
||||
stderr: (result.stderr || "").trim(),
|
||||
};
|
||||
}
|
||||
|
||||
async function probeHealth(baseUrl) {
|
||||
try {
|
||||
const response = await fetch(new URL("/api/monitoring/health", baseUrl), {
|
||||
@@ -110,8 +131,9 @@ if (args.models.length === 0) {
|
||||
}
|
||||
|
||||
const repoRoot = process.cwd();
|
||||
const localBuildSha = readBuildSha(repoRoot);
|
||||
const plan = planCanaryDeploy({
|
||||
buildSha: readBuildSha(repoRoot),
|
||||
buildSha: localBuildSha,
|
||||
isAncestorOfRelease: makeGitAncestryProbe(
|
||||
process.env.OMNIROUTE_RELEASE_REF || "origin/main",
|
||||
repoRoot
|
||||
@@ -147,7 +169,23 @@ try {
|
||||
console.log(`\n▶ upload: ${args.tarball} → ${args.host}:${remoteTarball}`);
|
||||
execFileSync("scp", [args.tarball, `${args.host}:${remoteTarball}`], { stdio: "inherit" });
|
||||
|
||||
run(install);
|
||||
const installResult = runCapturing(install);
|
||||
const outcome = classifyInstallOutcome({
|
||||
exitCode: installResult.exitCode,
|
||||
stderr: installResult.stderr,
|
||||
installedSha: run(verify),
|
||||
expectedSha: localBuildSha,
|
||||
});
|
||||
if (!outcome.installed) {
|
||||
if (installResult.stderr) console.error(installResult.stderr);
|
||||
fail(`install did not land: ${outcome.reason}`);
|
||||
}
|
||||
if (outcome.kind === "installed-with-cleanup-failure") {
|
||||
console.warn(` ⚠️ ${outcome.reason}`);
|
||||
} else {
|
||||
console.log(` ${outcome.reason}`);
|
||||
}
|
||||
|
||||
run(restart);
|
||||
|
||||
const installedSha = run(verify);
|
||||
|
||||
@@ -152,3 +152,80 @@ export function buildRemoteSteps(input: RemoteStepsInput): RemoteStep[] {
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
export type InstallOutcomeInput = {
|
||||
exitCode: number;
|
||||
stderr: string;
|
||||
/** BUILD_SHA read back from the installed package AFTER the install ran. */
|
||||
installedSha: string | null | undefined;
|
||||
/** BUILD_SHA of the artifact being shipped. */
|
||||
expectedSha: string;
|
||||
};
|
||||
|
||||
export type InstallOutcome = {
|
||||
installed: boolean;
|
||||
kind: "installed" | "installed-with-cleanup-failure" | "failed";
|
||||
reason: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Decide whether the global install actually landed.
|
||||
*
|
||||
* The exit code alone is not trustworthy in either direction:
|
||||
*
|
||||
* - `npm install -g` on the .17 gateway writes the whole package and *then* fails renaming
|
||||
* the old tree into its staging directory (`ENOTEMPTY`, exit 217). Treating that as a
|
||||
* failure aborts the deploy after the artifact is already on disk — which happened twice
|
||||
* on 2026-08-18, each time leaving the host with new files and an old running process.
|
||||
* - The 2026-08-14 outage went the other way: the install exited 0 while shipping a package
|
||||
* built from the wrong branch.
|
||||
*
|
||||
* So the SHA on disk decides, and it must match exactly. An absent or unreadable SHA fails
|
||||
* closed — an artifact that cannot be identified is never attested (same rule as the
|
||||
* provenance gate).
|
||||
*/
|
||||
export function classifyInstallOutcome(input: InstallOutcomeInput): InstallOutcome {
|
||||
const { exitCode, stderr, installedSha, expectedSha } = input;
|
||||
const onDisk = (installedSha ?? "").trim();
|
||||
|
||||
if (!onDisk) {
|
||||
return {
|
||||
installed: false,
|
||||
kind: "failed",
|
||||
reason: "no BUILD_SHA could be read from the installed package after the install",
|
||||
};
|
||||
}
|
||||
if (onDisk !== expectedSha) {
|
||||
return {
|
||||
installed: false,
|
||||
kind: "failed",
|
||||
reason: `installed BUILD_SHA is ${onDisk}, expected ${expectedSha}`,
|
||||
};
|
||||
}
|
||||
if (exitCode === 0) {
|
||||
return { installed: true, kind: "installed", reason: `installed ${onDisk}` };
|
||||
}
|
||||
|
||||
const staging = orphanStagingDirFromStderr(stderr);
|
||||
const enotempty = /ENOTEMPTY/.test(stderr);
|
||||
return {
|
||||
installed: true,
|
||||
kind: "installed-with-cleanup-failure",
|
||||
reason:
|
||||
`npm exited ${exitCode} but ${onDisk} is on disk — the package installed and npm failed ` +
|
||||
`during its own cleanup${enotempty ? " (ENOTEMPTY on the staging rename)" : ""}` +
|
||||
(staging ? `; orphaned staging dir left behind: ${staging}` : ""),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The staging directory npm failed to rename into, if it named one. It blocks the NEXT
|
||||
* install with the same error (npm reuses the name), so the operator has to clear it —
|
||||
* surfacing the exact path is the whole point. Deliberately not removed automatically:
|
||||
* this is a path under /usr/lib and a blind `rm -rf` there is not something a deploy
|
||||
* script should do on its own.
|
||||
*/
|
||||
export function orphanStagingDirFromStderr(stderr: string): string | null {
|
||||
const match = /npm error dest (\/\S*\/\.\S+)/.exec(stderr || "");
|
||||
return match ? match[1] : null;
|
||||
}
|
||||
|
||||
106
tests/unit/canary-install-outcome-10429.test.ts
Normal file
106
tests/unit/canary-install-outcome-10429.test.ts
Normal file
@@ -0,0 +1,106 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { test } from "node:test";
|
||||
import {
|
||||
classifyInstallOutcome,
|
||||
orphanStagingDirFromStderr,
|
||||
} from "../../scripts/ops/deployCanary.ts";
|
||||
|
||||
/**
|
||||
* `npm install -g <tarball>` on the .17 gateway finishes writing the package and then fails
|
||||
* while renaming the old tree into its staging directory:
|
||||
*
|
||||
* npm error code ENOTEMPTY
|
||||
* npm error syscall rename
|
||||
* npm error path /usr/lib/node_modules/omniroute
|
||||
* npm error dest /usr/lib/node_modules/.omniroute-h797OOZa
|
||||
*
|
||||
* Exit status is 217, but `dist/BUILD_SHA`, the package version and every dependency are the
|
||||
* new ones. The canary treated the non-zero exit as "install failed", aborted before the
|
||||
* restart, and discarded npm's stderr — so the deploy stopped half-done twice (2026-08-18)
|
||||
* with no clue in the log about why.
|
||||
*
|
||||
* The exit code is not the source of truth here; the SHA on disk is. These cases pin that,
|
||||
* including the inverse trap: a ZERO exit that installed the wrong artifact must still fail.
|
||||
*/
|
||||
|
||||
const ENOTEMPTY_STDERR = [
|
||||
"npm warn deprecated boolean@3.2.0: Package no longer supported.",
|
||||
"npm error code ENOTEMPTY",
|
||||
"npm error syscall rename",
|
||||
"npm error path /usr/lib/node_modules/omniroute",
|
||||
"npm error dest /usr/lib/node_modules/.omniroute-h797OOZa",
|
||||
"npm error ENOTEMPTY: directory not empty, rename '/usr/lib/node_modules/omniroute' -> " +
|
||||
"'/usr/lib/node_modules/.omniroute-h797OOZa'",
|
||||
].join("\n");
|
||||
|
||||
test("non-zero exit with the expected SHA on disk is a cleanup failure, not an install failure", () => {
|
||||
const outcome = classifyInstallOutcome({
|
||||
exitCode: 217,
|
||||
stderr: ENOTEMPTY_STDERR,
|
||||
installedSha: "22b89a273b",
|
||||
expectedSha: "22b89a273b",
|
||||
});
|
||||
assert.equal(outcome.installed, true, "the artifact is on disk — the deploy must continue");
|
||||
assert.equal(outcome.kind, "installed-with-cleanup-failure");
|
||||
assert.match(outcome.reason, /ENOTEMPTY|cleanup/i);
|
||||
});
|
||||
|
||||
test("a clean install is reported as such", () => {
|
||||
const outcome = classifyInstallOutcome({
|
||||
exitCode: 0,
|
||||
stderr: "",
|
||||
installedSha: "22b89a273b",
|
||||
expectedSha: "22b89a273b",
|
||||
});
|
||||
assert.equal(outcome.installed, true);
|
||||
assert.equal(outcome.kind, "installed");
|
||||
});
|
||||
|
||||
test("non-zero exit with a stale SHA is a real failure", () => {
|
||||
const outcome = classifyInstallOutcome({
|
||||
exitCode: 217,
|
||||
stderr: ENOTEMPTY_STDERR,
|
||||
installedSha: "e05ac345da",
|
||||
expectedSha: "22b89a273b",
|
||||
});
|
||||
assert.equal(outcome.installed, false);
|
||||
assert.equal(outcome.kind, "failed");
|
||||
});
|
||||
|
||||
test("a ZERO exit that left the wrong artifact still fails", () => {
|
||||
// The 2026-08-14 outage shipped a package built from the wrong branch. An install that
|
||||
// "succeeds" while the SHA does not match must never be waved through.
|
||||
const outcome = classifyInstallOutcome({
|
||||
exitCode: 0,
|
||||
stderr: "",
|
||||
installedSha: "178febc50f",
|
||||
expectedSha: "22b89a273b",
|
||||
});
|
||||
assert.equal(outcome.installed, false);
|
||||
assert.equal(outcome.kind, "failed");
|
||||
});
|
||||
|
||||
test("an unreadable SHA fails closed", () => {
|
||||
for (const installedSha of ["", null, undefined]) {
|
||||
const outcome = classifyInstallOutcome({
|
||||
exitCode: 0,
|
||||
stderr: "",
|
||||
installedSha: installedSha as string | null,
|
||||
expectedSha: "22b89a273b",
|
||||
});
|
||||
assert.equal(outcome.installed, false, `installedSha=${JSON.stringify(installedSha)}`);
|
||||
assert.equal(outcome.kind, "failed");
|
||||
}
|
||||
});
|
||||
|
||||
test("the orphaned staging directory is extracted so the operator can clear it", () => {
|
||||
assert.equal(
|
||||
orphanStagingDirFromStderr(ENOTEMPTY_STDERR),
|
||||
"/usr/lib/node_modules/.omniroute-h797OOZa"
|
||||
);
|
||||
});
|
||||
|
||||
test("no staging directory is invented when npm did not report one", () => {
|
||||
assert.equal(orphanStagingDirFromStderr(""), null);
|
||||
assert.equal(orphanStagingDirFromStderr("npm error code EACCES"), null);
|
||||
});
|
||||
Reference in New Issue
Block a user