From 6e0d5396120ed34ff63740ba4a2dc162272eff91 Mon Sep 17 00:00:00 2001
From: Jeff MAURY <jmaury@redhat.com>
Date: Wed, 6 Mar 2024 20:51:12 +0100
Subject: [PATCH] fix: image names include the recipe id not only the container
 name (#454)

Fixes #283

Signed-off-by: Jeff MAURY <jmaury@redhat.com>
---
 .../src/managers/applicationManager.spec.ts   | 21 ++++++++++++-------
 .../src/managers/applicationManager.ts        | 12 +++++++++--
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/packages/backend/src/managers/applicationManager.spec.ts b/packages/backend/src/managers/applicationManager.spec.ts
index 8cb61bb7c..367d56b81 100644
--- a/packages/backend/src/managers/applicationManager.spec.ts
+++ b/packages/backend/src/managers/applicationManager.spec.ts
@@ -211,7 +211,7 @@ describe('pullApplication', () => {
     mocks.buildImageMock.mockResolvedValue(undefined);
     mocks.listImagesMock.mockResolvedValue([
       {
-        RepoTags: ['container1:latest'],
+        RepoTags: ['recipe1-container1:latest'],
         engineId: 'engine',
         Id: 'id1',
       },
@@ -298,7 +298,7 @@ describe('pullApplication', () => {
       expect.anything(),
       {
         containerFile: 'Containerfile',
-        tag: 'container1:latest',
+        tag: 'recipe1-container1:latest',
         labels: {
           [LABEL_RECIPE_ID]: 'recipe1',
         },
@@ -739,6 +739,9 @@ describe('getRandomName', () => {
 });
 
 describe('buildImages', () => {
+  const recipe = {
+    id: 'recipe1',
+  } as Recipe;
   const containers: ContainerConfig[] = [
     {
       name: 'container1',
@@ -764,13 +767,15 @@ describe('buildImages', () => {
   test('setTaskState should be called with error if context does not exist', async () => {
     vi.spyOn(fs, 'existsSync').mockReturnValue(false);
     mocks.listImagesMock.mockRejectedValue([]);
-    await expect(manager.buildImages(containers, 'config')).rejects.toThrow('Context configured does not exist.');
+    await expect(manager.buildImages(recipe, containers, 'config')).rejects.toThrow(
+      'Context configured does not exist.',
+    );
   });
   test('setTaskState should be called with error if buildImage executon fails', async () => {
     vi.spyOn(fs, 'existsSync').mockReturnValue(true);
     mocks.buildImageMock.mockRejectedValue('error');
     mocks.listImagesMock.mockRejectedValue([]);
-    await expect(manager.buildImages(containers, 'config')).rejects.toThrow(
+    await expect(manager.buildImages(recipe, containers, 'config')).rejects.toThrow(
       'Something went wrong while building the image: error',
     );
     expect(mocks.updateTaskMock).toBeCalledWith({
@@ -785,7 +790,9 @@ describe('buildImages', () => {
     vi.spyOn(fs, 'existsSync').mockReturnValue(true);
     mocks.buildImageMock.mockResolvedValue({});
     mocks.listImagesMock.mockResolvedValue([]);
-    await expect(manager.buildImages(containers, 'config')).rejects.toThrow('no image found for container1:latest');
+    await expect(manager.buildImages(recipe, containers, 'config')).rejects.toThrow(
+      'no image found for container1:latest',
+    );
     expect(mocks.updateTaskMock).toBeCalledWith({
       error: 'no image found for container1:latest',
       name: 'Building container1',
@@ -799,12 +806,12 @@ describe('buildImages', () => {
     mocks.buildImageMock.mockResolvedValue({});
     mocks.listImagesMock.mockResolvedValue([
       {
-        RepoTags: ['container1:latest'],
+        RepoTags: ['recipe1-container1:latest'],
         engineId: 'engine',
         Id: 'id1',
       },
     ]);
-    const imageInfoList = await manager.buildImages(containers, 'config');
+    const imageInfoList = await manager.buildImages(recipe, containers, 'config');
     expect(mocks.updateTaskMock).toBeCalledWith({
       name: 'Building container1',
       id: expect.any(String),
diff --git a/packages/backend/src/managers/applicationManager.ts b/packages/backend/src/managers/applicationManager.ts
index c63b0e468..f6b651373 100644
--- a/packages/backend/src/managers/applicationManager.ts
+++ b/packages/backend/src/managers/applicationManager.ts
@@ -138,6 +138,7 @@ export class ApplicationManager {
 
       // build all images, one per container (for a basic sample we should have 2 containers = sample app + model service)
       const images = await this.buildImages(
+        recipe,
         configAndFilteredContainers.containers,
         configAndFilteredContainers.aiConfigFile.path,
         {
@@ -371,6 +372,7 @@ export class ApplicationManager {
   }
 
   async buildImages(
+    recipe: Recipe,
     containers: ContainerConfig[],
     configPath: string,
     labels?: { [key: string]: string },
@@ -400,9 +402,10 @@ export class ApplicationManager {
           throw new Error('Context configured does not exist.');
         }
 
+        const imageTag = this.getImageTag(recipe, container);
         const buildOptions = {
           containerFile: container.containerfile,
-          tag: `${container.name}:latest`,
+          tag: imageTag,
           labels: {
             [LABEL_RECIPE_ID]: labels !== undefined && 'recipe-id' in labels ? labels['recipe-id'] : undefined,
           },
@@ -434,9 +437,10 @@ export class ApplicationManager {
     await Promise.all(
       containers.map(async container => {
         const task = containerTasks[container.name];
+        const imageTag = this.getImageTag(recipe, container);
 
         const image = images.find(im => {
-          return im.RepoTags?.some(tag => tag.endsWith(`${container.name}:latest`));
+          return im.RepoTags?.some(tag => tag.endsWith(imageTag));
         });
 
         if (!image) {
@@ -460,6 +464,10 @@ export class ApplicationManager {
     return imageInfoList;
   }
 
+  private getImageTag(recipe: Recipe, container: ContainerConfig) {
+    return `${recipe.id}-${container.name}:latest`;
+  }
+
   getConfigAndFilterContainers(
     recipeConfig: string,
     localFolder: string,