Skip to content

Commit

Permalink
new field from_name in from_name in ImageTagInfo. Use the tag.from.na…
Browse files Browse the repository at this point in the history
…me as value of env JUPYTER_IMAGE while using the combination of imagestream/image name and tag as the image source for notebook; achieving independence from the internal openshift docker registry
  • Loading branch information
shalberd committed Oct 30, 2023
1 parent c0dca01 commit 1f6c127
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 25 deletions.
9 changes: 4 additions & 5 deletions backend/src/utils/notebookUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,13 @@ export const assembleNotebook = async (

const notebookSize = getNotebookSize(notebookSizeName);

let imageUrl = ``;
let imageSelection = ``;

try {
const image = await getImageInfo(fastify, imageName);

const selectedImage = getImageTag(image, imageTagName);

imageUrl = `${selectedImage.image?.dockerImageRepo}:${selectedImage.tag?.name}`;
imageSelection = `${selectedImage.image?.name}:${selectedImage.tag?.name}`;
} catch (e) {
fastify.log.error(`Error getting the image for ${imageName}:${imageTagName}`);
Expand Down Expand Up @@ -268,6 +266,7 @@ export const assembleNotebook = async (
'opendatahub.io/username': username,
'kubeflow-resource-stopped': null,
'opendatahub.io/accelerator-name': accelerator.accelerator?.metadata.name || '',
'image.openshift.io/triggers': `[{"from":{"kind":"ImageStreamTag","name":"${imageSelection}", "namespace":"${namespace}"},"fieldPath":"spec.template.spec.containers[?(@.name==\\"${name}\\")].image"}]`,
},
name: name,
namespace: namespace,
Expand All @@ -279,8 +278,8 @@ export const assembleNotebook = async (
enableServiceLinks: false,
containers: [
{
image: imageUrl,
imagePullPolicy: 'Always',
image: name,
imagePullPolicy: 'IfNotPresent',
workingDir: MOUNT_PATH,
name: name,
env: [
Expand All @@ -295,7 +294,7 @@ export const assembleNotebook = async (
},
{
name: 'JUPYTER_IMAGE',
value: imageUrl,
value: imageSelection,
},
...configMapEnvs,
...secretEnvs,
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/__mocks__/mockNotebookK8sResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export const mockNotebookK8sResource = ({
],
image:
'image-registry.openshift-image-registry.svc:5000/redhat-ods-applications/s2i-minimal-notebook:py3.8-v1',
imagePullPolicy: 'Always',
imagePullPolicy: 'IfNotPresent',
livenessProbe: {
failureThreshold: 3,
httpGet: {
Expand Down Expand Up @@ -164,7 +164,7 @@ export const mockNotebookK8sResource = ({
],
image:
'registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4bef31eb993feb6f1096b51b4876c65a6fb1f4401fee97fa4f4542b6b7c9bc46',
imagePullPolicy: 'Always',
imagePullPolicy: 'IfNotPresent',
livenessProbe: {
failureThreshold: 3,
httpGet: {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/__mocks__/mockPodK8sResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export const mockPodK8sResource = ({
},
terminationMessagePath: '/dev/termination-log',
terminationMessagePolicy: 'File',
imagePullPolicy: 'Always',
imagePullPolicy: 'IfNotPresent',
securityContext: {
capabilities: {
drop: ['ALL'],
Expand Down Expand Up @@ -302,7 +302,7 @@ export const mockPodK8sResource = ({
},
terminationMessagePath: '/dev/termination-log',
terminationMessagePolicy: 'File',
imagePullPolicy: 'Always',
imagePullPolicy: 'IfNotPresent',
securityContext: {
capabilities: {
drop: ['ALL'],
Expand Down
19 changes: 11 additions & 8 deletions frontend/src/api/k8s/notebooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { assemblePodSpecOptions, getshmVolume, getshmVolumeMount } from './utils
const assembleNotebook = (
data: StartNotebookData,
username: string,
dashboardNamespace: string,
canEnablePipelines?: boolean,
): NotebookKind => {
const {
Expand All @@ -48,9 +49,7 @@ const assembleNotebook = (
existingResources,
} = data;
const notebookId = overrideNotebookId || translateDisplayNameForK8s(notebookName);
const imageUrl = `${image.imageStream?.status?.dockerImageRepository}:${image.imageVersion?.name}`;
const imageSelection = `${image.imageStream?.metadata.name}:${image.imageVersion?.name}`;

const { affinity, tolerations, resources } = assemblePodSpecOptions(
notebookSize.resources,
accelerator,
Expand Down Expand Up @@ -106,6 +105,7 @@ const assembleNotebook = (
'notebooks.opendatahub.io/inject-oauth': 'true',
'opendatahub.io/username': username,
'opendatahub.io/accelerator-name': accelerator.accelerator?.metadata.name || '',
'image.openshift.io/triggers': `[{"from":{"kind":"ImageStreamTag","name":"${imageSelection}", "namespace":"${dashboardNamespace}"},"fieldPath":"spec.template.spec.containers[?(@.name==\\"${notebookId}\\")].image"}]`,
},
name: notebookId,
namespace: projectName,
Expand All @@ -117,8 +117,8 @@ const assembleNotebook = (
enableServiceLinks: false,
containers: [
{
image: imageUrl,
imagePullPolicy: 'Always',
image: notebookId,
imagePullPolicy: 'IfNotPresent',
workingDir: ROOT_MOUNT_PATH,
name: notebookId,
env: [
Expand All @@ -133,7 +133,7 @@ const assembleNotebook = (
},
{
name: 'JUPYTER_IMAGE',
value: imageUrl,
value: imageSelection,
},
],
envFrom,
Expand Down Expand Up @@ -240,9 +240,10 @@ export const startNotebook = async (
export const createNotebook = (
data: StartNotebookData,
username: string,
dashboardNamespace: string,
canEnablePipelines?: boolean,
): Promise<NotebookKind> => {
const notebook = assembleNotebook(data, username, canEnablePipelines);
const notebook = assembleNotebook(data, username, dashboardNamespace, canEnablePipelines);

const notebookPromise = k8sCreateResource<NotebookKind>({
model: NotebookModel,
Expand All @@ -262,9 +263,10 @@ export const updateNotebook = (
existingNotebook: NotebookKind,
data: StartNotebookData,
username: string,
dashboardNamespace: string,
): Promise<NotebookKind> => {
data.notebookId = existingNotebook.metadata.name;
const notebook = assembleNotebook(data, username);
const notebook = assembleNotebook(data, username, dashboardNamespace);

const oldNotebook = structuredClone(existingNotebook);
const container = oldNotebook.spec.template.spec.containers[0];
Expand All @@ -285,9 +287,10 @@ export const updateNotebook = (
export const createNotebookWithoutStarting = (
data: StartNotebookData,
username: string,
dashboardNamespace: string,
): Promise<NotebookKind> =>
new Promise((resolve, reject) =>
createNotebook(data, username).then((notebook) =>
createNotebook(data, username, dashboardNamespace).then((notebook) =>
setTimeout(
() =>
stopNotebook(notebook.metadata.name, notebook.metadata.namespace)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
import { NotebookContainer } from '~/types';
import {
getDescriptionForTag,
getImageTagByContainer,
getImageAndTagByContainerEnvJupyterImage,
getNameVersionString,
} from '~/utilities/imageUtils';
import { useAppContext } from '~/app/AppContext';
Expand Down Expand Up @@ -42,7 +42,7 @@ const NotebookServerDetails: React.FC = () => {
);
}

const { image, tag } = getImageTagByContainer(images, container);
const { image, tag } = getImageAndTagByContainerEnvJupyterImage(images, container);

const tagSoftware = getDescriptionForTag(tag);
const tagDependencies = tag?.content.dependencies ?? [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ const useNotebookImageData = (
const container: NotebookContainer | undefined = notebook.spec.template.spec.containers.find(
(container) => container.name === notebook.metadata.name,
);
const imageTag = container?.image.split('/').at(-1)?.split(':');

const imageStreamTagAndName =
container?.env?.find((i) => i?.name === 'JUPYTER_IMAGE')?.value ?? '';
const imageTag = imageStreamTagAndName.toString().split('/').at(-1)?.split(':');

if (!imageTag || imageTag.length < 2 || !container) {
return [null, true];
Expand Down
6 changes: 4 additions & 2 deletions frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
DataConnectionData,
} from '~/pages/projects/types';
import { useUser } from '~/redux/selectors';
import { useDashboardNamespace } from '~/redux/selectors';
import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext';
import { AppContext } from '~/app/AppContext';
import { fireTrackingEvent } from '~/utilities/segmentIOUtils';
Expand Down Expand Up @@ -61,6 +62,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
);
const editNotebook = notebookState?.notebook;
const { projectName } = startNotebookData;
const { dashboardNamespace } = useDashboardNamespace();
const navigate = useNavigate();
const [createInProgress, setCreateInProgress] = React.useState(false);
const isButtonDisabled =
Expand Down Expand Up @@ -154,7 +156,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
envFrom,
tolerationSettings,
};
updateNotebook(editNotebook, newStartNotebookData, username)
updateNotebook(editNotebook, newStartNotebookData, username, dashboardNamespace)
.then((notebook) => afterStart(notebook.metadata.name, 'updated'))
.catch(handleError);
}
Expand Down Expand Up @@ -212,7 +214,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
tolerationSettings,
};

createNotebook(newStartData, username, canEnablePipelines)
createNotebook(newStartData, username, dashboardNamespace, canEnablePipelines)
.then((notebook) => afterStart(notebook.metadata.name, 'created'))
.catch(handleError);
};
Expand Down
7 changes: 5 additions & 2 deletions frontend/src/utilities/imageUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,14 @@ export const getDescriptionForTag = (imageTag?: ImageTagInfo): string => {
return softwareDescriptions.join(', ');
};

export const getImageTagByContainer = (
export const getImageAndTagByContainerEnvJupyterImage = (
images: ImageInfo[],
container?: NotebookContainer,
): ImageTag => {
const imageTag = container?.image.split('/').at(-1)?.split(':');
const imageStreamTagAndName =
container?.env?.find((i) => i?.name === 'JUPYTER_IMAGE')?.value ?? '';
const imageTag = imageStreamTagAndName.toString().split('/').at(-1)?.split(':');

if (!imageTag || imageTag.length < 2) {
return { image: undefined, tag: undefined };
}
Expand Down
2 changes: 1 addition & 1 deletion manifests/base/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ spec:
containers:
- name: odh-dashboard
image: odh-dashboard
imagePullPolicy: Always
imagePullPolicy: IfNotPresent
ports:
- containerPort: 8080
resources:
Expand Down

0 comments on commit 1f6c127

Please sign in to comment.