fix(preflight): suppress node-state checks when the Docker snapshot is unavailable (#1423)

When the Docker daemon is unreachable the node snapshot collection fails and
returns empty sets. The preflight rules read those empty sets as "resource
absent", so a stack referencing an external network or volume got false "not
found" blockers while real host-port and container_name conflicts went
undetected.

Add a nodeStateAvailable flag to the preflight context, mirroring the existing
sourceReadable gate. The six node-state rules now suppress themselves when the
snapshot could not be collected, and a single info advisory reports that the
node-state checks were skipped so a clean pass during an outage is not mistaken
for full coverage.
This commit is contained in:
Anso
2026-06-24 19:48:34 -04:00
committed by GitHub
parent 96b3c49359
commit 2ed01641c8
6 changed files with 158 additions and 21 deletions
@@ -22,8 +22,13 @@ let nodeId: number;
function db() { return DatabaseService.getInstance(); }
function doctor() { return ComposeDoctorService.getInstance(); }
/** Mock the two Docker calls; render returns the given effective model JSON. */
function stubDocker(rendered: object | null, stderr = '', snapshot = { containers: [], networks: [], volumes: [] }) {
/** Mock the two Docker calls; render returns the given effective model JSON.
* Pass `'reject'` as the snapshot to simulate an unreachable Docker daemon. */
function stubDocker(
rendered: object | null,
stderr = '',
snapshot: { containers: unknown[]; networks: unknown[]; volumes: unknown[] } | 'reject' = { containers: [], networks: [], volumes: [] },
) {
vi.spyOn(ComposeService, 'getInstance').mockReturnValue({
renderConfig: vi.fn().mockResolvedValue({
rendered: rendered === null ? null : JSON.stringify(rendered),
@@ -33,7 +38,9 @@ function stubDocker(rendered: object | null, stderr = '', snapshot = { container
}),
} as unknown as ComposeService);
vi.spyOn(DockerController, 'getInstance').mockReturnValue({
getDependencySnapshot: vi.fn().mockResolvedValue(snapshot),
getDependencySnapshot: snapshot === 'reject'
? vi.fn().mockRejectedValue(new Error('docker down'))
: vi.fn().mockResolvedValue(snapshot),
} as unknown as DockerController);
}
@@ -164,19 +171,55 @@ describe('runPreflight', () => {
});
it('degrades to model-only findings when the node snapshot fails', async () => {
vi.spyOn(ComposeService, 'getInstance').mockReturnValue({
renderConfig: vi.fn().mockResolvedValue({
rendered: JSON.stringify({ name: STACK, services: { web: { image: 'nginx:latest' } }, networks: {}, volumes: {} }),
stderr: '', code: 0, timedOut: false,
}),
} as unknown as ComposeService);
vi.spyOn(DockerController, 'getInstance').mockReturnValue({
getDependencySnapshot: vi.fn().mockRejectedValue(new Error('docker down')),
} as unknown as DockerController);
stubDocker({ name: STACK, services: { web: { image: 'nginx:latest' } }, networks: {}, volumes: {} }, '', 'reject');
const report = await doctor().runPreflight(nodeId, STACK, null);
expect(report.renderable).toBe(true);
expect(report.findings.map(f => f.ruleId)).toContain('image-latest'); // model rule still ran
expect(report.findings.map(f => f.ruleId)).not.toContain('port-conflict-node'); // node-state skipped
expect(report.findings.map(f => f.ruleId)).toContain('node-state-unavailable'); // partial coverage is surfaced
});
it('does not flag external resources as missing when the snapshot is unavailable', async () => {
stubDocker(
{
name: STACK,
services: { web: { image: 'nginx:1.27', restart: 'always', healthcheck: { test: ['CMD', 'true'] } } },
networks: { ext: { name: 'shared', external: true } },
volumes: { v: { name: 'data', external: true } },
},
'',
'reject',
);
const report = await doctor().runPreflight(nodeId, STACK, null);
expect(report.renderable).toBe(true);
const ruleIds = report.findings.map(f => f.ruleId);
// An empty snapshot must not be read as "the external resource is absent".
expect(ruleIds).not.toContain('external-network-missing');
expect(ruleIds).not.toContain('external-volume-missing');
expect(ruleIds).toContain('node-state-unavailable');
});
it('marks the run partial (info) when only node state is unavailable', async () => {
stubDocker({ name: STACK, services: { web: { image: 'nginx:1.27', restart: 'always', healthcheck: { test: ['CMD', 'true'] } } }, networks: {}, volumes: {} }, '', 'reject');
const report = await doctor().runPreflight(nodeId, STACK, null);
// A clean model with the daemon down yields only the advisory, so the clean
// 'pass' becomes 'info': the operator sees the result is partial.
expect(report.findings.map(f => f.ruleId)).toEqual(['node-state-unavailable']);
expect(report.status).toBe('info');
expect(report.highestSeverity).toBe('info');
});
it('runs node-state rules against a collected snapshot', async () => {
stubDocker(
{ name: STACK, services: { web: { image: 'nginx:1.27', container_name: 'dup', restart: 'always', healthcheck: { test: ['CMD', 'true'] } } }, networks: {}, volumes: {} },
'',
{ containers: [{ name: 'dup', stack: 'other', ports: [] }], networks: [], volumes: [] },
);
const report = await doctor().runPreflight(nodeId, STACK, null);
const ruleIds = report.findings.map(f => f.ruleId);
// A successful snapshot sets nodeStateAvailable true, so the node-state rule runs.
expect(ruleIds).toContain('container-name-collision');
expect(ruleIds).not.toContain('node-state-unavailable');
});
it('orders findings by severity, highest first', async () => {
+64 -1
View File
@@ -29,7 +29,7 @@ function ctx(over: Partial<PreflightContext> = {}): PreflightContext {
missingEnvFiles: [],
sourceServiceNames: m ? m.services.map(s => s.name) : [], sourceReadable: true,
nodePorts: [], existingNetworkNames: new Set(), existingVolumeNames: new Set(),
existingContainers: [], bindChecks: [],
existingContainers: [], nodeStateAvailable: true, bindChecks: [],
stackIntent: null, serviceIntents: {}, accessUrlPorts: new Set(), hasAccessUrls: false, ...over,
};
}
@@ -342,6 +342,68 @@ describe('exposure-intent rules', () => {
});
});
describe('node-state availability', () => {
// When the node's Docker snapshot could not be collected, the empty sets must not
// be read as "resource absent" or "no conflict". Every node-state rule suppresses
// itself, and one advisory explains the partial coverage.
const externalRes = model([svc()], {
networks: { ext: { name: 'shared', external: true, internal: false } },
volumes: { v: { name: 'data', external: true, internal: false } },
});
const newRes = model([svc()], {
networks: { backend: { name: 'backend', external: false, internal: false } },
volumes: { data: { name: 'data', external: false, internal: false } },
});
const portModel = model([svc({ ports: [{ startPort: 8080, endPort: 8080, hostIp: '', protocol: 'tcp' }] })]);
const nameModel = model([svc({ containerName: 'taken' })]);
it('does not assert an external network/volume is absent', () => {
const f = runRules(ctx({ model: externalRes, nodeStateAvailable: false }));
expect(ids(f, 'external-network-missing')).toHaveLength(0);
expect(ids(f, 'external-volume-missing')).toHaveLength(0);
});
it('does not claim a network/volume is new', () => {
const f = runRules(ctx({ model: newRes, nodeStateAvailable: false }));
expect(ids(f, 'new-network')).toHaveLength(0);
expect(ids(f, 'new-volume')).toHaveLength(0);
});
it('does not report a clean all-clear over a real port conflict', () => {
const f = runRules(ctx({ model: portModel, nodeStateAvailable: false,
nodePorts: [{ publishedPort: 8080, protocol: 'tcp', ip: '', stack: 'other' }] }));
expect(ids(f, 'port-conflict-node')).toHaveLength(0);
});
it('does not report a clean all-clear over a real container_name collision', () => {
const f = runRules(ctx({ model: nameModel, nodeStateAvailable: false,
existingContainers: [{ name: 'taken', stack: 'other' }] }));
expect(ids(f, 'container-name-collision')).toHaveLength(0);
});
it('still runs node-state rules when the snapshot is available', () => {
const f = runRules(ctx({ model: externalRes, nodeStateAvailable: true }));
expect(ids(f, 'external-network-missing')).toHaveLength(1);
expect(ids(f, 'external-volume-missing')).toHaveLength(1);
});
it('does not suppress higher-severity model findings while node state is unavailable', () => {
const f = runRules(ctx({ model: model([svc({ privileged: true })]), nodeStateAvailable: false }));
expect(ids(f, 'node-state-unavailable')).toHaveLength(1);
expect(ids(f, 'privileged')).toHaveLength(1); // a real model finding is still reported alongside the advisory
});
describe('node-state-unavailable advisory', () => {
it('fires one info finding when the model rendered but node state is unavailable', () => {
const f = ids(runRules(ctx({ model: model([svc()]), nodeStateAvailable: false })), 'node-state-unavailable');
expect(f).toHaveLength(1);
expect(f[0].severity).toBe('info');
});
it('stays silent when node state is available', () => {
expect(ids(runRules(ctx({ model: model([svc()]), nodeStateAvailable: true })), 'node-state-unavailable')).toHaveLength(0);
});
it('stays silent when the model is unrenderable (render-failed already covers it)', () => {
const f = runRules(ctx({ model: null, renderable: false, renderError: 'boom', nodeStateAvailable: false }));
expect(ids(f, 'node-state-unavailable')).toHaveLength(0);
});
});
});
describe('rule registry completeness', () => {
// The canonical rule set. Adding or removing a rule must update this list,
// which forces a deliberate pass over the docs and the frontend severity map.
@@ -349,6 +411,7 @@ describe('rule registry completeness', () => {
'render-failed', 'env-unset', 'env-file-missing', 'port-conflict-node', 'port-conflict-internal', 'port-exposed-all-interfaces',
'bind-path-missing', 'bind-path-permission', 'docker-socket-mount', 'privileged', 'network-mode-host',
'uid-gid-risk', 'image-latest', 'no-restart-policy', 'no-healthcheck', 'deploy-swarm-only',
'node-state-unavailable',
'external-network-missing', 'external-volume-missing', 'new-network', 'new-volume', 'anonymous-volume',
'container-name-internal-dup', 'container-name-collision',
'exposure-internal-published', 'sensitive-service-broad-exposure', 'exposure-unclassified',
+5 -2
View File
@@ -175,7 +175,7 @@ export class ComposeDoctorService {
|| 'Sencho could not run docker compose on this node.';
}
const { nodePorts, existingNetworkNames, existingVolumeNames, existingContainers } = await this.nodeState(nodeId, fsSvc, stackName);
const { nodePorts, existingNetworkNames, existingVolumeNames, existingContainers, nodeStateAvailable } = await this.nodeState(nodeId, fsSvc, stackName);
const bindChecks = model ? await this.resolveBindChecks(model, baseDir) : [];
const { stackIntent, serviceIntents, accessUrlPorts, hasAccessUrls } = this.exposureState(nodeId, stackName);
@@ -207,6 +207,7 @@ export class ComposeDoctorService {
existingNetworkNames,
existingVolumeNames,
existingContainers,
nodeStateAvailable,
bindChecks,
stackIntent,
serviceIntents,
@@ -254,6 +255,7 @@ export class ComposeDoctorService {
existingNetworkNames: Set<string>;
existingVolumeNames: Set<string>;
existingContainers: { name: string; stack: string | null }[];
nodeStateAvailable: boolean;
}> {
try {
const knownStacks = await fsSvc.getStacks();
@@ -265,11 +267,12 @@ export class ComposeDoctorService {
existingNetworkNames: new Set(snapshot.networks.map(n => n.name)),
existingVolumeNames: new Set(snapshot.volumes.map(v => v.name)),
existingContainers: snapshot.containers.map(c => ({ name: c.name, stack: c.stack })),
nodeStateAvailable: true,
};
} catch (error) {
console.warn('[ComposeDoctor] Node snapshot unavailable for %s; node-state checks skipped:',
sanitizeForLog(stackName), sanitizeForLog(getErrorMessage(error, 'unknown')));
return { nodePorts: [], existingNetworkNames: new Set(), existingVolumeNames: new Set(), existingContainers: [] };
return { nodePorts: [], existingNetworkNames: new Set(), existingVolumeNames: new Set(), existingContainers: [], nodeStateAvailable: false };
}
}
+28 -6
View File
@@ -105,7 +105,7 @@ const envFileMissing: PreflightRule = {
const portConflictNode: PreflightRule = {
id: 'port-conflict-node',
run(ctx) {
if (!ctx.model) return [];
if (!ctx.model || !ctx.nodeStateAvailable) return [];
const byPort = new Map<number, NodePortBinding[]>();
for (const b of ctx.nodePorts) {
const list = byPort.get(b.publishedPort);
@@ -388,10 +388,31 @@ const deploySwarmOnly: PreflightRule = {
},
};
const nodeStateUnavailable: PreflightRule = {
id: 'node-state-unavailable',
run(ctx) {
// Emit only when the model rendered but the node's Docker snapshot could not be
// read: an unrenderable model already raises its own render-failed blocker, so a
// second advisory there would be noise. The rules that read node state suppress
// themselves in this state; this finding tells the operator why, so a clean pass
// during an outage is not mistaken for full coverage. (The info-only
// new-network / new-volume notices are gated too, but left out of the message
// below: they preview a deploy action rather than flag a problem.)
if (!ctx.model || ctx.nodeStateAvailable) return [];
return [{
ruleId: 'node-state-unavailable',
severity: 'info',
title: 'Node-state checks skipped',
message: 'The node\'s Docker state could not be read, so the external-resource, host-port, and container_name checks did not run. This result is partial.',
remediation: 'Confirm the Docker daemon is reachable on this node, then re-run preflight.',
}];
},
};
const externalNetworkMissing: PreflightRule = {
id: 'external-network-missing',
run(ctx) {
if (!ctx.model) return [];
if (!ctx.model || !ctx.nodeStateAvailable) return [];
const findings: PreflightFinding[] = [];
for (const [key, net] of Object.entries(ctx.model.networks)) {
if (!net.external || ctx.existingNetworkNames.has(net.name)) continue;
@@ -411,7 +432,7 @@ const externalNetworkMissing: PreflightRule = {
const externalVolumeMissing: PreflightRule = {
id: 'external-volume-missing',
run(ctx) {
if (!ctx.model) return [];
if (!ctx.model || !ctx.nodeStateAvailable) return [];
const findings: PreflightFinding[] = [];
for (const [key, vol] of Object.entries(ctx.model.volumes)) {
if (!vol.external || ctx.existingVolumeNames.has(vol.name)) continue;
@@ -431,7 +452,7 @@ const externalVolumeMissing: PreflightRule = {
const newNetwork: PreflightRule = {
id: 'new-network',
run(ctx) {
if (!ctx.model) return [];
if (!ctx.model || !ctx.nodeStateAvailable) return [];
const findings: PreflightFinding[] = [];
for (const [key, net] of Object.entries(ctx.model.networks)) {
if (net.external || key === 'default') continue;
@@ -452,7 +473,7 @@ const newNetwork: PreflightRule = {
const newVolume: PreflightRule = {
id: 'new-volume',
run(ctx) {
if (!ctx.model) return [];
if (!ctx.model || !ctx.nodeStateAvailable) return [];
const findings: PreflightFinding[] = [];
for (const [key, vol] of Object.entries(ctx.model.volumes)) {
if (vol.external) continue;
@@ -521,7 +542,7 @@ const containerNameInternalDup: PreflightRule = {
const containerNameCollision: PreflightRule = {
id: 'container-name-collision',
run(ctx) {
if (!ctx.model) return [];
if (!ctx.model || !ctx.nodeStateAvailable) return [];
const findings: PreflightFinding[] = [];
for (const s of ctx.model.services) {
if (!s.containerName) continue;
@@ -723,6 +744,7 @@ export const PREFLIGHT_RULES: PreflightRule[] = [
noRestartPolicy,
noHealthcheck,
deploySwarmOnly,
nodeStateUnavailable,
externalNetworkMissing,
externalVolumeMissing,
newNetwork,
+3
View File
@@ -100,6 +100,9 @@ export interface PreflightContext {
existingNetworkNames: Set<string>;
existingVolumeNames: Set<string>;
existingContainers: { name: string; stack: string | null }[];
/** Whether the node's Docker snapshot was collected; gates node-state checks so
* an unavailable snapshot cannot be mistaken for an empty node. */
nodeStateAvailable: boolean;
bindChecks: BindCheck[];
/** Stack-level exposure classification, or null when unset. */
stackIntent: ExposureIntent | null;
+3
View File
@@ -60,6 +60,9 @@ Preflight runs against the **active node**, so selecting a remote node checks th
<Accordion title="A port conflict is flagged for a port my own stack uses">
Preflight ignores ports already held by the stack you are checking, so redeploying a running stack does not flag its own bindings. A conflict means a *different* stack, or an unmanaged container, holds that host port on this node.
</Accordion>
<Accordion title="The report says node-state checks were skipped">
The node-state checks (external networks and volumes, host-port conflicts, and `container_name` collisions) read the node's live Docker state. When the Docker daemon on the target node is unreachable, Sencho cannot read that state, so those checks are skipped and the report shows an info notice rather than guessing. The model checks still run. Confirm the daemon is reachable on the node, then run preflight again for full coverage.
</Accordion>
<Accordion title="The Doctor tab is not there on a remote node">
The tab appears when the active node reports that it supports Compose Doctor. A node running an older version of Sencho does not advertise it, so the tab is hidden for that node until it is updated.
</Accordion>