Skip to content

Commit

Permalink
fix: use machine name to execute podman cli
Browse files Browse the repository at this point in the history
Signed-off-by: lstocchi <[email protected]>
  • Loading branch information
lstocchi committed Feb 29, 2024
1 parent a0af678 commit c957f6d
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 86 deletions.
4 changes: 2 additions & 2 deletions packages/backend/src/managers/playground.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/managers/playground.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions packages/backend/src/managers/podmanConnection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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
Expand All @@ -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({
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions packages/backend/src/managers/podmanConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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) => {
Expand All @@ -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;
Expand Down
19 changes: 15 additions & 4 deletions packages/backend/src/models/WSLUploader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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',
Expand Down
14 changes: 4 additions & 10 deletions packages/backend/src/models/WSLUploader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}
Expand All @@ -56,7 +50,7 @@ export class WSLUploader implements UploadWorker {
await podmanDesktopApi.process.exec(getPodmanCli(), [
'machine',
'ssh',
connection.connection.name,
machine.Name,
'cp',
convertToMntPath,
remotePath,
Expand Down
84 changes: 31 additions & 53 deletions packages/backend/src/utils/podman.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import { beforeEach } from 'node:test';
const mocks = vi.hoisted(() => {
return {
getConfigurationMock: vi.fn(),
execMock: vi.fn(),
getContainerConnectionsMock: vi.fn(),
};
});
Expand All @@ -42,9 +41,6 @@ vi.mock('@podman-desktop/api', () => ({
configuration: {
getConfiguration: () => config,
},
process: {
exec: mocks.execMock,
},
provider: {
getContainerConnections: mocks.getContainerConnectionsMock,
},
Expand Down Expand Up @@ -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',
},
Expand All @@ -142,7 +120,7 @@ describe('getFirstRunningPodmanConnection', () => {
{
connection: {
name: 'machine2',
status: vi.fn(),
status: () => 'started',
endpoint: {
socketPath: '/endpoint.sock',
},
Expand All @@ -151,7 +129,7 @@ describe('getFirstRunningPodmanConnection', () => {
providerId: 'podman2',
},
]);
const result = await utils.getFirstRunningPodmanConnection();
const result = utils.getFirstRunningPodmanConnection();
expect(result.connection.name).equal('machine2');
});
});
25 changes: 16 additions & 9 deletions packages/backend/src/utils/podman.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,25 @@ async function getJSONMachineList(): Promise<string> {
return stdout;
}

export async function getFirstRunningPodmanConnection(): Promise<ProviderContainerConnection | undefined> {
let engine: ProviderContainerConnection;
export async function getFirstRunningMachine(): Promise<MachineJSON | undefined> {
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);
}
Expand Down

0 comments on commit c957f6d

Please sign in to comment.