Skip to content

Commit

Permalink
fix: fix failing test, format, lint
Browse files Browse the repository at this point in the history 
Signed-off-by: lstocchi <[email protected]>
  • Loading branch information
@lstocchi
lstocchi committed Jan 24, 2024
1 parent 8e2305e commit 39b3860
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 56 deletions.
44 changes: 39 additions & 5 deletions packages/backend/src/managers/applicationManager.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ const mocks = vi.hoisted(() => {
return {
parseYamlMock: vi.fn(),
builImageMock: vi.fn(),
listImagesMock: vi.fn(),
getImageInspectMock: vi.fn(),
createPodMock: vi.fn(),
createContainerMock: vi.fn(),
replicatePodmanContainerMock: vi.fn(),
};
});

Expand All @@ -23,6 +28,11 @@ vi.mock('../models/AIConfig', () => ({
vi.mock('@podman-desktop/api', () => ({
containerEngine: {
buildImage: mocks.builImageMock,
listImages: mocks.listImagesMock,
getImageInspect: mocks.getImageInspectMock,
createPod: mocks.createPodMock,
createContainer: mocks.createContainerMock,
replicatePodmanContainer: mocks.replicatePodmanContainerMock,
},
}));

Expand All @@ -38,8 +48,9 @@ describe('pullApplication', () => {
const setStatusMock = vi.fn();
const cloneRepositoryMock = vi.fn();
const isModelOnDiskMock = vi.fn();
const getLocalModelPathMock = vi.fn();
let manager: ApplicationManager;
let downloadModelMainSpy: MockInstance<
let doDownloadModelWrapperSpy: MockInstance<
[modelId: string, url: string, taskUtil: RecipeStatusUtils, destFileName?: string],
Promise<string>
>;
Expand Down Expand Up @@ -83,6 +94,27 @@ describe('pullApplication', () => {
},
});
mocks.builImageMock.mockResolvedValue(undefined);
mocks.listImagesMock.mockResolvedValue([
{
RepoTags: ['container1:latest'],
engineId: 'engine',
Id: 'id1',
},
]);
mocks.getImageInspectMock.mockResolvedValue({
Config: {
ExposedPorts: {
'8080': '8080',
},
},
});
mocks.createPodMock.mockResolvedValue({
engineId: 'engine',
Id: 'id',
});
mocks.createContainerMock.mockResolvedValue({
id: 'id',
});

manager = new ApplicationManager(
'/home/user/aistudio',
Expand All @@ -94,11 +126,12 @@ describe('pullApplication', () => {
} as unknown as RecipeStatusRegistry,
{
isModelOnDisk: isModelOnDiskMock,
getLocalModelPath: getLocalModelPathMock,
} as unknown as ModelsManager,
);

downloadModelMainSpy = vi.spyOn(manager, 'doDownloadModelWrapper');
downloadModelMainSpy.mockResolvedValue('');
doDownloadModelWrapperSpy = vi.spyOn(manager, 'doDownloadModelWrapper');
doDownloadModelWrapperSpy.mockResolvedValue('path');
}

test('pullApplication should clone repository and call downloadModelMain and buildImage', async () => {
Expand Down Expand Up @@ -132,7 +165,7 @@ describe('pullApplication', () => {
} else {
expect(cloneRepositoryMock).toHaveBeenNthCalledWith(1, 'repo', '/home/user/aistudio/recipe1');
}
expect(downloadModelMainSpy).toHaveBeenCalledOnce();
expect(doDownloadModelWrapperSpy).toHaveBeenCalledOnce();
expect(mocks.builImageMock).toHaveBeenCalledOnce();
});

Expand Down Expand Up @@ -170,6 +203,7 @@ describe('pullApplication', () => {
recipeFolderExists: true,
});
isModelOnDiskMock.mockReturnValue(true);
getLocalModelPathMock.mockReturnValue('path');
const recipe: Recipe = {
id: 'recipe1',
name: 'Recipe 1',
Expand All @@ -191,6 +225,6 @@ describe('pullApplication', () => {

await manager.pullApplication(recipe, model);
expect(cloneRepositoryMock).not.toHaveBeenCalled();
expect(downloadModelMainSpy).not.toHaveBeenCalled();
expect(doDownloadModelWrapperSpy).not.toHaveBeenCalled();
});
});
86 changes: 53 additions & 33 deletions packages/backend/src/managers/applicationManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import type { GitManager } from './gitManager';
import fs from 'fs';
import * as https from 'node:https';
import * as path from 'node:path';
import { PodCreatePortOptions, containerEngine } from '@podman-desktop/api';
import { type PodCreatePortOptions, containerEngine } from '@podman-desktop/api';
import type { RecipeStatusRegistry } from '../registries/RecipeStatusRegistry';
import type { AIConfig, AIConfigFile, ContainerConfig } from '../models/AIConfig';
import { parseYaml } from '../models/AIConfig';
Expand Down Expand Up @@ -83,20 +83,30 @@ export class ApplicationManager {

// create a pod containing all the containers to run the application
await this.createApplicationPod(images, modelPath, taskUtil);

}

async createApplicationPod(images: ImageInfo[], modelPath: string, taskUtil: RecipeStatusUtils) {
// create empty pod
const pod = await this.createPod(images, taskUtil);
let pod: Pod;
try {
pod = await this.createPod(images);
} catch (e) {
console.error('error when creating pod');
taskUtil.setTask({
id: pod.Id,
state: 'error',
name: 'Creating application',
});
throw e;
}

taskUtil.setTask({
id: pod.Id,
state: 'loading',
name: `Creating application`,
});

await this.createAndAddContainersToPod(pod, images, modelPath, taskUtil);
await this.createAndAddContainersToPod(pod, images, modelPath);

taskUtil.setTask({
id: pod.Id,
Expand All @@ -105,10 +115,9 @@ export class ApplicationManager {
});
}

async createAndAddContainersToPod(pod: Pod, images: ImageInfo[], modelPath: string, taskUtil: RecipeStatusUtils) {
async createAndAddContainersToPod(pod: Pod, images: ImageInfo[], modelPath: string) {
await Promise.all(
images.map(async image => {

let hostConfig: unknown;
let envs: string[] = [];
// if it's a model service we mount the model as a volume
Expand All @@ -128,45 +137,47 @@ export class ApplicationManager {
} else {
hostConfig = {
AutoRemove: true,
}
};
// TODO: remove static port
const modelService = images.find(image => image.modelService);
if (modelService && modelService.ports.length > 0) {
envs = [`MODEL_ENDPOINT=http://localhost:${modelService.ports[0]}`]
}
envs = [`MODEL_ENDPOINT=http://localhost:${modelService.ports[0]}`];
}
}
const createdContainer = await containerEngine.createContainer(pod.engineId, {
Image: image.id,
Detach: true,
HostConfig: hostConfig,
Env: envs,
start: false,
}).catch(e => console.error(e));
const createdContainer = await containerEngine
.createContainer(pod.engineId, {
Image: image.id,
Detach: true,
HostConfig: hostConfig,
Env: envs,
start: false,
})
.catch((e: unknown) => console.error(e));

// now, for each container, put it in the pod
if (createdContainer) {
try {
await containerEngine.replicatePodmanContainer(
{
{
id: createdContainer.id,
engineId: pod.engineId,
engineId: pod.engineId,
},
{ engineId: pod.engineId },
{ pod: pod.Id, name: this.getRandomName(`${image.appName}-podified`) },
);
} catch (error) {
console.error(error);
}
}
}
}),
);
}

async createPod(images: ImageInfo[], taskUtil: RecipeStatusUtils): Promise<Pod> {
async createPod(images: ImageInfo[]): Promise<Pod> {
// find the exposed port of the sample app so we can open its ports on the new pod
const sampleAppImageInfo = images.find(image => !image.modelService);
if (!sampleAppImageInfo) {
console.error('no image found')
console.error('no image found');
throw new Error('no sample app found');
}

Expand All @@ -179,22 +190,26 @@ export class ApplicationManager {
host_port: parseInt(localPorts),
host_ip: '',
protocol: '',
range: 1
})
range: 1,
});
}

// create new pod
return await containerEngine.createPod({
name: this.getRandomName(`pod-${sampleAppImageInfo.appName}`),
portmappings: portmappings,
})
});
}

getRandomName(base: string): string {
return `${base ?? ''}-${new Date().getTime()}`;
}

async buildImages(containers: ContainerConfig[], configPath: string, taskUtil: RecipeStatusUtils): Promise<ImageInfo[]> {
async buildImages(
containers: ContainerConfig[],
configPath: string,
taskUtil: RecipeStatusUtils,
): Promise<ImageInfo[]> {
containers.forEach(container => {
taskUtil.setTask({
id: container.name,
Expand Down Expand Up @@ -248,11 +263,11 @@ export class ApplicationManager {
await Promise.all(
containers.map(async container => {
const image = images.find(im => {
return im.RepoTags?.some(tag => tag.endsWith(`${container.name}:latest`))
return im.RepoTags?.some(tag => tag.endsWith(`${container.name}:latest`));
});

if (!image) {
console.error('no image found')
console.error('no image found');
taskUtil.setTaskState(container.name, 'error');
return;
}
Expand All @@ -270,10 +285,10 @@ export class ApplicationManager {
modelService: container.modelService,
ports: exposedPorts,
appName: container.name,
})
});

taskUtil.setTaskState(container.name, 'success');
})
}),
);

return imageInfoList;
Expand Down Expand Up @@ -311,7 +326,7 @@ export class ApplicationManager {
}
}

getConfiguration(recipeConfig: string, localFolder: string,taskUtil: RecipeStatusUtils): AIConfigFile {
getConfiguration(recipeConfig: string, localFolder: string, taskUtil: RecipeStatusUtils): AIConfigFile {
// Adding loading configuration task
const loadingConfiguration: Task = {
id: 'loading-config',
Expand Down Expand Up @@ -361,7 +376,7 @@ export class ApplicationManager {
return {
aiConfig,
path: configFile,
}
};
}

async doCheckout(repository: string, localFolder: string, taskUtil: RecipeStatusUtils) {
Expand Down Expand Up @@ -396,7 +411,12 @@ export class ApplicationManager {
taskUtil.setTask(checkoutTask);
}

doDownloadModelWrapper(modelId: string, url: string, taskUtil: RecipeStatusUtils, destFileName?: string): Promise<string> {
doDownloadModelWrapper(
modelId: string,
url: string,
taskUtil: RecipeStatusUtils,
destFileName?: string,
): Promise<string> {
return new Promise((resolve, reject) => {
const downloadCallback = (result: DownloadModelResult) => {
if (result.result) {
Expand Down
39 changes: 22 additions & 17 deletions packages/backend/src/managers/modelsManager.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,24 @@ beforeEach(() => {
vi.resetAllMocks();
});

const dirent = [
{
isDirectory: () => true,
path: '/home/user/appstudio-dir',
name: 'model-id-1',
},
{
isDirectory: () => true,
path: '/home/user/appstudio-dir',
name: 'model-id-2',
},
{
isDirectory: () => false,
path: '/home/user/appstudio-dir',
name: 'other-file-should-be-ignored.txt',
},
] as fs.Dirent[];

function mockFiles(now: Date) {
vi.spyOn(os, 'homedir').mockReturnValue('/home/user');
const existsSyncSpy = vi.spyOn(fs, 'existsSync');
Expand All @@ -36,23 +54,7 @@ function mockFiles(now: Date) {
const base = path.basename(dir);
return [base + '-model'];
} else {
return [
{
isDirectory: () => true,
path: '/home/user/appstudio-dir',
name: 'model-id-1',
},
{
isDirectory: () => true,
path: '/home/user/appstudio-dir',
name: 'model-id-2',
},
{
isDirectory: () => false,
path: '/home/user/appstudio-dir',
name: 'other-file-should-be-ignored.txt',
},
] as fs.Dirent[];
return dirent;
}
});
}
Expand All @@ -68,12 +70,14 @@ test('getLocalModelsFromDisk should get models in local directory', () => {
file: 'model-id-1-model',
size: 32000,
creation: now,
path: path.resolve(dirent[0].path, dirent[0].name, 'model-id-1-model'),
},
{
id: 'model-id-2',
file: 'model-id-2-model',
size: 32000,
creation: now,
path: path.resolve(dirent[1].path, dirent[1].name, 'model-id-2-model'),
},
]);
});
Expand Down Expand Up @@ -133,6 +137,7 @@ test('loadLocalModels should post a message with the message on disk and on cata
file: 'model-id-1-model',
id: 'model-id-1',
size: 32000,
path: path.resolve(dirent[0].path, dirent[0].name, 'model-id-1-model'),
},
id: 'model-id-1',
},
Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/utils/ports.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export async function getPortsInfo(portDescriptor: string): Promise<string | und
return `${localPort}`;
}
}

/**
* return a range of the same length as portDescriptor containing free ports
* undefined if the portDescriptor range is not valid
Expand Down

0 comments on commit 39b3860

Please sign in to comment.