fix(notifications): neutralize satellite-local node names in alert bodies (#1640)

* fix(notifications): neutralize satellite-local node names in alert bodies

Fleet-aggregated alerts embedded each instance seed name (often Local)
while the hub badge already named the remote. Drop identity prefixes and
use type-aware local wording so attribution stays on the badge.

* fix(docs): correct image-update default check cadence

Operator docs still said six-hour polling; the seeded default is two
hours in interval mode, and the cadence is configurable or cron-based.

* test(notifications): assert hub stamps roster name on neutral remote bodies

Cover the fan-in path that attaches hub roster identity while leaving the
satellite message body unchanged.
This commit is contained in:
Anso
2026-07-16 15:51:52 -04:00
committed by GitHub
parent b91025dc8b
commit d8e4ede94f
12 changed files with 288 additions and 48 deletions
+103
View File
@@ -469,3 +469,106 @@ describe('BlueprintService marker parsing + name-conflict guard', () => {
expect(BlueprintService.parseMarker(JSON.stringify({ blueprintId: 'nope', revision: 1 }))).toBeNull();
});
});
describe('BlueprintReconciler drift alert node wording', () => {
type ReconcilerWithDrift = {
handleDrift: (blueprint: Blueprint, node: Node, reason: string) => Promise<void>;
};
function seedRemoteNode(name: string): number {
counter += 1;
const db = DatabaseService.getInstance().getDb();
const result = db.prepare(
`INSERT INTO nodes (name, type, mode, compose_dir, is_default, status, created_at)
VALUES (?, 'remote', 'proxy', '/tmp/compose', 0, 'online', ?)`
).run(name, Date.now());
return result.lastInsertRowid as number;
}
it('suggest-mode local drift uses on this node', async () => {
const { NotificationService } = await import('../services/NotificationService');
const dispatchSpy = vi.spyOn(NotificationService.getInstance(), 'dispatchAlert').mockResolvedValue(undefined);
const nodeId = seedNode();
const bp = seedBlueprint({ name: 'drift-local', drift_mode: 'suggest', nodeIds: [nodeId] });
const node = DatabaseService.getInstance().getNode(nodeId)!;
const reconciler = BlueprintReconciler.getInstance() as unknown as ReconcilerWithDrift;
await reconciler.handleDrift(bp, node, 'compose changed');
expect(dispatchSpy).toHaveBeenCalledWith(
'warning',
'blueprint_drift_detected',
'Blueprint "drift-local" drifted on this node: compose changed',
{ stackName: 'drift-local', actor: 'system:blueprint' },
);
});
it('suggest-mode remote drift keeps the authoritative node name', async () => {
const { NotificationService } = await import('../services/NotificationService');
const dispatchSpy = vi.spyOn(NotificationService.getInstance(), 'dispatchAlert').mockResolvedValue(undefined);
const nodeId = seedRemoteNode('sencho-test-02');
const bp = seedBlueprint({ name: 'drift-remote', drift_mode: 'suggest', nodeIds: [nodeId] });
const node = DatabaseService.getInstance().getNode(nodeId)!;
const reconciler = BlueprintReconciler.getInstance() as unknown as ReconcilerWithDrift;
await reconciler.handleDrift(bp, node, 'compose changed');
expect(dispatchSpy).toHaveBeenCalledWith(
'warning',
'blueprint_drift_detected',
'Blueprint "drift-remote" drifted on node "sencho-test-02": compose changed',
{ stackName: 'drift-remote', actor: 'system:blueprint' },
);
});
it('stateful marker-loss on local uses on this node', async () => {
const { NotificationService } = await import('../services/NotificationService');
const dispatchSpy = vi.spyOn(NotificationService.getInstance(), 'dispatchAlert').mockResolvedValue(undefined);
vi.spyOn(BlueprintService.getInstance(), 'readMarker').mockResolvedValue(null);
const nodeId = seedNode();
const bp = seedBlueprint({
name: 'marker-local',
drift_mode: 'enforce',
classification: 'stateful',
nodeIds: [nodeId],
});
const node = DatabaseService.getInstance().getNode(nodeId)!;
const reconciler = BlueprintReconciler.getInstance() as unknown as ReconcilerWithDrift;
await reconciler.handleDrift(bp, node, 'volumes diverged');
expect(dispatchSpy).toHaveBeenCalledWith(
'warning',
'blueprint_drift_detected',
'Blueprint "marker-local" lost its marker on this node; auto-fix declined to avoid stomping unowned data. Reason: volumes diverged',
{ stackName: 'marker-local', actor: 'system:blueprint' },
);
});
it('enforce correction-failure on local uses on this node', async () => {
const { NotificationService } = await import('../services/NotificationService');
const dispatchSpy = vi.spyOn(NotificationService.getInstance(), 'dispatchAlert').mockResolvedValue(undefined);
vi.spyOn(BlueprintService.getInstance(), 'deployToNode').mockResolvedValue({
status: 'failed',
error: 'compose up failed',
} as Awaited<ReturnType<typeof BlueprintService.prototype.deployToNode>>);
const nodeId = seedNode();
const bp = seedBlueprint({
name: 'fix-local',
drift_mode: 'enforce',
classification: 'stateless',
nodeIds: [nodeId],
});
const node = DatabaseService.getInstance().getNode(nodeId)!;
const reconciler = BlueprintReconciler.getInstance() as unknown as ReconcilerWithDrift;
await reconciler.handleDrift(bp, node, 'compose changed');
expect(dispatchSpy).toHaveBeenCalledWith(
'error',
'blueprint_drift_correction_failed',
'Auto-fix for "fix-local" on this node failed: compose up failed',
{ stackName: 'fix-local', actor: 'system:blueprint' },
);
});
});
@@ -137,7 +137,7 @@ describe('DockerEventService - die classification', () => {
expect(mockDispatchAlert).toHaveBeenCalledWith(
'error',
'monitor_alert',
expect.stringContaining('Container Crash Detected'),
'Container Crash Detected: web exited unexpectedly (Code: 1).',
expect.objectContaining({ containerName: 'web' }),
);
});
@@ -588,8 +588,8 @@ describe('DockerEventService - reconnect', () => {
const warn = mockDispatchAlert.mock.calls.find(c => c[0] === 'warning');
const info = mockDispatchAlert.mock.calls.find(c => c[0] === 'info');
expect(warn?.[2]).toContain('Lost connection');
expect(info?.[2]).toContain('Reconnected');
expect(warn?.[2]).toBe('Lost connection to Docker daemon; monitoring paused.');
expect(info?.[2]).toBe('Reconnected to Docker daemon.');
});
it('shutdown cancels pending reconnect', async () => {
@@ -452,13 +452,13 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, true);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockDispatchAlert).toHaveBeenCalledTimes(1);
expect(mockDispatchAlert).toHaveBeenCalledWith(
'info',
'image_update_available',
expect.stringContaining('stackA'),
'Stack "stackA" has image updates available.',
{ stackName: 'stackA', actor: 'system:image-update' },
);
expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', true, expect.any(Number), 'ok', null);
@@ -469,7 +469,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, true);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockDispatchAlert).not.toHaveBeenCalled();
});
@@ -483,7 +483,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, true);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockDispatchAlert).toHaveBeenCalledTimes(2);
const dispatched = mockDispatchAlert.mock.calls.map(call => (call[3] as any)?.stackName);
@@ -497,7 +497,7 @@ services:
mockGetStackUpdateStatus.mockReturnValue({ stackA: true, stackB: true });
stubCheckImage(service, true);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockDispatchAlert).not.toHaveBeenCalled();
});
@@ -508,11 +508,11 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, true);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockAddNotificationHistory).toHaveBeenCalledWith(1, expect.objectContaining({
level: 'error',
message: expect.stringContaining('webhook timeout'),
message: 'Failed to notify about image updates for stack "stackA": webhook timeout',
}));
});
});
@@ -567,7 +567,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImageByRef(service, { 'nginx:latest': { hasUpdate: false, error: 'Registry unreachable for registry-1.docker.io/library/nginx:latest' } });
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockRecordStackCheckFailure).toHaveBeenCalledWith(1, 'stackA', expect.stringContaining('Registry unreachable'), expect.any(Number));
expect(mockUpsertStackUpdateStatus).not.toHaveBeenCalled();
@@ -584,7 +584,7 @@ services:
'postgres:15': { hasUpdate: false, error: 'Registry unreachable for registry-1.docker.io/library/postgres:15' },
});
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', true, expect.any(Number), 'partial', expect.stringContaining('Registry unreachable'));
expect(mockRecordStackCheckFailure).not.toHaveBeenCalled();
@@ -605,7 +605,7 @@ services:
'postgres:15': { hasUpdate: false },
});
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', true, expect.any(Number), 'partial', expect.stringContaining('Registry unreachable'));
expect(mockRecordStackCheckFailure).not.toHaveBeenCalled();
@@ -619,7 +619,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImageByRef(service, { 'nginx:latest': { hasUpdate: false, notCheckable: true } });
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', false, expect.any(Number), 'ok', null);
expect(mockRecordStackCheckFailure).not.toHaveBeenCalled();
@@ -665,7 +665,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, false);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockEnvExists).toHaveBeenCalledWith('stackA');
expect(mockGetEnvContent).not.toHaveBeenCalled();
@@ -677,7 +677,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, false);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockEnvExists).toHaveBeenCalledWith('stackA');
expect(mockGetEnvContent).toHaveBeenCalledWith('stackA');
@@ -689,7 +689,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, false);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
// Should not throw; should still complete and write status
expect(mockUpsertStackUpdateStatus).toHaveBeenCalled();
@@ -1070,7 +1070,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, false);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockClearStackUpdateStatus).toHaveBeenCalledWith(1, 'stackB');
});
@@ -1080,7 +1080,7 @@ services:
const service = ImageUpdateService.getInstance();
stubCheckImage(service, false);
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
expect(mockClearStackUpdateStatus).not.toHaveBeenCalled();
});
@@ -1125,7 +1125,7 @@ services:
const checkImageSpy = vi.fn().mockResolvedValue({ hasUpdate: false });
(service as any).checkImage = checkImageSpy;
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
// Should check both the compose image and the container image
const checkedImages = checkImageSpy.mock.calls.map((c: any[]) => c[1]);
@@ -1143,7 +1143,7 @@ services:
const checkImageSpy = vi.fn().mockResolvedValue({ hasUpdate: false });
(service as any).checkImage = checkImageSpy;
await (service as any).checkNode(1, 'local', fakeDb());
await (service as any).checkNode(1, fakeDb());
const checkedImages = checkImageSpy.mock.calls.map((c: any[]) => c[1]);
expect(checkedImages).not.toContain('someapp:v2');
@@ -895,7 +895,12 @@ describe('MonitorService - breach state machine', () => {
const svc = MonitorService.getInstance();
await (svc as any).evaluate();
expect(mockDispatchAlert).toHaveBeenCalledWith('warning', 'monitor_alert', expect.stringContaining('CPU'), { stackName: 'my-stack', actor: 'system:monitor' });
expect(mockDispatchAlert).toHaveBeenCalledWith(
'warning',
'monitor_alert',
'The **CPU usage** for **my-stack** has exceeded your threshold of **80%** (Currently: 90%).',
{ stackName: 'my-stack', actor: 'system:monitor' },
);
expect(mockUpdateStackAlertLastFired).toHaveBeenCalledWith(1, expect.any(Number));
});
@@ -2159,3 +2159,97 @@ describe('SchedulerService - delete_after_run', () => {
expect(mockUpdateScheduledTask).toHaveBeenCalledWith(300, expect.objectContaining({ last_status: 'success' }));
});
});
// ── Node-neutral failure alerts (local vs remote) ──────────────────────
describe('SchedulerService - node-neutral failure alerts', () => {
function makeContainerTask(overrides: Partial<ScheduledTask> & { id: number; name: string }) {
return {
action: 'restart',
target_type: 'container',
target_id: 'web',
cron_expression: '0 * * * *',
enabled: true,
node_id: 1,
created_by: 'admin',
last_status: null,
...overrides,
};
}
it('dispatches local offline failure without the seed node name', async () => {
mockGetScheduledTask.mockReturnValue(makeContainerTask({ id: 400, name: 'offline-local', node_id: 1 }));
mockGetNode.mockReturnValue({ id: 1, name: 'Local', type: 'local', status: 'offline' });
await SchedulerService.getInstance().triggerTask(400);
expect(mockDispatchAlert).toHaveBeenCalledWith(
'error',
'system',
'Scheduled task "offline-local" (restart) failed: Target node is offline',
{ stackName: 'web', actor: 'system:scheduler' },
);
});
it('dispatches remote offline failure with the authoritative node name', async () => {
mockGetScheduledTask.mockReturnValue(makeContainerTask({ id: 401, name: 'offline-remote', node_id: 2 }));
mockGetNode.mockReturnValue({ id: 2, name: 'sencho-test-02', type: 'remote', status: 'offline' });
await SchedulerService.getInstance().triggerTask(401);
expect(mockDispatchAlert).toHaveBeenCalledWith(
'error',
'system',
'Scheduled task "offline-remote" (restart) failed: Target node "sencho-test-02" is offline',
{ stackName: 'web', actor: 'system:scheduler' },
);
});
it('dispatches local missing-container failure with this-node wording', async () => {
mockGetScheduledTask.mockReturnValue(makeContainerTask({ id: 402, name: 'missing-local', node_id: 1 }));
mockGetNode.mockReturnValue({ id: 1, name: 'Local', type: 'local', status: 'online' });
mockFindContainerByName.mockResolvedValue(null);
await SchedulerService.getInstance().triggerTask(402);
expect(mockDispatchAlert).toHaveBeenCalledWith(
'error',
'system',
'Scheduled task "missing-local" (restart) failed: Container "web" not found on this node. It may have been renamed or removed.',
{ stackName: 'web', actor: 'system:scheduler' },
);
});
it('dispatches remote missing-container failure with the authoritative node name', async () => {
mockGetScheduledTask.mockReturnValue(makeContainerTask({ id: 403, name: 'missing-remote', node_id: 2 }));
mockGetNode.mockReturnValue({ id: 2, name: 'sencho-test-02', type: 'remote', status: 'online' });
mockGetProxyTarget.mockReturnValue({
apiUrl: 'http://remote:1852',
apiToken: 'test-token',
});
const mockFetch = vi.fn().mockResolvedValue({
ok: true,
json: async () => ([]),
});
vi.stubGlobal('fetch', mockFetch);
await SchedulerService.getInstance().triggerTask(403);
expect(mockDispatchAlert).toHaveBeenCalledWith(
'error',
'system',
'Scheduled task "missing-remote" (restart) failed: Container "web" not found on node "sencho-test-02". It may have been renamed or removed.',
{ stackName: 'web', actor: 'system:scheduler' },
);
});
it('missing node lookup keeps the id fallback, not this-node wording', () => {
mockGetNode.mockReturnValue(undefined);
const svc = SchedulerService.getInstance() as unknown as {
containerNotFoundMessage: (containerName: string, nodeId: number) => string;
};
expect(svc.containerNotFoundMessage('web', 99)).toBe(
'Container "web" not found on node "99". It may have been renamed or removed.',
);
});
});