Skip to content

Commit

Permalink
Fix for untrue flag
Browse files Browse the repository at this point in the history
  • Loading branch information
ashley-o0o committed Aug 6, 2024
1 parent 8e59efb commit 39e6700
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand All @@ -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,
Expand All @@ -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;

0 comments on commit 39e6700

Please sign in to comment.