From af6cd5efa3dd850bc0e0f543c77681306750f11f Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Wed, 29 May 2024 17:01:46 +0200 Subject: [PATCH] refactor: moving pod events method to PodManager (#1131) * refactor: moving pod events method to PodManager Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: prettier Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: applicationManager.spec.ts and imports Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: prettier Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --------- Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --- .../src/managers/applicationManager.spec.ts | 42 +++--- .../src/managers/applicationManager.ts | 4 +- .../backend/src/managers/podmanConnection.ts | 70 ---------- .../src/managers/recipes/PodManager.spec.ts | 127 +++++++++++++++++- .../src/managers/recipes/PodManager.ts | 66 ++++++++- packages/backend/src/studio.ts | 1 + 6 files changed, 211 insertions(+), 99 deletions(-) diff --git a/packages/backend/src/managers/applicationManager.spec.ts b/packages/backend/src/managers/applicationManager.spec.ts index 369f0a86f..10a4214bd 100644 --- a/packages/backend/src/managers/applicationManager.spec.ts +++ b/packages/backend/src/managers/applicationManager.spec.ts @@ -29,21 +29,14 @@ import * as portsUtils from '../utils/ports'; import { goarch } from '../utils/arch'; import * as utils from '../utils/utils'; import * as podman from '../utils/podman'; -import type { Webview, TelemetryLogger, PodInfo } from '@podman-desktop/api'; +import type { Webview, TelemetryLogger, PodInfo, Disposable } from '@podman-desktop/api'; import type { CatalogManager } from './catalogManager'; import type { LocalRepositoryRegistry } from '../registries/LocalRepositoryRegistry'; -import type { - PodmanConnection, - machineStopHandle, - podRemoveHandle, - podStartHandle, - podStopHandle, - startupHandle, -} from './podmanConnection'; +import type { PodmanConnection, machineStopHandle, startupHandle } from './podmanConnection'; import { TaskRegistry } from '../registries/TaskRegistry'; import type { CancellationTokenRegistry } from '../registries/CancellationTokenRegistry'; import type { BuilderManager } from './recipes/BuilderManager'; -import type { PodManager } from './recipes/PodManager'; +import type { PodEvent, PodManager } from './recipes/PodManager'; const mocks = vi.hoisted(() => { return { @@ -61,9 +54,6 @@ const mocks = vi.hoisted(() => { pullImageMock: vi.fn(), stopContainerMock: vi.fn(), containerRegistrySubscribeMock: vi.fn(), - onPodStartMock: vi.fn(), - onPodStopMock: vi.fn(), - onPodRemoveMock: vi.fn(), startupSubscribeMock: vi.fn(), onMachineStopMock: vi.fn(), listContainersMock: vi.fn(), @@ -142,6 +132,9 @@ const podManager = { stopPod: vi.fn(), removePod: vi.fn(), startPod: vi.fn(), + onStartPodEvent: vi.fn(), + onStopPodEvent: vi.fn(), + onRemovePodEvent: vi.fn(), } as unknown as PodManager; const localRepositoryRegistry = { @@ -994,9 +987,6 @@ describe('pod detection', async () => { 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, @@ -1071,7 +1061,7 @@ describe('pod detection', async () => { test('onPodStart updates the applications state with the started pod', async () => { vi.mocked(podManager.getAllPods).mockResolvedValue([]); mocks.onMachineStopMock.mockImplementation((_f: machineStopHandle) => {}); - mocks.onPodStartMock.mockImplementation((f: podStartHandle) => { + vi.mocked(podManager.onStartPodEvent).mockImplementation((f: (e: PodInfo) => void): Disposable => { f({ engineId: 'engine-1', engineName: 'Engine 1', @@ -1081,6 +1071,7 @@ describe('pod detection', async () => { 'ai-lab-model-id': 'model-id-1', }, } as unknown as PodInfo); + return { dispose: vi.fn() }; }); const sendApplicationStateSpy = vi.spyOn(manager, 'notify').mockResolvedValue(); manager.adoptRunningApplications(); @@ -1090,12 +1081,13 @@ describe('pod detection', async () => { test('onPodStart does no update the applications state with the started pod without labels', async () => { vi.mocked(podManager.getAllPods).mockResolvedValue([]); mocks.onMachineStopMock.mockImplementation((_f: machineStopHandle) => {}); - mocks.onPodStartMock.mockImplementation((f: podStartHandle) => { + vi.mocked(podManager.onStartPodEvent).mockImplementation((f: (e: PodInfo) => void): Disposable => { f({ engineId: 'engine-1', engineName: 'Engine 1', kind: 'podman', } as unknown as PodInfo); + return { dispose: vi.fn() }; }); const sendApplicationStateSpy = vi.spyOn(manager, 'notify').mockResolvedValue(); manager.adoptRunningApplications(); @@ -1105,7 +1097,7 @@ describe('pod detection', async () => { test('onPodStart does no update the applications state with the started pod without specific labels', async () => { vi.mocked(podManager.getAllPods).mockResolvedValue([]); mocks.onMachineStopMock.mockImplementation((_f: machineStopHandle) => {}); - mocks.onPodStartMock.mockImplementation((f: podStartHandle) => { + vi.mocked(podManager.onStartPodEvent).mockImplementation((f: (e: PodInfo) => void): Disposable => { f({ engineId: 'engine-1', engineName: 'Engine 1', @@ -1114,6 +1106,7 @@ describe('pod detection', async () => { label1: 'value1', }, } as unknown as PodInfo); + return { dispose: vi.fn() }; }); const sendApplicationStateSpy = vi.spyOn(manager, 'notify').mockResolvedValue(); manager.adoptRunningApplications(); @@ -1133,7 +1126,7 @@ describe('pod detection', async () => { } as unknown as PodInfo, ]); mocks.onMachineStopMock.mockImplementation((_f: machineStopHandle) => {}); - mocks.onPodStopMock.mockImplementation((f: podStopHandle) => { + vi.mocked(podManager.onStopPodEvent).mockImplementation((f: (e: PodInfo) => void): Disposable => { setTimeout(() => { f({ engineId: 'engine-1', @@ -1144,7 +1137,9 @@ describe('pod detection', async () => { 'ai-lab-model-id': 'model-id-1', }, } as unknown as PodInfo); + return { dispose: vi.fn() }; }, 1); + return { dispose: vi.fn() }; }); const sendApplicationStateSpy = vi.spyOn(manager, 'notify').mockResolvedValue(); manager.adoptRunningApplications(); @@ -1166,10 +1161,13 @@ describe('pod detection', async () => { } as unknown as PodInfo, ]); mocks.onMachineStopMock.mockImplementation((_f: machineStopHandle) => {}); - mocks.onPodRemoveMock.mockImplementation((f: podRemoveHandle) => { + vi.mocked(podManager.onRemovePodEvent).mockImplementation((f: (e: PodEvent) => void): Disposable => { setTimeout(() => { - f('pod-id-1'); + f({ + podId: 'pod-id-1', + }); }, 1); + return { dispose: vi.fn() }; }); const sendApplicationStateSpy = vi.spyOn(manager, 'notify').mockResolvedValue(); manager.adoptRunningApplications(); diff --git a/packages/backend/src/managers/applicationManager.ts b/packages/backend/src/managers/applicationManager.ts index 893de0413..42f8e2c01 100644 --- a/packages/backend/src/managers/applicationManager.ts +++ b/packages/backend/src/managers/applicationManager.ts @@ -578,10 +578,10 @@ export class ApplicationManager extends Publisher implements this.notify(); }); - this.podmanConnection.onPodStart((pod: PodInfo) => { + this.podManager.onStartPodEvent((pod: PodInfo) => { this.adoptPod(pod); }); - this.podmanConnection.onPodRemove((podId: string) => { + this.podManager.onRemovePodEvent(({ podId }) => { this.forgetPodById(podId); }); diff --git a/packages/backend/src/managers/podmanConnection.ts b/packages/backend/src/managers/podmanConnection.ts index bac11b7ef..7509aeb6d 100644 --- a/packages/backend/src/managers/podmanConnection.ts +++ b/packages/backend/src/managers/podmanConnection.ts @@ -20,8 +20,6 @@ import { type RegisterContainerConnectionEvent, provider, type UpdateContainerConnectionEvent, - containerEngine, - type PodInfo, type Disposable, } from '@podman-desktop/api'; import { getFirstRunningPodmanConnection } from '../utils/podman'; @@ -29,25 +27,18 @@ import { getFirstRunningPodmanConnection } from '../utils/podman'; export type startupHandle = () => void; export type machineStartHandle = () => void; export type machineStopHandle = () => void; -export type podStartHandle = (pod: PodInfo) => void; -export type podStopHandle = (pod: PodInfo) => void; -export type podRemoveHandle = (podId: string) => void; export class PodmanConnection implements Disposable { #firstFound = false; #toExecuteAtStartup: startupHandle[] = []; #toExecuteAtMachineStop: machineStopHandle[] = []; #toExecuteAtMachineStart: machineStartHandle[] = []; - #toExecuteAtPodStart: podStartHandle[] = []; - #toExecuteAtPodStop: podStopHandle[] = []; - #toExecuteAtPodRemove: podRemoveHandle[] = []; #onEventDisposable: Disposable | undefined; init(): void { this.listenRegistration(); this.listenMachine(); - this.watchPods(); } dispose(): void { @@ -114,65 +105,4 @@ export class PodmanConnection implements Disposable { onMachineStop(f: machineStopHandle) { this.#toExecuteAtMachineStop.push(f); } - - watchPods() { - if (this.#onEventDisposable !== undefined) throw new Error('already watching pods.'); - - this.#onEventDisposable = containerEngine.onEvent(event => { - if (event.Type !== 'pod') { - return; - } - switch (event.status) { - case 'remove': - for (const f of this.#toExecuteAtPodRemove) { - f(event.id); - } - break; - case 'start': - containerEngine - .listPods() - .then((pods: PodInfo[]) => { - const pod = pods.find((p: PodInfo) => p.Id === event.id); - if (!pod) { - return; - } - for (const f of this.#toExecuteAtPodStart) { - f(pod); - } - }) - .catch((err: unknown) => { - console.error(err); - }); - break; - case 'stop': - containerEngine - .listPods() - .then((pods: PodInfo[]) => { - const pod = pods.find((p: PodInfo) => p.Id === event.id); - if (!pod) { - return; - } - for (const f of this.#toExecuteAtPodStop) { - f(pod); - } - }) - .catch((err: unknown) => { - console.error(err); - }); - break; - } - }); - } - - onPodStart(f: podStartHandle) { - this.#toExecuteAtPodStart.push(f); - } - - onPodStop(f: podStopHandle) { - this.#toExecuteAtPodStop.push(f); - } - - onPodRemove(f: podRemoveHandle) { - this.#toExecuteAtPodRemove.push(f); - } } diff --git a/packages/backend/src/managers/recipes/PodManager.spec.ts b/packages/backend/src/managers/recipes/PodManager.spec.ts index e2992364d..d152ea346 100644 --- a/packages/backend/src/managers/recipes/PodManager.spec.ts +++ b/packages/backend/src/managers/recipes/PodManager.spec.ts @@ -18,8 +18,8 @@ import { beforeEach, describe, vi, expect, test } from 'vitest'; import { PodManager } from './PodManager'; -import type { ContainerInspectInfo, PodCreateOptions, PodInfo } from '@podman-desktop/api'; -import { containerEngine } from '@podman-desktop/api'; +import type { ContainerInspectInfo, ContainerJSONEvent, PodCreateOptions, PodInfo } from '@podman-desktop/api'; +import { EventEmitter, containerEngine } from '@podman-desktop/api'; vi.mock('@podman-desktop/api', () => ({ containerEngine: { @@ -29,7 +29,9 @@ vi.mock('@podman-desktop/api', () => ({ startPod: vi.fn(), createPod: vi.fn(), inspectContainer: vi.fn(), + onEvent: vi.fn(), }, + EventEmitter: vi.fn(), })); beforeEach(() => { @@ -45,6 +47,18 @@ beforeEach(() => { }, } as unknown as ContainerInspectInfo; }); + + // mocking the EventEmitter mechanism + const listeners: ((value: unknown) => void)[] = []; + + vi.mocked(EventEmitter).mockReturnValue({ + event: vi.fn().mockImplementation(callback => { + listeners.push(callback); + }), + fire: vi.fn().mockImplementation((content: unknown) => { + listeners.forEach(listener => listener(content)); + }), + } as unknown as EventEmitter); }); test('getAllPods should use container engine list pods method', async () => { @@ -234,3 +248,112 @@ test('createPod should call containerEngine.createPod', async () => { await new PodManager().createPod(options); expect(containerEngine.createPod).toHaveBeenCalledWith(options); }); + +test('dispose should dispose onEvent disposable', () => { + const disposableMock = vi.fn(); + vi.mocked(containerEngine.onEvent).mockImplementation(() => { + return { dispose: disposableMock }; + }); + + const podManager = new PodManager(); + podManager.init(); + + podManager.dispose(); + + expect(containerEngine.onEvent).toHaveBeenCalled(); + expect(disposableMock).toHaveBeenCalled(); +}); + +const getInitializedPodManager = (): { + onEventListener: (e: ContainerJSONEvent) => unknown; + podManager: PodManager; +} => { + let func: ((e: ContainerJSONEvent) => unknown) | undefined = undefined; + vi.mocked(containerEngine.onEvent).mockImplementation(fn => { + func = fn; + return { dispose: vi.fn() }; + }); + + const podManager = new PodManager(); + podManager.init(); + + if (!func) throw new Error('listener should be defined'); + + return { onEventListener: func, podManager }; +}; + +describe('events', () => { + test('onStartPodEvent listener should be called on start pod event', async () => { + vi.mocked(containerEngine.listPods).mockResolvedValue([ + { + Id: 'pod-id-1', + Labels: { + 'dummy-key': 'dummy-value', + hello: 'world', + }, + }, + ] as unknown as PodInfo[]); + + const { onEventListener, podManager } = getInitializedPodManager(); + + const startListenerMock = vi.fn(); + podManager.onStartPodEvent(startListenerMock); + + onEventListener({ id: 'pod-id-1', Type: 'pod', type: '', status: 'start' }); + + await vi.waitFor(() => { + expect(startListenerMock).toHaveBeenCalledWith({ + Id: 'pod-id-1', + Labels: { + 'dummy-key': 'dummy-value', + hello: 'world', + }, + }); + }); + }); + + test('onStopPodEvent listener should be called on start pod event', async () => { + vi.mocked(containerEngine.listPods).mockResolvedValue([ + { + Id: 'pod-id-1', + Labels: { + 'dummy-key': 'dummy-value', + hello: 'world', + }, + }, + ] as unknown as PodInfo[]); + + const { onEventListener, podManager } = getInitializedPodManager(); + + const stopListenerMock = vi.fn(); + podManager.onStopPodEvent(stopListenerMock); + + onEventListener({ id: 'pod-id-1', Type: 'pod', type: '', status: 'stop' }); + + await vi.waitFor(() => { + expect(stopListenerMock).toHaveBeenCalledWith({ + Id: 'pod-id-1', + Labels: { + 'dummy-key': 'dummy-value', + hello: 'world', + }, + }); + }); + }); + + test('onRemovePodEvent listener should be called on start pod event', async () => { + const { onEventListener, podManager } = getInitializedPodManager(); + + const removeListenerMock = vi.fn(); + podManager.onRemovePodEvent(removeListenerMock); + + onEventListener({ id: 'pod-id-1', Type: 'pod', type: '', status: 'remove' }); + + await vi.waitFor(() => { + expect(removeListenerMock).toHaveBeenCalledWith({ + podId: 'pod-id-1', + }); + }); + expect(containerEngine.listPods).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/backend/src/managers/recipes/PodManager.ts b/packages/backend/src/managers/recipes/PodManager.ts index d9172dcf5..fdb3e0e9c 100644 --- a/packages/backend/src/managers/recipes/PodManager.ts +++ b/packages/backend/src/managers/recipes/PodManager.ts @@ -15,13 +15,60 @@ * * SPDX-License-Identifier: Apache-2.0 ***********************************************************************/ -import type { Disposable, PodCreateOptions, PodInfo } from '@podman-desktop/api'; -import { containerEngine } from '@podman-desktop/api'; +import type { Disposable, PodCreateOptions, PodInfo, Event } from '@podman-desktop/api'; +import { containerEngine, EventEmitter } from '@podman-desktop/api'; import type { PodHealth } from '@shared/src/models/IApplicationState'; import { getPodHealth } from '../../utils/podsUtils'; +export interface PodEvent { + podId: string; +} + export class PodManager implements Disposable { - dispose(): void {} + #eventDisposable: Disposable | undefined; + + // start pod events + private readonly _onStartPodEvent = new EventEmitter(); + readonly onStartPodEvent: Event = this._onStartPodEvent.event; + + // stop pod events + private readonly _onStopPodEvent = new EventEmitter(); + readonly onStopPodEvent: Event = this._onStopPodEvent.event; + + // remove pod events + private readonly _onRemovePodEvent = new EventEmitter(); + readonly onRemovePodEvent: Event = this._onRemovePodEvent.event; + + constructor() {} + + dispose(): void { + this.#eventDisposable?.dispose(); + } + + init(): void { + this.#eventDisposable = containerEngine.onEvent(async event => { + // filter on pod event type + if (event.Type !== 'pod') { + return; + } + + if (event.status === 'remove') { + return this._onRemovePodEvent.fire({ + podId: event.id, + }); + } + + const pod: PodInfo = await this.getPodById(event.id); + switch (event.status) { + case 'start': + this._onStartPodEvent.fire(pod); + break; + case 'stop': + this._onStopPodEvent.fire(pod); + break; + } + }); + } /** * Utility method to get all the pods @@ -75,6 +122,19 @@ export class PodManager implements Disposable { return getPodHealth(containerStates); } + /** + * This handy method is private as we do not want expose method not providing + * the engineId, but this is required because PodEvent do not provide the engineId + * @param id + * @private + */ + private async getPodById(id: string): Promise { + const pods = await this.getAllPods(); + const result = pods.find(pod => pod.Id === id); + if (!result) throw new Error(`pod with Id ${id} cannot be found.`); + return result; + } + async getPod(engineId: string, Id: string): Promise { const pods = await this.getAllPods(); const result = pods.find(pod => pod.engineId === engineId && pod.Id === Id); diff --git a/packages/backend/src/studio.ts b/packages/backend/src/studio.ts index 58b2bdeba..f7950141b 100644 --- a/packages/backend/src/studio.ts +++ b/packages/backend/src/studio.ts @@ -164,6 +164,7 @@ export class Studio { this.#extensionContext.subscriptions.push(builderManager); const podManager = new PodManager(); + podManager.init(); this.#extensionContext.subscriptions.push(podManager); this.modelsManager = new ModelsManager(