fix: move docker mutations to the mutations resolver (#1333)

Thanks to @S3ppo on Discord for flagging this issue


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced a dedicated mutation field that centralizes Docker
operations via the API.
- Added a new method for Docker-related mutations in the mutation
resolver.

- **Refactor**
- Streamlined Docker container controls by renaming the start/stop
operations for improved clarity.
	- Removed legacy fields to provide a more unified mutation interface.

- **Bug Fixes**
- Enhanced error handling during container start/stop operations to
ensure consistent behavior.

- **Tests**
- Updated test cases to reflect the new naming and behavior, ensuring
reliable Docker operation validations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Eli Bosley
2025-04-04 12:43:34 -04:00
committed by GitHub
parent bc3ca92fb0
commit 1bbe7d27b0
10 changed files with 102 additions and 83 deletions

View File

@@ -507,7 +507,6 @@ export function DockerSchema(): z.ZodObject<Properties<Docker>> {
__typename: z.literal('Docker').optional(),
containers: z.array(DockerContainerSchema()).nullish(),
id: z.string(),
mutations: DockerMutationsSchema(),
networks: z.array(DockerNetworkSchema()).nullish()
})
}

View File

@@ -574,7 +574,6 @@ export type Docker = Node & {
__typename?: 'Docker';
containers?: Maybe<Array<DockerContainer>>;
id: Scalars['ID']['output'];
mutations: DockerMutations;
networks?: Maybe<Array<DockerNetwork>>;
};
@@ -850,6 +849,7 @@ export type Mutation = {
deleteNotification: NotificationOverview;
/** Delete a user */
deleteUser?: Maybe<User>;
docker?: Maybe<DockerMutations>;
enableDynamicRemoteAccess: Scalars['Boolean']['output'];
login?: Maybe<Scalars['String']['output']>;
/** Pause parity check */
@@ -2558,7 +2558,6 @@ export type DisplayResolvers<ContextType = Context, ParentType extends Resolvers
export type DockerResolvers<ContextType = Context, ParentType extends ResolversParentTypes['Docker'] = ResolversParentTypes['Docker']> = ResolversObject<{
containers?: Resolver<Maybe<Array<ResolversTypes['DockerContainer']>>, ParentType, ContextType>;
id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
mutations?: Resolver<ResolversTypes['DockerMutations'], ParentType, ContextType>;
networks?: Resolver<Maybe<Array<ResolversTypes['DockerNetwork']>>, ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
}>;
@@ -2776,6 +2775,7 @@ export type MutationResolvers<ContextType = Context, ParentType extends Resolver
deleteArchivedNotifications?: Resolver<ResolversTypes['NotificationOverview'], ParentType, ContextType>;
deleteNotification?: Resolver<ResolversTypes['NotificationOverview'], ParentType, ContextType, RequireFields<MutationdeleteNotificationArgs, 'id' | 'type'>>;
deleteUser?: Resolver<Maybe<ResolversTypes['User']>, ParentType, ContextType, RequireFields<MutationdeleteUserArgs, 'input'>>;
docker?: Resolver<Maybe<ResolversTypes['DockerMutations']>, ParentType, ContextType>;
enableDynamicRemoteAccess?: Resolver<ResolversTypes['Boolean'], ParentType, ContextType, RequireFields<MutationenableDynamicRemoteAccessArgs, 'input'>>;
login?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType, RequireFields<MutationloginArgs, 'password' | 'username'>>;
pauseParityCheck?: Resolver<Maybe<ResolversTypes['JSON']>, ParentType, ContextType>;

View File

@@ -9,10 +9,12 @@ type Query {
}
type DockerMutations {
startContainer(id: ID!): DockerContainer!
stopContainer(id: ID!): DockerContainer!
""" Stop a container """
stop(id: ID!): DockerContainer!
""" Start a container """
start(id: ID!): DockerContainer!
}
extend type Docker {
mutations: DockerMutations!
}
type Mutation {
docker: DockerMutations
}

View File

@@ -19,8 +19,8 @@ describe('DockerMutationsResolver', () => {
{
provide: DockerService,
useValue: {
startContainer: vi.fn(),
stopContainer: vi.fn(),
start: vi.fn(),
stop: vi.fn(),
},
},
],
@@ -34,7 +34,7 @@ describe('DockerMutationsResolver', () => {
expect(resolver).toBeDefined();
});
it('should start container', async () => {
it('should start', async () => {
const mockContainer: DockerContainer = {
id: '1',
autoStart: false,
@@ -46,14 +46,14 @@ describe('DockerMutationsResolver', () => {
state: ContainerState.RUNNING,
status: 'Up 2 hours',
};
vi.mocked(dockerService.startContainer).mockResolvedValue(mockContainer);
vi.mocked(dockerService.start).mockResolvedValue(mockContainer);
const result = await resolver.startContainer('1');
const result = await resolver.start('1');
expect(result).toEqual(mockContainer);
expect(dockerService.startContainer).toHaveBeenCalledWith('1');
expect(dockerService.start).toHaveBeenCalledWith('1');
});
it('should stop container', async () => {
it('should stop', async () => {
const mockContainer: DockerContainer = {
id: '1',
autoStart: false,
@@ -65,10 +65,10 @@ describe('DockerMutationsResolver', () => {
state: ContainerState.EXITED,
status: 'Exited',
};
vi.mocked(dockerService.stopContainer).mockResolvedValue(mockContainer);
vi.mocked(dockerService.stop).mockResolvedValue(mockContainer);
const result = await resolver.stopContainer('1');
const result = await resolver.stop('1');
expect(result).toEqual(mockContainer);
expect(dockerService.stopContainer).toHaveBeenCalledWith('1');
expect(dockerService.stop).toHaveBeenCalledWith('1');
});
});

View File

@@ -9,23 +9,23 @@ import { DockerService } from '@app/unraid-api/graph/resolvers/docker/docker.ser
export class DockerMutationsResolver {
constructor(private readonly dockerService: DockerService) {}
@ResolveField('startContainer')
@ResolveField('start')
@UsePermissions({
action: AuthActionVerb.UPDATE,
resource: Resource.DOCKER,
possession: AuthPossession.ANY,
})
public async startContainer(@Args('id') id: string) {
return this.dockerService.startContainer(id);
public async start(@Args('id') id: string) {
return this.dockerService.start(id);
}
@ResolveField('stopContainer')
@ResolveField('stop')
@UsePermissions({
action: AuthActionVerb.UPDATE,
resource: Resource.DOCKER,
possession: AuthPossession.ANY,
})
public async stopContainer(@Args('id') id: string) {
return this.dockerService.stopContainer(id);
public async stop(@Args('id') id: string) {
return this.dockerService.stop(id);
}
}

View File

@@ -69,9 +69,4 @@ describe('DockerResolver', () => {
expect(result).toEqual(mockContainers);
expect(dockerService.getContainers).toHaveBeenCalledWith({ useCache: false });
});
it('should return mutations object with id', () => {
const result = resolver.mutations();
expect(result).toEqual({ id: 'docker-mutations' });
});
});

View File

@@ -35,11 +35,4 @@ export class DockerResolver {
public async networks() {
return this.dockerService.getNetworks({ useCache: false });
}
@ResolveField()
public mutations() {
return {
id: 'docker-mutations',
};
}
}

View File

@@ -183,7 +183,7 @@ describe('DockerService', () => {
mockListContainers.mockResolvedValue(mockContainers);
mockContainer.start.mockResolvedValue(undefined);
const result = await service.startContainer('abc123def456');
const result = await service.start('abc123def456');
expect(result).toEqual({
id: 'abc123def456',
@@ -234,7 +234,7 @@ describe('DockerService', () => {
mockListContainers.mockResolvedValue(mockContainers);
mockContainer.stop.mockResolvedValue(undefined);
const result = await service.stopContainer('abc123def456');
const result = await service.stop('abc123def456');
expect(result).toEqual({
id: 'abc123def456',
@@ -254,7 +254,7 @@ describe('DockerService', () => {
mounts: [],
});
expect(mockContainer.stop).toHaveBeenCalled();
expect(mockContainer.stop).toHaveBeenCalledWith({ t: 10 });
expect(mockListContainers).toHaveBeenCalledWith({
all: true,
size: true,
@@ -265,8 +265,8 @@ describe('DockerService', () => {
mockListContainers.mockResolvedValue([]);
mockContainer.start.mockResolvedValue(undefined);
await expect(service.startContainer('abc123def456')).rejects.toThrow(
'Container abc123def456 not found after starting'
await expect(service.start('not-found')).rejects.toThrow(
'Container not-found not found after starting'
);
});
@@ -274,8 +274,8 @@ describe('DockerService', () => {
mockListContainers.mockResolvedValue([]);
mockContainer.stop.mockResolvedValue(undefined);
await expect(service.stopContainer('abc123def456')).rejects.toThrow(
'Container abc123def456 not found after stopping'
await expect(service.stop('not-found')).rejects.toThrow(
'Container not-found not found after stopping'
);
});

View File

@@ -11,10 +11,11 @@ import type { ContainerPort, DockerContainer, DockerNetwork } from '@app/graphql
import { dockerLogger } from '@app/core/log.js';
import { pubsub, PUBSUB_CHANNEL } from '@app/core/pubsub.js';
import { catchHandlers } from '@app/core/utils/misc/catch-handlers.js';
import { sleep } from '@app/core/utils/misc/sleep.js';
import { ContainerPortType, ContainerState } from '@app/graphql/generated/api/types.js';
import { getters } from '@app/store/index.js';
interface ContainerListingOptions {
interface ContainerListingOptions extends Docker.ContainerListOptions {
useCache: boolean;
}
@@ -26,6 +27,7 @@ interface NetworkListingOptions {
export class DockerService implements OnModuleInit {
private client: Docker;
private containerCache: Array<DockerContainer> = [];
private autoStarts: string[] = [];
private dockerWatcher: null | typeof DockerEE = null;
private readonly logger = new Logger(DockerService.name);
@@ -124,53 +126,62 @@ export class DockerService implements OnModuleInit {
await pubsub.publish(PUBSUB_CHANNEL.INFO, this.appUpdateEvent);
}, 500);
public async getContainers({ useCache }: ContainerListingOptions): Promise<DockerContainer[]> {
public transformContainer(container: Docker.ContainerInfo): DockerContainer {
return camelCaseKeys<DockerContainer>(
{
labels: container.Labels ?? {},
sizeRootFs: undefined,
imageId: container.ImageID,
state:
typeof container.State === 'string'
? (ContainerState[container.State.toUpperCase()] ?? ContainerState.EXITED)
: ContainerState.EXITED,
autoStart: this.autoStarts.includes(container.Names[0].split('/')[1]),
ports: container.Ports.map<ContainerPort>((port) => ({
...port,
type: ContainerPortType[port.Type.toUpperCase()],
})),
command: container.Command,
created: container.Created,
mounts: container.Mounts,
networkSettings: container.NetworkSettings,
hostConfig: {
networkMode: container.HostConfig.NetworkMode,
},
id: container.Id,
image: container.Image,
status: container.Status,
},
{ deep: true }
);
}
public async getContainers(
{
useCache = false,
all = true,
size = true,
...listOptions
}: Partial<ContainerListingOptions> = { useCache: false }
): Promise<DockerContainer[]> {
if (useCache && this.containerCache.length > 0) {
this.logger.debug('Using docker container cache');
return this.containerCache;
}
this.logger.debug('Updating docker container cache');
const rawContainers = await this.client
.listContainers({
all: true,
size: true,
all,
size,
...listOptions,
})
// If docker throws an error return no containers
.catch(catchHandlers.docker);
const autoStarts = await this.getAutoStarts();
this.autoStarts = await this.getAutoStarts();
// Cleanup container object
this.containerCache = rawContainers.map((container) => {
const names = container.Names[0];
const containerData: DockerContainer = camelCaseKeys<DockerContainer>(
{
labels: container.Labels ?? {},
sizeRootFs: undefined,
imageId: container.ImageID,
state:
typeof container.State === 'string'
? (ContainerState[container.State.toUpperCase()] ?? ContainerState.EXITED)
: ContainerState.EXITED,
autoStart: autoStarts.includes(names.split('/')[1]),
ports: container.Ports.map<ContainerPort>((port) => ({
...port,
type: ContainerPortType[port.Type.toUpperCase()],
})),
command: container.Command,
created: container.Created,
mounts: container.Mounts,
networkSettings: container.NetworkSettings,
hostConfig: {
networkMode: container.HostConfig.NetworkMode,
},
id: container.Id,
image: container.Image,
status: container.Status,
},
{ deep: true }
);
const containerData: DockerContainer = this.transformContainer(container);
return containerData;
});
return this.containerCache;
@@ -193,7 +204,7 @@ export class DockerService implements OnModuleInit {
);
}
public async startContainer(id: string): Promise<DockerContainer> {
public async start(id: string): Promise<DockerContainer> {
const container = this.client.getContainer(id);
await container.start();
const containers = await this.getContainers({ useCache: false });
@@ -204,11 +215,23 @@ export class DockerService implements OnModuleInit {
return updatedContainer;
}
public async stopContainer(id: string): Promise<DockerContainer> {
public async stop(id: string): Promise<DockerContainer> {
const container = this.client.getContainer(id);
await container.stop();
const containers = await this.getContainers({ useCache: false });
const updatedContainer = containers.find((c) => c.id === id);
await container.stop({ t: 10 });
let containers = await this.getContainers({ useCache: false });
let updatedContainer: DockerContainer | undefined;
for (let i = 0; i < 5; i++) {
await sleep(500);
// Refresh the containers list on each attempt
containers = await this.getContainers({ useCache: false });
updatedContainer = containers.find((c) => c.id === id);
this.logger.debug(
`Container ${id} state after stop attempt ${i + 1}: ${updatedContainer?.state}`
);
if (updatedContainer?.state === ContainerState.EXITED) {
return updatedContainer;
}
}
if (!updatedContainer) {
throw new Error(`Container ${id} not found after stopping`);
}

View File

@@ -8,4 +8,11 @@ export class MutationResolver {
__typename: 'ArrayMutations',
};
}
@ResolveField()
public async docker() {
return {
__typename: 'DockerMutations',
};
}
}