Skip to content

Commit 9e0e89e

Browse files
elibosleyelibosley
authored
fix(onboarding): surface drive warnings for internal boot (#1963)
## Summary - replace the internal boot "already configured" hard stop with structured `driveWarnings` on `internalBootContext` - show selected-drive warnings in the onboarding UI while keeping warning drives selectable - sync Redux into Nest `ConfigService` immediately so `assignableDisks` matches fresh `devs.ini` state without a stale debounce window ## Why - onboarding needed to warn about internal boot partitions instead of blocking the flow outright - `ConfigService` could lag Redux after a synchronous `devs.ini` refresh, which let assigned drives from stale state leak into `assignableDisks` - the endpoint should reflect `devs.ini` exactly and treat internal boot partitions as warnings, not implicit exclusion ## Testing - `pnpm --filter ./api test src/unraid-api/config/store-sync.service.spec.ts src/unraid-api/graph/resolvers/disks/disks.service.spec.ts src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.spec.ts src/unraid-api/graph/resolvers/onboarding/onboarding.query.spec.ts src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.ts` - `pnpm test /Users/elibosley/Code/api/web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts /Users/elibosley/Code/api/web/__test__/components/Onboarding/OnboardingModal.test.ts /Users/elibosley/Code/api/web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts` - deployed API changes to `devgen-dev1.local` and verified `internalBootContext.assignableDisks` matched `devs.ini` instead of `disks.ini` - probed `createInternalBootPool` on `devgen-dev1.local` and confirmed the old "internal boot is already configured" short-circuit no longer triggers <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added drive warning indicators to identify selected drives that already contain internal boot partitions. * Enhanced onboarding interface with amber alerts displaying warnings for drives with existing internal boot data. * **Improvements** * Refined internal boot configuration validation logic to support additional setup scenarios while preserving safety checks. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent c30a0c0 commit 9e0e89e

18 files changed

+242
-123
lines changed

‎api/generated-schema.graphql‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,6 +1078,13 @@ type OnboardingInternalBootResult {
10781078
output: String!
10791079
}
10801080

1081+
"""Warning metadata for an assignable internal boot drive"""
1082+
type OnboardingInternalBootDriveWarning {
1083+
diskId: String!
1084+
device: String!
1085+
warnings: [String!]!
1086+
}
1087+
10811088
"""Current onboarding context for configuring internal boot"""
10821089
type OnboardingInternalBootContext {
10831090
arrayStopped: Boolean!
@@ -1088,6 +1095,7 @@ type OnboardingInternalBootContext {
10881095
shareNames: [String!]!
10891096
poolNames: [String!]!
10901097
assignableDisks: [Disk!]!
1098+
driveWarnings: [OnboardingInternalBootDriveWarning!]!
10911099
}
10921100

10931101
type RCloneDrive {

‎api/src/unraid-api/config/store-sync.service.spec.ts‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ describe('StoreSyncService', () => {
2424
let subscriber: (() => void) | undefined;
2525

2626
beforeEach(() => {
27-
vi.useFakeTimers();
2827
subscribe.mockReset();
2928
getState.mockReset();
3029

@@ -38,50 +37,52 @@ describe('StoreSyncService', () => {
3837

3938
configService = new ConfigService();
4039
setSpy = vi.spyOn(configService, 'set');
41-
42-
service = new StoreSyncService(configService);
4340
});
4441

4542
afterEach(() => {
46-
vi.runOnlyPendingTimers();
47-
vi.useRealTimers();
43+
service?.onModuleDestroy();
4844
});
4945

50-
it('debounces sync operations and writes once after rapid updates', () => {
51-
getState.mockReturnValue({ count: 2 });
46+
it('syncs the current store state immediately on construction', () => {
47+
getState.mockReturnValue({ count: 1 });
5248

53-
subscriber?.();
54-
vi.advanceTimersByTime(500);
55-
subscriber?.();
49+
service = new StoreSyncService(configService);
5650

57-
expect(setSpy).not.toHaveBeenCalled();
51+
expect(setSpy).toHaveBeenCalledTimes(1);
52+
expect(setSpy).toHaveBeenCalledWith('store', { count: 1 });
53+
});
5854

59-
vi.advanceTimersByTime(1000);
55+
it('writes immediately when the store updates', () => {
56+
getState.mockReturnValue({ count: 1 });
57+
service = new StoreSyncService(configService);
58+
setSpy.mockClear();
59+
getState.mockReturnValue({ count: 2 });
60+
61+
subscriber?.();
6062

6163
expect(setSpy).toHaveBeenCalledTimes(1);
6264
expect(setSpy).toHaveBeenCalledWith('store', { count: 2 });
6365
});
6466

6567
it('skips writes when serialized state is unchanged', () => {
6668
getState.mockReturnValue({ count: 1 });
69+
service = new StoreSyncService(configService);
70+
setSpy.mockClear();
71+
getState.mockReturnValue({ count: 2 });
6772

6873
subscriber?.();
69-
vi.advanceTimersByTime(1000);
70-
74+
getState.mockReturnValue({ count: 2 });
7175
subscriber?.();
72-
vi.advanceTimersByTime(1000);
7376

7477
expect(setSpy).toHaveBeenCalledTimes(1);
7578
});
7679

77-
it('unsubscribes and clears timer on module destroy', () => {
80+
it('unsubscribes on module destroy', () => {
7881
getState.mockReturnValue({ count: 1 });
79-
80-
subscriber?.();
82+
service = new StoreSyncService(configService);
83+
setSpy.mockClear();
8184
service.onModuleDestroy();
82-
vi.advanceTimersByTime(1000);
8385

8486
expect(unsubscribe).toHaveBeenCalledTimes(1);
85-
expect(setSpy).not.toHaveBeenCalled();
8687
});
8788
});

‎api/src/unraid-api/config/store-sync.service.ts‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,46 +5,32 @@ import type { Unsubscribe } from '@reduxjs/toolkit';
55

66
import { store } from '@app/store/index.js';
77

8-
const STORE_SYNC_DEBOUNCE_MS = 1000;
9-
108
@Injectable()
119
export class StoreSyncService implements OnModuleDestroy {
1210
private unsubscribe: Unsubscribe;
1311
private logger = new Logger(StoreSyncService.name);
14-
private syncTimer: NodeJS.Timeout | null = null;
1512
private lastSyncedState: string | null = null;
1613

1714
constructor(private configService: ConfigService) {
15+
this.syncStore();
1816
this.unsubscribe = store.subscribe(() => {
19-
this.scheduleSync();
17+
this.syncStore();
2018
});
2119
}
2220

23-
private scheduleSync() {
24-
if (this.syncTimer) {
25-
clearTimeout(this.syncTimer);
21+
private syncStore() {
22+
const state = store.getState();
23+
const serializedState = JSON.stringify(state);
24+
if (serializedState === this.lastSyncedState) {
25+
return;
2626
}
2727

28-
this.syncTimer = setTimeout(() => {
29-
this.syncTimer = null;
30-
const state = store.getState();
31-
const serializedState = JSON.stringify(state);
32-
if (serializedState === this.lastSyncedState) {
33-
return;
34-
}
35-
36-
this.configService.set('store', state);
37-
this.lastSyncedState = serializedState;
38-
this.logger.verbose('Synced store to NestJS Config');
39-
}, STORE_SYNC_DEBOUNCE_MS);
28+
this.configService.set('store', state);
29+
this.lastSyncedState = serializedState;
30+
this.logger.verbose('Synced store to NestJS Config');
4031
}
4132

4233
onModuleDestroy() {
43-
if (this.syncTimer) {
44-
clearTimeout(this.syncTimer);
45-
this.syncTimer = null;
46-
}
47-
4834
this.unsubscribe();
4935
}
5036
}

‎api/src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.spec.ts‎

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,15 @@ describe('OnboardingInternalBootService', () => {
4040
};
4141
const disksService = {
4242
getAssignableDisks: vi.fn(),
43-
} satisfies Pick<DisksService, 'getAssignableDisks'>;
43+
getInternalBootDeviceNames: vi.fn(),
44+
} satisfies Pick<DisksService, 'getAssignableDisks' | 'getInternalBootDeviceNames'>;
4445

4546
beforeEach(() => {
4647
vi.clearAllMocks();
4748
internalBootStateService.getBootedFromFlashWithInternalBootSetup.mockResolvedValue(false);
4849
internalBootStateService.invalidateCachedInternalBootDeviceState.mockResolvedValue(undefined);
4950
disksService.getAssignableDisks.mockResolvedValue([]);
51+
disksService.getInternalBootDeviceNames.mockResolvedValue(new Set());
5052
vi.mocked(getters.emhttp).mockReturnValue({
5153
var: {},
5254
devices: [],
@@ -121,10 +123,43 @@ describe('OnboardingInternalBootService', () => {
121123
shareNames: ['media', 'diskshare'],
122124
poolNames: ['cache', 'nvme'],
123125
assignableDisks,
126+
driveWarnings: [],
124127
});
125128
expect(vi.mocked(loadStateFileSync)).not.toHaveBeenCalled();
126129
});
127130

131+
it('flags assignable disks that already contain the internal boot partition layout', async () => {
132+
const assignableDisks = [
133+
{
134+
id: 'disk-1',
135+
device: '/dev/sda',
136+
serialNum: 'SERIAL-1',
137+
size: 1,
138+
interfaceType: 'SATA',
139+
},
140+
{
141+
id: 'disk-2',
142+
device: '/dev/sdb',
143+
serialNum: 'SERIAL-2',
144+
size: 1,
145+
interfaceType: 'SATA',
146+
},
147+
];
148+
disksService.getAssignableDisks.mockResolvedValue(assignableDisks);
149+
disksService.getInternalBootDeviceNames.mockResolvedValue(new Set(['sdb']));
150+
151+
const service = createService();
152+
const result = await service.getInternalBootContext();
153+
154+
expect(result.driveWarnings).toEqual([
155+
{
156+
diskId: 'disk-2',
157+
device: 'sdb',
158+
warnings: ['HAS_INTERNAL_BOOT_PARTITIONS'],
159+
},
160+
]);
161+
});
162+
128163
it('refreshes internal boot context from disk and invalidates cached device state', async () => {
129164
const assignableDisks = [
130165
{
@@ -399,8 +434,9 @@ describe('OnboardingInternalBootService', () => {
399434
expect(vi.mocked(emcmd)).not.toHaveBeenCalled();
400435
});
401436

402-
it('returns validation error when internal boot is already configured while booted from flash', async () => {
437+
it('continues setup when booted from flash and internal boot partitions already exist', async () => {
403438
internalBootStateService.getBootedFromFlashWithInternalBootSetup.mockResolvedValue(true);
439+
vi.mocked(emcmd).mockResolvedValue({ ok: true } as Awaited<ReturnType<typeof emcmd>>);
404440
const service = createService();
405441

406442
const result = await service.createInternalBootPool({
@@ -410,34 +446,12 @@ describe('OnboardingInternalBootService', () => {
410446
updateBios: false,
411447
});
412448

413-
expect(result).toMatchObject({
414-
ok: false,
415-
code: 3,
416-
});
417-
expect(result.output).toContain('internal boot is already configured');
418-
expect(internalBootStateService.invalidateCachedInternalBootDeviceState).not.toHaveBeenCalled();
419-
expect(vi.mocked(emcmd)).not.toHaveBeenCalled();
420-
});
421-
422-
it('returns failure output when the internal boot state lookup throws', async () => {
423-
internalBootStateService.getBootedFromFlashWithInternalBootSetup.mockRejectedValue(
424-
new Error('state lookup failed')
449+
expect(result.ok).toBe(true);
450+
expect(result.code).toBe(0);
451+
expect(vi.mocked(emcmd)).toHaveBeenCalledTimes(4);
452+
expect(internalBootStateService.invalidateCachedInternalBootDeviceState).toHaveBeenCalledTimes(
453+
1
425454
);
426-
const service = createService();
427-
428-
const result = await service.createInternalBootPool({
429-
poolName: 'cache',
430-
devices: ['disk-1'],
431-
bootSizeMiB: 16384,
432-
updateBios: false,
433-
});
434-
435-
expect(result.ok).toBe(false);
436-
expect(result.code).toBe(1);
437-
expect(result.output).toContain('mkbootpool: command failed or timed out');
438-
expect(result.output).toContain('state lookup failed');
439-
expect(internalBootStateService.invalidateCachedInternalBootDeviceState).not.toHaveBeenCalled();
440-
expect(vi.mocked(emcmd)).not.toHaveBeenCalled();
441455
});
442456

443457
it('returns failure output when emcmd command throws', async () => {

‎api/src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.ts‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ import { Injectable, Logger } from '@nestjs/common';
22

33
import { execa } from 'execa';
44

5+
import type { Disk } from '@app/unraid-api/graph/resolvers/disks/disks.model.js';
56
import type {
67
CreateInternalBootPoolInput,
78
OnboardingInternalBootContext,
9+
OnboardingInternalBootDriveWarning,
810
OnboardingInternalBootResult,
911
} from '@app/unraid-api/graph/resolvers/onboarding/onboarding.model.js';
1012
import { emcmd } from '@app/core/utils/clients/emcmd.js';
@@ -34,10 +36,6 @@ export class OnboardingInternalBootService {
3436
private readonly disksService: DisksService
3537
) {}
3638

37-
private async isBootedFromFlashWithInternalBootSetup(): Promise<boolean> {
38-
return this.internalBootStateService.getBootedFromFlashWithInternalBootSetup();
39-
}
40-
4139
private async runStep(
4240
commandText: string,
4341
command: Record<string, string>,
@@ -181,6 +179,30 @@ export class OnboardingInternalBootService {
181179
.filter((entry) => entry.length > 0);
182180
}
183181

182+
private normalizeDeviceName(value: string | null | undefined): string {
183+
if (typeof value !== 'string') {
184+
return '';
185+
}
186+
187+
const trimmed = value.trim();
188+
return trimmed.startsWith('/dev/') ? trimmed.slice('/dev/'.length) : trimmed;
189+
}
190+
191+
private async getAssignableDiskWarnings(
192+
assignableDisks: Array<Pick<Disk, 'id' | 'device'>>
193+
): Promise<OnboardingInternalBootDriveWarning[]> {
194+
const internalBootDeviceNames = await this.disksService.getInternalBootDeviceNames();
195+
196+
return assignableDisks
197+
.filter((disk) => internalBootDeviceNames.has(this.normalizeDeviceName(disk.device)))
198+
.map((disk) => ({
199+
diskId: disk.id.trim(),
200+
device: this.normalizeDeviceName(disk.device),
201+
warnings: ['HAS_INTERNAL_BOOT_PARTITIONS'],
202+
}))
203+
.filter((warning) => warning.diskId.length > 0 && warning.device.length > 0);
204+
}
205+
184206
private getPoolNamesFromEmhttpState(): string[] {
185207
const emhttpState = getters.emhttp();
186208
const names = new Set<string>();
@@ -211,6 +233,7 @@ export class OnboardingInternalBootService {
211233
this.loadEmhttpBootContext();
212234

213235
const vars = getters.emhttp().var ?? {};
236+
const assignableDisks = await this.disksService.getAssignableDisks();
214237
let bootedFromFlashWithInternalBootSetup = false;
215238

216239
try {
@@ -221,6 +244,8 @@ export class OnboardingInternalBootService {
221244
this.logger.warn(`Failed to resolve internal boot context boot state: ${message}`);
222245
}
223246

247+
const driveWarnings = await this.getAssignableDiskWarnings(assignableDisks);
248+
224249
return {
225250
arrayStopped: vars.mdState === ArrayState.STOPPED || vars.fsState === 'Stopped',
226251
bootEligible:
@@ -235,7 +260,8 @@ export class OnboardingInternalBootService {
235260
reservedNames: this.splitCsvValues(vars.reservedNames),
236261
shareNames: this.getShareNames(),
237262
poolNames: this.getPoolNamesFromEmhttpState(),
238-
assignableDisks: await this.disksService.getAssignableDisks(),
263+
assignableDisks,
264+
driveWarnings,
239265
};
240266
}
241267

@@ -498,17 +524,6 @@ export class OnboardingInternalBootService {
498524
}
499525

500526
try {
501-
if (await this.isBootedFromFlashWithInternalBootSetup()) {
502-
this.logger.warn(
503-
'createInternalBootPool aborted because internal boot is already configured'
504-
);
505-
return {
506-
ok: false,
507-
code: 3,
508-
output: 'mkbootpool: internal boot is already configured while the system is still booted from flash',
509-
};
510-
}
511-
512527
const resolvedBootDevices = input.updateBios
513528
? await this.resolveRequestedBootDevices(input.devices, output)
514529
: new Map<string, ResolvedBootDevice>();

‎api/src/unraid-api/graph/resolvers/onboarding/onboarding.model.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,20 @@ export class OnboardingInternalBootResult {
333333
output!: string;
334334
}
335335

336+
@ObjectType({
337+
description: 'Warning metadata for an assignable internal boot drive',
338+
})
339+
export class OnboardingInternalBootDriveWarning {
340+
@Field(() => String)
341+
diskId!: string;
342+
343+
@Field(() => String)
344+
device!: string;
345+
346+
@Field(() => [String])
347+
warnings!: string[];
348+
}
349+
336350
@ObjectType({
337351
description: 'Current onboarding context for configuring internal boot',
338352
})
@@ -360,4 +374,7 @@ export class OnboardingInternalBootContext {
360374

361375
@Field(() => [Disk])
362376
assignableDisks!: Disk[];
377+
378+
@Field(() => [OnboardingInternalBootDriveWarning])
379+
driveWarnings!: OnboardingInternalBootDriveWarning[];
363380
}

0 commit comments

Comments
 (0)