Skip to content

fix(docker): remove aggressive caching to ensure data correctness #1864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pujitm merged 1 commit into from
Jan 9, 2026
Merged

fix(docker): remove aggressive caching to ensure data correctness #1864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ vi.mock('./utils/docker-client.js', () => ({
vi.mock('./docker.service.js', () => ({
DockerService: vi.fn().mockImplementation(() => ({
getDockerClient: vi.fn(),
clearContainerCache: vi.fn(),
getAppInfo: vi.fn().mockResolvedValue({ info: { apps: { installed: 1, running: 1 } } }),
})),
}));
Expand All @@ -76,7 +75,6 @@ describe('DockerEventService', () => {
// Create a mock Docker service *instance*
const mockDockerServiceImpl = {
getDockerClient: vi.fn(),
clearContainerCache: vi.fn(),
getAppInfo: vi.fn().mockResolvedValue({ info: { apps: { installed: 1, running: 1 } } }),
};

Expand Down Expand Up @@ -132,7 +130,6 @@ describe('DockerEventService', () => {

await waitForEventProcessing();

expect(dockerService.clearContainerCache).toHaveBeenCalled();
expect(dockerService.getAppInfo).toHaveBeenCalled();
expect(pubsub.publish).toHaveBeenCalledWith(PUBSUB_CHANNEL.INFO, expect.any(Object));
});
Expand All @@ -154,7 +151,6 @@ describe('DockerEventService', () => {

await waitForEventProcessing();

expect(dockerService.clearContainerCache).not.toHaveBeenCalled();
expect(dockerService.getAppInfo).not.toHaveBeenCalled();
expect(pubsub.publish).not.toHaveBeenCalled();
});
Expand All @@ -172,7 +168,6 @@ describe('DockerEventService', () => {
await waitForEventProcessing();

expect(service.isActive()).toBe(true);
expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1);
expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1);
expect(pubsub.publish).toHaveBeenCalledTimes(1);
});
Expand All @@ -190,7 +185,6 @@ describe('DockerEventService', () => {

await waitForEventProcessing();

expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(2);
expect(dockerService.getAppInfo).toHaveBeenCalledTimes(2);
expect(pubsub.publish).toHaveBeenCalledTimes(2);
});
Expand All @@ -206,7 +200,6 @@ describe('DockerEventService', () => {

await waitForEventProcessing();

expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1);
expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1);
expect(pubsub.publish).toHaveBeenCalledTimes(1);

Expand All @@ -223,7 +216,6 @@ describe('DockerEventService', () => {

await waitForEventProcessing();

expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1);
expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1);
expect(pubsub.publish).toHaveBeenCalledTimes(1);

Expand Down
4 changes: 1 addition & 3 deletions api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,12 @@ export class DockerEventService implements OnModuleDestroy, OnModuleInit {
if (shouldProcess) {
this.logger.debug(`[${dockerEvent.from}] ${dockerEvent.Type}->${actionName}`);

// For container lifecycle events, update the container cache
// For container lifecycle events, publish updated app info
if (
dockerEvent.Type === DockerEventType.CONTAINER &&
typeof actionName === 'string' &&
this.containerActions.includes(actionName as DockerEventAction)
) {
await this.dockerService.clearContainerCache();
// Get updated counts and publish
const appInfo = await this.dockerService.getAppInfo();
await pubsub.publish(PUBSUB_CHANNEL.INFO, appInfo);
this.logger.debug(`Published app info update due to event: ${actionName}`);
Expand Down
41 changes: 7 additions & 34 deletions api/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { CACHE_MANAGER } from '@nestjs/cache-manager';
import { Test, TestingModule } from '@nestjs/testing';

import { beforeEach, describe, expect, it, vi } from 'vitest';
Expand All @@ -17,27 +16,14 @@ vi.mock('@app/unraid-api/graph/resolvers/docker/utils/docker-client.js', () => (
getDockerClient: vi.fn().mockReturnValue(mockDockerInstance),
}));

const mockCacheManager = {
get: vi.fn(),
set: vi.fn(),
};

describe('DockerNetworkService', () => {
let service: DockerNetworkService;

beforeEach(async () => {
mockListNetworks.mockReset();
mockCacheManager.get.mockReset();
mockCacheManager.set.mockReset();

const module: TestingModule = await Test.createTestingModule({
providers: [
DockerNetworkService,
{
provide: CACHE_MANAGER,
useValue: mockCacheManager,
},
],
providers: [DockerNetworkService],
}).compile();

service = module.get<DockerNetworkService>(DockerNetworkService);
Expand All @@ -48,16 +34,7 @@ describe('DockerNetworkService', () => {
});

describe('getNetworks', () => {
it('should return cached networks if available and not skipped', async () => {
const cached = [{ id: 'net1', name: 'test-net' }];
mockCacheManager.get.mockResolvedValue(cached);

const result = await service.getNetworks({ skipCache: false });
expect(result).toEqual(cached);
expect(mockListNetworks).not.toHaveBeenCalled();
});

it('should fetch networks from docker if cache skipped', async () => {
it('should fetch networks from docker', async () => {
const rawNetworks = [
{
Id: 'net1',
Expand All @@ -67,22 +44,18 @@ describe('DockerNetworkService', () => {
];
mockListNetworks.mockResolvedValue(rawNetworks);

const result = await service.getNetworks({ skipCache: true });
const result = await service.getNetworks();
expect(result).toHaveLength(1);
expect(result[0].id).toBe('net1');
expect(result[0].name).toBe('test-net');
expect(mockListNetworks).toHaveBeenCalled();
expect(mockCacheManager.set).toHaveBeenCalledWith(
DockerNetworkService.NETWORK_CACHE_KEY,
expect.anything(),
expect.anything()
);
});

it('should fetch networks from docker if cache miss', async () => {
mockCacheManager.get.mockResolvedValue(undefined);
it('should return empty array when no networks exist', async () => {
mockListNetworks.mockResolvedValue([]);

await service.getNetworks({ skipCache: false });
const result = await service.getNetworks();
expect(result).toEqual([]);
expect(mockListNetworks).toHaveBeenCalled();
});
});
Expand Down
33 changes: 3 additions & 30 deletions api/src/unraid-api/graph/resolvers/docker/docker-network.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,20 @@
import { CACHE_MANAGER } from '@nestjs/cache-manager';
import { Inject, Injectable, Logger } from '@nestjs/common';

import { type Cache } from 'cache-manager';
import { Injectable, Logger } from '@nestjs/common';

import { catchHandlers } from '@app/core/utils/misc/catch-handlers.js';
import { DockerNetwork } from '@app/unraid-api/graph/resolvers/docker/docker.model.js';
import { getDockerClient } from '@app/unraid-api/graph/resolvers/docker/utils/docker-client.js';

interface NetworkListingOptions {
skipCache: boolean;
}

@Injectable()
export class DockerNetworkService {
private readonly logger = new Logger(DockerNetworkService.name);
private readonly client = getDockerClient();

public static readonly NETWORK_CACHE_KEY = 'docker_networks';
private static readonly CACHE_TTL_SECONDS = 60;

constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {}

/**
* Get all Docker networks
* @returns All the in/active Docker networks on the system.
*/
public async getNetworks({ skipCache }: NetworkListingOptions): Promise<DockerNetwork[]> {
if (!skipCache) {
const cachedNetworks = await this.cacheManager.get<DockerNetwork[]>(
DockerNetworkService.NETWORK_CACHE_KEY
);
if (cachedNetworks) {
this.logger.debug('Using docker network cache');
return cachedNetworks;
}
}

this.logger.debug('Updating docker network cache');
public async getNetworks(): Promise<DockerNetwork[]> {
this.logger.debug('Fetching docker networks');
const rawNetworks = await this.client.listNetworks().catch(catchHandlers.docker);
const networks = rawNetworks.map(
(network) =>
Expand All @@ -59,11 +37,6 @@ export class DockerNetworkService {
}) as DockerNetwork
);

await this.cacheManager.set(
DockerNetworkService.NETWORK_CACHE_KEY,
networks,
DockerNetworkService.CACHE_TTL_SECONDS * 1000
);
return networks;
}
}
2 changes: 1 addition & 1 deletion api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class DockerTemplateScannerService {
const templates = await this.loadAllTemplates(result);

try {
const containers = await this.dockerService.getContainers({ skipCache: true });
const containers = await this.dockerService.getContainers();
const config = this.dockerConfigService.getConfig();
const currentMappings = config.templateMappings || {};
const skipSet = new Set(config.skipTemplatePaths || []);
Expand Down
2 changes: 1 addition & 1 deletion api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('DockerModule', () => {
const module: TestingModule = await Test.createTestingModule({
providers: [
DockerResolver,
{ provide: DockerService, useValue: { clearContainerCache: vi.fn() } },
{ provide: DockerService, useValue: {} },
{
provide: DockerConfigService,
useValue: {
Expand Down
18 changes: 9 additions & 9 deletions api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ describe('DockerResolver', () => {
getNetworks: vi.fn(),
getContainerLogSizes: vi.fn(),
getContainerLogs: vi.fn(),
clearContainerCache: vi.fn(),
},
},
{
Expand Down Expand Up @@ -161,7 +160,7 @@ describe('DockerResolver', () => {
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRw');
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeLog');
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false });
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
});

it('should request size when sizeRootFs field is requested', async () => {
Expand Down Expand Up @@ -191,7 +190,7 @@ describe('DockerResolver', () => {
const result = await resolver.containers(false, mockInfo);
expect(result).toEqual(mockContainers);
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true });
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
});

it('should request size when sizeRw field is requested', async () => {
Expand All @@ -205,7 +204,7 @@ describe('DockerResolver', () => {

await resolver.containers(false, mockInfo);
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRw');
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true });
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
});

it('should fetch log sizes when sizeLog field is requested', async () => {
Expand Down Expand Up @@ -240,7 +239,7 @@ describe('DockerResolver', () => {
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeLog');
expect(dockerService.getContainerLogSizes).toHaveBeenCalledWith(['test-container']);
expect(result[0]?.sizeLog).toBe(42);
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false });
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
});

it('should request size when GraphQLFieldHelper indicates sizeRootFs is requested', async () => {
Expand All @@ -254,7 +253,7 @@ describe('DockerResolver', () => {

await resolver.containers(false, mockInfo);
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true });
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
});

it('should not request size when GraphQLFieldHelper indicates sizeRootFs is not requested', async () => {
Expand All @@ -266,18 +265,19 @@ describe('DockerResolver', () => {

await resolver.containers(false, mockInfo);
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false });
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
});

it('should handle skipCache parameter', async () => {
it('skipCache parameter is deprecated and ignored', async () => {
const mockContainers: DockerContainer[] = [];
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockReturnValue(false);

const mockInfo = {} as any;

// skipCache parameter is now deprecated and ignored
await resolver.containers(true, mockInfo);
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: true, size: false });
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
});

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