Skip to content

Commit

Permalink
Fix for untrue flag (#3049)
Browse files Browse the repository at this point in the history
  • Loading branch information
ashley-o0o authored Aug 9, 2024
1 parent bd0f9a9 commit b9e2a52
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,38 @@ 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);
});

it('should test an image defined via sha', () => {
const imageName = 'jupyter-datascience-notebook';
const imageSha = 'sha256:a138838e1c9acd7708462e420bf939e03296b97e9cf6c0aa0fd9a5d20361ab75';
const notebook = mockNotebookK8sResource({
lastImageSelection: `${imageName}:${imageSha}`,
image: `quay.io/opendatahub/${imageName}@${imageSha}`,
});
const images = [
mockImageStreamK8sResource({
imageTag: `quay.io/opendatahub/${imageName}@${imageSha}`,
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,59 +24,52 @@ 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
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 {
imageAvailability: NotebookImageAvailability.DELETED,
imageDisplayName,
};
const notebookImageInternalRegistry = getNotebookImageInternalRegistry(
notebook,
images,
imageName,
versionName,
);
if (
notebookImageInternalRegistry &&
notebookImageInternalRegistry.imageAvailability !== NotebookImageAvailability.DELETED
) {
return notebookImageInternalRegistry;
}

const versions = imageStream.spec.tags || [];
const imageVersion = versions.find(
(version) => version.name === versionName || version.from?.name === container.image,
const notebookImageNoInternalRegistry = getNotebookImageNoInternalRegistry(
notebook,
images,
lastImageSelectionName,
container.image,
);

// because the image stream was found, get its display name
const imageDisplayName = getImageStreamDisplayName(imageStream);

// if the image version is not found, consider the image stream deleted
if (!imageVersion) {
return {
imageAvailability: NotebookImageAvailability.DELETED,
imageDisplayName,
};
if (
notebookImageNoInternalRegistry &&
notebookImageNoInternalRegistry.imageAvailability !== NotebookImageAvailability.DELETED
) {
return notebookImageNoInternalRegistry;
}
const notebookImageNoInternalRegistryNoSHA = getNotebookImageNoInternalRegistryNoSHA(
notebook,
images,
lastImageSelectionTag,
container.image,
);
if (
notebookImageNoInternalRegistryNoSHA &&
notebookImageNoInternalRegistryNoSHA.imageAvailability !== NotebookImageAvailability.DELETED
) {
return notebookImageNoInternalRegistryNoSHA;
}

// if the image stream exists and the image version exists, return the image data
return {
imageStream,
imageVersion,
imageAvailability:
imageStream.metadata.labels?.['opendatahub.io/notebook-image'] === 'true'
? NotebookImageAvailability.ENABLED
: NotebookImageAvailability.DISABLED,
imageDisplayName,
imageAvailability: NotebookImageAvailability.DELETED,
imageDisplayName:
notebookImageInternalRegistry?.imageDisplayName ||
notebookImageNoInternalRegistry?.imageDisplayName ||
notebookImageNoInternalRegistryNoSHA?.imageDisplayName,
};
};

Expand All @@ -98,4 +91,113 @@ const useNotebookImageData = (notebook?: NotebookKind): NotebookImageData => {
}, [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)
return getDeletedImageData(
notebook.metadata.annotations?.['opendatahub.io/image-display-name'],
);
}

const versions = imageStream.spec.tags || [];
const imageVersion = versions.find((version) => version.name === versionName);
const imageDisplayName = getImageStreamDisplayName(imageStream);
if (!imageVersion) {
return getDeletedImageData(imageDisplayName);
}
return {
imageStream,
imageVersion,
imageAvailability: getImageAvailability(imageStream),
imageDisplayName,
};
};

const getNotebookImageNoInternalRegistry = (
notebook: NotebookKind,
images: ImageStreamKind[],
lastImageSelectionName: string,
containerImage: string,
): NotebookImageData[0] => {
const imageStream = images.find(
(image) =>
image.metadata.name === lastImageSelectionName &&
image.spec.tags?.find((version) => version.from?.name === containerImage),
);

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)
return getDeletedImageData(
notebook.metadata.annotations?.['opendatahub.io/image-display-name'],
);
}

const versions = imageStream.spec.tags || [];
const imageVersion = versions.find((version) => version.from?.name === containerImage);
const imageDisplayName = getImageStreamDisplayName(imageStream);
if (!imageVersion) {
return getDeletedImageData(imageDisplayName);
}
return {
imageStream,
imageVersion,
imageAvailability: getImageAvailability(imageStream),
imageDisplayName,
};
};

const getNotebookImageNoInternalRegistryNoSHA = (
notebook: NotebookKind,
images: ImageStreamKind[],
lastImageSelectionTag: 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 (!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)
return getDeletedImageData(
notebook.metadata.annotations?.['opendatahub.io/image-display-name'],
);
}

const versions = imageStream.spec.tags || [];
const imageVersion = versions.find((version) => version.name === lastImageSelectionTag);
const imageDisplayName = getImageStreamDisplayName(imageStream);
if (!imageVersion) {
return getDeletedImageData(imageDisplayName);
}
return {
imageStream,
imageVersion,
imageAvailability: getImageAvailability(imageStream),
imageDisplayName,
};
};

export const getImageAvailability = (imageStream: ImageStreamKind): NotebookImageAvailability =>
imageStream.metadata.labels?.['opendatahub.io/notebook-image'] === 'true'
? NotebookImageAvailability.ENABLED
: NotebookImageAvailability.DISABLED;

export const getDeletedImageData = (
imageDisplayName: string | undefined,
): NotebookImageData[0] => ({
imageAvailability: NotebookImageAvailability.DELETED,
imageDisplayName,
});

export default useNotebookImageData;

0 comments on commit b9e2a52

Please sign in to comment.