diff --git a/src/cli/observe.ts b/src/cli/observe.ts index 58b9973..abbf435 100644 --- a/src/cli/observe.ts +++ b/src/cli/observe.ts @@ -115,14 +115,14 @@ async function connectHost( ssh.disconnect(); const message = cause instanceof Error ? cause.message : String(cause); return { - observer: unreachableObserver(host.name, `Failed to connect: ${message}`), + observer: unreachableObserver(host.name, `Failed to connect: ${message}`, host.apps), apps: host.apps, accessoryNames: host.accessoryNames, }; } } -function unreachableObserver(serverName: string, message: string): ObserveTarget['observer'] { +function unreachableObserver(serverName: string, message: string, planned: ShipnodeApp[]): ObserveTarget['observer'] { return { serverName, async collect(): Promise { @@ -133,6 +133,7 @@ function unreachableObserver(serverName: string, message: string): ObserveTarget deployLock: null, apps: [], error: message, + plannedApps: planned.map((app) => ({ app: app.name, appType: app.appType })), }; }, }; diff --git a/src/domain/observe/collector.ts b/src/domain/observe/collector.ts index eef24ed..7484973 100644 --- a/src/domain/observe/collector.ts +++ b/src/domain/observe/collector.ts @@ -59,12 +59,12 @@ export class MetricsCollector { // An unreachable host is a state the fleet view has to render, not an // exception to unwind — the same stance `rollFleet` takes on a replica // that fails mid-roll. - return this.unreachable(timestamp, cause instanceof Error ? cause.message : String(cause)); + return this.unreachable(timestamp, cause instanceof Error ? cause.message : String(cause), request.apps); } const sections = splitSections(stdout); if (sections.size === 0) { - return this.unreachable(timestamp, 'Observe poll returned no data'); + return this.unreachable(timestamp, 'Observe poll returned no data', request.apps); } const accessoriesSection = sections.get('accessories'); @@ -78,7 +78,7 @@ export class MetricsCollector { }; } - private unreachable(timestamp: string, error: string): ServerSnapshot { + private unreachable(timestamp: string, error: string, planned: ShipnodeApp[]): ServerSnapshot { return { server: this.server, timestamp, @@ -86,6 +86,7 @@ export class MetricsCollector { deployLock: null, apps: [], error, + plannedApps: planned.map((app) => ({ app: app.name, appType: app.appType })), }; } } diff --git a/src/domain/observe/pivot.ts b/src/domain/observe/pivot.ts index a13ba7a..b92e464 100644 --- a/src/domain/observe/pivot.ts +++ b/src/domain/observe/pivot.ts @@ -26,11 +26,32 @@ export function pivotByApp(snapshots: ServerSnapshot[]): FleetView[] { } } - // A server whose poll failed reports no apps at all, so it cannot say which - // apps it was meant to be running. It is attributed to every app another - // replica proves exists — enough to stop a partial observation from reading - // as a converged fleet, without inventing apps for a host we never reached. - const unreachable = snapshots.flatMap((server) => (server.error === undefined ? [] : [server.server])); + // A server whose poll failed reports no apps at all. When the caller recorded + // which apps it was planned to run, the failure is blamed on exactly those + // apps — and an app whose every server failed still gets a view, so it cannot + // vanish. Without that record (an older snapshot shape) the server is blamed + // on every app another replica proves exists: enough to stop a partial + // observation from reading as a converged fleet, without inventing apps. + const unreachableByApp = new Map(); + const plannedType = new Map(); + const unattributed: string[] = []; + for (const server of snapshots) { + if (server.error === undefined) continue; + if (server.plannedApps === undefined) { + unattributed.push(server.server); + continue; + } + for (const planned of server.plannedApps) { + const servers = unreachableByApp.get(planned.app); + if (servers === undefined) unreachableByApp.set(planned.app, [server.server]); + else servers.push(server.server); + plannedType.set(planned.app, planned.appType); + if (!byApp.has(planned.app)) { + byApp.set(planned.app, []); + order.push(planned.app); + } + } + } return order.map((appName) => { const replicas = byApp.get(appName) ?? []; @@ -41,10 +62,10 @@ export function pivotByApp(snapshots: ServerSnapshot[]): FleetView[] { return { app: appName, - appType: replicas[0]?.snapshot.appType ?? 'backend', + appType: replicas[0]?.snapshot.appType ?? plannedType.get(appName) ?? 'backend', replicas, convergence: assessConvergence(observations), - unreachable, + unreachable: [...(unreachableByApp.get(appName) ?? []), ...(replicas.length > 0 ? unattributed : [])], }; }); } diff --git a/src/domain/observe/snapshot.ts b/src/domain/observe/snapshot.ts index 84fc17a..d6f9209 100644 --- a/src/domain/observe/snapshot.ts +++ b/src/domain/observe/snapshot.ts @@ -28,6 +28,19 @@ export interface ServerSnapshot { apps: AppSnapshot[]; /** Whole-server failure — unreachable, timed out, script produced nothing. */ error?: string; + /** + * The apps this server is configured to run. Set only on a failed poll: a + * server that reports nothing cannot say which apps it should have run, but + * the caller that asked for the poll knows, and the fleet view needs it to + * blame the failure on those apps and no others. + */ + plannedApps?: PlannedApp[]; +} + +/** An app a server is configured to run, as known without reaching the server. */ +export interface PlannedApp { + app: string; + appType: 'backend' | 'frontend'; } /** The app-scoped slice of one server's poll. */ diff --git a/tests/unit/observe-collector.test.ts b/tests/unit/observe-collector.test.ts index 5b7f75c..5f9e6d8 100644 --- a/tests/unit/observe-collector.test.ts +++ b/tests/unit/observe-collector.test.ts @@ -150,6 +150,23 @@ describe('MetricsCollector', () => { expect(snapshot.server).toBe('b'); }); + it('records which apps a failed poll was meant to cover', async () => { + const executor = new (class extends FakeRemoteExecutor { + override async exec(): Promise { + throw new Error('connect ETIMEDOUT'); + } + })(); + const planned = config.apps.map((a) => ({ app: a.name, appType: a.appType })); + + const timedOut = await new MetricsCollector(executor, 'b', config).collect({ apps: config.apps }); + const empty = await new MetricsCollector( + new FakeRemoteExecutor().when(() => true, { stdout: '', stderr: '', exitCode: 0 }), 'c', config, + ).collect({ apps: config.apps }); + + expect(timedOut.plannedApps).toEqual(planned); + expect(empty.plannedApps).toEqual(planned); + }); + it('reports empty output as an error rather than an empty snapshot', async () => { const executor = new FakeRemoteExecutor().when(() => true, { stdout: '', stderr: '', exitCode: 0 }); const snapshot = await new MetricsCollector(executor, 'c', config).collect({ apps: config.apps }); diff --git a/tests/unit/observe-pivot.test.ts b/tests/unit/observe-pivot.test.ts index 95f9e91..6018eb0 100644 --- a/tests/unit/observe-pivot.test.ts +++ b/tests/unit/observe-pivot.test.ts @@ -94,6 +94,52 @@ describe('pivotByApp', () => { expect(views).toEqual([]); }); + it('blames a failed server only on the apps planned for it', () => { + // `web` runs on a alone; `api` runs on a and b. b failing says nothing + // about `web`, which must not turn into a fleet with a missing replica. + const views = pivotByApp([ + server('a', [app('web', '1'), app('api', '1')]), + server('b', [], { error: 'down', plannedApps: [{ app: 'api', appType: 'backend' }] }), + ]); + + expect(views.find((v) => v.app === 'web')?.unreachable).toEqual([]); + expect(views.find((v) => v.app === 'api')?.unreachable).toEqual(['b']); + }); + + it('ignores a failed server that only hosts accessories', () => { + const views = pivotByApp([ + server('a', [app('api', '1')]), + server('data', [], { error: 'down', plannedApps: [] }), + ]); + + expect(views[0].unreachable).toEqual([]); + }); + + it('still shows an app whose every server failed', () => { + const views = pivotByApp([ + server('a', [app('web', '1')]), + server('b', [], { error: 'down', plannedApps: [{ app: 'site', appType: 'frontend' }] }), + server('c', [], { error: 'down', plannedApps: [{ app: 'site', appType: 'frontend' }] }), + ]); + + const site = views.find((v) => v.app === 'site'); + expect(site).toBeDefined(); + expect(site?.replicas).toEqual([]); + expect(site?.unreachable).toEqual(['b', 'c']); + expect(site?.appType).toBe('frontend'); + }); + + it('lists reachable replicas and unreachable servers of the same app together', () => { + const views = pivotByApp([ + server('a', [app('api', '1')]), + server('b', [], { error: 'down', plannedApps: [{ app: 'api', appType: 'backend' }] }), + ]); + + expect(views).toHaveLength(1); + expect(views[0].replicas.map((r) => r.server)).toEqual(['a']); + expect(views[0].unreachable).toEqual(['b']); + }); + it('keeps a per-app error visible on the replica', () => { const views = pivotByApp([ server('a', [app('api', '1', { error: 'PM2 command failed' })]),