Fix issue that fetch the wrong Notebook Image if two Imagestream Vers…

…ion shared the same sha
opendatahub-io · May 31, 2024 · aab08fb · aab08fb
1 parent 09aab22
commit aab08fb
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 14 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
@@ -12,6 +12,8 @@ type MockResourceConfigType = {
   user?: string;
   description?: string;
   resources?: ContainerResources;
+  image?: string;
+  lastImageSelection?: string;
   opts?: RecursivePartial<NotebookKind>;
 };
 
@@ -22,6 +24,8 @@ export const mockNotebookK8sResource = ({
   user = 'test-user',
   description = '',
   resources = DEFAULT_NOTEBOOK_SIZES[0].resources,
+  image = 'test-imagestream:1.2',
+  lastImageSelection = 's2i-minimal-notebook:py3.8-v1',
   opts = {},
 }: MockResourceConfigType): NotebookKind =>
   _.merge(
@@ -32,7 +36,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',
@@ -96,8 +100,7 @@ export const mockNotebookK8sResource = ({
                     },
                   },
                 ],
-                image:
-                  'image-registry.openshift-image-registry.svc:5000/redhat-ods-applications/s2i-minimal-notebook:py3.8-v1',
+                image,
                 imagePullPolicy: 'Always',
                 livenessProbe: {
                   failureThreshold: 3,

diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts b/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts
@@ -173,7 +173,10 @@ class ServingRuntimeModal extends Modal {
   }
 }
 
+// Expect KServeModal to inherit both modal classes.
+// eslint-disable-next-line @typescript-eslint/no-unsafe-declaration-merging
 interface KServeModal extends ServingRuntimeModal, InferenceServiceModal {}
+// eslint-disable-next-line @typescript-eslint/no-unsafe-declaration-merging
 class KServeModal extends InferenceServiceModal {}
 mixin(KServeModal, [ServingRuntimeModal, InferenceServiceModal]);
 

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
@@ -1,24 +1,98 @@
-import { mockNotebookK8sResource } from '~/__mocks__';
+import { mockNotebookK8sResource } from '~/__mocks__/mockNotebookK8sResource';
 import { mockImageStreamK8sResource } from '~/__mocks__/mockImageStreamK8sResource';
 import { getNotebookImageData } from '~/pages/projects/screens/detail/notebooks/useNotebookImageData';
 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) {