From bcc014e4df3c861ca392b9fe5d9ceb7fa9de170b Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Thu, 18 Jan 2024 16:37:54 +0100 Subject: [PATCH] feat: adding BuildOption (#5533) * feat: adding BuildOption Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * test: ensuring getFirstRunningConnection is properly called Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: code comment Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * Update packages/extension-api/src/extension-api.d.ts Co-authored-by: Florent BENOIT Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: resolving name and comments Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * Update packages/extension-api/src/extension-api.d.ts Co-authored-by: Florent BENOIT Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --------- Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> Co-authored-by: Florent BENOIT --- packages/extension-api/src/extension-api.d.ts | 22 +++++++++++----- .../src/plugin/container-registry.spec.ts | 25 +++++++++++++------ .../main/src/plugin/container-registry.ts | 20 +++++++++------ packages/main/src/plugin/extension-loader.ts | 20 ++++++--------- packages/main/src/plugin/index.ts | 8 +++--- 5 files changed, 59 insertions(+), 36 deletions(-) diff --git a/packages/extension-api/src/extension-api.d.ts b/packages/extension-api/src/extension-api.d.ts index 9236c78105ede..79fd10c76ae58 100644 --- a/packages/extension-api/src/extension-api.d.ts +++ b/packages/extension-api/src/extension-api.d.ts @@ -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; } @@ -1953,13 +1966,10 @@ declare module '@podman-desktop/api' { export function stopContainer(engineId: string, id: string): Promise; export function deleteContainer(engineId: string, id: string): Promise; 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; export function listImages(): Promise; diff --git a/packages/main/src/plugin/container-registry.spec.ts b/packages/main/src/plugin/container-registry.spec.ts index 441abe5ae0836..76206a6264bdf 100644 --- a/packages/main/src/plugin/container-registry.spec.ts +++ b/packages/main/src/plugin/container-registry.spec.ts @@ -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; @@ -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', ); }); @@ -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', ); }); @@ -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', ); }); @@ -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: {}, @@ -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: {}, @@ -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: {}, diff --git a/packages/main/src/plugin/container-registry.ts b/packages/main/src/plugin/container-registry.ts index 5a50b66898fa3..f54596f5a0997 100644 --- a/packages/main/src/plugin/container-registry.ts +++ b/packages/main/src/plugin/container-registry.ts @@ -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 { 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(); @@ -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, '/'); } diff --git a/packages/main/src/plugin/extension-loader.ts b/packages/main/src/plugin/extension-loader.ts index 037a69abeab02..f147504c4b0a7 100644 --- a/packages/main/src/plugin/extension-loader.ts +++ b/packages/main/src/plugin/extension-loader.ts @@ -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 { diff --git a/packages/main/src/plugin/index.ts b/packages/main/src/plugin/index.ts index 99a4f4348fb9d..23323ac6dd092 100644 --- a/packages/main/src/plugin/index.ts +++ b/packages/main/src/plugin/index.ts @@ -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', @@ -1221,6 +1217,10 @@ export class PluginSystem { data, ); }, + relativeContainerfilePath, + imageName, + platform, + selectedProvider, abortController, ); },