From bfeac07c4c38fb55db99d5f8204e85ec54552564 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Sat, 1 Mar 2025 21:58:49 +0100 Subject: [PATCH] fix: Implement systemd-notify directly (#26456) Co-authored-by: Nerivec <62446222+Nerivec@users.noreply.github.com> Co-authored-by: Koen Kanters --- .github/workflows/ci.yml | 4 +- lib/controller.ts | 25 ++---- lib/types/unix-dgram.d.ts | 14 +++ lib/util/sd-notify.ts | 68 +++++++++++++++ package.json | 4 +- pnpm-lock.yaml | 24 +----- test/controller.test.ts | 33 +++++-- test/sd-notify.test.ts | 175 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 293 insertions(+), 54 deletions(-) create mode 100644 lib/types/unix-dgram.d.ts create mode 100644 lib/util/sd-notify.ts create mode 100644 test/sd-notify.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64a4388df..016a33da6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -188,8 +188,8 @@ jobs: cache: pnpm - name: Install dependencies - # --ignore-scripts prevents the serialport build which often fails on Windows - run: pnpm i --frozen-lockfile --ignore-scripts + # --ignore-scripts prevents build on Windows (only for unix-dgram, so doesn't matter, others have pre-builds) + run: pnpm i --frozen-lockfile ${{ matrix.os == 'windows-latest' && '--ignore-scripts' || '' }} - name: Build run: pnpm run build diff --git a/lib/controller.ts b/lib/controller.ts index 0a86ff7bb..3be8cd555 100644 --- a/lib/controller.ts +++ b/lib/controller.ts @@ -1,5 +1,4 @@ import type {IClientPublishOptions} from 'mqtt'; -import type * as SdNotify from 'sd-notify'; import type {Zigbee2MQTTAPI} from './types/api'; @@ -30,12 +29,11 @@ import ExtensionReceive from './extension/receive'; import MQTT from './mqtt'; import State from './state'; import logger from './util/logger'; +import {initSdNotify} from './util/sd-notify'; import * as settings from './util/settings'; import utils from './util/utils'; import Zigbee from './zigbee'; -type SdNotifyType = typeof SdNotify; - const AllExtensions = [ ExtensionPublish, ExtensionReceive, @@ -73,7 +71,7 @@ export class Controller { private exitCallback: (code: number, restart: boolean) => Promise; private extensions: Extension[]; private extensionArgs: ExtensionArgs; - private sdNotify: SdNotifyType | undefined; + private sdNotify: Awaited>; constructor(restartCallback: () => Promise, exitCallback: (code: number, restart: boolean) => Promise) { logger.init(); @@ -128,15 +126,6 @@ export class Controller { const info = await utils.getZigbee2MQTTVersion(); logger.info(`Starting Zigbee2MQTT version ${info.version} (commit #${info.commitHash})`); - try { - this.sdNotify = process.env.NOTIFY_SOCKET ? await import('sd-notify') : undefined; - logger.debug('sd-notify loaded'); - /* v8 ignore start */ - } catch { - logger.debug('sd-notify is not installed'); - } - /* v8 ignore stop */ - // Start zigbee try { await this.zigbee.start(); @@ -198,11 +187,7 @@ export class Controller { logger.info(`Zigbee2MQTT started!`); - const watchdogInterval = this.sdNotify?.watchdogInterval() || 0; - if (watchdogInterval > 0) { - this.sdNotify?.startWatchdogMode(Math.floor(watchdogInterval / 2)); - } - this.sdNotify?.ready(); + this.sdNotify = await initSdNotify(); } @bind async enableDisableExtension(enable: boolean, name: string): Promise { @@ -227,7 +212,7 @@ export class Controller { } async stop(restart = false): Promise { - this.sdNotify?.stopping(process.pid); + this.sdNotify?.notifyStopping(); // Call extensions await this.callExtensions('stop', this.extensions); @@ -246,7 +231,7 @@ export class Controller { code = 1; } - this.sdNotify?.stopWatchdogMode(); + this.sdNotify?.stop(); return await this.exit(code, restart); } diff --git a/lib/types/unix-dgram.d.ts b/lib/types/unix-dgram.d.ts new file mode 100644 index 000000000..52b7ff5e3 --- /dev/null +++ b/lib/types/unix-dgram.d.ts @@ -0,0 +1,14 @@ +declare module 'unix-dgram' { + import {EventEmitter} from 'events'; + import {Buffer} from 'buffer'; + + export class UnixDgramSocket extends EventEmitter { + send(buf: Buffer, callback?: (err?: Error) => void): void; + send(buf: Buffer, offset: number, length: number, path: string, callback?: (err?: Error) => void): void; + bind(path: string): void; + connect(remotePath: string): void; + close(): void; + } + + export function createSocket(type: 'unix_dgram', listener?: (msg: Buffer) => void): UnixDgramSocket; +} diff --git a/lib/util/sd-notify.ts b/lib/util/sd-notify.ts new file mode 100644 index 000000000..aea588ff9 --- /dev/null +++ b/lib/util/sd-notify.ts @@ -0,0 +1,68 @@ +import type {UnixDgramSocket} from 'unix-dgram'; + +import {platform} from 'node:os'; + +import logger from './logger'; + +/** + * Handle sd_notify protocol, @see https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html + * No-op if running on unsupported platforms or without Type=notify + * Soft-fails if improperly setup (this is not necessary for Zigbee2MQTT to function properly) + */ +export async function initSdNotify(): Promise<{notifyStopping: () => void; stop: () => void} | undefined> { + if (!process.env.NOTIFY_SOCKET) { + return; + } + + let socket: UnixDgramSocket | undefined; + + try { + const {createSocket} = await import('unix-dgram'); + socket = createSocket('unix_dgram'); + } catch (error) { + if (platform() !== 'win32' || process.env.WSL_DISTRO_NAME) { + // not on plain Windows + logger.error(`Could not init sd_notify: ${(error as Error).message}`); + logger.debug((error as Error).stack!); + } else { + // this should not happen + logger.warning(`NOTIFY_SOCKET env is set: ${(error as Error).message}`); + } + + return; + } + + const sendToSystemd = (msg: string): void => { + const buffer = Buffer.from(msg); + + socket.send(buffer, 0, buffer.byteLength, process.env.NOTIFY_SOCKET!, (err) => { + if (err) { + logger.warning(`Failed to send "${msg}" to systemd: ${err.message}`); + } + }); + }; + const notifyStopping = (): void => sendToSystemd('STOPPING=1'); + + sendToSystemd('READY=1'); + + const wdUSec = process.env.WATCHDOG_USEC !== undefined ? Math.max(0, parseInt(process.env.WATCHDOG_USEC, 10)) : -1; + + if (wdUSec > 0) { + // Convert us to ms, send twice as frequently as the timeout + const watchdogInterval = setInterval(() => sendToSystemd('WATCHDOG=1'), wdUSec / 1000 / 2); + + return { + notifyStopping, + stop: (): void => clearInterval(watchdogInterval), + }; + } + + if (wdUSec !== -1) { + logger.warning(`WATCHDOG_USEC invalid: "${process.env.WATCHDOG_USEC}", parsed to "${wdUSec}"`); + } + + return { + notifyStopping, + stop: (): void => {}, + }; +} diff --git a/package.json b/package.json index 0afaf7988..597d42326 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,6 @@ "@types/node": "^22.13.5", "@types/object-assign-deep": "^0.4.3", "@types/readable-stream": "4.0.18", - "@types/sd-notify": "^2.8.2", "@types/serve-static": "^1.15.7", "@types/ws": "8.5.14", "@vitest/coverage-v8": "^3.0.7", @@ -95,7 +94,6 @@ "onlyBuiltDependencies": [ "@serialport/bindings-cpp", "esbuild", - "sd-notify", "unix-dgram" ] }, @@ -103,6 +101,6 @@ "zigbee2mqtt": "cli.js" }, "optionalDependencies": { - "sd-notify": "^2.8.0" + "unix-dgram": "^2.0.6" } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d232d8730..9d8801110 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -114,9 +114,6 @@ importers: '@types/readable-stream': specifier: 4.0.18 version: 4.0.18 - '@types/sd-notify': - specifier: ^2.8.2 - version: 2.8.2 '@types/serve-static': specifier: ^1.15.7 version: 1.15.7 @@ -148,9 +145,9 @@ importers: specifier: ^3.0.7 version: 3.0.7(@types/node@22.13.5) optionalDependencies: - sd-notify: - specifier: ^2.8.0 - version: 2.8.0 + unix-dgram: + specifier: ^2.0.6 + version: 2.0.6 packages: @@ -609,9 +606,6 @@ packages: '@types/readable-stream@4.0.18': resolution: {integrity: sha512-21jK/1j+Wg+7jVw1xnSwy/2Q1VgVjWuFssbYGTREPUBeZ+rqVFl2udq0IkxzPC0ZhOzVceUbyIACFZKLqKEBlA==} - '@types/sd-notify@2.8.2': - resolution: {integrity: sha512-LVWtuGvzso9z3N89NISzseq8RVHkEeg2h275370yQYx8/CoNaV2NnG17TTjDavy2FrmcUBFaR6OymlPQjqfb2g==} - '@types/send@0.17.4': resolution: {integrity: sha512-x2EM6TJOybec7c52BX0ZspPodMsQUd5L6PRwOunVyVUhXiBSKf3AezDL8Dgvgt5o0UfKNfuA0eMLr2wLT4AiBA==} @@ -1505,11 +1499,6 @@ packages: safer-buffer@2.1.2: resolution: {integrity: sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==} - sd-notify@2.8.0: - resolution: {integrity: sha512-e+D1v0Y6UzmqXcPlaTkHk1QMdqk36mF/jIYv5gwry/N2Tb8/UNnpfG6ktGLpeBOR6TCC5hPKgqA+0hTl9sm2tA==} - engines: {node: '>=8.0.0'} - os: [linux, darwin, win32] - semver@7.7.1: resolution: {integrity: sha512-hlq8tAfn0m/61p4BVRcPzIGr6LKiMwo4VM6dGi6pt4qcRkmNzTcWq6eCEjEh+qXjkMDvPlOFFSGwQjoEa6gyMA==} engines: {node: '>=10'} @@ -2206,8 +2195,6 @@ snapshots: '@types/node': 22.13.5 safe-buffer: 5.1.2 - '@types/sd-notify@2.8.2': {} - '@types/send@0.17.4': dependencies: '@types/mime': 1.3.5 @@ -3182,11 +3169,6 @@ snapshots: safer-buffer@2.1.2: {} - sd-notify@2.8.0: - dependencies: - bindings: 1.5.0 - optional: true - semver@7.7.1: {} send@0.19.0: diff --git a/test/controller.test.ts b/test/controller.test.ts index c04ee4340..560c42c87 100644 --- a/test/controller.test.ts +++ b/test/controller.test.ts @@ -14,6 +14,7 @@ import {devices, mockController as mockZHController, events as mockZHEvents, ret import type {Mock, MockInstance} from 'vitest'; import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import stringify from 'json-stable-stringify-without-jsonify'; @@ -24,15 +25,12 @@ import {Controller as ZHController} from 'zigbee-herdsman'; import {Controller} from '../lib/controller'; import * as settings from '../lib/util/settings'; -process.env.NOTIFY_SOCKET = 'mocked'; const LOG_MQTT_NS = 'z2m:mqtt'; -vi.mock('sd-notify', () => ({ - watchdogInterval: vi.fn(() => 3000), - startWatchdogMode: vi.fn(), - stopWatchdogMode: vi.fn(), - ready: vi.fn(), - stopping: vi.fn(), +const mockUnixDgramSend = vi.fn(); + +vi.mock('unix-dgram', () => ({ + createSocket: vi.fn(() => ({send: mockUnixDgramSend})), })); const mocksClear = [ @@ -49,6 +47,7 @@ const mocksClear = [ mockLogger.debug, mockLogger.info, mockLogger.error, + mockUnixDgramSend, ]; describe('Controller', () => { @@ -338,7 +337,7 @@ describe('Controller', () => { expect(mockExit).toHaveBeenCalledWith(0, true); }); - it('Start controller and stop', async () => { + it('Start controller and stop without SdNotify', async () => { mockZHController.stop.mockRejectedValueOnce('failed'); await controller.start(); await controller.stop(); @@ -346,6 +345,24 @@ describe('Controller', () => { expect(mockZHController.stop).toHaveBeenCalledTimes(1); expect(mockExit).toHaveBeenCalledTimes(1); expect(mockExit).toHaveBeenCalledWith(1, false); + expect(mockUnixDgramSend).toHaveBeenCalledTimes(0); + }); + + it('Start controller and stop with SdNotify', async () => { + vi.spyOn(os, 'platform').mockImplementationOnce(() => 'linux'); + + process.env.NOTIFY_SOCKET = 'mocked'; // coverage + + mockZHController.stop.mockRejectedValueOnce('failed'); + await controller.start(); + await controller.stop(); + expect(mockMQTTEndAsync).toHaveBeenCalledTimes(1); + expect(mockZHController.stop).toHaveBeenCalledTimes(1); + expect(mockExit).toHaveBeenCalledTimes(1); + expect(mockExit).toHaveBeenCalledWith(1, false); + expect(mockUnixDgramSend).toHaveBeenCalledTimes(2); + + delete process.env.NOTIFY_SOCKET; }); it('Start controller adapter disconnects', async () => { diff --git a/test/sd-notify.test.ts b/test/sd-notify.test.ts new file mode 100644 index 000000000..727a2b16f --- /dev/null +++ b/test/sd-notify.test.ts @@ -0,0 +1,175 @@ +import {mockLogger} from './mocks/logger'; + +import {initSdNotify} from '../lib/util/sd-notify'; + +const mockPlatform = vi.fn(() => 'linux'); + +vi.mock('node:os', () => ({ + platform: vi.fn(() => mockPlatform()), +})); + +const mockUnixDgramSocket = { + send: vi.fn(), +}; +const mockCreateSocket = vi.fn(() => { + if (mockPlatform() !== 'win32') { + return mockUnixDgramSocket; + } + + throw new Error('Unix datagrams not available on this platform'); +}); + +vi.mock('unix-dgram', () => ({ + createSocket: mockCreateSocket, +})); + +const mocksClear = [ + mockLogger.log, + mockLogger.debug, + mockLogger.info, + mockLogger.warning, + mockLogger.error, + mockUnixDgramSocket.send, + mockCreateSocket, + mockPlatform, +]; + +describe('sd-notify', () => { + const expectSocketNthSend = (nth: number, message: string): void => { + expect(mockUnixDgramSocket.send).toHaveBeenNthCalledWith(nth, Buffer.from(message), 0, expect.any(Number), 'mocked', expect.any(Function)); + }; + + beforeAll(async () => { + vi.useFakeTimers(); + }); + + afterAll(async () => { + vi.useRealTimers(); + }); + + beforeEach(() => { + mocksClear.forEach((m) => m.mockClear()); + delete process.env.NOTIFY_SOCKET; + delete process.env.WATCHDOG_USEC; + delete process.env.WSL_DISTRO_NAME; + }); + + it('No socket', async () => { + const res = await initSdNotify(); + + expect(mockCreateSocket).toHaveBeenCalledTimes(0); + expect(res).toBeUndefined(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(0); + }); + + it('Error on unsupported platform', async () => { + // also called by `mockCreateSocket` + mockPlatform.mockImplementationOnce(() => 'win32').mockImplementationOnce(() => 'win32'); + + process.env.NOTIFY_SOCKET = 'mocked'; + const res = await initSdNotify(); + + expect(mockCreateSocket).toHaveBeenCalledTimes(1); + expect(res).toBeUndefined(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(0); + expect(mockLogger.warning).toHaveBeenCalledWith(`NOTIFY_SOCKET env is set: Unix datagrams not available on this platform`); + }); + + it('Error on supported platform', async () => { + // NOTE: `import('unix-dgram')` can also fail in similar way when bindings are missing (not compiled) + mockCreateSocket.mockImplementationOnce(() => { + throw new Error('Error create socket'); + }); + + process.env.NOTIFY_SOCKET = 'mocked'; + const res = await initSdNotify(); + + expect(mockCreateSocket).toHaveBeenCalledTimes(1); + expect(res).toBeUndefined(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(0); + expect(mockLogger.error).toHaveBeenCalledWith('Could not init sd_notify: Error create socket'); + }); + + it('Socket only', async () => { + process.env.NOTIFY_SOCKET = 'mocked'; + const res = await initSdNotify(); + + expect(res).toStrictEqual({notifyStopping: expect.any(Function), stop: expect.any(Function)}); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(1); + expectSocketNthSend(1, 'READY=1'); + + await vi.advanceTimersByTimeAsync(7500); + + res!.notifyStopping(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(2); + expectSocketNthSend(2, 'STOPPING=1'); + + res!.stop(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(2); + }); + + it('Invalid watchdog timeout - socket only', async () => { + process.env.NOTIFY_SOCKET = 'mocked'; + process.env.WATCHDOG_USEC = 'mocked'; + const res = await initSdNotify(); + + expect(res).toStrictEqual({notifyStopping: expect.any(Function), stop: expect.any(Function)}); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(1); + expectSocketNthSend(1, 'READY=1'); + + await vi.advanceTimersByTimeAsync(7500); + + res!.notifyStopping(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(2); + expectSocketNthSend(2, 'STOPPING=1'); + + res!.stop(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(2); + }); + + it('Socket and watchdog', async () => { + process.env.NOTIFY_SOCKET = 'mocked'; + process.env.WATCHDOG_USEC = '10000000'; + const res = await initSdNotify(); + + expect(res).toStrictEqual({notifyStopping: expect.any(Function), stop: expect.any(Function)}); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(1); + expectSocketNthSend(1, 'READY=1'); + + await vi.advanceTimersByTimeAsync(7500); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(2); + expectSocketNthSend(2, 'WATCHDOG=1'); + + res!.notifyStopping(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(3); + expectSocketNthSend(3, 'STOPPING=1'); + + await vi.advanceTimersByTimeAsync(6000); + + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(4); + expectSocketNthSend(4, 'WATCHDOG=1'); + + res!.stop(); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(4); + + await vi.advanceTimersByTimeAsync(10000); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(4); + }); + + it('Fails to send', async () => { + mockUnixDgramSocket.send.mockImplementationOnce( + (buf: Buffer, offset: number, length: number, path: string, callback?: (err?: Error) => void) => { + callback!(new Error('Failure')); + }, + ); + + process.env.NOTIFY_SOCKET = 'mocked'; + const res = await initSdNotify(); + + expect(res).toStrictEqual({notifyStopping: expect.any(Function), stop: expect.any(Function)}); + expect(mockUnixDgramSocket.send).toHaveBeenCalledTimes(1); + expectSocketNthSend(1, 'READY=1'); + + expect(mockLogger.warning).toHaveBeenCalledWith(`Failed to send "READY=1" to systemd: Failure`); + }); +});