mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-17 14:08:19 +00:00
fix: stop compose-ps spam after deleting a stack (#1831)
* fix: stop compose-ps spam after deleting a stack A live logs WebSocket kept calling docker compose ps against the removed stack directory every 2s. Missing cwd surfaces as spawn ENOENT, which was logged as Docker CLI unavailable. Skip compose when the dir is gone and idle the log stream instead of retrying. * fix: contain stack directory stats inside the compose root Resolve the stack path against the node's compose directory and refuse paths that escape it before calling fs.stat, matching the existing filesystem containment pattern at other sinks.
This commit is contained in:
@@ -35,6 +35,7 @@ const {
|
||||
mockCompensateWithCandidate,
|
||||
mockBuildUnifiedHeldImagePredicate,
|
||||
mockGetRecovery,
|
||||
mockFsStat,
|
||||
} = vi.hoisted(() => ({
|
||||
mockSpawn: vi.fn(),
|
||||
mockGetContainersByStack: vi.fn().mockResolvedValue([]),
|
||||
@@ -87,6 +88,7 @@ const {
|
||||
mockCompensateWithCandidate: vi.fn().mockResolvedValue(true),
|
||||
mockBuildUnifiedHeldImagePredicate: vi.fn().mockReturnValue(() => false),
|
||||
mockGetRecovery: vi.fn().mockReturnValue(undefined),
|
||||
mockFsStat: vi.fn().mockResolvedValue({ isDirectory: () => true }),
|
||||
}));
|
||||
|
||||
vi.mock('child_process', () => ({ spawn: mockSpawn, execFile: vi.fn() }));
|
||||
@@ -97,11 +99,13 @@ vi.mock('fs', () => ({
|
||||
writeFileSync: (...args: unknown[]) => mockWriteFileSync(...args),
|
||||
unlinkSync: (...args: unknown[]) => mockUnlinkSync(...args),
|
||||
rmdirSync: (...args: unknown[]) => mockRmdirSync(...args),
|
||||
promises: { stat: (...args: unknown[]) => mockFsStat(...args) },
|
||||
},
|
||||
mkdtempSync: (...args: unknown[]) => mockMkdtempSync(...args),
|
||||
writeFileSync: (...args: unknown[]) => mockWriteFileSync(...args),
|
||||
unlinkSync: (...args: unknown[]) => mockUnlinkSync(...args),
|
||||
rmdirSync: (...args: unknown[]) => mockRmdirSync(...args),
|
||||
promises: { stat: (...args: unknown[]) => mockFsStat(...args) },
|
||||
}));
|
||||
|
||||
vi.mock('../services/NodeRegistry', () => ({
|
||||
@@ -316,6 +320,7 @@ beforeEach(() => {
|
||||
mockIsMeshStackEnabled.mockReturnValue(false);
|
||||
mockCompensateWithCandidate.mockResolvedValue(true);
|
||||
mockBuildUnifiedHeldImagePredicate.mockReturnValue(() => false);
|
||||
mockFsStat.mockResolvedValue({ isDirectory: () => true });
|
||||
delete process.env.SENCHO_MODE;
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
});
|
||||
@@ -1709,6 +1714,84 @@ describe('ComposeService - streamLogs', () => {
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not reschedule when the stack directory is gone', async () => {
|
||||
mockFsStat.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }));
|
||||
const ws = createMockWs();
|
||||
const svc = ComposeService.getInstance(1);
|
||||
|
||||
svc.streamLogs('mystack', ws);
|
||||
await vi.waitFor(() => expect(ws.send).toHaveBeenCalled());
|
||||
|
||||
const sent = (ws.send as ReturnType<typeof vi.fn>).mock.calls
|
||||
.flatMap((c) => String(c[0]).split(/\r?\n/))
|
||||
.filter(Boolean);
|
||||
expect(sent.some((line) => line.includes('Stack directory is gone'))).toBe(true);
|
||||
expect(mockGetContainersByStack).not.toHaveBeenCalled();
|
||||
expect(ws.send).toHaveBeenCalledTimes(1);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(4000);
|
||||
expect(mockGetContainersByStack).not.toHaveBeenCalled();
|
||||
expect(ws.send).toHaveBeenCalledTimes(1);
|
||||
expect(ws.close).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('retries every 2s when the stack dir exists and no containers are found', async () => {
|
||||
mockGetContainersByStack.mockResolvedValue([]);
|
||||
const ws = createMockWs();
|
||||
const svc = ComposeService.getInstance(1);
|
||||
|
||||
svc.streamLogs('mystack', ws);
|
||||
await vi.waitFor(() => expect(mockGetContainersByStack).toHaveBeenCalledTimes(1));
|
||||
|
||||
await vi.advanceTimersByTimeAsync(2000);
|
||||
expect(mockGetContainersByStack).toHaveBeenCalledTimes(2);
|
||||
|
||||
const sent = (ws.send as ReturnType<typeof vi.fn>).mock.calls
|
||||
.flatMap((c) => String(c[0]))
|
||||
.join('');
|
||||
expect(sent).toContain('No containers found. Waiting for activity');
|
||||
});
|
||||
|
||||
it('stops retrying after the websocket closes', async () => {
|
||||
mockGetContainersByStack.mockResolvedValue([]);
|
||||
const ws = createMockWs();
|
||||
const svc = ComposeService.getInstance(1);
|
||||
|
||||
svc.streamLogs('mystack', ws);
|
||||
await vi.waitFor(() => expect(mockGetContainersByStack).toHaveBeenCalledTimes(1));
|
||||
|
||||
ws.readyState = 3;
|
||||
ws.emit('close');
|
||||
await vi.advanceTimersByTimeAsync(4000);
|
||||
expect(mockGetContainersByStack).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('stops the empty-container retry after the stack directory disappears', async () => {
|
||||
mockGetContainersByStack.mockResolvedValue([]);
|
||||
const ws = createMockWs();
|
||||
const svc = ComposeService.getInstance(1);
|
||||
|
||||
svc.streamLogs('mystack', ws);
|
||||
await vi.waitFor(() => expect(mockGetContainersByStack).toHaveBeenCalledTimes(1));
|
||||
|
||||
mockFsStat.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }));
|
||||
await vi.advanceTimersByTimeAsync(2000);
|
||||
|
||||
expect(mockGetContainersByStack).toHaveBeenCalledTimes(1);
|
||||
const sent = (ws.send as ReturnType<typeof vi.fn>).mock.calls
|
||||
.flatMap((c) => String(c[0]))
|
||||
.join('');
|
||||
expect(sent).toContain('No containers found. Waiting for activity');
|
||||
expect(sent).toContain('Stack directory is gone');
|
||||
expect(ws.close).not.toHaveBeenCalled();
|
||||
|
||||
const sendsAfterGone = (ws.send as ReturnType<typeof vi.fn>).mock.calls.length;
|
||||
await vi.advanceTimersByTimeAsync(4000);
|
||||
expect(mockGetContainersByStack).toHaveBeenCalledTimes(1);
|
||||
expect(ws.send).toHaveBeenCalledTimes(sendsAfterGone);
|
||||
expect(ws.close).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
|
||||
@@ -78,6 +78,7 @@ beforeEach(() => {
|
||||
// holds re-spy after this beforeEach runs.
|
||||
vi.spyOn(StackUpdateRecoveryService.getInstance(), 'getHeldImageIds').mockReturnValue(new Set());
|
||||
vi.spyOn(ServiceUpdateRecoveryService.getInstance(), 'getHeldImageIds').mockReturnValue(new Set());
|
||||
vi.spyOn(fs, 'stat').mockResolvedValue({ isDirectory: () => true } as Awaited<ReturnType<typeof fs.stat>>);
|
||||
});
|
||||
|
||||
/** Hold specific images via the stack-side recovery service for one test. */
|
||||
@@ -2478,3 +2479,73 @@ describe('DockerController - smartFallback stack-dir evidence', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('DockerController - missing stack directory', () => {
|
||||
const enoent = () => Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' });
|
||||
|
||||
beforeEach(() => {
|
||||
vi.spyOn(fs, 'stat').mockRejectedValue(enoent());
|
||||
});
|
||||
|
||||
it('getContainersByStack returns [] without spawning compose', async () => {
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
const dc = DockerController.getInstance(1);
|
||||
const fetchSpy = spyOrphanDc(dc, 'fetchComposePsContainers');
|
||||
|
||||
await expect(dc.getContainersByStack('gone-stack')).resolves.toEqual([]);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
expect(mockExecFileAsync).not.toHaveBeenCalled();
|
||||
expect(errSpy).not.toHaveBeenCalled();
|
||||
fetchSpy.mockRestore();
|
||||
errSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('getLegacyOrphanContainersByStack returns [] without spawning compose', async () => {
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
const dc = DockerController.getInstance(1);
|
||||
const fetchSpy = spyOrphanDc(dc, 'fetchComposePsContainers');
|
||||
const fallbackSpy = spyOrphanDc(dc, 'smartFallback');
|
||||
|
||||
await expect(dc.getLegacyOrphanContainersByStack('gone-stack')).resolves.toEqual([]);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
expect(fallbackSpy).not.toHaveBeenCalled();
|
||||
expect(mockExecFileAsync).not.toHaveBeenCalled();
|
||||
expect(errSpy).not.toHaveBeenCalled();
|
||||
fetchSpy.mockRestore();
|
||||
fallbackSpy.mockRestore();
|
||||
errSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('classifyLegacyOrphansForUpdate returns classification_failed without spawning compose', async () => {
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
const dc = DockerController.getInstance(1);
|
||||
const fetchSpy = spyOrphanDc(dc, 'fetchComposePsContainers');
|
||||
|
||||
await expect(dc.classifyLegacyOrphansForUpdate('gone-stack')).resolves.toEqual({
|
||||
status: 'classification_failed',
|
||||
error: 'Stack directory is gone',
|
||||
});
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
expect(mockExecFileAsync).not.toHaveBeenCalled();
|
||||
expect(errSpy).not.toHaveBeenCalled();
|
||||
fetchSpy.mockRestore();
|
||||
errSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('still logs Docker CLI unavailable when the stack dir exists and compose ENOENT fires', async () => {
|
||||
vi.spyOn(fs, 'stat').mockResolvedValue({ isDirectory: () => true } as Awaited<ReturnType<typeof fs.stat>>);
|
||||
vi.spyOn(os, 'freemem').mockReturnValue(2 * 1024 * 1024 * 1024);
|
||||
vi.spyOn(os, 'totalmem').mockReturnValue(8 * 1024 * 1024 * 1024);
|
||||
mockExecFileAsync.mockRejectedValue(Object.assign(new Error('spawn docker ENOENT'), { code: 'ENOENT' }));
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
|
||||
const dc = DockerController.getInstance(1);
|
||||
await dc.getContainersByStack('my-stack');
|
||||
|
||||
expect(mockExecFileAsync).toHaveBeenCalled();
|
||||
const logged = errSpy.mock.calls.map((c) => c.map(String).join(' ')).join('\n');
|
||||
expect(logged).toMatch(/Docker Compose Error for/);
|
||||
expect(logged).toContain('Docker CLI unavailable on this node');
|
||||
errSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+1653
-1631
File diff suppressed because it is too large
Load Diff
@@ -2480,6 +2480,25 @@ class DockerController {
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the stack directory is missing (ENOENT) or is not a directory.
|
||||
* Other stat failures return false so callers keep the existing compose-ps path.
|
||||
*/
|
||||
private async isStackDirectoryAbsent(stackName: string): Promise<boolean> {
|
||||
// Canonical js/path-injection barrier at the fs.stat sink.
|
||||
const baseResolved = path.resolve(NodeRegistry.getInstance().getComposeDir(this.nodeId));
|
||||
const stackDir = path.resolve(baseResolved, stackName);
|
||||
if (!stackDir.startsWith(baseResolved + path.sep)) {
|
||||
return true;
|
||||
}
|
||||
try {
|
||||
const st = await fs.stat(stackDir);
|
||||
return !st.isDirectory();
|
||||
} catch (error) {
|
||||
return (error as NodeJS.ErrnoException).code === 'ENOENT';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Containers visible to `docker compose ps` for this stack. Empty when Compose
|
||||
* does not manage any containers (including first deploy or mislabeled legacy).
|
||||
@@ -2530,6 +2549,7 @@ class DockerController {
|
||||
*/
|
||||
public async getLegacyOrphanContainersByStack(stackName: string): Promise<Array<{ Id: string }>> {
|
||||
const stackDir = path.join(NodeRegistry.getInstance().getComposeDir(this.nodeId), stackName);
|
||||
if (await this.isStackDirectoryAbsent(stackName)) return [];
|
||||
const toIds = (list: Array<{ Id?: string }>) =>
|
||||
list.filter((c): c is { Id: string } => typeof c.Id === 'string' && c.Id.length > 0)
|
||||
.map((c) => ({ Id: c.Id }));
|
||||
@@ -2573,6 +2593,9 @@ class DockerController {
|
||||
| { status: 'classification_failed'; error: string }
|
||||
> {
|
||||
const stackDir = path.join(NodeRegistry.getInstance().getComposeDir(this.nodeId), stackName);
|
||||
if (await this.isStackDirectoryAbsent(stackName)) {
|
||||
return { status: 'classification_failed', error: 'Stack directory is gone' };
|
||||
}
|
||||
const toIds = (list: Array<{ Id?: string }>) =>
|
||||
list.filter((c): c is { Id: string } => typeof c.Id === 'string' && c.Id.length > 0)
|
||||
.map((c) => c.Id);
|
||||
@@ -2614,6 +2637,9 @@ class DockerController {
|
||||
// not the process default, so a non-default local node sees its own stack dir
|
||||
// and deploy spec.
|
||||
const stackDir = path.join(NodeRegistry.getInstance().getComposeDir(this.nodeId), stackName);
|
||||
if (await this.isStackDirectoryAbsent(stackName)) {
|
||||
return this.enrichContainers([]);
|
||||
}
|
||||
|
||||
try {
|
||||
const containers = await this.fetchComposePsContainers(stackName, stackDir);
|
||||
|
||||
Reference in New Issue
Block a user