Merge pull request #2867 from lucferbux/rhoaieng-5414-fix

Fix issue that fetch the wrong Notebook Image if two Imagestream Version shared the same sha
opendatahub-io · May 31, 2024 · 3392cb8 · 3392cb8
2 parents 4aa0a8c + 7ffb101
commit 3392cb8
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 11 deletions.
diff --git a/frontend/src/__mocks__/mockImageStreamK8sResource.ts b/frontend/src/__mocks__/mockImageStreamK8sResource.ts
@@ -7,6 +7,7 @@ type MockResourceConfigType = {
   namespace?: string;
   displayName?: string;
   imageTag?: string;
+  tagName?: string;
   opts?: RecursivePartial<ImageStreamKind>;
 };
 
@@ -15,6 +16,7 @@ export const mockImageStreamK8sResource = ({
   namespace = 'test-project',
   displayName = 'Test Image',
   imageTag = 'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+  tagName = '1.2',
   opts = {},
 }: MockResourceConfigType): ImageStreamKind =>
   _.mergeWith(
@@ -50,7 +52,7 @@ export const mockImageStreamK8sResource = ({
         },
         tags: [
           {
-            name: '1.2',
+            name: tagName,
             annotations: {
               'opendatahub.io/notebook-python-dependencies':
                 '[{"name":"JupyterLab","version": "3.2"}, {"name": "Notebook","version": "6.4"}]',

diff --git a/frontend/src/__mocks__/mockNotebookK8sResource.ts b/frontend/src/__mocks__/mockNotebookK8sResource.ts
@@ -14,6 +14,7 @@ type MockResourceConfigType = {
   envFromName?: string;
   resources?: ContainerResources;
   image?: string;
+  lastImageSelection?: string;
   opts?: RecursivePartial<NotebookKind>;
   uid?: string;
 };
@@ -27,6 +28,7 @@ export const mockNotebookK8sResource = ({
   description = '',
   resources = DEFAULT_NOTEBOOK_SIZES[0].resources,
   image = 'test-imagestream:1.2',
+  lastImageSelection = 's2i-minimal-notebook:py3.8-v1',
   opts = {},
   uid = genUID('notebook'),
 }: MockResourceConfigType): NotebookKind =>
@@ -38,7 +40,7 @@ export const mockNotebookK8sResource = ({
         annotations: {
           'notebooks.kubeflow.org/last-activity': '2023-02-14T21:45:14Z',
           'notebooks.opendatahub.io/inject-oauth': 'true',
-          'notebooks.opendatahub.io/last-image-selection': 's2i-minimal-notebook:py3.8-v1',
+          'notebooks.opendatahub.io/last-image-selection': lastImageSelection,
           'notebooks.opendatahub.io/last-size-selection': 'Small',
           'notebooks.opendatahub.io/oauth-logout-url':
             'http://localhost:4010/projects/project?notebookLogout=workbench',

diff --git a/frontend/src/pages/projects/screens/detail/notebooks/__tests__/useNotebookImageData.spec.ts b/frontend/src/pages/projects/screens/detail/notebooks/__tests__/useNotebookImageData.spec.ts
@@ -4,21 +4,95 @@ import { getNotebookImageData } from '~/pages/projects/screens/detail/notebooks/
 import { NotebookImageAvailability } from '~/pages/projects/screens/detail/notebooks/const';
 
 describe('getNotebookImageData', () => {
-  it('should return image data when image stream exists and image version exists', () => {
+  it('should return image data when image stream exists and image version exists with internal registry', () => {
     const notebook = mockNotebookK8sResource({
       image:
         'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+      lastImageSelection: 'jupyter-datascience-notebook',
     });
     const images = [
       mockImageStreamK8sResource({
         imageTag:
           'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+        name: 'jupyter-datascience-notebook',
       }),
     ];
     const result = getNotebookImageData(notebook, images);
     expect(result?.imageAvailability).toBe(NotebookImageAvailability.ENABLED);
   });
 
+  it('should return image data when image stream exists and image version exists without internal registry', () => {
+    const imageName = 'jupyter-datascience-notebook';
+    const tagName = '2024.1';
+    const notebook = mockNotebookK8sResource({
+      image: `image-registry.openshift-image-registry.svc:5000/opendatahub/${imageName}:${tagName}`,
+      lastImageSelection: 'jupyter-datascience-notebook',
+    });
+    const images = [
+      mockImageStreamK8sResource({
+        imageTag:
+          'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+        tagName,
+        name: imageName,
+      }),
+    ];
+    const result = getNotebookImageData(notebook, images);
+    expect(result?.imageAvailability).toBe(NotebookImageAvailability.ENABLED);
+  });
+
+  it('should return the right image if multiple ImageStreams have the same image with internal registry', () => {
+    const displayName = 'Jupyter Data Science Notebook';
+    const notebook = mockNotebookK8sResource({
+      image:
+        'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+      lastImageSelection: 'jupyter-datascience-notebook',
+    });
+    const images = [
+      mockImageStreamK8sResource({
+        imageTag:
+          'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+        name: 'jupyter-datascience-notebook',
+        displayName,
+      }),
+      mockImageStreamK8sResource({
+        imageTag:
+          'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+        name: 'custom-notebook',
+        displayName: 'Custom Notebook',
+      }),
+    ];
+    const result = getNotebookImageData(notebook, images);
+    expect(result?.imageDisplayName).toBe(displayName);
+  });
+
+  it('should return the right image if multiple ImageStreams have the same tag without internal registry', () => {
+    const imageName = 'jupyter-datascience-notebook';
+    const tagName = '2024.1';
+    const displayName = 'Jupyter Data Science Notebook';
+    const notebook = mockNotebookK8sResource({
+      image: `image-registry.openshift-image-registry.svc:5000/opendatahub/${imageName}:${tagName}`,
+      lastImageSelection: 'jupyter-datascience-notebook',
+    });
+    const images = [
+      mockImageStreamK8sResource({
+        imageTag:
+          'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+        tagName,
+        name: 'code-server',
+        displayName: 'Code Server',
+      }),
+      mockImageStreamK8sResource({
+        imageTag:
+          'quay.io/opendatahub/notebooks@sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75',
+        tagName,
+        name: imageName,
+        displayName,
+      }),
+    ];
+    const result = getNotebookImageData(notebook, images);
+    expect(result?.imageDisplayName).toBe(displayName);
+  });
+
   it('should return image data when image stream exists and image version does not exist', () => {
     const notebook = mockNotebookK8sResource({
       image:

diff --git a/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts b/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts
@@ -23,14 +23,23 @@ export const getNotebookImageData = (
     };
   }
 
-  const [, versionName] = imageTag;
-  const imageStream = images.find((image) =>
-    image.spec.tags
-      ? image.spec.tags.find(
-          (version) => version.name === versionName || version.from?.name === container.image,
-        )
-      : false,
-  );
+  const [imageName, versionName] = imageTag;
+  const [lastImageSelectionName] =
+    notebook.metadata.annotations?.['notebooks.opendatahub.io/last-image-selection']?.split(':') ??
+    [];
+
+  // Fallback for non internal registry clusters
+  const imageStream =
+    images.find((image) => image.metadata.name === imageName) ||
+    images.find((image) =>
+      image.spec.tags
+        ? image.spec.tags.find(
+            (version) =>
+              version.from?.name === container.image &&
+              image.metadata.name === lastImageSelectionName,
+          )
+        : false,
+    );
 
   // if the image stream is not found, consider it deleted
   if (!imageStream) {