From 963bad844968e664cce19fdddb5d5fafa1d710c3 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 30 Sep 2024 14:44:05 +0200 Subject: [PATCH] feat: improve podman cli execution Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --- .../src/managers/modelsManager.spec.ts | 22 +-- .../backend/src/managers/modelsManager.ts | 32 +++- .../src/managers/podmanConnection.spec.ts | 155 +++++++++++++++++- .../backend/src/managers/podmanConnection.ts | 95 ++++++++++- .../backend/src/utils/modelsUtils.spec.ts | 61 +------ packages/backend/src/utils/modelsUtils.ts | 32 ---- packages/backend/src/utils/podman.ts | 8 +- packages/backend/src/utils/uploader.spec.ts | 10 +- packages/backend/src/utils/uploader.ts | 4 +- .../src/workers/uploader/WSLUploader.spec.ts | 38 ++--- .../src/workers/uploader/WSLUploader.ts | 52 +++--- 11 files changed, 336 insertions(+), 173 deletions(-) diff --git a/packages/backend/src/managers/modelsManager.spec.ts b/packages/backend/src/managers/modelsManager.spec.ts index a519bbe16..723f32316 100644 --- a/packages/backend/src/managers/modelsManager.spec.ts +++ b/packages/backend/src/managers/modelsManager.spec.ts @@ -21,7 +21,7 @@ import os from 'node:os'; import fs, { type Stats, type PathLike } from 'node:fs'; import path from 'node:path'; import { ModelsManager } from './modelsManager'; -import { env, process as coreProcess } from '@podman-desktop/api'; +import { env } from '@podman-desktop/api'; import type { RunResult, TelemetryLogger, Webview, ContainerProviderConnection } from '@podman-desktop/api'; import type { CatalogManager } from './catalogManager'; import type { ModelInfo } from '@shared/src/models/IModelInfo'; @@ -33,7 +33,6 @@ import type { GGUFParseOutput } from '@huggingface/gguf'; import { gguf } from '@huggingface/gguf'; import type { PodmanConnection } from './podmanConnection'; import { VMType } from '@shared/src/models/IPodman'; -import { getPodmanMachineName } from '../utils/podman'; import type { ConfigurationRegistry } from '../registries/ConfigurationRegistry'; import { Uploader } from '../utils/uploader'; @@ -47,7 +46,6 @@ const mocks = vi.hoisted(() => { getTargetMock: vi.fn(), getDownloaderCompleter: vi.fn(), isCompletionEventMock: vi.fn(), - getPodmanCliMock: vi.fn(), }; }); @@ -59,11 +57,6 @@ vi.mock('@huggingface/gguf', () => ({ gguf: vi.fn(), })); -vi.mock('../utils/podman', () => ({ - getPodmanCli: mocks.getPodmanCliMock, - getPodmanMachineName: vi.fn(), -})); - vi.mock('@podman-desktop/api', () => { return { Disposable: { @@ -72,9 +65,6 @@ vi.mock('@podman-desktop/api', () => { env: { isWindows: false, }, - process: { - exec: vi.fn(), - }, fs: { createFileSystemWatcher: (): unknown => ({ onDidCreate: vi.fn(), @@ -102,6 +92,7 @@ vi.mock('../utils/downloader', () => ({ const podmanConnectionMock = { getContainerProviderConnections: vi.fn(), + executeSSH: vi.fn(), } as unknown as PodmanConnection; const cancellationTokenRegistryMock = { @@ -598,8 +589,7 @@ describe('deleting models', () => { }); test('deleting on windows should check for all connections', async () => { - vi.mocked(coreProcess.exec).mockResolvedValue({} as RunResult); - mocks.getPodmanCliMock.mockReturnValue('dummyCli'); + vi.mocked(podmanConnectionMock.executeSSH).mockResolvedValue({} as RunResult); vi.mocked(env).isWindows = true; const connections: ContainerProviderConnection[] = [ { @@ -622,7 +612,6 @@ describe('deleting models', () => { }, ]; vi.mocked(podmanConnectionMock.getContainerProviderConnections).mockReturnValue(connections); - vi.mocked(getPodmanMachineName).mockReturnValue('machine-2'); const rmSpy = vi.spyOn(fs.promises, 'rm'); rmSpy.mockResolvedValue(undefined); @@ -659,10 +648,7 @@ describe('deleting models', () => { expect(podmanConnectionMock.getContainerProviderConnections).toHaveBeenCalledOnce(); - expect(coreProcess.exec).toHaveBeenCalledWith('dummyCli', [ - 'machine', - 'ssh', - 'machine-2', + expect(podmanConnectionMock.executeSSH).toHaveBeenCalledWith(connections[1], [ 'rm', '-f', '/home/user/ai-lab/models/dummyFile', diff --git a/packages/backend/src/managers/modelsManager.ts b/packages/backend/src/managers/modelsManager.ts index 5e42c0129..53d171b8a 100644 --- a/packages/backend/src/managers/modelsManager.ts +++ b/packages/backend/src/managers/modelsManager.ts @@ -30,8 +30,7 @@ import type { Task } from '@shared/src/models/ITask'; import type { BaseEvent } from '../models/baseEvent'; import { isCompletionEvent, isProgressEvent } from '../models/baseEvent'; import { Uploader } from '../utils/uploader'; -import { deleteRemoteModel, getLocalModelFile, isModelUploaded } from '../utils/modelsUtils'; -import { getPodmanMachineName } from '../utils/podman'; +import { getLocalModelFile, getRemoteModelFile } from '../utils/modelsUtils'; import type { CancellationTokenRegistry } from '../registries/CancellationTokenRegistry'; import { getHash, hasValidSha } from '../utils/sha'; import type { GGUFParseOutput } from '@huggingface/gguf'; @@ -231,17 +230,32 @@ export class ModelsManager implements Disposable { for (const connection of connections) { // ignore non-wsl machines if (connection.vmType !== VMType.WSL) continue; - // Get the corresponding machine name - const machineName = getPodmanMachineName(connection); - // check if model already loaded on the podman machine - const existsRemote = await isModelUploaded(machineName, modelInfo); - if (!existsRemote) return; + // check if remote model is + try { + await this.podmanConnection.executeSSH(connection, ['stat', getRemoteModelFile(modelInfo)]); + } catch (err: unknown) { + console.warn(err); + continue; + } - await deleteRemoteModel(machineName, modelInfo); + await this.deleteRemoteModelByConnection(connection, modelInfo); } } + /** + * Delete a model given a {@link ContainerProviderConnection} + * @param connection + * @param modelInfo + * @protected + */ + protected async deleteRemoteModelByConnection( + connection: ContainerProviderConnection, + modelInfo: ModelInfo, + ): Promise { + await this.podmanConnection.executeSSH(connection, ['rm', '-f', getRemoteModelFile(modelInfo)]); + } + /** * This method will resolve when the provided model will be downloaded. * @@ -439,7 +453,7 @@ export class ModelsManager implements Disposable { connection: connection.name, }); - const uploader = new Uploader(connection, model); + const uploader = new Uploader(this.podmanConnection, connection, model); uploader.onEvent(event => this.onDownloadUploadEvent(event, 'upload'), this); // perform download diff --git a/packages/backend/src/managers/podmanConnection.spec.ts b/packages/backend/src/managers/podmanConnection.spec.ts index e9a85f864..f234b9a16 100644 --- a/packages/backend/src/managers/podmanConnection.spec.ts +++ b/packages/backend/src/managers/podmanConnection.spec.ts @@ -20,6 +20,7 @@ import { beforeEach, describe, expect, test, vi } from 'vitest'; import { PodmanConnection } from './podmanConnection'; import type { ContainerProviderConnection, + Extension, ProviderConnectionStatus, ProviderContainerConnection, ProviderEvent, @@ -29,10 +30,11 @@ import type { UpdateContainerConnectionEvent, Webview, } from '@podman-desktop/api'; -import { containerEngine, process, provider, EventEmitter, env } from '@podman-desktop/api'; +import { containerEngine, extensions, process, provider, EventEmitter, env } from '@podman-desktop/api'; import { VMType } from '@shared/src/models/IPodman'; import { Messages } from '@shared/Messages'; import type { ModelInfo } from '@shared/src/models/IModelInfo'; +import { getPodmanCli, getPodmanMachineName } from '../utils/podman'; const webviewMock = { postMessage: vi.fn(), @@ -51,6 +53,9 @@ vi.mock('@podman-desktop/api', async () => { process: { exec: vi.fn(), }, + extensions: { + getExtension: vi.fn(), + }, containerEngine: { listInfos: vi.fn(), }, @@ -64,6 +69,7 @@ vi.mock('@podman-desktop/api', async () => { vi.mock('../utils/podman', () => { return { getPodmanCli: vi.fn(), + getPodmanMachineName: vi.fn(), MIN_CPUS_VALUE: 4, }; }); @@ -73,6 +79,8 @@ beforeEach(() => { vi.mocked(webviewMock.postMessage).mockResolvedValue(true); vi.mocked(provider.getContainerConnections).mockReturnValue([]); + vi.mocked(getPodmanCli).mockReturnValue('podman-executable'); + vi.mocked(getPodmanMachineName).mockImplementation(connection => connection.name); const listeners: ((value: unknown) => void)[] = []; @@ -86,6 +94,151 @@ beforeEach(() => { } as unknown as EventEmitter); }); +const providerContainerConnectionMock: ProviderContainerConnection = { + connection: { + type: 'podman', + status: () => 'started', + name: 'Podman Machine', + endpoint: { + socketPath: './socket-path', + }, + }, + providerId: 'podman', +}; + +describe('execute', () => { + test('execute should get the podman extension from api', async () => { + vi.mocked(extensions.getExtension).mockReturnValue(undefined); + const manager = new PodmanConnection(webviewMock); + await manager.execute(providerContainerConnectionMock.connection, ['ls']); + expect(extensions.getExtension).toHaveBeenCalledWith('podman-desktop.podman'); + }); + + test('execute should call getPodmanCli if extension not available', async () => { + vi.mocked(extensions.getExtension).mockReturnValue(undefined); + const manager = new PodmanConnection(webviewMock); + await manager.execute(providerContainerConnectionMock.connection, ['ls']); + + expect(getPodmanCli).toHaveBeenCalledOnce(); + expect(process.exec).toHaveBeenCalledWith('podman-executable', ['ls'], undefined); + }); + + test('options should be propagated to process execution when provided', async () => { + vi.mocked(extensions.getExtension).mockReturnValue(undefined); + const manager = new PodmanConnection(webviewMock); + await manager.execute(providerContainerConnectionMock.connection, ['ls'], { + isAdmin: true, + }); + + expect(getPodmanCli).toHaveBeenCalledOnce(); + expect(process.exec).toHaveBeenCalledWith('podman-executable', ['ls'], { + isAdmin: true, + }); + }); + + test('execute should use extension exec if available', async () => { + vi.mocked(provider.getContainerConnections).mockReturnValue([providerContainerConnectionMock]); + const podmanAPI = { + exec: vi.fn(), + }; + vi.mocked(extensions.getExtension).mockReturnValue({ exports: podmanAPI } as unknown as Extension); + const manager = new PodmanConnection(webviewMock); + await manager.execute(providerContainerConnectionMock.connection, ['ls']); + + expect(getPodmanCli).not.toHaveBeenCalledOnce(); + expect(podmanAPI.exec).toHaveBeenCalledWith(['ls'], { + connection: providerContainerConnectionMock, + }); + }); + + test('an error should be throw if the provided container connection do not exists', async () => { + vi.mocked(provider.getContainerConnections).mockReturnValue([]); + const podmanAPI = { + exec: vi.fn(), + }; + vi.mocked(extensions.getExtension).mockReturnValue({ exports: podmanAPI } as unknown as Extension); + const manager = new PodmanConnection(webviewMock); + + await expect(async () => { + await manager.execute(providerContainerConnectionMock.connection, ['ls'], { + isAdmin: true, + }); + }).rejects.toThrowError('cannot find podman provider with connection name Podman Machine'); + }); + + test('execute should propagate options to extension exec if available', async () => { + vi.mocked(provider.getContainerConnections).mockReturnValue([providerContainerConnectionMock]); + const podmanAPI = { + exec: vi.fn(), + }; + vi.mocked(extensions.getExtension).mockReturnValue({ exports: podmanAPI } as unknown as Extension); + const manager = new PodmanConnection(webviewMock); + await manager.execute(providerContainerConnectionMock.connection, ['ls'], { + isAdmin: true, + }); + + expect(getPodmanCli).not.toHaveBeenCalledOnce(); + expect(podmanAPI.exec).toHaveBeenCalledWith(['ls'], { + isAdmin: true, + connection: providerContainerConnectionMock, + }); + }); +}); + +describe('executeSSH', () => { + test('executeSSH should call getPodmanCli if extension not available', async () => { + vi.mocked(extensions.getExtension).mockReturnValue(undefined); + const manager = new PodmanConnection(webviewMock); + await manager.executeSSH(providerContainerConnectionMock.connection, ['ls']); + + expect(getPodmanCli).toHaveBeenCalledOnce(); + expect(process.exec).toHaveBeenCalledWith( + 'podman-executable', + ['machine', 'ssh', providerContainerConnectionMock.connection.name, 'ls'], + undefined, + ); + }); + + test('executeSSH should use extension exec if available', async () => { + vi.mocked(provider.getContainerConnections).mockReturnValue([providerContainerConnectionMock]); + const podmanAPI = { + exec: vi.fn(), + }; + vi.mocked(extensions.getExtension).mockReturnValue({ exports: podmanAPI } as unknown as Extension); + const manager = new PodmanConnection(webviewMock); + await manager.executeSSH(providerContainerConnectionMock.connection, ['ls']); + + expect(getPodmanCli).not.toHaveBeenCalledOnce(); + expect(podmanAPI.exec).toHaveBeenCalledWith( + ['machine', 'ssh', providerContainerConnectionMock.connection.name, 'ls'], + { + connection: providerContainerConnectionMock, + }, + ); + }); + + test('executeSSH should propagate options to extension exec if available', async () => { + vi.mocked(provider.getContainerConnections).mockReturnValue([providerContainerConnectionMock]); + const podmanAPI = { + exec: vi.fn(), + }; + vi.mocked(extensions.getExtension).mockReturnValue({ exports: podmanAPI } as unknown as Extension); + const manager = new PodmanConnection(webviewMock); + await manager.executeSSH(providerContainerConnectionMock.connection, ['ls'], { + isAdmin: true, + }); + + expect(getPodmanCli).not.toHaveBeenCalledOnce(); + expect(podmanAPI.exec).toHaveBeenCalledWith( + ['machine', 'ssh', providerContainerConnectionMock.connection.name, 'ls'], + { + isAdmin: true, + connection: providerContainerConnectionMock, + }, + ); + }); +}); + describe('podman connection initialization', () => { test('init should notify publisher', () => { const manager = new PodmanConnection(webviewMock); diff --git a/packages/backend/src/managers/podmanConnection.ts b/packages/backend/src/managers/podmanConnection.ts index 377fb2011..79ac06abf 100644 --- a/packages/backend/src/managers/podmanConnection.ts +++ b/packages/backend/src/managers/podmanConnection.ts @@ -23,10 +23,12 @@ import type { RegisterContainerConnectionEvent, UpdateContainerConnectionEvent, Webview, + RunResult, + RunOptions, + ProviderContainerConnection, } from '@podman-desktop/api'; -import { containerEngine, env, navigation, EventEmitter, process, provider } from '@podman-desktop/api'; -import type { MachineJSON } from '../utils/podman'; -import { MIN_CPUS_VALUE, getPodmanCli } from '../utils/podman'; +import { containerEngine, env, navigation, EventEmitter, process, provider, extensions } from '@podman-desktop/api'; +import { getPodmanMachineName, type MachineJSON, MIN_CPUS_VALUE, getPodmanCli } from '../utils/podman'; import { VMType } from '@shared/src/models/IPodman'; import { Publisher } from '../utils/Publisher'; import type { @@ -40,6 +42,10 @@ export interface PodmanConnectionEvent { status: 'stopped' | 'started' | 'unregister' | 'register'; } +export interface PodmanRunOptions extends RunOptions { + connection?: ProviderContainerConnection; +} + export class PodmanConnection extends Publisher implements Disposable { // Map of providerId with corresponding connections #providers: Map; @@ -54,6 +60,71 @@ export class PodmanConnection extends Publisher { + const podman = extensions.getExtension('podman-desktop.podman'); + if (!podman) { + console.warn('cannot find podman extension api'); + return this.executeLegacy(args, options); + } + + const podmanApi: { + exec(args: string[], options?: PodmanRunOptions): Promise; + } = podman.exports; + + return podmanApi.exec(args, { + ...options, + connection: this.getProviderContainerConnection(connection), + }); + } + + /** + * Execute a command inside the podman machine + * + * @example + * ``` + * const result = await podman.executeSSH(connection, ['ls', '/dev']); + * ``` + * @param connection + * @param args + * @param options + */ + executeSSH(connection: ContainerProviderConnection, args: string[], options?: RunOptions): Promise { + return this.execute(connection, ['machine', 'ssh', this.getNameLegacyCompatibility(connection), ...args], options); + } + + /** + * Before 1.13, the podman extension was not exposing any api. + * + * Therefore, to support old version we need to get the podman executable ourself + * @deprecated + */ + protected executeLegacy(args: string[], options?: RunOptions): Promise { + return process.exec(getPodmanCli(), [...args], options); + } + + /** + * Before 1.13, the {@link ContainerProviderConnection.name} field was used as friendly user + * field also. + * + * Therefore, we could have `Podman Machine Default` as name, where the real machine was `podman-machine-default`. + * @param connection + * @deprecated + */ + protected getNameLegacyCompatibility(connection: ContainerProviderConnection): string { + return getPodmanMachineName(connection); + } + getContainerProviderConnections(): ContainerProviderConnection[] { return Array.from(this.#providers.values()).flat(); } @@ -92,11 +163,27 @@ export class PodmanConnection extends Publisher disposable.dispose()); } + /** + * This method allow us to get the ProviderContainerConnection given a ContainerProviderConnection + * @param connection + * @protected + */ + protected getProviderContainerConnection(connection: ContainerProviderConnection): ProviderContainerConnection { + const providers: ProviderContainerConnection[] = provider.getContainerConnections(); + + const podmanProvider = providers + .filter(({ connection }) => connection.type === 'podman') + .find(provider => provider.connection.name === connection.name); + if (!podmanProvider) throw new Error(`cannot find podman provider with connection name ${connection.name}`); + + return podmanProvider; + } + protected refreshProviders(): void { // clear all providers this.#providers.clear(); - const providers = provider.getContainerConnections(); + const providers: ProviderContainerConnection[] = provider.getContainerConnections(); // register the podman container connection providers diff --git a/packages/backend/src/utils/modelsUtils.spec.ts b/packages/backend/src/utils/modelsUtils.spec.ts index 0bb9cc1ba..8d8482f42 100644 --- a/packages/backend/src/utils/modelsUtils.spec.ts +++ b/packages/backend/src/utils/modelsUtils.spec.ts @@ -16,16 +16,8 @@ * SPDX-License-Identifier: Apache-2.0 ***********************************************************************/ import { beforeEach, describe, expect, test, vi } from 'vitest'; -import { process as apiProcess } from '@podman-desktop/api'; -import { - deleteRemoteModel, - getLocalModelFile, - getRemoteModelFile, - isModelUploaded, - MACHINE_BASE_FOLDER, -} from './modelsUtils'; +import { getLocalModelFile, getRemoteModelFile, MACHINE_BASE_FOLDER } from './modelsUtils'; import type { ModelInfo } from '@shared/src/models/IModelInfo'; -import { getPodmanCli } from './podman'; vi.mock('@podman-desktop/api', () => { return { @@ -35,14 +27,8 @@ vi.mock('@podman-desktop/api', () => { }; }); -vi.mock('./podman', () => ({ - getPodmanCli: vi.fn(), -})); - beforeEach(() => { vi.resetAllMocks(); - - vi.mocked(getPodmanCli).mockReturnValue('dummyPodmanCli'); }); describe('getLocalModelFile', () => { @@ -94,48 +80,3 @@ describe('getRemoteModelFile', () => { expect(path).toBe(`${MACHINE_BASE_FOLDER}dummy.guff`); }); }); - -describe('isModelUploaded', () => { - test('execute stat on targeted machine', async () => { - expect( - await isModelUploaded('dummyMachine', { - id: 'dummyModelId', - file: { - path: 'dummyPath', - file: 'dummy.guff', - }, - } as unknown as ModelInfo), - ).toBeTruthy(); - - expect(getPodmanCli).toHaveBeenCalled(); - expect(apiProcess.exec).toHaveBeenCalledWith('dummyPodmanCli', [ - 'machine', - 'ssh', - 'dummyMachine', - 'stat', - expect.anything(), - ]); - }); -}); - -describe('deleteRemoteModel', () => { - test('execute stat on targeted machine', async () => { - await deleteRemoteModel('dummyMachine', { - id: 'dummyModelId', - file: { - path: 'dummyPath', - file: 'dummy.guff', - }, - } as unknown as ModelInfo); - - expect(getPodmanCli).toHaveBeenCalled(); - expect(apiProcess.exec).toHaveBeenCalledWith('dummyPodmanCli', [ - 'machine', - 'ssh', - 'dummyMachine', - 'rm', - '-f', - expect.anything(), - ]); - }); -}); diff --git a/packages/backend/src/utils/modelsUtils.ts b/packages/backend/src/utils/modelsUtils.ts index 5e9ed2f5b..7cefb8bad 100644 --- a/packages/backend/src/utils/modelsUtils.ts +++ b/packages/backend/src/utils/modelsUtils.ts @@ -17,8 +17,6 @@ ***********************************************************************/ import type { ModelInfo } from '@shared/src/models/IModelInfo'; import { join, posix } from 'node:path'; -import { getPodmanCli } from './podman'; -import { process } from '@podman-desktop/api'; export const MACHINE_BASE_FOLDER = '/home/user/ai-lab/models/'; @@ -42,36 +40,6 @@ export function getRemoteModelFile(modelInfo: ModelInfo): string { return posix.join(MACHINE_BASE_FOLDER, modelInfo.file.file); } -/** - * utility method to determine if a model is already uploaded to the podman machine - * @param machine - * @param modelInfo - */ -export async function isModelUploaded(machine: string, modelInfo: ModelInfo): Promise { - try { - const remotePath = getRemoteModelFile(modelInfo); - await process.exec(getPodmanCli(), ['machine', 'ssh', machine, 'stat', remotePath]); - return true; - } catch (err: unknown) { - console.error('Something went wrong while trying to stat remote model path', err); - return false; - } -} - -/** - * Given a machine and a modelInfo, delete the corresponding file on the podman machine - * @param machine the machine to target - * @param modelInfo the model info - */ -export async function deleteRemoteModel(machine: string, modelInfo: ModelInfo): Promise { - try { - const remotePath = getRemoteModelFile(modelInfo); - await process.exec(getPodmanCli(), ['machine', 'ssh', machine, 'rm', '-f', remotePath]); - } catch (err: unknown) { - console.error('Something went wrong while trying to stat remote model path', err); - } -} - export function getModelPropertiesForEnvironment(modelInfo: ModelInfo): string[] { const envs: string[] = []; if (modelInfo.properties) { diff --git a/packages/backend/src/utils/podman.ts b/packages/backend/src/utils/podman.ts index d66718ca8..cffc56fab 100644 --- a/packages/backend/src/utils/podman.ts +++ b/packages/backend/src/utils/podman.ts @@ -32,6 +32,11 @@ export type MachineJSON = { VMType?: string; }; +/** + * We should be using the {@link @podman-desktop/api#extensions.getExtensions} function to get podman + * exec method + * @deprecated + */ export function getPodmanCli(): string { // If we have a custom binary path regardless if we are running Windows or not const customBinaryPath = getCustomBinaryPath(); @@ -52,8 +57,9 @@ export function getCustomBinaryPath(): string | undefined { } /** - * In the ${link ContainerProviderConnection.name} property the name is not usage, and we need to transform it + * In the {@link ContainerProviderConnection.name} property the name is not usage, and we need to transform it * @param connection + * @deprecated */ export function getPodmanMachineName(connection: ContainerProviderConnection): string { const runningConnectionName = connection.name; diff --git a/packages/backend/src/utils/uploader.spec.ts b/packages/backend/src/utils/uploader.spec.ts index a4d87ce83..191002f4c 100644 --- a/packages/backend/src/utils/uploader.spec.ts +++ b/packages/backend/src/utils/uploader.spec.ts @@ -24,15 +24,13 @@ import { Uploader } from './uploader'; import type { ModelInfo } from '@shared/src/models/IModelInfo'; import type { ContainerProviderConnection } from '@podman-desktop/api'; import { VMType } from '@shared/src/models/IPodman'; +import type { PodmanConnection } from '../managers/podmanConnection'; vi.mock('@podman-desktop/api', async () => { return { env: { isWindows: false, }, - process: { - exec: vi.fn(), - }, EventEmitter: vi.fn().mockImplementation(() => { return { fire: vi.fn(), @@ -41,6 +39,10 @@ vi.mock('@podman-desktop/api', async () => { }; }); +const podmanConnectionMock: PodmanConnection = { + executeSSH: vi.fn(), +} as unknown as PodmanConnection; + const connectionMock: ContainerProviderConnection = { name: 'machine2', type: 'podman', @@ -51,7 +53,7 @@ const connectionMock: ContainerProviderConnection = { }, }; -const uploader = new Uploader(connectionMock, { +const uploader = new Uploader(podmanConnectionMock, connectionMock, { id: 'dummyModelId', file: { file: 'dummyFile.guff', diff --git a/packages/backend/src/utils/uploader.ts b/packages/backend/src/utils/uploader.ts index fbb13a872..a38ffb858 100644 --- a/packages/backend/src/utils/uploader.ts +++ b/packages/backend/src/utils/uploader.ts @@ -24,6 +24,7 @@ import type { ModelInfo } from '@shared/src/models/IModelInfo'; import { getLocalModelFile } from './modelsUtils'; import type { IWorker } from '../workers/IWorker'; import type { UploaderOptions } from '../workers/uploader/UploaderOptions'; +import type { PodmanConnection } from '../managers/podmanConnection'; export class Uploader { readonly #_onEvent = new EventEmitter(); @@ -31,11 +32,12 @@ export class Uploader { readonly #workers: IWorker[] = []; constructor( + podman: PodmanConnection, private connection: ContainerProviderConnection, private modelInfo: ModelInfo, private abortSignal?: AbortSignal, ) { - this.#workers = [new WSLUploader()]; + this.#workers = [new WSLUploader(podman)]; } /** diff --git a/packages/backend/src/workers/uploader/WSLUploader.spec.ts b/packages/backend/src/workers/uploader/WSLUploader.spec.ts index 3f07ac147..13896e5cf 100644 --- a/packages/backend/src/workers/uploader/WSLUploader.spec.ts +++ b/packages/backend/src/workers/uploader/WSLUploader.spec.ts @@ -21,6 +21,7 @@ import { WSLUploader } from './WSLUploader'; import type { ModelInfo } from '@shared/src/models/IModelInfo'; import { configuration, env, process, type ContainerProviderConnection, type RunResult } from '@podman-desktop/api'; import { VMType } from '@shared/src/models/IPodman'; +import type { PodmanConnection } from '../../managers/podmanConnection'; vi.mock('@podman-desktop/api', () => ({ env: { @@ -34,6 +35,10 @@ vi.mock('@podman-desktop/api', () => ({ }, })); +const podmanConnectionMock: PodmanConnection = { + executeSSH: vi.fn(), +} as unknown as PodmanConnection; + const connectionMock: ContainerProviderConnection = { name: 'machine2', type: 'podman', @@ -44,11 +49,17 @@ const connectionMock: ContainerProviderConnection = { }, }; -const wslUploader = new WSLUploader(); +const wslUploader = new WSLUploader(podmanConnectionMock); beforeEach(() => { vi.resetAllMocks(); + vi.mocked(podmanConnectionMock.executeSSH).mockResolvedValue({ + stderr: '', + stdout: '', + command: '', + } as RunResult); + vi.mocked(configuration.getConfiguration).mockReturnValue({ get: () => 'podman.exe', has: vi.fn(), @@ -98,7 +109,7 @@ describe('upload', () => { }); test('copy model if not exists on podman machine', async () => { - vi.mocked(process.exec).mockRejectedValueOnce('error'); + vi.mocked(podmanConnectionMock.executeSSH).mockRejectedValueOnce('error'); await wslUploader.perform({ connection: connectionMock, model: { @@ -106,25 +117,16 @@ describe('upload', () => { file: { path: 'C:\\Users\\podman\\folder', file: 'dummy.guff' }, } as unknown as ModelInfo, }); - expect(process.exec).toBeCalledWith('podman.exe', [ - 'machine', - 'ssh', - 'machine2', + expect(podmanConnectionMock.executeSSH).toBeCalledWith(connectionMock, [ 'stat', '/home/user/ai-lab/models/dummy.guff', ]); - expect(process.exec).toBeCalledWith('podman.exe', [ - 'machine', - 'ssh', - 'machine2', + expect(podmanConnectionMock.executeSSH).toBeCalledWith(connectionMock, [ 'mkdir', '-p', '/home/user/ai-lab/models/', ]); - expect(process.exec).toBeCalledWith('podman.exe', [ - 'machine', - 'ssh', - 'machine2', + expect(podmanConnectionMock.executeSSH).toBeCalledWith(connectionMock, [ 'cp', '/mnt/c/Users/podman/folder/dummy.guff', '/home/user/ai-lab/models/dummy.guff', @@ -132,7 +134,6 @@ describe('upload', () => { }); test('do not copy model if it exists on podman machine', async () => { - vi.mocked(process.exec).mockResolvedValue({} as RunResult); await wslUploader.perform({ connection: connectionMock, model: { @@ -140,13 +141,10 @@ describe('upload', () => { file: { path: 'C:\\Users\\podman\\folder', file: 'dummy.guff' }, } as unknown as ModelInfo, }); - expect(process.exec).toBeCalledWith('podman.exe', [ - 'machine', - 'ssh', - 'machine2', + expect(podmanConnectionMock.executeSSH).toBeCalledWith(connectionMock, [ 'stat', '/home/user/ai-lab/models/dummy.guff', ]); - expect(process.exec).toBeCalledTimes(1); + expect(podmanConnectionMock.executeSSH).toBeCalledTimes(1); }); }); diff --git a/packages/backend/src/workers/uploader/WSLUploader.ts b/packages/backend/src/workers/uploader/WSLUploader.ts index d8c6ec72e..dbdff6d20 100644 --- a/packages/backend/src/workers/uploader/WSLUploader.ts +++ b/packages/backend/src/workers/uploader/WSLUploader.ts @@ -16,14 +16,35 @@ * SPDX-License-Identifier: Apache-2.0 ***********************************************************************/ -import * as podmanDesktopApi from '@podman-desktop/api'; -import { getPodmanCli, getPodmanMachineName } from '../../utils/podman'; -import { getLocalModelFile, getRemoteModelFile, isModelUploaded, MACHINE_BASE_FOLDER } from '../../utils/modelsUtils'; +import { getLocalModelFile, getRemoteModelFile, MACHINE_BASE_FOLDER } from '../../utils/modelsUtils'; import { WindowsWorker } from '../WindowsWorker'; import { VMType } from '@shared/src/models/IPodman'; import type { UploaderOptions } from './UploaderOptions'; +import type { PodmanConnection } from '../../managers/podmanConnection'; +import type { ContainerProviderConnection } from '@podman-desktop/api'; +import type { ModelInfo } from '@shared/src/models/IModelInfo'; export class WSLUploader extends WindowsWorker { + constructor(private podman: PodmanConnection) { + super(); + } + + /** + * @param connection + * @param model + * @protected + */ + protected async isModelUploaded(connection: ContainerProviderConnection, model: ModelInfo): Promise { + const remoteFile = getRemoteModelFile(model); + try { + const result = await this.podman.executeSSH(connection, ['stat', remoteFile]); + return (result.stderr ?? '').length === 0; + } catch (err: unknown) { + console.debug(err); + return false; + } + } + async perform(options: UploaderOptions): Promise { const localPath = getLocalModelFile(options.model); @@ -33,36 +54,21 @@ export class WSLUploader extends WindowsWorker { return localPath; } - // the connection name cannot be used as it is - const machineName = getPodmanMachineName(options.connection); - const driveLetter = localPath.charAt(0); const convertToMntPath = localPath .replace(`${driveLetter}:\\`, `/mnt/${driveLetter.toLowerCase()}/`) .replace(/\\/g, '/'); // check if model already loaded on the podman machine - const existsRemote = await isModelUploaded(machineName, options.model); + const existsRemote = await this.isModelUploaded(options.connection, options.model); + + // get the name of the file on the machine const remoteFile = getRemoteModelFile(options.model); // if not exists remotely it copies it from the local path if (!existsRemote) { - await podmanDesktopApi.process.exec(getPodmanCli(), [ - 'machine', - 'ssh', - machineName, - 'mkdir', - '-p', - MACHINE_BASE_FOLDER, - ]); - await podmanDesktopApi.process.exec(getPodmanCli(), [ - 'machine', - 'ssh', - machineName, - 'cp', - convertToMntPath, - remoteFile, - ]); + await this.podman.executeSSH(options.connection, ['mkdir', '-p', MACHINE_BASE_FOLDER]); + await this.podman.executeSSH(options.connection, ['cp', convertToMntPath, remoteFile]); } return remoteFile;