Skip to content

Commit 38a6f0c

Browse files
pujitmclaude
pujitm
and
claude
authored
fix(docker): sync template mappings in organizer to prevent false orphan warnings (#1866)
## Summary - Fixes race condition where newly installed containers showed "Orphan Container" warning until page refresh - Refactored orphan status computation to reduce Docker API calls from 3 to 1 when syncing ## Changes - Added `RawDockerContainer` type (containers without `isOrphaned`/`templatePath`) - Added `enrichWithOrphanStatus()` to compute orphan status from current config - Added `getRawContainers()` to fetch containers without orphan enrichment - Updated resolvers and organizer service to use raw → sync → enrich pattern ## Context When a new container is installed, the GraphQL query fetches both `docker.containers` and `docker.organizer` data in parallel. Previously, `isOrphaned` was computed inside `getContainers()` based on `config.templateMappings`, requiring a refetch after sync to get updated values. This caused: 1. Newly installed containers to temporarily appear as orphans 2. Redundant Docker API calls (3 calls when syncing) Now orphan status enrichment is a separate step that reads the current config, eliminating redundant API calls while ensuring newly installed containers are correctly identified. ## Test plan - [x] Install a new Docker template and verify no "Orphan Container" warning appears - [x] Verify existing orphan detection still works for actual orphan containers - [x] Verify that deletion of orphan container from the alert works and displays any errors inline 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Automatic synchronization of missing Docker containers and visible orphan/template status for containers. * Inline error alert with dismiss for container removal failures. * **Bug Fixes** * Removal requests can opt out of retry behavior to surface immediate errors. * **Deprecations** * Deprecated `skipCache` parameter on Docker queries; it is now ignored. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1c1bae8 commit 38a6f0c

File tree

10 files changed

+141
-76
lines changed

10 files changed

+141
-76
lines changed

‎api/generated-schema.graphql‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,16 +1342,16 @@ type TailscaleStatus {
13421342

13431343
type Docker implements Node {
13441344
id: PrefixedID!
1345-
containers(skipCache: Boolean! = false): [DockerContainer!]!
1346-
networks(skipCache: Boolean! = false): [DockerNetwork!]!
1347-
portConflicts(skipCache: Boolean! = false): DockerPortConflicts!
1345+
containers(skipCache: Boolean! = false @deprecated(reason: "Caching has been removed; this parameter is now ignored")): [DockerContainer!]!
1346+
networks(skipCache: Boolean! = false @deprecated(reason: "Caching has been removed; this parameter is now ignored")): [DockerNetwork!]!
1347+
portConflicts(skipCache: Boolean! = false @deprecated(reason: "Caching has been removed; this parameter is now ignored")): DockerPortConflicts!
13481348

13491349
"""
13501350
Access container logs. Requires specifying a target container id through resolver arguments.
13511351
"""
13521352
logs(id: PrefixedID!, since: DateTime, tail: Int): DockerContainerLogs!
13531353
container(id: PrefixedID!): DockerContainer
1354-
organizer(skipCache: Boolean! = false): ResolvedOrganizerV1!
1354+
organizer(skipCache: Boolean! = false @deprecated(reason: "Caching has been removed; this parameter is now ignored")): ResolvedOrganizerV1!
13551355
containerUpdateStatuses: [ExplicitStatusItem!]!
13561356
}
13571357

‎api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.spec.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ describe('DockerTemplateScannerService', () => {
2424
await mkdir(testTemplateDir, { recursive: true });
2525

2626
const mockDockerService = {
27-
getContainers: vi.fn(),
27+
getRawContainers: vi.fn(),
2828
};
2929

3030
const mockDockerConfigService = {
@@ -196,7 +196,7 @@ describe('DockerTemplateScannerService', () => {
196196
} as DockerContainer,
197197
];
198198

199-
vi.mocked(dockerService.getContainers).mockResolvedValue(containers);
199+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(containers);
200200
vi.mocked(dockerConfigService.getConfig).mockReturnValue({
201201
updateCheckCronSchedule: '0 6 * * *',
202202
templateMappings: {},
@@ -236,7 +236,7 @@ describe('DockerTemplateScannerService', () => {
236236
} as DockerContainer,
237237
];
238238

239-
vi.mocked(dockerService.getContainers).mockResolvedValue(containers);
239+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(containers);
240240
vi.mocked(dockerConfigService.getConfig).mockReturnValue({
241241
updateCheckCronSchedule: '0 6 * * *',
242242
templateMappings: {},
@@ -254,7 +254,7 @@ describe('DockerTemplateScannerService', () => {
254254

255255
const containers: DockerContainer[] = [];
256256

257-
vi.mocked(dockerService.getContainers).mockResolvedValue(containers);
257+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(containers);
258258
vi.mocked(dockerConfigService.getConfig).mockReturnValue({
259259
updateCheckCronSchedule: '0 6 * * *',
260260
templateMappings: {},
@@ -268,7 +268,7 @@ describe('DockerTemplateScannerService', () => {
268268
});
269269

270270
it('should handle docker service errors gracefully', async () => {
271-
vi.mocked(dockerService.getContainers).mockRejectedValue(new Error('Docker error'));
271+
vi.mocked(dockerService.getRawContainers).mockRejectedValue(new Error('Docker error'));
272272
vi.mocked(dockerConfigService.getConfig).mockReturnValue({
273273
updateCheckCronSchedule: '0 6 * * *',
274274
templateMappings: {},
@@ -290,7 +290,7 @@ describe('DockerTemplateScannerService', () => {
290290
} as DockerContainer,
291291
];
292292

293-
vi.mocked(dockerService.getContainers).mockResolvedValue(containers);
293+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(containers);
294294
vi.mocked(dockerConfigService.getConfig).mockReturnValue({
295295
updateCheckCronSchedule: '0 6 * * *',
296296
templateMappings: {},
@@ -325,7 +325,7 @@ describe('DockerTemplateScannerService', () => {
325325
skipTemplatePaths: [],
326326
});
327327

328-
vi.mocked(dockerService.getContainers).mockResolvedValue(containers);
328+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(containers);
329329

330330
const scanSpy = vi.spyOn(service, 'scanTemplates').mockResolvedValue({
331331
scanned: 0,

‎api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ import { XMLParser } from 'fast-xml-parser';
88
import { ENABLE_NEXT_DOCKER_RELEASE, PATHS_DOCKER_TEMPLATES } from '@app/environment.js';
99
import { DockerConfigService } from '@app/unraid-api/graph/resolvers/docker/docker-config.service.js';
1010
import { DockerTemplateSyncResult } from '@app/unraid-api/graph/resolvers/docker/docker-template-scanner.model.js';
11-
import { DockerContainer } from '@app/unraid-api/graph/resolvers/docker/docker.model.js';
12-
import { DockerService } from '@app/unraid-api/graph/resolvers/docker/docker.service.js';
11+
import {
12+
DockerService,
13+
RawDockerContainer,
14+
} from '@app/unraid-api/graph/resolvers/docker/docker.service.js';
1315

1416
interface ParsedTemplate {
1517
filePath: string;
@@ -56,7 +58,7 @@ export class DockerTemplateScannerService {
5658
}
5759
}
5860

59-
async syncMissingContainers(containers: DockerContainer[]): Promise<boolean> {
61+
async syncMissingContainers(containers: RawDockerContainer[]): Promise<boolean> {
6062
const config = this.dockerConfigService.getConfig();
6163
const mappings = config.templateMappings || {};
6264
const skipSet = new Set(config.skipTemplatePaths || []);
@@ -87,7 +89,7 @@ export class DockerTemplateScannerService {
8789
const templates = await this.loadAllTemplates(result);
8890

8991
try {
90-
const containers = await this.dockerService.getContainers();
92+
const containers = await this.dockerService.getRawContainers();
9193
const config = this.dockerConfigService.getConfig();
9294
const currentMappings = config.templateMappings || {};
9395
const skipSet = new Set(config.skipTemplatePaths || []);
@@ -244,7 +246,7 @@ export class DockerTemplateScannerService {
244246
}
245247

246248
private matchContainerToTemplate(
247-
container: DockerContainer,
249+
container: RawDockerContainer,
248250
templates: ParsedTemplate[]
249251
): ParsedTemplate | null {
250252
const containerName = this.normalizeContainerName(container.names[0]);

‎api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,14 @@ describe('DockerResolver', () => {
3636
{
3737
provide: DockerService,
3838
useValue: {
39-
getContainers: vi.fn(),
39+
getRawContainers: vi.fn(),
40+
enrichWithOrphanStatus: vi.fn().mockImplementation((containers) =>
41+
containers.map((c: Record<string, unknown>) => ({
42+
...c,
43+
isOrphaned: false,
44+
templatePath: '/path/to/template.xml',
45+
}))
46+
),
4047
getNetworks: vi.fn(),
4148
getContainerLogSizes: vi.fn(),
4249
getContainerLogs: vi.fn(),
@@ -122,7 +129,7 @@ describe('DockerResolver', () => {
122129
});
123130

124131
it('should return containers from service', async () => {
125-
const mockContainers: DockerContainer[] = [
132+
const mockRawContainers = [
126133
{
127134
id: '1',
128135
autoStart: false,
@@ -134,7 +141,6 @@ describe('DockerResolver', () => {
134141
ports: [],
135142
state: ContainerState.EXITED,
136143
status: 'Exited',
137-
isOrphaned: false,
138144
},
139145
{
140146
id: '2',
@@ -147,24 +153,25 @@ describe('DockerResolver', () => {
147153
ports: [],
148154
state: ContainerState.RUNNING,
149155
status: 'Up 2 hours',
150-
isOrphaned: false,
151156
},
152157
];
153-
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
158+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(mockRawContainers);
154159
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockImplementation(() => false);
155160

156161
const mockInfo = {} as any;
157162

158163
const result = await resolver.containers(false, mockInfo);
159-
expect(result).toEqual(mockContainers);
164+
expect(result).toHaveLength(2);
165+
expect(result[0]).toMatchObject({ id: '1', isOrphaned: false });
166+
expect(result[1]).toMatchObject({ id: '2', isOrphaned: false });
160167
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
161168
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRw');
162169
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeLog');
163-
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
170+
expect(dockerService.getRawContainers).toHaveBeenCalledWith({ size: false });
164171
});
165172

166173
it('should request size when sizeRootFs field is requested', async () => {
167-
const mockContainers: DockerContainer[] = [
174+
const mockRawContainers = [
168175
{
169176
id: '1',
170177
autoStart: false,
@@ -177,25 +184,25 @@ describe('DockerResolver', () => {
177184
sizeRootFs: 1024000,
178185
state: ContainerState.EXITED,
179186
status: 'Exited',
180-
isOrphaned: false,
181187
},
182188
];
183-
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
189+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(mockRawContainers);
184190
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockImplementation((_, field) => {
185191
return field === 'sizeRootFs';
186192
});
187193

188194
const mockInfo = {} as any;
189195

190196
const result = await resolver.containers(false, mockInfo);
191-
expect(result).toEqual(mockContainers);
197+
expect(result).toHaveLength(1);
198+
expect(result[0]).toMatchObject({ id: '1', sizeRootFs: 1024000, isOrphaned: false });
192199
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
193-
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
200+
expect(dockerService.getRawContainers).toHaveBeenCalledWith({ size: true });
194201
});
195202

196203
it('should request size when sizeRw field is requested', async () => {
197204
const mockContainers: DockerContainer[] = [];
198-
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
205+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(mockContainers);
199206
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockImplementation((_, field) => {
200207
return field === 'sizeRw';
201208
});
@@ -204,7 +211,7 @@ describe('DockerResolver', () => {
204211

205212
await resolver.containers(false, mockInfo);
206213
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRw');
207-
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
214+
expect(dockerService.getRawContainers).toHaveBeenCalledWith({ size: true });
208215
});
209216

210217
it('should fetch log sizes when sizeLog field is requested', async () => {
@@ -223,7 +230,7 @@ describe('DockerResolver', () => {
223230
isOrphaned: false,
224231
},
225232
];
226-
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
233+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(mockContainers);
227234
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockImplementation((_, field) => {
228235
if (field === 'sizeLog') return true;
229236
return false;
@@ -239,12 +246,12 @@ describe('DockerResolver', () => {
239246
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeLog');
240247
expect(dockerService.getContainerLogSizes).toHaveBeenCalledWith(['test-container']);
241248
expect(result[0]?.sizeLog).toBe(42);
242-
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
249+
expect(dockerService.getRawContainers).toHaveBeenCalledWith({ size: false });
243250
});
244251

245252
it('should request size when GraphQLFieldHelper indicates sizeRootFs is requested', async () => {
246253
const mockContainers: DockerContainer[] = [];
247-
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
254+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(mockContainers);
248255
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockImplementation((_, field) => {
249256
return field === 'sizeRootFs';
250257
});
@@ -253,31 +260,31 @@ describe('DockerResolver', () => {
253260

254261
await resolver.containers(false, mockInfo);
255262
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
256-
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
263+
expect(dockerService.getRawContainers).toHaveBeenCalledWith({ size: true });
257264
});
258265

259266
it('should not request size when GraphQLFieldHelper indicates sizeRootFs is not requested', async () => {
260267
const mockContainers: DockerContainer[] = [];
261-
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
268+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(mockContainers);
262269
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockImplementation(() => false);
263270

264271
const mockInfo = {} as any;
265272

266273
await resolver.containers(false, mockInfo);
267274
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
268-
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
275+
expect(dockerService.getRawContainers).toHaveBeenCalledWith({ size: false });
269276
});
270277

271278
it('skipCache parameter is deprecated and ignored', async () => {
272279
const mockContainers: DockerContainer[] = [];
273-
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
280+
vi.mocked(dockerService.getRawContainers).mockResolvedValue(mockContainers);
274281
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockReturnValue(false);
275282

276283
const mockInfo = {} as any;
277284

278285
// skipCache parameter is now deprecated and ignored
279286
await resolver.containers(true, mockInfo);
280-
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
287+
expect(dockerService.getRawContainers).toHaveBeenCalledWith({ size: false });
281288
});
282289

283290
it('should fetch container logs with provided arguments', async () => {

‎api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,27 +98,29 @@ export class DockerResolver {
9898
const requestsRootFsSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeRootFs');
9999
const requestsRwSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeRw');
100100
const requestsLogSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeLog');
101-
const containers = await this.dockerService.getContainers({
101+
const rawContainers = await this.dockerService.getRawContainers({
102102
size: requestsRootFsSize || requestsRwSize,
103103
});
104104

105105
if (requestsLogSize) {
106106
const names = Array.from(
107107
new Set(
108-
containers
108+
rawContainers
109109
.map((container) => container.names?.[0]?.replace(/^\//, '') || null)
110110
.filter((name): name is string => Boolean(name))
111111
)
112112
);
113113
const logSizes = await this.dockerService.getContainerLogSizes(names);
114-
containers.forEach((container) => {
114+
rawContainers.forEach((container) => {
115115
const normalized = container.names?.[0]?.replace(/^\//, '') || '';
116-
container.sizeLog = normalized ? (logSizes.get(normalized) ?? 0) : 0;
116+
(container as { sizeLog?: number }).sizeLog = normalized
117+
? (logSizes.get(normalized) ?? 0)
118+
: 0;
117119
});
118120
}
119121

120-
const wasSynced = await this.dockerTemplateScannerService.syncMissingContainers(containers);
121-
return wasSynced ? await this.dockerService.getContainers() : containers;
122+
await this.dockerTemplateScannerService.syncMissingContainers(rawContainers);
123+
return this.dockerService.enrichWithOrphanStatus(rawContainers);
122124
}
123125

124126
@UsePermissions({

0 commit comments

Comments
 (0)