From b130cbf5bfb3672e7d4cde6417528f4c80173f3a Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Tue, 21 May 2024 11:54:36 +0200 Subject: [PATCH] refactor: some cleanup application manager Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --- .../src/managers/applicationManager.spec.ts | 100 +----------------- .../src/managers/applicationManager.ts | 75 +++---------- .../backend/src/utils/imagesUtils.spec.ts | 37 +++++++ packages/backend/src/utils/imagesUtils.ts | 28 +++++ packages/backend/src/utils/podsUtils.ts | 32 ++++++ packages/backend/src/utils/ports.ts | 52 ++++----- packages/backend/src/utils/randomUtils.ts | 4 + 7 files changed, 139 insertions(+), 189 deletions(-) create mode 100644 packages/backend/src/utils/imagesUtils.spec.ts create mode 100644 packages/backend/src/utils/imagesUtils.ts create mode 100644 packages/backend/src/utils/podsUtils.ts diff --git a/packages/backend/src/managers/applicationManager.spec.ts b/packages/backend/src/managers/applicationManager.spec.ts index 602b78239..0610777ec 100644 --- a/packages/backend/src/managers/applicationManager.spec.ts +++ b/packages/backend/src/managers/applicationManager.spec.ts @@ -68,7 +68,6 @@ const mocks = vi.hoisted(() => { getContainerConnectionsMock: vi.fn(), pullImageMock: vi.fn(), stopContainerMock: vi.fn(), - getFreePortMock: vi.fn(), containerRegistrySubscribeMock: vi.fn(), onPodStartMock: vi.fn(), onPodStopMock: vi.fn(), @@ -655,40 +654,6 @@ describe('filterContainers', () => { }); }); -describe('getRandomName', () => { - test('return base name plus random string', () => { - const manager = new ApplicationManager( - '/home/user/aistudio', - {} as unknown as GitManager, - taskRegistry, - {} as Webview, - {} as PodmanConnection, - {} as CatalogManager, - {} as unknown as ModelsManager, - telemetryLogger, - localRepositoryRegistry, - ); - const randomName = manager.getRandomName('base'); - expect(randomName).not.equal('base'); - expect(randomName.length).toBeGreaterThan(4); - }); - test('return random string when base is empty', () => { - const manager = new ApplicationManager( - '/home/user/aistudio', - {} as unknown as GitManager, - taskRegistry, - {} as Webview, - {} as PodmanConnection, - {} as CatalogManager, - {} as unknown as ModelsManager, - telemetryLogger, - localRepositoryRegistry, - ); - const randomName = manager.getRandomName(''); - expect(randomName.length).toBeGreaterThan(0); - }); -}); - describe('buildImages', () => { const recipe = { id: 'recipe1', @@ -807,7 +772,6 @@ describe('createPod', async () => { }); test('call createPod with sample app exposed port', async () => { const images = [imageInfo1, imageInfo2]; - vi.spyOn(manager, 'getRandomName').mockReturnValue('name'); vi.spyOn(portsUtils, 'getPortsInfo').mockResolvedValueOnce('9000'); vi.spyOn(portsUtils, 'getPortsInfo').mockResolvedValueOnce('9001'); vi.spyOn(portsUtils, 'getPortsInfo').mockResolvedValueOnce('9002'); @@ -817,7 +781,7 @@ describe('createPod', async () => { }); await manager.createPod({ id: 'recipe-id' } as Recipe, { id: 'model-id' } as ModelInfo, images); expect(mocks.createPodMock).toBeCalledWith({ - name: 'name', + name: expect.anything(), portmappings: [ { container_port: 8080, @@ -1021,14 +985,13 @@ describe('createAndAddContainersToPod', () => { id: 'container-1', }); vi.spyOn(podman, 'isQEMUMachine').mockResolvedValue(false); - vi.spyOn(manager, 'getRandomName').mockReturnValue('name'); await manager.createAndAddContainersToPod(pod, [imageInfo1, imageInfo2], modelInfo, 'path'); expect(mocks.createContainerMock).toHaveBeenNthCalledWith(1, 'engine', { Image: 'id', Detach: true, Env: ['MODEL_ENDPOINT=http://localhost:8085'], start: false, - name: 'name', + name: expect.anything(), pod: 'id', HealthCheck: { Interval: 5000000000, @@ -1042,7 +1005,7 @@ describe('createAndAddContainersToPod', () => { Detach: true, Env: ['MODEL_PATH=/path', ...extraEnvs], start: false, - name: 'name', + name: expect.anything(), pod: 'id', HostConfig: { Mounts: [ @@ -1354,62 +1317,6 @@ describe('pod detection', async () => { expect(mocks.stopPodMock).toHaveBeenCalledWith('engine-1', 'pod-1'); expect(mocks.removePodMock).toHaveBeenCalledWith('engine-1', 'pod-1'); }); -}); - -describe('getImageTag', () => { - const manager = new ApplicationManager( - '/path/to/user/dir', - {} as GitManager, - taskRegistry, - { - postMessage: mocks.postMessageMock, - } as unknown as Webview, - { - onPodStart: mocks.onPodStartMock, - onPodStop: mocks.onPodStopMock, - onPodRemove: mocks.onPodRemoveMock, - startupSubscribe: mocks.startupSubscribeMock, - onMachineStop: mocks.onMachineStopMock, - } as unknown as PodmanConnection, - { - getRecipeById: vi.fn().mockReturnValue({ name: 'MyRecipe' } as Recipe), - } as unknown as CatalogManager, - {} as ModelsManager, - {} as TelemetryLogger, - localRepositoryRegistry, - ); - test('return recipe-container tag if container image prop is not defined', () => { - const recipe = { - id: 'recipe1', - } as Recipe; - const container = { - name: 'name', - } as ContainerConfig; - const imageTag = manager.getImageTag(recipe, container); - expect(imageTag).equals('recipe1-name:latest'); - }); - test('return container image prop is defined', () => { - const recipe = { - id: 'recipe1', - } as Recipe; - const container = { - name: 'name', - image: 'quay.io/repo/image:v1', - } as ContainerConfig; - const imageTag = manager.getImageTag(recipe, container); - expect(imageTag).equals('quay.io/repo/image:v1'); - }); - test('append latest tag to container image prop if it has no tag', () => { - const recipe = { - id: 'recipe1', - } as Recipe; - const container = { - name: 'name', - image: 'quay.io/repo/image', - } as ContainerConfig; - const imageTag = manager.getImageTag(recipe, container); - expect(imageTag).equals('quay.io/repo/image:latest'); - }); test('adoptRunningApplications should check pods health', async () => { mocks.listPodsMock.mockResolvedValue([ @@ -1474,3 +1381,4 @@ describe('getImageTag', () => { expect(state[0].health).toEqual('healthy'); }); }); + diff --git a/packages/backend/src/managers/applicationManager.ts b/packages/backend/src/managers/applicationManager.ts index 95cfeb822..c43dca589 100644 --- a/packages/backend/src/managers/applicationManager.ts +++ b/packages/backend/src/managers/applicationManager.ts @@ -36,7 +36,7 @@ import type { Task } from '@shared/src/models/ITask'; import { getParentDirectory } from '../utils/pathUtils'; import type { ModelInfo } from '@shared/src/models/IModelInfo'; import type { ModelsManager } from './modelsManager'; -import { getPortsInfo } from '../utils/ports'; +import { getPortsFromLabel, getPortsInfo } from '../utils/ports'; import { goarch } from '../utils/arch'; import { getDurationSecondsSince, timeout } from '../utils/utils'; import type { LocalRepositoryRegistry } from '../registries/LocalRepositoryRegistry'; @@ -50,6 +50,9 @@ import { Publisher } from '../utils/Publisher'; import { isQEMUMachine } from '../utils/podman'; import { SECOND } from '../utils/inferenceUtils'; import { getModelPropertiesForEnvironment } from '../utils/modelsUtils'; +import { getPodHealth } from '../utils/podsUtils'; +import { getImageTag } from '../utils/imagesUtils'; +import { getRandomName } from '../utils/randomUtils'; export const LABEL_MODEL_ID = 'ai-lab-model-id'; export const LABEL_MODEL_PORTS = 'ai-lab-model-ports'; @@ -318,7 +321,7 @@ export class ApplicationManager extends Publisher implements }; } - const podifiedName = this.getRandomName(`${image.appName}-podified`); + const podifiedName = getRandomName(`${image.appName}-podified`); await containerEngine.createContainer(podInfo.engineId, { Image: image.id, name: podifiedName, @@ -384,7 +387,7 @@ export class ApplicationManager extends Publisher implements labels[LABEL_APP_PORTS] = appPorts.join(','); } const pod = await containerEngine.createPod({ - name: this.getRandomName(`pod-${sampleAppImageInfo.appName}`), + name: getRandomName(`pod-${sampleAppImageInfo.appName}`), portmappings: portmappings, labels, }); @@ -395,10 +398,6 @@ export class ApplicationManager extends Publisher implements }; } - getRandomName(base: string): string { - return `${base ?? ''}-${new Date().getTime()}`; - } - async buildImages( recipe: Recipe, containers: ContainerConfig[], @@ -432,7 +431,7 @@ export class ApplicationManager extends Publisher implements throw new Error('Context configured does not exist.'); } - const imageTag = this.getImageTag(recipe, container); + const imageTag = getImageTag(recipe, container); const buildOptions: BuildImageOptions = { containerFile: container.containerfile, tag: imageTag, @@ -479,7 +478,7 @@ export class ApplicationManager extends Publisher implements await Promise.all( containers.map(async container => { const task = containerTasks[container.name]; - const imageTag = this.getImageTag(recipe, container); + const imageTag = getImageTag(recipe, container); const image = images.find(im => { return im.RepoTags?.some(tag => tag.endsWith(imageTag)); @@ -506,14 +505,6 @@ export class ApplicationManager extends Publisher implements return imageInfoList; } - getImageTag(recipe: Recipe, container: ContainerConfig) { - let tag = container.image ?? `${recipe.id}-${container.name}`; - if (!tag.includes(':')) { - tag += ':latest'; - } - return tag; - } - getConfigAndFilterContainers( recipeBaseDir: string | undefined, localFolder: string, @@ -672,7 +663,7 @@ export class ApplicationManager extends Publisher implements ); } - adoptPod(pod: PodInfo) { + private adoptPod(pod: PodInfo) { if (!pod.Labels) { return; } @@ -681,8 +672,8 @@ export class ApplicationManager extends Publisher implements if (!recipeId || !modelId) { return; } - const appPorts = this.getPortsFromLabel(pod.Labels, LABEL_APP_PORTS); - const modelPorts = this.getPortsFromLabel(pod.Labels, LABEL_MODEL_PORTS); + const appPorts = getPortsFromLabel(pod.Labels, LABEL_APP_PORTS); + const modelPorts = getPortsFromLabel(pod.Labels, LABEL_MODEL_PORTS); if (this.#applications.has({ recipeId, modelId })) { return; } @@ -697,7 +688,7 @@ export class ApplicationManager extends Publisher implements this.updateApplicationState(recipeId, modelId, state); } - forgetPod(pod: PodInfo) { + private forgetPod(pod: PodInfo) { if (!pod.Labels) { return; } @@ -723,7 +714,7 @@ export class ApplicationManager extends Publisher implements } } - forgetPodById(podId: string) { + private forgetPodById(podId: string) { const app = Array.from(this.#applications.values()).find(p => p.pod.Id === podId); if (!app) { return; @@ -753,7 +744,7 @@ export class ApplicationManager extends Publisher implements } } - async checkPodsHealth() { + private async checkPodsHealth() { const pods = await containerEngine .listPods() .then(pods => pods.filter(pod => LABEL_RECIPE_ID in pod.Labels && LABEL_MODEL_ID in pod.Labels)); @@ -770,7 +761,7 @@ export class ApplicationManager extends Publisher implements containerEngine.inspectContainer(pod.engineId, container.Id).then(data => data.State.Health?.Status), ), ); - const podHealth = this.getPodHealth(containerStates); + const podHealth = getPodHealth(containerStates); const state = this.#applications.get({ recipeId, modelId }); if (state.health !== podHealth) { state.health = podHealth; @@ -788,20 +779,6 @@ export class ApplicationManager extends Publisher implements } } - getPodHealth(infos: (string | undefined)[]): PodHealth { - const checked = infos.filter(info => !!info && info !== 'none' && info !== ''); - if (!checked.length) { - return 'none'; - } - if (infos.some(info => info === 'unhealthy')) { - return 'unhealthy'; - } - if (infos.some(info => info === 'starting')) { - return 'starting'; - } - return 'healthy'; - } - updateApplicationState(recipeId: string, modelId: string, state: ApplicationState): void { this.#applications.set({ recipeId, modelId }, state); this.notify(); @@ -890,29 +867,7 @@ export class ApplicationManager extends Publisher implements ); } - getPortsFromLabel(labels: { [key: string]: string }, key: string): number[] { - if (!(key in labels)) { - return []; - } - const value = labels[key]; - const portsStr = value.split(','); - const result: number[] = []; - for (const portStr of portsStr) { - const port = parseInt(portStr, 10); - if (isNaN(port)) { - // malformed label, just ignore it - return []; - } - result.push(port); - } - return result; - } - dispose(): void { - this.cleanDisposables(); - } - - private cleanDisposables(): void { this.#disposables.forEach(disposable => disposable.dispose()); } } diff --git a/packages/backend/src/utils/imagesUtils.spec.ts b/packages/backend/src/utils/imagesUtils.spec.ts new file mode 100644 index 000000000..2059eb42e --- /dev/null +++ b/packages/backend/src/utils/imagesUtils.spec.ts @@ -0,0 +1,37 @@ +import { expect, test } from 'vitest'; +import type { Recipe } from '@shared/src/models/IRecipe'; +import type { ContainerConfig } from '../models/AIConfig'; +import { getImageTag } from './imagesUtils'; + +test('return recipe-container tag if container image prop is not defined', () => { + const recipe = { + id: 'recipe1', + } as Recipe; + const container = { + name: 'name', + } as ContainerConfig; + const imageTag = getImageTag(recipe, container); + expect(imageTag).equals('recipe1-name:latest'); +}); +test('return container image prop is defined', () => { + const recipe = { + id: 'recipe1', + } as Recipe; + const container = { + name: 'name', + image: 'quay.io/repo/image:v1', + } as ContainerConfig; + const imageTag = getImageTag(recipe, container); + expect(imageTag).equals('quay.io/repo/image:v1'); +}); +test('append latest tag to container image prop if it has no tag', () => { + const recipe = { + id: 'recipe1', + } as Recipe; + const container = { + name: 'name', + image: 'quay.io/repo/image', + } as ContainerConfig; + const imageTag = getImageTag(recipe, container); + expect(imageTag).equals('quay.io/repo/image:latest'); +}); diff --git a/packages/backend/src/utils/imagesUtils.ts b/packages/backend/src/utils/imagesUtils.ts new file mode 100644 index 000000000..ec22d6eb2 --- /dev/null +++ b/packages/backend/src/utils/imagesUtils.ts @@ -0,0 +1,28 @@ +/********************************************************************** + * Copyright (C) 2024 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ + +import type { Recipe } from '@shared/src/models/IRecipe'; +import type { ContainerConfig } from '../models/AIConfig'; + +export function getImageTag(recipe: Recipe, container: ContainerConfig) { + let tag = container.image ?? `${recipe.id}-${container.name}`; + if (!tag.includes(':')) { + tag += ':latest'; + } + return tag; +} diff --git a/packages/backend/src/utils/podsUtils.ts b/packages/backend/src/utils/podsUtils.ts new file mode 100644 index 000000000..98b752d01 --- /dev/null +++ b/packages/backend/src/utils/podsUtils.ts @@ -0,0 +1,32 @@ +/********************************************************************** + * Copyright (C) 2024 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ +import type { PodHealth } from '@shared/src/models/IApplicationState'; + +export function getPodHealth(infos: (string | undefined)[]): PodHealth { + const checked = infos.filter(info => !!info && info !== 'none' && info !== ''); + if (!checked.length) { + return 'none'; + } + if (infos.some(info => info === 'unhealthy')) { + return 'unhealthy'; + } + if (infos.some(info => info === 'starting')) { + return 'starting'; + } + return 'healthy'; +} diff --git a/packages/backend/src/utils/ports.ts b/packages/backend/src/utils/ports.ts index 151bfddc6..3765aab5f 100644 --- a/packages/backend/src/utils/ports.ts +++ b/packages/backend/src/utils/ports.ts @@ -1,5 +1,5 @@ /********************************************************************** - * Copyright (C) 2022 Red Hat, Inc. + * Copyright (C) 2024 Red Hat, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,38 +41,6 @@ export async function getFreeRandomPort(address: string): Promise { ); } -/** - * Find a free port starting from the given port - */ -export async function getFreePort(port = 0): Promise { - if (port < 1024) { - port = 9000; - } - let isFree = false; - while (!isFree) { - isFree = await isFreePort(port); - if (!isFree) { - port++; - } - } - - return port; -} - -function isFreeAddressPort(address: string, port: number): Promise { - const server = net.createServer(); - return new Promise((resolve, reject) => - server - .on('error', (error: NodeJS.ErrnoException) => (error.code === 'EADDRINUSE' ? resolve(false) : reject(error))) - .on('listening', () => server.close(() => resolve(true))) - .listen(port, address), - ); -} - -export async function isFreePort(port: number): Promise { - return (await isFreeAddressPort('127.0.0.1', port)) && (await isFreeAddressPort('0.0.0.0', port)); -} - export async function getPortsInfo(portDescriptor: string): Promise { const localPort = await getPort(portDescriptor); if (!localPort) { @@ -99,3 +67,21 @@ async function getPort(portDescriptor: string): Promise { return undefined; } } + +export function getPortsFromLabel(labels: { [key: string]: string }, key: string): number[] { + if (!(key in labels)) { + return []; + } + const value = labels[key]; + const portsStr = value.split(','); + const result: number[] = []; + for (const portStr of portsStr) { + const port = parseInt(portStr, 10); + if (isNaN(port)) { + // malformed label, just ignore it + return []; + } + result.push(port); + } + return result; +} diff --git a/packages/backend/src/utils/randomUtils.ts b/packages/backend/src/utils/randomUtils.ts index 5fc930ea7..272625024 100644 --- a/packages/backend/src/utils/randomUtils.ts +++ b/packages/backend/src/utils/randomUtils.ts @@ -19,3 +19,7 @@ export const getRandomString = (): string => { return (Math.random() + 1).toString(36).substring(7); }; + +export function getRandomName(prefix: string): string { + return `${prefix ?? ''}-${new Date().getTime()}`; +}