Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for untrue !Deleted flag #3049

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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}`,
ashley-o0o marked this conversation as resolved.
Show resolved Hide resolved
});
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)
ashley-o0o marked this conversation as resolved.
Show resolved Hide resolved
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;
Loading