Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: delete containers outside the pod #160

Merged
merged 1 commit into from
Jan 26, 2024
Merged

fix: delete containers outside the pod #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions packages/backend/src/managers/applicationManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const mocks = vi.hoisted(() => {
replicatePodmanContainerMock: vi.fn(),
startContainerMock: vi.fn(),
startPod: vi.fn(),
deleteContainerMock: vi.fn(),
};
});
vi.mock('../models/AIConfig', () => ({
Expand All @@ -41,6 +42,7 @@ vi.mock('@podman-desktop/api', () => ({
replicatePodmanContainer: mocks.replicatePodmanContainerMock,
startContainer: mocks.startContainerMock,
startPod: mocks.startPod,
deleteContainer: mocks.deleteContainerMock,
},
}));
let setTaskMock: MockInstance;
Expand Down Expand Up @@ -731,6 +733,20 @@ describe('createApplicationPod', () => {
name: 'Creating application',
});
});
test('throw if createAndAddContainersToPod fails', async () => {
const pod: PodInfo = {
engineId: 'engine',
Id: 'id',
};
vi.spyOn(manager, 'createPod').mockResolvedValue(pod);
vi.spyOn(manager, 'createAndAddContainersToPod').mockRejectedValue('error');
await expect(() => manager.createApplicationPod(images, 'path', taskUtils)).rejects.toThrowError('error');
expect(setTaskMock).toBeCalledWith({
id: 'id',
state: 'error',
name: 'Creating application',
});
});
});

describe('doDownloadModelWrapper', () => {
Expand Down Expand Up @@ -829,3 +845,52 @@ describe('runApplication', () => {
});
});
});

describe('createAndAddContainersToPod', () => {
const manager = new ApplicationManager(
'/home/user/aistudio',
{} as unknown as GitManager,
{} as unknown as RecipeStatusRegistry,
{} as unknown as ModelsManager,
);
const pod: PodInfo = {
engineId: 'engine',
Id: 'id',
};
const imageInfo1: ImageInfo = {
id: 'id',
appName: 'appName',
modelService: false,
ports: ['8080'],
};
test('check that after the creation and copy inside the pod, the container outside the pod is actually deleted', async () => {
mocks.createContainerMock.mockResolvedValue({
id: 'container-1',
});
vi.spyOn(manager, 'getRandomName').mockReturnValue('name');
await manager.createAndAddContainersToPod(pod, [imageInfo1], 'path');
expect(mocks.createContainerMock).toBeCalledWith('engine', {
Image: 'id',
Detach: true,
HostConfig: {
AutoRemove: true,
},
Env: [],
start: false,
});
expect(mocks.replicatePodmanContainerMock).toBeCalledWith(
{
id: 'container-1',
engineId: 'engine',
},
{
engineId: 'engine',
},
{
pod: 'id',
name: 'name',
},
);
expect(mocks.deleteContainerMock).toBeCalledWith('engine', 'container-1');
});
});
63 changes: 36 additions & 27 deletions packages/backend/src/managers/applicationManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,18 @@ export class ApplicationManager {
name: `Creating application`,
});

const attachedContainers = await this.createAndAddContainersToPod(pod, images, modelPath);
let attachedContainers: ContainerAttachedInfo[];
try {
attachedContainers = await this.createAndAddContainersToPod(pod, images, modelPath);
} catch (e) {
console.error(`error when creating pod ${pod.Id}`);
taskUtil.setTask({
id: pod.Id,
state: 'error',
name: 'Creating application',
});
throw e;
}

taskUtil.setTask({
id: pod.Id,
Expand Down Expand Up @@ -219,35 +230,33 @@ export class ApplicationManager {
envs = [`MODEL_ENDPOINT=${endPoint}`];
}
}
const createdContainer = await containerEngine
.createContainer(pod.engineId, {
Image: image.id,
Detach: true,
HostConfig: hostConfig,
Env: envs,
start: false,
})
.catch((e: unknown) => console.error(e));
const createdContainer = await containerEngine.createContainer(pod.engineId, {
Image: image.id,
Detach: true,
HostConfig: hostConfig,
Env: envs,
start: false,
});

// now, for each container, put it in the pod
if (createdContainer) {
try {
const podifiedName = this.getRandomName(`${image.appName}-podified`);
await containerEngine.replicatePodmanContainer(
{
id: createdContainer.id,
engineId: pod.engineId,
},
{ engineId: pod.engineId },
{ pod: pod.Id, name: podifiedName },
);
containers.push({
name: podifiedName,
endPoint,
});
} catch (error) {
console.error(error);
}
const podifiedName = this.getRandomName(`${image.appName}-podified`);
await containerEngine.replicatePodmanContainer(
{
id: createdContainer.id,
engineId: pod.engineId,
},
{ engineId: pod.engineId },
{ pod: pod.Id, name: podifiedName },
);
containers.push({
name: podifiedName,
endPoint,
});
// remove the external container
await containerEngine.deleteContainer(pod.engineId, createdContainer.id);
} else {
throw new Error(`failed at creating container for image ${image.id}`);
}
}),
);
Expand Down
Loading