From 39e67007af1e20e8656558697792a96af9ecc52e Mon Sep 17 00:00:00 2001 From: Ashley McEntee Date: Wed, 31 Jul 2024 14:23:12 -0400 Subject: [PATCH] Fix for untrue flag --- .../__tests__/useNotebookImageData.spec.ts | 17 ++ .../detail/notebooks/useNotebookImageData.ts | 192 ++++++++++++++---- 2 files changed, 172 insertions(+), 37 deletions(-) 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 index 8685653586..b04f95d9f3 100644 --- a/frontend/src/pages/projects/screens/detail/notebooks/__tests__/useNotebookImageData.spec.ts +++ b/frontend/src/pages/projects/screens/detail/notebooks/__tests__/useNotebookImageData.spec.ts @@ -106,4 +106,21 @@ describe('getNotebookImageData', () => { const result = getNotebookImageData(notebook, images); expect(result?.imageAvailability).toBe(NotebookImageAvailability.DELETED); }); + + it('should fail when custom image shows unexpected Deleted flag', () => { + const imageName = 'jupyter-datascience-notebook'; + const tagName = '2024.1'; + const notebook = mockNotebookK8sResource({ + lastImageSelection: `${imageName}:${tagName}`, + image: `quay.io/opendatahub/${imageName}:${tagName}`, + }); + const images = [ + mockImageStreamK8sResource({ + tagName, + name: imageName, + }), + ]; + const result = getNotebookImageData(notebook, images); + expect(result?.imageAvailability).toBe(NotebookImageAvailability.ENABLED); + }); }); diff --git a/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts b/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts index 17f4078783..1a1ab249ae 100644 --- a/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts +++ b/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts @@ -24,24 +24,83 @@ export const getNotebookImageData = ( } const [imageName, versionName] = imageTag; - const [lastImageSelectionName] = + const [lastImageSelectionName, lastImageSelectionTag] = 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 + const notebookImageInternalRegistry = getNotebookImageInternalRegistry( + notebook, + images, + imageName, + versionName, + ); + if ( + notebookImageInternalRegistry && + notebookImageInternalRegistry.imageAvailability !== NotebookImageAvailability.DELETED + ) { + return notebookImageInternalRegistry; + } + const notebookImageNoInternalRegistry = getNotebookImageNoInternalRegistry( + notebook, + images, + lastImageSelectionName, + versionName, + imageName, + ); + if ( + notebookImageNoInternalRegistry && + notebookImageNoInternalRegistry.imageAvailability !== NotebookImageAvailability.DELETED + ) { + return notebookImageNoInternalRegistry; + } + const notebookImageNoInternalRegistryNoSHA = getNotebookImageNoInternalRegistryNoSHA( + notebook, + images, + lastImageSelectionTag, + versionName, + imageName, + ); + if ( + notebookImageNoInternalRegistryNoSHA && + notebookImageNoInternalRegistryNoSHA.imageAvailability !== NotebookImageAvailability.DELETED + ) { + return notebookImageNoInternalRegistryNoSHA; + } + return { + imageAvailability: NotebookImageAvailability.DELETED, + imageDisplayName: + notebookImageInternalRegistry?.imageDisplayName || + notebookImageNoInternalRegistry?.imageDisplayName || + notebookImageNoInternalRegistryNoSHA?.imageDisplayName, + }; +}; + +const useNotebookImageData = (notebook?: NotebookKind): NotebookImageData => { + const { dashboardNamespace } = useNamespaces(); + const [images, loaded, loadError] = useImageStreams(dashboardNamespace, true); + + return React.useMemo(() => { + if (!loaded || !notebook) { + return [null, false, loadError]; + } + const data = getNotebookImageData(notebook, images); + + if (data === null) { + return [null, false, loadError]; + } + + return [data, true, undefined]; + }, [images, notebook, loaded, loadError]); +}; + +const getNotebookImageInternalRegistry = ( + notebook: NotebookKind, + images: ImageStreamKind[], + imageName: string, + versionName: string, +): NotebookImageData[0] => { + const imageStream = images.find((image) => image.metadata.name === imageName); + if (!imageStream) { // Get the image display name from the notebook metadata if we can't find the image stream. (this is a fallback and could still be undefined) const imageDisplayName = notebook.metadata.annotations?.['opendatahub.io/image-display-name']; @@ -51,24 +110,57 @@ export const getNotebookImageData = ( imageDisplayName, }; } - const versions = imageStream.spec.tags || []; - const imageVersion = versions.find( - (version) => version.name === versionName || version.from?.name === container.image, + const imageVersion = versions.find((version) => version.name === versionName); + const imageDisplayName = getImageStreamDisplayName(imageStream); + if (!imageVersion) { + return { + imageAvailability: NotebookImageAvailability.DELETED, + imageDisplayName, + }; + } + return { + imageStream, + imageVersion, + imageAvailability: + imageStream.metadata.labels?.['opendatahub.io/notebook-image'] === 'true' + ? NotebookImageAvailability.ENABLED + : NotebookImageAvailability.DISABLED, + imageDisplayName, + }; +}; + +const getNotebookImageNoInternalRegistry = ( + notebook: NotebookKind, + images: ImageStreamKind[], + lastImageSelectionName: string, + versionName: string, + containerImage: string, +): NotebookImageData[0] => { + const imageStream = images.find( + (image) => + image.metadata.name === lastImageSelectionName && + image.spec.tags?.find((version) => version.from?.name === containerImage), ); - // because the image stream was found, get its display name - const imageDisplayName = getImageStreamDisplayName(imageStream); + if (!imageStream) { + // Get the image display name from the notebook metadata if we can't find the image stream. (this is a fallback and could still be undefined) + const imageDisplayName = notebook.metadata.annotations?.['opendatahub.io/image-display-name']; - // if the image version is not found, consider the image stream deleted + return { + imageAvailability: NotebookImageAvailability.DELETED, + imageDisplayName, + }; + } + const versions = imageStream.spec.tags || []; + const imageVersion = versions.find((version) => version.from?.name === containerImage); + const imageDisplayName = getImageStreamDisplayName(imageStream); if (!imageVersion) { return { imageAvailability: NotebookImageAvailability.DELETED, imageDisplayName, }; } - - // if the image stream exists and the image version exists, return the image data return { imageStream, imageVersion, @@ -80,22 +172,48 @@ export const getNotebookImageData = ( }; }; -const useNotebookImageData = (notebook?: NotebookKind): NotebookImageData => { - const { dashboardNamespace } = useNamespaces(); - const [images, loaded, loadError] = useImageStreams(dashboardNamespace, true); - - return React.useMemo(() => { - if (!loaded || !notebook) { - return [null, false, loadError]; - } - const data = getNotebookImageData(notebook, images); +const getNotebookImageNoInternalRegistryNoSHA = ( + notebook: NotebookKind, + images: ImageStreamKind[], + lastImageSelectionTag: string, + versionName: string, + containerImage: string, +): NotebookImageData[0] => { + const imageStream = images.find((image) => + image.status?.tags?.find( + (version) => + version.tag === lastImageSelectionTag && + version.items?.find((item) => item.dockerImageReference === containerImage), + ), + ); - if (data === null) { - return [null, false, loadError]; - } + if (!imageStream) { + // Get the image display name from the notebook metadata if we can't find the image stream. (this is a fallback and could still be undefined) + const imageDisplayName = notebook.metadata.annotations?.['opendatahub.io/image-display-name']; - return [data, true, undefined]; - }, [images, notebook, loaded, loadError]); + return { + imageAvailability: NotebookImageAvailability.DELETED, + imageDisplayName, + }; + } + const versions = imageStream.spec.tags || []; + const imageVersion = versions.find((version) => version.name === lastImageSelectionTag); + const imageDisplayName = getImageStreamDisplayName(imageStream); + if (!imageVersion) { + return { + imageAvailability: NotebookImageAvailability.DELETED, + imageDisplayName, + }; + } + return { + imageStream, + imageVersion, + imageAvailability: + imageStream.metadata.labels?.['opendatahub.io/notebook-image'] === 'true' + ? NotebookImageAvailability.ENABLED + : NotebookImageAvailability.DISABLED, + imageDisplayName, + }; }; export default useNotebookImageData;