From 8af82c313f886c8e22a8f0d8983dae5fa5b5e1df Mon Sep 17 00:00:00 2001 From: Sven Thoms <21118431+shalberd@users.noreply.github.com> Date: Thu, 27 Jul 2023 12:36:53 +0200 Subject: [PATCH] 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 --- backend/src/utils/notebookUtils.ts | 9 ++++----- .../src/__mocks__/mockNotebookK8sResource.ts | 4 ++-- frontend/src/__mocks__/mockPodK8sResource.ts | 4 ++-- frontend/src/api/k8s/notebooks.ts | 19 +++++++++++-------- .../screens/server/NotebookServerDetails.tsx | 4 ++-- .../detail/notebooks/useNotebookImageData.ts | 5 ++++- .../screens/spawner/SpawnerFooter.tsx | 6 ++++-- frontend/src/utilities/imageUtils.ts | 7 +++++-- manifests/base/deployment.yaml | 2 +- 9 files changed, 35 insertions(+), 25 deletions(-) diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index af3c9e2703..d921c23861 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -160,7 +160,6 @@ export const assembleNotebook = async ( const notebookSize = getNotebookSize(notebookSizeName); - let imageUrl = ``; let imageSelection = ``; try { @@ -168,7 +167,6 @@ export const assembleNotebook = async ( 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}`); @@ -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, @@ -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: [ @@ -295,7 +294,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 index 37a6497e72..067c9ca046 100644 --- 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 index 0823f4c823..f54933e20c 100644 --- 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 index 3fd58721a8..51170e3be3 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -30,6 +30,7 @@ import { assemblePodSpecOptions, getshmVolume, getshmVolumeMount } from './utils const assembleNotebook = ( data: StartNotebookData, username: string, + dashboardNamespace: string, canEnablePipelines?: boolean, ): NotebookKind => { const { @@ -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, @@ -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, @@ -117,8 +117,8 @@ const assembleNotebook = ( enableServiceLinks: false, containers: [ { - image: imageUrl, - imagePullPolicy: 'Always', + image: notebookId, + imagePullPolicy: 'IfNotPresent', workingDir: ROOT_MOUNT_PATH, name: notebookId, env: [ @@ -133,7 +133,7 @@ const assembleNotebook = ( }, { name: 'JUPYTER_IMAGE', - value: imageUrl, + value: imageSelection, }, ], envFrom, @@ -240,9 +240,10 @@ export const startNotebook = async ( export const createNotebook = ( data: StartNotebookData, username: string, + dashboardNamespace: string, canEnablePipelines?: boolean, ): Promise => { - const notebook = assembleNotebook(data, username, canEnablePipelines); + const notebook = assembleNotebook(data, username, dashboardNamespace, canEnablePipelines); const notebookPromise = k8sCreateResource({ model: NotebookModel, @@ -262,9 +263,10 @@ export const updateNotebook = ( existingNotebook: NotebookKind, data: StartNotebookData, username: string, + dashboardNamespace: string, ): Promise => { 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]; @@ -285,9 +287,10 @@ export const updateNotebook = ( export const createNotebookWithoutStarting = ( data: StartNotebookData, username: string, + dashboardNamespace: string, ): Promise => 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 index edd6ebc690..9fd8837129 100644 --- 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, } from '~/utilities/imageUtils'; import { useAppContext } from '~/app/AppContext'; @@ -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 ?? []; diff --git a/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts b/frontend/src/pages/projects/screens/detail/notebooks/useNotebookImageData.ts index 716ca584b4..f2ca00d91e 100644 --- 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 index ab93e4d235..195044e788 100644 --- 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 = ({ ); const editNotebook = notebookState?.notebook; const { projectName } = startNotebookData; + const { dashboardNamespace } = useDashboardNamespace(); const navigate = useNavigate(); const [createInProgress, setCreateInProgress] = React.useState(false); const isButtonDisabled = @@ -154,7 +156,7 @@ const SpawnerFooter: React.FC = ({ envFrom, tolerationSettings, }; - updateNotebook(editNotebook, newStartNotebookData, username) + updateNotebook(editNotebook, newStartNotebookData, username, dashboardNamespace) .then((notebook) => afterStart(notebook.metadata.name, 'updated')) .catch(handleError); } @@ -212,7 +214,7 @@ const SpawnerFooter: React.FC = ({ 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 index 6b1a04d17e..04dbdc4e8b 100644 --- a/frontend/src/utilities/imageUtils.ts +++ b/frontend/src/utilities/imageUtils.ts @@ -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 }; } diff --git a/manifests/base/deployment.yaml b/manifests/base/deployment.yaml index 8b2aeab8ae..e6867d9319 100644 --- 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: