From 3aa855e87ff3ae5312d8638f4ed7d81a9da35ad9 Mon Sep 17 00:00:00 2001 From: lstocchi Date: Wed, 24 Jan 2024 12:20:35 +0100 Subject: [PATCH] fix: fix failing test, format, lint Signed-off-by: lstocchi --- .../src/managers/applicationManager.spec.ts | 41 +++++++-- .../src/managers/applicationManager.ts | 86 ++++++++++++------- .../src/managers/modelsManager.spec.ts | 37 ++++---- packages/backend/src/utils/ports.ts | 2 +- 4 files changed, 110 insertions(+), 56 deletions(-) diff --git a/packages/backend/src/managers/applicationManager.spec.ts b/packages/backend/src/managers/applicationManager.spec.ts index eb84160e5..f713da728 100644 --- a/packages/backend/src/managers/applicationManager.spec.ts +++ b/packages/backend/src/managers/applicationManager.spec.ts @@ -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(), }; }); @@ -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, }, })); @@ -39,7 +49,7 @@ describe('pullApplication', () => { const cloneRepositoryMock = vi.fn(); const isModelOnDiskMock = vi.fn(); let manager: ApplicationManager; - let downloadModelMainSpy: MockInstance< + let doDownloadModelWrapperSpy: MockInstance< [modelId: string, url: string, taskUtil: RecipeStatusUtils, destFileName?: string], Promise >; @@ -83,6 +93,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', @@ -97,8 +128,8 @@ describe('pullApplication', () => { } 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 () => { @@ -132,7 +163,7 @@ describe('pullApplication', () => { } else { expect(cloneRepositoryMock).toHaveBeenNthCalledWith(1, 'repo', '/home/user/aistudio/recipe1'); } - expect(downloadModelMainSpy).toHaveBeenCalledOnce(); + expect(doDownloadModelWrapperSpy).toHaveBeenCalledOnce(); expect(mocks.builImageMock).toHaveBeenCalledOnce(); }); @@ -191,6 +222,6 @@ describe('pullApplication', () => { await manager.pullApplication(recipe, model); expect(cloneRepositoryMock).not.toHaveBeenCalled(); - expect(downloadModelMainSpy).not.toHaveBeenCalled(); + expect(doDownloadModelWrapperSpy).not.toHaveBeenCalled(); }); }); diff --git a/packages/backend/src/managers/applicationManager.ts b/packages/backend/src/managers/applicationManager.ts index 9eb034797..dd949b1a7 100644 --- a/packages/backend/src/managers/applicationManager.ts +++ b/packages/backend/src/managers/applicationManager.ts @@ -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'; @@ -83,12 +83,22 @@ 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, @@ -96,7 +106,7 @@ export class ApplicationManager { name: `Creating application`, }); - await this.createAndAddContainersToPod(pod, images, modelPath, taskUtil); + await this.createAndAddContainersToPod(pod, images, modelPath); taskUtil.setTask({ id: pod.Id, @@ -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 @@ -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 { + async createPod(images: ImageInfo[]): Promise { // 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'); } @@ -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 { + async buildImages( + containers: ContainerConfig[], + configPath: string, + taskUtil: RecipeStatusUtils, + ): Promise { containers.forEach(container => { taskUtil.setTask({ id: container.name, @@ -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; } @@ -270,10 +285,10 @@ export class ApplicationManager { modelService: container.modelService, ports: exposedPorts, appName: container.name, - }) + }); taskUtil.setTaskState(container.name, 'success'); - }) + }), ); return imageInfoList; @@ -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', @@ -361,7 +376,7 @@ export class ApplicationManager { return { aiConfig, path: configFile, - } + }; } async doCheckout(repository: string, localFolder: string, taskUtil: RecipeStatusUtils) { @@ -396,7 +411,12 @@ export class ApplicationManager { taskUtil.setTask(checkoutTask); } - doDownloadModelWrapper(modelId: string, url: string, taskUtil: RecipeStatusUtils, destFileName?: string): Promise { + doDownloadModelWrapper( + modelId: string, + url: string, + taskUtil: RecipeStatusUtils, + destFileName?: string, + ): Promise { return new Promise((resolve, reject) => { const downloadCallback = (result: DownloadModelResult) => { if (result.result) { diff --git a/packages/backend/src/managers/modelsManager.spec.ts b/packages/backend/src/managers/modelsManager.spec.ts index f57b7ef4f..51cfa8791 100644 --- a/packages/backend/src/managers/modelsManager.spec.ts +++ b/packages/backend/src/managers/modelsManager.spec.ts @@ -27,6 +27,23 @@ function mockFiles(now: Date) { info.size = 32000; info.mtime = now; statSyncSpy.mockReturnValue(info); + 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[]; const readdirSyncMock = vi.spyOn(fs, 'readdirSync') as unknown as MockInstance< [path: string], string[] | fs.Dirent[] @@ -36,23 +53,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; } }); } @@ -68,12 +69,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'), }, ]); }); diff --git a/packages/backend/src/utils/ports.ts b/packages/backend/src/utils/ports.ts index 8f73d9b5e..9a5c33b00 100644 --- a/packages/backend/src/utils/ports.ts +++ b/packages/backend/src/utils/ports.ts @@ -77,7 +77,7 @@ export async function getPortsInfo(portDescriptor: string): Promise