Skip to content

Commit

Permalink
Fix issue that fetch the wrong Notebook Image if two Imagestream Vers…
Browse files Browse the repository at this point in the history
…ion shared the same sha
  • Loading branch information
lucferbux committed May 31, 2024
1 parent 4aa0a8c commit 7ffb101
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 11 deletions.
4 changes: 3 additions & 1 deletion frontend/src/__mocks__/mockImageStreamK8sResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ type MockResourceConfigType = {
namespace?: string;
displayName?: string;
imageTag?: string;
tagName?: string;
opts?: RecursivePartial<ImageStreamKind>;
};

Expand All @@ -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(
Expand Down Expand Up @@ -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"}]',
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/__mocks__/mockNotebookK8sResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type MockResourceConfigType = {
envFromName?: string;
resources?: ContainerResources;
image?: string;
lastImageSelection?: string;
opts?: RecursivePartial<NotebookKind>;
uid?: string;
};
Expand All @@ -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 =>
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(':') ??
[];

Check warning on line 29 in frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts#L29

Added line #L29 was not covered by tests

// 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,

Check warning on line 41 in frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts#L41

Added line #L41 was not covered by tests
);

// if the image stream is not found, consider it deleted
if (!imageStream) {
Expand Down

0 comments on commit 7ffb101

Please sign in to comment.