From c957f6d67be3cedc4a24ac17b77b40f0053ccbcf Mon Sep 17 00:00:00 2001 From: lstocchi Date: Thu, 29 Feb 2024 18:31:39 +0100 Subject: [PATCH] fix: use machine name to execute podman cli Signed-off-by: lstocchi --- .../backend/src/managers/playground.spec.ts | 4 +- packages/backend/src/managers/playground.ts | 2 +- .../src/managers/podmanConnection.spec.ts | 8 +- .../backend/src/managers/podmanConnection.ts | 6 +- .../backend/src/models/WSLUploader.spec.ts | 19 ++++- packages/backend/src/models/WSLUploader.ts | 14 +--- packages/backend/src/utils/podman.spec.ts | 84 +++++++------------ packages/backend/src/utils/podman.ts | 25 ++++-- 8 files changed, 76 insertions(+), 86 deletions(-) diff --git a/packages/backend/src/managers/playground.spec.ts b/packages/backend/src/managers/playground.spec.ts index cb865d663..d3c040f08 100644 --- a/packages/backend/src/managers/playground.spec.ts +++ b/packages/backend/src/managers/playground.spec.ts @@ -106,7 +106,7 @@ test('startPlayground should fail if no provider', async () => { test('startPlayground should download image if not present then create container', async () => { mocks.postMessage.mockResolvedValue(undefined); - mocks.getFirstRunningPodmanConnectionMock.mockResolvedValue({ + mocks.getFirstRunningPodmanConnectionMock.mockReturnValue({ connection: { type: 'podman', status: () => 'started', @@ -162,7 +162,7 @@ test('stopPlayground should fail if no playground is running', async () => { test('stopPlayground should stop a started playground', async () => { mocks.postMessage.mockResolvedValue(undefined); - mocks.getFirstRunningPodmanConnectionMock.mockResolvedValue({ + mocks.getFirstRunningPodmanConnectionMock.mockReturnValue({ connection: { type: 'podman', status: () => 'started', diff --git a/packages/backend/src/managers/playground.ts b/packages/backend/src/managers/playground.ts index 448efd866..9cbd280aa 100644 --- a/packages/backend/src/managers/playground.ts +++ b/packages/backend/src/managers/playground.ts @@ -157,7 +157,7 @@ export class PlayGroundManager { this.setPlaygroundStatus(modelId, 'starting'); - const connection = await getFirstRunningPodmanConnection(); + const connection = getFirstRunningPodmanConnection(); if (!connection) { const error = 'Unable to find an engine to start playground'; this.setPlaygroundError(modelId, error); diff --git a/packages/backend/src/managers/podmanConnection.spec.ts b/packages/backend/src/managers/podmanConnection.spec.ts index 16d402bd7..015e4db31 100644 --- a/packages/backend/src/managers/podmanConnection.spec.ts +++ b/packages/backend/src/managers/podmanConnection.spec.ts @@ -44,7 +44,7 @@ vi.mock('../utils/podman', () => { test('startupSubscribe should execute immediately if provider already registered', async () => { const manager = new PodmanConnection(); // one provider is already registered - mocks.getFirstRunningPodmanConnectionMock.mockResolvedValue({ + mocks.getFirstRunningPodmanConnectionMock.mockReturnValue({ connection: { type: 'podman', status: () => 'started', @@ -53,7 +53,7 @@ test('startupSubscribe should execute immediately if provider already registered mocks.onDidRegisterContainerConnection.mockReturnValue({ dispose: vi.fn, }); - await manager.listenRegistration(); + manager.listenRegistration(); const handler = vi.fn(); manager.startupSubscribe(handler); // the handler is called immediately @@ -64,7 +64,7 @@ test('startupSubscribe should execute when provider is registered', async () => const manager = new PodmanConnection(); // no provider is already registered - mocks.getFirstRunningPodmanConnectionMock.mockResolvedValue(undefined); + mocks.getFirstRunningPodmanConnectionMock.mockReturnValue(undefined); mocks.onDidRegisterContainerConnection.mockImplementation((f: (e: RegisterContainerConnectionEvent) => void) => { setTimeout(() => { f({ @@ -78,7 +78,7 @@ test('startupSubscribe should execute when provider is registered', async () => dispose: vi.fn(), }; }); - await manager.listenRegistration(); + manager.listenRegistration(); const handler = vi.fn(); manager.startupSubscribe(handler); // the handler is not called immediately diff --git a/packages/backend/src/managers/podmanConnection.ts b/packages/backend/src/managers/podmanConnection.ts index 36f6dbefa..8b6be0641 100644 --- a/packages/backend/src/managers/podmanConnection.ts +++ b/packages/backend/src/managers/podmanConnection.ts @@ -45,7 +45,7 @@ export class PodmanConnection implements Disposable { #onEventDisposable: Disposable | undefined; init(): void { - this.listenRegistration().catch((e: unknown) => console.error(String(e))); + this.listenRegistration(); this.listenMachine(); this.watchPods(); } @@ -54,7 +54,7 @@ export class PodmanConnection implements Disposable { this.#onEventDisposable?.dispose(); } - async listenRegistration() { + listenRegistration() { // In case the extension has not yet registered, we listen for new registrations // and retain the first started podman provider const disposable = provider.onDidRegisterContainerConnection((e: RegisterContainerConnectionEvent) => { @@ -73,7 +73,7 @@ export class PodmanConnection implements Disposable { }); // In case at least one extension has already registered, we get one started podman provider - const engine = await getFirstRunningPodmanConnection(); + const engine = getFirstRunningPodmanConnection(); if (engine) { disposable.dispose(); this.#firstFound = true; diff --git a/packages/backend/src/models/WSLUploader.spec.ts b/packages/backend/src/models/WSLUploader.spec.ts index 4f0d1a74f..04c6ff385 100644 --- a/packages/backend/src/models/WSLUploader.spec.ts +++ b/packages/backend/src/models/WSLUploader.spec.ts @@ -57,6 +57,15 @@ describe('canUpload', () => { }); describe('upload', () => { + const machine2: utils.MachineJSON = { + Name: 'machine2', + CPUs: 2, + Memory: '2000', + DiskSize: '100', + Running: true, + Starting: false, + Default: true, + }; vi.spyOn(utils, 'getPodmanCli').mockReturnValue('podman'); vi.spyOn(utils, 'getFirstRunningPodmanConnection').mockResolvedValue({ connection: { @@ -73,18 +82,20 @@ describe('upload', () => { await expect(wslUploader.upload('')).rejects.toThrowError('invalid local path'); }); test('copy model if not exists on podman machine', async () => { - mocks.execMock.mockRejectedValueOnce(''); + mocks.execMock.mockRejectedValueOnce('error'); + vi.spyOn(utils, 'getFirstRunningMachine').mockResolvedValue(machine2); await wslUploader.upload('C:\\Users\\podman\\folder\\file'); - expect(mocks.execMock).toBeCalledWith('podman', ['machine', 'ssh', 'test', 'stat', '/home/user/file']); + expect(mocks.execMock).toBeCalledWith('podman', ['machine', 'ssh', 'machine2', 'stat', '/home/user/file']); }); test('do not copy model if it exists on podman machine', async () => { mocks.execMock.mockResolvedValue(''); + vi.spyOn(utils, 'getFirstRunningMachine').mockResolvedValue(machine2); await wslUploader.upload('C:\\Users\\podman\\folder\\file'); - expect(mocks.execMock).toBeCalledWith('podman', ['machine', 'ssh', 'test', 'stat', '/home/user/file']); + expect(mocks.execMock).toBeCalledWith('podman', ['machine', 'ssh', 'machine2', 'stat', '/home/user/file']); expect(mocks.execMock).toBeCalledWith('podman', [ 'machine', 'ssh', - 'test', + 'machine2', 'cp', '/mnt/c/Users/podman/folder/file', '/home/user/file', diff --git a/packages/backend/src/models/WSLUploader.ts b/packages/backend/src/models/WSLUploader.ts index f03845d3f..e70ec3124 100644 --- a/packages/backend/src/models/WSLUploader.ts +++ b/packages/backend/src/models/WSLUploader.ts @@ -18,7 +18,7 @@ import path from 'node:path'; import * as podmanDesktopApi from '@podman-desktop/api'; -import { getFirstRunningPodmanConnection, getPodmanCli } from '../utils/podman'; +import { getFirstRunningMachine, getPodmanCli } from '../utils/podman'; import type { UploadWorker } from './uploader'; export class WSLUploader implements UploadWorker { @@ -36,17 +36,11 @@ export class WSLUploader implements UploadWorker { .replace(`${driveLetter}:\\`, `/mnt/${driveLetter.toLowerCase()}/`) .replace(/\\/g, '/'); const remotePath = `/home/user/${path.basename(convertToMntPath)}`; - const connection = await getFirstRunningPodmanConnection(); + const machine = await getFirstRunningMachine(); // check if model already loaded on the podman machine let existsRemote = true; try { - await podmanDesktopApi.process.exec(getPodmanCli(), [ - 'machine', - 'ssh', - connection.connection.name, - 'stat', - remotePath, - ]); + await podmanDesktopApi.process.exec(getPodmanCli(), ['machine', 'ssh', machine.Name, 'stat', remotePath]); } catch (e) { existsRemote = false; } @@ -56,7 +50,7 @@ export class WSLUploader implements UploadWorker { await podmanDesktopApi.process.exec(getPodmanCli(), [ 'machine', 'ssh', - connection.connection.name, + machine.Name, 'cp', convertToMntPath, remotePath, diff --git a/packages/backend/src/utils/podman.spec.ts b/packages/backend/src/utils/podman.spec.ts index 0b856fa78..7a5a49251 100644 --- a/packages/backend/src/utils/podman.spec.ts +++ b/packages/backend/src/utils/podman.spec.ts @@ -24,7 +24,6 @@ import { beforeEach } from 'node:test'; const mocks = vi.hoisted(() => { return { getConfigurationMock: vi.fn(), - execMock: vi.fn(), getContainerConnectionsMock: vi.fn(), }; }); @@ -42,9 +41,6 @@ vi.mock('@podman-desktop/api', () => ({ configuration: { getConfiguration: () => config, }, - process: { - exec: mocks.execMock, - }, provider: { getContainerConnections: mocks.getContainerConnectionsMock, }, @@ -75,63 +71,45 @@ describe('getPodmanCli', () => { }); describe('getFirstRunningPodmanConnection', () => { - test('should return undefined if failing at retrieving machine list', async () => { - mocks.execMock.mockRejectedValue('error'); - const result = await utils.getFirstRunningPodmanConnection(); + test('should return undefined if failing at retrieving connection', async () => { + mocks.getConfigurationMock.mockRejectedValue('error'); + const result = utils.getFirstRunningPodmanConnection(); expect(result).toBeUndefined(); }); test('should return undefined if default podman machine is not running', async () => { - const machine = { - Name: 'machine', - CPUs: 2, - Memory: 2000, - DiskSize: '100', - Running: false, - Starting: false, - Default: true, - }; - const machine2 = { - Name: 'machine2', - CPUs: 2, - Memory: 2000, - DiskSize: '100', - Running: true, - Starting: false, - Default: false, - }; - mocks.execMock.mockResolvedValue({ - stdout: JSON.stringify([machine2, machine]), - }); - const result = await utils.getFirstRunningPodmanConnection(); + mocks.getContainerConnectionsMock.mockReturnValue([ + { + connection: { + name: 'machine', + status: () => 'stopped', + endpoint: { + socketPath: '/endpoint.sock', + }, + type: 'podman', + }, + providerId: 'podman', + }, + { + connection: { + name: 'machine2', + status: () => 'stopped', + endpoint: { + socketPath: '/endpoint.sock', + }, + type: 'podman', + }, + providerId: 'podman2', + }, + ]); + const result = utils.getFirstRunningPodmanConnection(); expect(result).toBeUndefined(); }); test('should return default running podman connection', async () => { - const machine = { - Name: 'machine', - CPUs: 2, - Memory: 2000, - DiskSize: '100', - Running: false, - Starting: false, - Default: false, - }; - const machine2 = { - Name: 'machine2', - CPUs: 2, - Memory: 2000, - DiskSize: '100', - Running: true, - Starting: false, - Default: true, - }; - mocks.execMock.mockResolvedValue({ - stdout: JSON.stringify([machine, machine2]), - }); mocks.getContainerConnectionsMock.mockReturnValue([ { connection: { name: 'machine', - status: vi.fn(), + status: () => 'stopped', endpoint: { socketPath: '/endpoint.sock', }, @@ -142,7 +120,7 @@ describe('getFirstRunningPodmanConnection', () => { { connection: { name: 'machine2', - status: vi.fn(), + status: () => 'started', endpoint: { socketPath: '/endpoint.sock', }, @@ -151,7 +129,7 @@ describe('getFirstRunningPodmanConnection', () => { providerId: 'podman2', }, ]); - const result = await utils.getFirstRunningPodmanConnection(); + const result = utils.getFirstRunningPodmanConnection(); expect(result.connection.name).equal('machine2'); }); }); diff --git a/packages/backend/src/utils/podman.ts b/packages/backend/src/utils/podman.ts index b874a319b..10b989393 100644 --- a/packages/backend/src/utils/podman.ts +++ b/packages/backend/src/utils/podman.ts @@ -53,18 +53,25 @@ async function getJSONMachineList(): Promise { return stdout; } -export async function getFirstRunningPodmanConnection(): Promise { - let engine: ProviderContainerConnection; +export async function getFirstRunningMachine(): Promise { try { const machineListOutput = await getJSONMachineList(); const machines = JSON.parse(machineListOutput) as MachineJSON[]; - const machine = machines.find(machine => machine.Default && machine.Running); - if (machine) { - engine = provider - .getContainerConnections() - .filter(connection => connection.connection.type === 'podman') - .find(connection => connection.connection.name === machine.Name); - } + return machines.find(machine => machine.Default && machine.Running); + } catch (e) { + console.log(e); + } + + return undefined; +} + +export function getFirstRunningPodmanConnection(): ProviderContainerConnection | undefined { + let engine: ProviderContainerConnection; + try { + engine = provider + .getContainerConnections() + .filter(connection => connection.connection.type === 'podman') + .find(connection => connection.connection.status() === 'started'); } catch (e) { console.log(e); }