Reconsider usage of dockerImageRepository from imagestream status field in image field of assembled Notebooks

[shalberd]

Feature description

https://github.com/opendatahub-io/odh-dashboard/search?q=dockerImageRepository

Currently, across the board in both frontend (Jupyter, same namespace for Notebooks as namespace of ODH Instance) and backend (different namespaces posssible), the imageUrl referenced in Notebook image: field is constructed from Imagestream status ' dockerImageRepository (varialbe imageUrl in the code).

That entry is empty and non-existent when there is no openshift-internal docker registry / dockerImageRepository used.

     from:
        kind: DockerImage
        name: 'quay.io/thoth-station/s2i-generic-data-science-notebook:v0.0.5'
      generation: 2
      importPolicy: {}
      referencePolicy:
        type: Source
status:
  dockerImageRepository: ''
  tags:
    - tag: v0.0.5
      items:
        - created: '2022-11-21T13:14:31Z'
          dockerImageReference: >-
            quay.io/thoth-station/s2i-generic-data-science-notebook@sha256:3f619c61501218f03d39d97541336dee024f446e64f3a47e2bc7e62cddeb2e58
          image: >-
            sha256:3f619c61501218f03d39d97541336dee024f446e64f3a47e2bc7e62cddeb2e58
          generation: 2

Consider instead to use imagestream name and tag for referencing images.

For cases where Notebook CRD is in a different namespaces than the rest of the software (odh notebook controller, odh dashboard, you need to allow the service account to access an imagestream in a different namespace (system:image-puller).

That seems to be the case, however, already,

odh-dashboard/frontend/src/utilities/notebookControllerUtils.ts

Line 159 in c683fef

const roleBindingName = `${notebookNamespace}-image-pullers`;

which makes the case for referencing imagestream name and tag in a Notebok container even more valid.

publicDockerImageRepository is not a solution, either, because that is dependent on the internal registry, too, plus it being exposed externally:

PublicDockerImageRepository represents the public location from where the image can be pulled outside the cluster. This field may be empty if the administrator has not exposed the integrated registry externally.

Already, in the types, those fields are not mandatory, optional, which is great.

Describe alternatives you've considered

No response

Anything else?

Absolute blocker on all environments that use no openshift-internal docker registry. Especially critical because Jupyterhub is no longer maintained and customers are encouraged to switch to Dashboard plus Notebook controller / Notebook CRDs.

[shalberd]

See also related PR #800

[andrewballantyne]

I believe @VaishnaviHire is working with you on this -- is that correct @shalberd ? I know we have had a bit of a hiccup during the start of the year, but the last I heard was there was something happening upstream on the Notebook Controller to help with this.

[shalberd]

Yes, that is correct, we have discussed this and I have prepared an upstream Kubeflow PR for the notebook controller and reconciliation util therein. kubeflow/kubeflow#6904

It is on the agenda for today's Kubeflow notebooks workgroup meeting https://docs.google.com/document/d/1YtSWRhdhyOgd6ZQcWLM38TGDy2H_EhXjr8U5lUi37_I/edit# I will attend at 9 am PST / 18:00 CET.

@LaVLaS We will see whether they are willing to merge this in upstream. In any case, we'd need to do a merge of upstream to downstream Kubeflow then. If they do not agree to merge, we can put the same logic in opendatahub-io/kubeflow and build a test docker image from there for the notebook controller.

[shalberd]

@harshad16 @andrewballantyne @lucferbux

Vaishnavi Hire is currently testing behavior of the new notebook controller, taking into account image change trigger annotations, and its impact on long-running notebooks when weekly images change behind an imagestream tag. Idea is to first merge in our opendatahub / downstream notebook PR and then later to merge in odh dashboard PR that makes use of the image change trigger annotation to enable notebook spawning both in environments with and without the internal openshift image registry.

@atheo89 Can we make the use of image content change trigger mechanism to resolve image references to planning?

[shalberd]

@harshad16 @jiridanek @jstourac @atheo89 @lucferbux

closed in favor odh odh notebook controller doing the notebook image field lookup in all cases (no openshift internal registry, openshift internal registry): opendatahub-io/kubeflow#336 and opendatahub-io/kubeflow#329

as well as dashboard changes #2867

Thank you all very much, neat work

minor remaining: changing notebook container imagePullPolicy: IfNotPresent from old imagePullPolicy: Always

as a nice-to-have feature for external registry and large images with a unique hash. Needs to be validated if that works with internal openshift registry, too. Saves times on workbench startup.