From d7e129aba4a1cb8aa5e17a196cde7495988b1319 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 8 Dec 2025 10:06:12 -0500 Subject: [PATCH 1/3] fix: keyfile watcher to poll instead of inotify on FAT32 --- .../store/watch/registration-watch.test.ts | 168 ++++++++++++++++++ api/src/store/watch/registration-watch.ts | 44 ++++- 2 files changed, 207 insertions(+), 5 deletions(-) create mode 100644 api/src/__test__/store/watch/registration-watch.test.ts diff --git a/api/src/__test__/store/watch/registration-watch.test.ts b/api/src/__test__/store/watch/registration-watch.test.ts new file mode 100644 index 0000000000..746ab3539b --- /dev/null +++ b/api/src/__test__/store/watch/registration-watch.test.ts @@ -0,0 +1,168 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { RegistrationType } from '@app/unraid-api/graph/resolvers/registration/registration.model.js'; + +// Mock the store module +vi.mock('@app/store/index.js', () => ({ + store: { + dispatch: vi.fn(), + }, + getters: { + emhttp: vi.fn(), + }, +})); + +// Mock the emhttp module +vi.mock('@app/store/modules/emhttp.js', () => ({ + loadSingleStateFile: vi.fn((key) => ({ type: 'emhttp/load-single-state-file', payload: key })), +})); + +// Mock the registration module +vi.mock('@app/store/modules/registration.js', () => ({ + loadRegistrationKey: vi.fn(() => ({ type: 'registration/load-registration-key' })), +})); + +// Mock the logger +vi.mock('@app/core/log.js', () => ({ + keyServerLogger: { + info: vi.fn(), + debug: vi.fn(), + }, +})); + +describe('reloadVarIniWithRetry', () => { + let store: { dispatch: ReturnType }; + let getters: { emhttp: ReturnType }; + let loadSingleStateFile: ReturnType; + + beforeEach(async () => { + vi.useFakeTimers(); + + const storeModule = await import('@app/store/index.js'); + const emhttpModule = await import('@app/store/modules/emhttp.js'); + + store = storeModule.store as unknown as typeof store; + getters = storeModule.getters as unknown as typeof getters; + loadSingleStateFile = emhttpModule.loadSingleStateFile as unknown as typeof loadSingleStateFile; + + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('returns early when registration state changes on first retry', async () => { + // Initial state is TRIAL + getters.emhttp + .mockReturnValueOnce({ var: { regTy: RegistrationType.TRIAL } }) // First call (beforeState) + .mockReturnValueOnce({ var: { regTy: RegistrationType.UNLEASHED } }); // After first reload + + const { reloadVarIniWithRetry } = await import('@app/store/watch/registration-watch.js'); + + const promise = reloadVarIniWithRetry(); + + // Advance past the first delay (500ms) + await vi.advanceTimersByTimeAsync(500); + await promise; + + // Should only dispatch once since state changed + expect(store.dispatch).toHaveBeenCalledTimes(1); + expect(loadSingleStateFile).toHaveBeenCalledWith('var'); + }); + + it('retries up to maxRetries when state does not change', async () => { + // State never changes + getters.emhttp.mockReturnValue({ var: { regTy: RegistrationType.TRIAL } }); + + const { reloadVarIniWithRetry } = await import('@app/store/watch/registration-watch.js'); + + const promise = reloadVarIniWithRetry(3); + + // Advance through all retries: 500ms, 1000ms, 2000ms + await vi.advanceTimersByTimeAsync(500); + await vi.advanceTimersByTimeAsync(1000); + await vi.advanceTimersByTimeAsync(2000); + await promise; + + // Should dispatch 3 times (maxRetries) + expect(store.dispatch).toHaveBeenCalledTimes(3); + }); + + it('stops retrying when state changes on second attempt', async () => { + getters.emhttp + .mockReturnValueOnce({ var: { regTy: RegistrationType.TRIAL } }) // beforeState + .mockReturnValueOnce({ var: { regTy: RegistrationType.TRIAL } }) // After first reload (no change) + .mockReturnValueOnce({ var: { regTy: RegistrationType.UNLEASHED } }); // After second reload (changed!) + + const { reloadVarIniWithRetry } = await import('@app/store/watch/registration-watch.js'); + + const promise = reloadVarIniWithRetry(3); + + // First retry + await vi.advanceTimersByTimeAsync(500); + // Second retry + await vi.advanceTimersByTimeAsync(1000); + await promise; + + // Should dispatch twice - stopped after state changed + expect(store.dispatch).toHaveBeenCalledTimes(2); + }); + + it('handles undefined regTy gracefully', async () => { + getters.emhttp.mockReturnValue({ var: {} }); + + const { reloadVarIniWithRetry } = await import('@app/store/watch/registration-watch.js'); + + const promise = reloadVarIniWithRetry(1); + + await vi.advanceTimersByTimeAsync(500); + await promise; + + // Should still dispatch even with undefined regTy + expect(store.dispatch).toHaveBeenCalledTimes(1); + }); + + it('uses exponential backoff delays', async () => { + getters.emhttp.mockReturnValue({ var: { regTy: RegistrationType.TRIAL } }); + + const { reloadVarIniWithRetry } = await import('@app/store/watch/registration-watch.js'); + + const promise = reloadVarIniWithRetry(3); + + // At 0ms, no dispatch yet + expect(store.dispatch).toHaveBeenCalledTimes(0); + + // At 500ms, first dispatch + await vi.advanceTimersByTimeAsync(500); + expect(store.dispatch).toHaveBeenCalledTimes(1); + + // At 1500ms (500 + 1000), second dispatch + await vi.advanceTimersByTimeAsync(1000); + expect(store.dispatch).toHaveBeenCalledTimes(2); + + // At 3500ms (500 + 1000 + 2000), third dispatch + await vi.advanceTimersByTimeAsync(2000); + expect(store.dispatch).toHaveBeenCalledTimes(3); + + await promise; + }); +}); + +describe('setupRegistrationKeyWatch', () => { + it('is configured with polling enabled for FAT32 compatibility', async () => { + // This is a design/documentation test - verifies the watcher is configured correctly + // The actual file watching behavior can't easily be tested in unit tests + + const fs = await import('fs'); + const path = await import('path'); + const watchFilePath = path.join(process.cwd(), 'src/store/watch/registration-watch.ts'); + + const fileContent = fs.readFileSync(watchFilePath, 'utf-8'); + + // Verify polling is always enabled (not dependent on env var) + expect(fileContent).toContain('usePolling: true'); + // Verify there's a comment explaining why + expect(fileContent).toMatch(/FAT32|inotify/i); + }); +}); diff --git a/api/src/store/watch/registration-watch.ts b/api/src/store/watch/registration-watch.ts index da63a03d2d..2aeeffb932 100644 --- a/api/src/store/watch/registration-watch.ts +++ b/api/src/store/watch/registration-watch.ts @@ -1,17 +1,51 @@ import { watch } from 'chokidar'; -import { CHOKIDAR_USEPOLLING } from '@app/environment.js'; -import { store } from '@app/store/index.js'; +import { keyServerLogger } from '@app/core/log.js'; +import { getters, store } from '@app/store/index.js'; +import { loadSingleStateFile } from '@app/store/modules/emhttp.js'; import { loadRegistrationKey } from '@app/store/modules/registration.js'; +import { StateFileKey } from '@app/store/types.js'; + +/** + * Reloads var.ini with retry logic to handle timing issues with emhttpd. + * When a key file changes, emhttpd needs time to process it and update var.ini. + * This function retries loading var.ini until the registration state changes + * or max retries are exhausted. + */ +export const reloadVarIniWithRetry = async (maxRetries = 3): Promise => { + const beforeState = getters.emhttp().var?.regTy; + + for (let attempt = 0; attempt < maxRetries; attempt++) { + const delay = 500 * Math.pow(2, attempt); // 500ms, 1s, 2s + await new Promise((resolve) => setTimeout(resolve, delay)); + + await store.dispatch(loadSingleStateFile(StateFileKey.var)); + + const afterState = getters.emhttp().var?.regTy; + if (beforeState !== afterState) { + keyServerLogger.info('Registration state updated: %s -> %s', beforeState, afterState); + return; + } + keyServerLogger.debug('Retry %d: var.ini regTy still %s', attempt + 1, afterState); + } + keyServerLogger.debug('var.ini regTy unchanged after %d retries (may be expected)', maxRetries); +}; export const setupRegistrationKeyWatch = () => { + // IMPORTANT: /boot/config is on FAT32 flash drive which does NOT support inotify + // Must use polling to detect file changes on FAT32 filesystems watch('/boot/config', { persistent: true, ignoreInitial: true, ignored: (path: string) => !path.endsWith('.key'), - usePolling: CHOKIDAR_USEPOLLING === true, - }).on('all', async () => { - // Load updated key into store + usePolling: true, // Required for FAT32 - inotify doesn't work + interval: 5000, // Poll every 5 seconds (balance between responsiveness and CPU usage) + }).on('all', async (event, path) => { + keyServerLogger.info('Key file %s: %s', event, path); + await store.dispatch(loadRegistrationKey()); + + // Reload var.ini to get updated registration metadata from emhttpd + await reloadVarIniWithRetry(); }); }; From a4bea906dc71ba11c4e5fa7c94133efa87c5a913 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 8 Dec 2025 10:16:40 -0500 Subject: [PATCH 2/3] remove the craziest test case i've seen in my lifetime --- .../store/watch/registration-watch.test.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/api/src/__test__/store/watch/registration-watch.test.ts b/api/src/__test__/store/watch/registration-watch.test.ts index 746ab3539b..42f1157bb5 100644 --- a/api/src/__test__/store/watch/registration-watch.test.ts +++ b/api/src/__test__/store/watch/registration-watch.test.ts @@ -148,21 +148,3 @@ describe('reloadVarIniWithRetry', () => { await promise; }); }); - -describe('setupRegistrationKeyWatch', () => { - it('is configured with polling enabled for FAT32 compatibility', async () => { - // This is a design/documentation test - verifies the watcher is configured correctly - // The actual file watching behavior can't easily be tested in unit tests - - const fs = await import('fs'); - const path = await import('path'); - const watchFilePath = path.join(process.cwd(), 'src/store/watch/registration-watch.ts'); - - const fileContent = fs.readFileSync(watchFilePath, 'utf-8'); - - // Verify polling is always enabled (not dependent on env var) - expect(fileContent).toContain('usePolling: true'); - // Verify there's a comment explaining why - expect(fileContent).toMatch(/FAT32|inotify/i); - }); -}); From ebdb0136af42ae58b6f9276b0410f29aa1e03246 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Mon, 8 Dec 2025 10:48:24 -0500 Subject: [PATCH 3/3] assert with enum instead of plain string --- api/src/__test__/store/watch/registration-watch.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/__test__/store/watch/registration-watch.test.ts b/api/src/__test__/store/watch/registration-watch.test.ts index 42f1157bb5..204fa5a79b 100644 --- a/api/src/__test__/store/watch/registration-watch.test.ts +++ b/api/src/__test__/store/watch/registration-watch.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { StateFileKey } from '@app/store/types.js'; import { RegistrationType } from '@app/unraid-api/graph/resolvers/registration/registration.model.js'; // Mock the store module @@ -68,7 +69,7 @@ describe('reloadVarIniWithRetry', () => { // Should only dispatch once since state changed expect(store.dispatch).toHaveBeenCalledTimes(1); - expect(loadSingleStateFile).toHaveBeenCalledWith('var'); + expect(loadSingleStateFile).toHaveBeenCalledWith(StateFileKey.var); }); it('retries up to maxRetries when state does not change', async () => {