env var JUPYTER_IMAGE, details on image from JUPYTER_IMAGE instead of…

… container image field, image change trigger annotation on assemble and update notebook, imagePullPolicy from Always to IfNotPresent
opendatahub-io · Jul 27, 2023 · d3f0d96 · d3f0d96
1 parent 2992130
commit d3f0d96
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 25 deletions.
diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts
@@ -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}`);
@@ -266,6 +264,7 @@ export const assembleNotebook = async (
         'notebooks.opendatahub.io/last-image-selection': imageSelection,
         'opendatahub.io/username': username,
         'kubeflow-resource-stopped': null,
+        'image.openshift.io/triggers': `[{"from":{"kind":"ImageStreamTag","name":"${imageSelection}", "namespace":"${namespace}"},"fieldPath":"spec.template.spec.containers[?(@.name==\\"${name}\\")].image"}]`,
       },
       name: name,
       namespace: namespace,
@@ -277,8 +276,8 @@ export const assembleNotebook = async (
           enableServiceLinks: false,
           containers: [
             {
-              image: imageUrl,
-              imagePullPolicy: 'Always',
+              image: name,
+              imagePullPolicy: 'IfNotPresent',
               workingDir: MOUNT_PATH,
               name: name,
               env: [
@@ -293,7 +292,7 @@ export const assembleNotebook = async (
                 },
                 {
                   name: 'JUPYTER_IMAGE',
-                  value: imageUrl,
+                  value: imageSelection,
                 },
                 ...configMapEnvs,
                 ...secretEnvs,

diff --git a/frontend/src/__mocks__/mockNotebookK8sResource.ts b/frontend/src/__mocks__/mockNotebookK8sResource.ts
@@ -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: {
@@ -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: {

diff --git a/frontend/src/__mocks__/mockPodK8sResource.ts b/frontend/src/__mocks__/mockPodK8sResource.ts
@@ -202,7 +202,7 @@ export const mockPodK8sResource = ({
         },
         terminationMessagePath: '/dev/termination-log',
         terminationMessagePolicy: 'File',
-        imagePullPolicy: 'Always',
+        imagePullPolicy: 'IfNotPresent',
         securityContext: {
           capabilities: {
             drop: ['ALL'],
@@ -302,7 +302,7 @@ export const mockPodK8sResource = ({
         },
         terminationMessagePath: '/dev/termination-log',
         terminationMessagePolicy: 'File',
-        imagePullPolicy: 'Always',
+        imagePullPolicy: 'IfNotPresent',
         securityContext: {
           capabilities: {
             drop: ['ALL'],

diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts
@@ -30,6 +30,7 @@ import { assemblePodSpecOptions } from './utils';
 const assembleNotebook = (
   data: StartNotebookData,
   username: string,
+  dashboardNamespace: string,
   canEnablePipelines?: boolean,
 ): NotebookKind => {
   const {
@@ -46,9 +47,7 @@ const assembleNotebook = (
     tolerationSettings,
   } = 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,
     gpus,
@@ -88,6 +87,7 @@ const assembleNotebook = (
         'notebooks.opendatahub.io/last-image-selection': imageSelection,
         'notebooks.opendatahub.io/inject-oauth': 'true',
         'opendatahub.io/username': username,
+        'image.openshift.io/triggers': `[{"from":{"kind":"ImageStreamTag","name":"${imageSelection}", "namespace":"${dashboardNamespace}"},"fieldPath":"spec.template.spec.containers[?(@.name==\\"${notebookId}\\")].image"}]`,
       },
       name: notebookId,
       namespace: projectName,
@@ -99,8 +99,8 @@ const assembleNotebook = (
           enableServiceLinks: false,
           containers: [
             {
-              image: imageUrl,
-              imagePullPolicy: 'Always',
+              image: notebookId,
+              imagePullPolicy: 'IfNotPresent',
               workingDir: ROOT_MOUNT_PATH,
               name: notebookId,
               env: [
@@ -115,7 +115,7 @@ const assembleNotebook = (
                 },
                 {
                   name: 'JUPYTER_IMAGE',
-                  value: imageUrl,
+                  value: imageSelection,
                 },
               ],
               envFrom,
@@ -229,9 +229,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,
@@ -251,9 +252,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];
@@ -274,9 +276,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)

diff --git a/frontend/src/pages/notebookController/screens/server/NotebookServerDetails.tsx b/frontend/src/pages/notebookController/screens/server/NotebookServerDetails.tsx
@@ -14,7 +14,7 @@ import {
 import { NotebookContainer } from '~/types';
 import {
   getDescriptionForTag,
-  getImageTagByContainer,
+  getImageAndTagByContainerEnvJupyterImage,
   getNameVersionString,
   getNumGpus,
 } from '~/utilities/imageUtils';
@@ -41,7 +41,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 ?? [];

diff --git a/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts b/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts
@@ -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];

diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx
@@ -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';
@@ -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 =
@@ -149,7 +151,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);
     }
@@ -207,7 +209,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);
   };

diff --git a/frontend/src/utilities/imageUtils.ts b/frontend/src/utilities/imageUtils.ts
@@ -156,11 +156,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 };
   }

diff --git a/manifests/base/deployment.yaml b/manifests/base/deployment.yaml
@@ -28,7 +28,7 @@ spec:
       containers:
       - name: odh-dashboard
         image: odh-dashboard
-        imagePullPolicy: Always
+        imagePullPolicy: IfNotPresent
         ports:
         - containerPort: 8080
         resources: