fix: delete containers outside the pod

Signed-off-by: lstocchi <[email protected]>
containers · Jan 26, 2024 · 30d8bdd · 30d8bdd
1 parent 6c2133e
commit 30d8bdd
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 27 deletions.
diff --git a/packages/backend/src/managers/applicationManager.spec.ts b/packages/backend/src/managers/applicationManager.spec.ts
@@ -26,6 +26,7 @@ const mocks = vi.hoisted(() => {
     replicatePodmanContainerMock: vi.fn(),
     startContainerMock: vi.fn(),
     startPod: vi.fn(),
+    deleteContainerMock: vi.fn(),
   };
 });
 vi.mock('../models/AIConfig', () => ({
@@ -41,6 +42,7 @@ vi.mock('@podman-desktop/api', () => ({
     replicatePodmanContainer: mocks.replicatePodmanContainerMock,
     startContainer: mocks.startContainerMock,
     startPod: mocks.startPod,
+    deleteContainer: mocks.deleteContainerMock,
   },
 }));
 let setTaskMock: MockInstance;
@@ -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('restartContainerWhenEndpointIsUp', () => {
@@ -783,3 +799,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');
+  });
+});
diff --git a/packages/backend/src/managers/applicationManager.ts b/packages/backend/src/managers/applicationManager.ts
@@ -164,7 +164,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,
@@ -212,35 +223,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}`);
         }
       }),
     );