Skip to content

Commit

Permalink
feat: adding BuildOption (podman-desktop#5533)
Browse files Browse the repository at this point in the history
* feat: adding BuildOption

Signed-off-by: axel7083 <[email protected]>

* test: ensuring getFirstRunningConnection is properly called

Signed-off-by: axel7083 <[email protected]>

* fix: code comment

Signed-off-by: axel7083 <[email protected]>

* Update packages/extension-api/src/extension-api.d.ts

Co-authored-by: Florent BENOIT <[email protected]>
Signed-off-by: axel7083 <[email protected]>

* fix: resolving name and comments

Signed-off-by: axel7083 <[email protected]>

* Update packages/extension-api/src/extension-api.d.ts

Co-authored-by: Florent BENOIT <[email protected]>
Signed-off-by: axel7083 <[email protected]>

---------

Signed-off-by: axel7083 <[email protected]>
Co-authored-by: Florent BENOIT <[email protected]>
  • Loading branch information
axel7083 and benoitf authored Jan 18, 2024
1 parent 6d3f69c commit bcc014e
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 36 deletions.
22 changes: 16 additions & 6 deletions packages/extension-api/src/extension-api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,19 @@ declare module '@podman-desktop/api' {
id: string;
}

export interface BuildImageOptions {
// Specifies a Containerfile which contains instructions for building the image
containerFile?: string;
// Specifies the name which is assigned to the resulting image if the build process completes successfully.
tag?: string;
// Set the os/arch of the built image (and its base image, when using one) to the provided value instead of using the current operating system and architecture of the host
platform?: string;
// Set the provider to use, if not we will try select the first one available (sorted in favor of Podman).
provider?: ProviderContainerConnectionInfo | containerDesktopAPI.ContainerProviderConnection;
// The abort controller for running the build image operation
abortController?: AbortController;
}

export interface NetworkCreateOptions {
Name: string;
}
Expand Down Expand Up @@ -1953,13 +1966,10 @@ declare module '@podman-desktop/api' {
export function stopContainer(engineId: string, id: string): Promise<void>;
export function deleteContainer(engineId: string, id: string): Promise<void>;
export function buildImage(
containerBuildContextDirectory: string,
relativeContainerfilePath: string,
imageName: string,
platform: string,
selectedProvider: ProviderContainerConnectionInfo | containerDesktopAPI.ContainerProviderConnection,
// build context directory
context: string,
eventCollect: (eventName: 'stream' | 'error' | 'finish', data: string) => void,
abortController?: AbortController,
options?: BuildImageOptions,
);
export function saveImage(engineId: string, id: string, filename: string): Promise<void>;
export function listImages(): Promise<ImageInfo[]>;
Expand Down
25 changes: 18 additions & 7 deletions packages/main/src/plugin/container-registry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -981,11 +981,22 @@ describe('buildImage', () => {
lifecycleMethods: undefined,
status: 'started',
};
await expect(containerRegistry.buildImage('context', 'file', 'name', '', connection, () => {})).rejects.toThrow(
await expect(containerRegistry.buildImage('context', () => {}, 'file', 'name', '', connection)).rejects.toThrow(
'no running provider for the matching container',
);
});

test('called getFirstRunningConnection when undefined provider', async () => {
const getFirstRunningConnection = vi.spyOn(containerRegistry, 'getFirstRunningConnection');
getFirstRunningConnection.mockImplementation(() => {
throw new Error('mocked');
});

await expect(containerRegistry.buildImage('context', () => {})).rejects.toThrow('mocked');

expect(getFirstRunningConnection).toHaveBeenCalledOnce();
});

test('throw if there is no running provider with containerProviderConnection input', async () => {
const fakeDockerode = {} as Dockerode;

Expand All @@ -1009,7 +1020,7 @@ describe('buildImage', () => {
},
status: () => 'started',
};
await expect(containerRegistry.buildImage('context', 'file', 'name', '', connection, () => {})).rejects.toThrow(
await expect(containerRegistry.buildImage('context', () => {}, 'file', 'name', '', connection)).rejects.toThrow(
'no running provider for the matching container',
);
});
Expand Down Expand Up @@ -1046,7 +1057,7 @@ describe('buildImage', () => {
vi.spyOn(tar, 'pack').mockReturnValue({} as NodeJS.ReadableStream);
vi.spyOn(dockerAPI, 'buildImage').mockRejectedValue('human error message');

await expect(containerRegistry.buildImage('context', 'file', 'name', '', connection, () => {})).rejects.toThrow(
await expect(containerRegistry.buildImage('context', () => {}, 'file', 'name', '', connection)).rejects.toThrow(
'human error message',
);
});
Expand Down Expand Up @@ -1082,7 +1093,7 @@ describe('buildImage', () => {
vi.spyOn(tar, 'pack').mockReturnValue({} as NodeJS.ReadableStream);
vi.spyOn(dockerAPI, 'buildImage').mockRejectedValue('human error message');

await expect(containerRegistry.buildImage('context', 'file', 'name', '', connection, () => {})).rejects.toThrow(
await expect(containerRegistry.buildImage('context', () => {}, 'file', 'name', '', connection)).rejects.toThrow(
'human error message',
);
});
Expand Down Expand Up @@ -1122,7 +1133,7 @@ describe('buildImage', () => {
return f(null, []);
});

await containerRegistry.buildImage('context', '\\path\\file', 'name', '', connection, () => {});
await containerRegistry.buildImage('context', () => {}, '\\path\\file', 'name', '', connection);

expect(dockerAPI.buildImage).toBeCalledWith({} as NodeJS.ReadableStream, {
registryconfig: {},
Expand Down Expand Up @@ -1166,7 +1177,7 @@ describe('buildImage', () => {
return f(null, []);
});

await containerRegistry.buildImage('context', '\\path\\file', 'name', '', connection, () => {});
await containerRegistry.buildImage('context', () => {}, '\\path\\file', 'name', '', connection);

expect(dockerAPI.buildImage).toBeCalledWith({} as NodeJS.ReadableStream, {
registryconfig: {},
Expand Down Expand Up @@ -1211,7 +1222,7 @@ describe('buildImage', () => {
return f(null, []);
});

await containerRegistry.buildImage('context', '/dir/dockerfile', 'name', '', connection, () => {});
await containerRegistry.buildImage('context', () => {}, '/dir/dockerfile', 'name', '', connection);

expect(dockerAPI.buildImage).toBeCalledWith({} as NodeJS.ReadableStream, {
registryconfig: {},
Expand Down
20 changes: 13 additions & 7 deletions packages/main/src/plugin/container-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1946,17 +1946,23 @@ export class ContainerProviderRegistry {

async buildImage(
containerBuildContextDirectory: string,
relativeContainerfilePath: string,
imageName: string,
platform: string,
selectedProvider: ProviderContainerConnectionInfo | containerDesktopAPI.ContainerProviderConnection,
eventCollect: (eventName: 'stream' | 'error' | 'finish', data: string) => void,
relativeContainerfilePath?: string,
imageName?: string,
platform?: string,
selectedProvider?: ProviderContainerConnectionInfo | containerDesktopAPI.ContainerProviderConnection,
abortController?: AbortController,
): Promise<unknown> {
let telemetryOptions = {};
try {
// grab all connections
const matchingContainerProviderApi = this.getMatchingEngineFromConnection(selectedProvider);
let matchingContainerProviderApi: Dockerode;
if (selectedProvider !== undefined) {
// grab all connections
matchingContainerProviderApi = this.getMatchingEngineFromConnection(selectedProvider);
} else {
// Get the first running connection (preference for podman)
matchingContainerProviderApi = this.getFirstRunningConnection()[1];
}

// grab auth for all registries
const registryconfig = this.imageRegistry.getRegistryConfig();
Expand All @@ -1965,7 +1971,7 @@ export class ContainerProviderRegistry {
`Uploading the build context from ${containerBuildContextDirectory}...Can take a while...\r\n`,
);
const tarStream = tar.pack(containerBuildContextDirectory);
if (isWindows()) {
if (isWindows() && relativeContainerfilePath !== undefined) {
relativeContainerfilePath = relativeContainerfilePath.replace(/\\/g, '/');
}

Expand Down
20 changes: 8 additions & 12 deletions packages/main/src/plugin/extension-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -907,22 +907,18 @@ export class ExtensionLoader {
return containerProviderRegistry.deleteContainer(engineId, id);
},
buildImage(
containerBuildContextDirectory: string,
relativeContainerfilePath: string,
imageName: string,
platform: string,
selectedProvider: ProviderContainerConnectionInfo | containerDesktopAPI.ContainerProviderConnection,
context: string,
eventCollect: (eventName: 'stream' | 'error' | 'finish', data: string) => void,
abortController?: AbortController,
options?: containerDesktopAPI.BuildImageOptions,
) {
return containerProviderRegistry.buildImage(
containerBuildContextDirectory,
relativeContainerfilePath,
imageName,
platform,
selectedProvider,
context,
eventCollect,
abortController,
options?.containerFile,
options?.tag,
options?.platform,
options?.provider,
options?.abortController,
);
},
listImages(): Promise<containerDesktopAPI.ImageInfo[]> {
Expand Down
8 changes: 4 additions & 4 deletions packages/main/src/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1209,10 +1209,6 @@ export class PluginSystem {

return containerProviderRegistry.buildImage(
containerBuildContextDirectory,
relativeContainerfilePath,
imageName,
platform,
selectedProvider,
(eventName: string, data: string) => {
this.getWebContentsSender().send(
'container-provider-registry:buildImage-onData',
Expand All @@ -1221,6 +1217,10 @@ export class PluginSystem {
data,
);
},
relativeContainerfilePath,
imageName,
platform,
selectedProvider,
abortController,
);
},
Expand Down

0 comments on commit bcc014e

Please sign in to comment.